tests-mbed_hal-sleep_manager: fix counter wraparound handling #12519
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fkjagodzinski
approved these changes
Feb 26, 2020
| @@ -141,7 +141,7 @@ void test_sleep_auto() | |||
| const ticker_info_t *lp_ticker_info = get_lp_ticker_data()->interface->get_info(); | |||
| const unsigned lp_ticker_mask = ((1 << lp_ticker_info->bits) - 1); | |||
| const ticker_irq_handler_type lp_ticker_irq_handler_org = set_lp_ticker_irq_handler(lp_ticker_isr); | |||
| us_timestamp_t us_ts1, us_ts2, lp_ts1, lp_ts2, us_diff1, us_diff2, lp_diff1, lp_diff2; | |||
| unsigned int us_ts1, us_ts2, lp_ts1, lp_ts2, us_diff1, us_diff2, lp_diff1, lp_diff2; | |||
There was a problem hiding this comment.
Suggested change
| unsigned int us_ts1, us_ts2, lp_ts1, lp_ts2, us_diff1, us_diff2, lp_diff1, lp_diff2; | |
| uint32_t us_ts1, us_ts2, lp_ts1, lp_ts2, us_diff1, us_diff2, lp_diff1, lp_diff2; |
There is a mismatch while handling counter wraparound in `test_sleep_auto` test case. In the test timestamps in ticks are first converted to us and then while counting the time difference wraparound is handled (us and ticks fields are mismatched). The ticker wraparound case must be handled in the field of ticks, and then the difference converted to us.
mprse
force-pushed
the
sleepmanaget_test_fix
branch
from
a9763f4 to
42a9cc1
Compare
stevew817
approved these changes
Feb 26, 2020
amq
approved these changes
Feb 26, 2020
|
@mprse, thank you for your changes. |
stevew817
approved these changes
Feb 26, 2020
fkjagodzinski
approved these changes
Feb 26, 2020
0xc0170
approved these changes
Feb 27, 2020
|
CI started |
Test run: SUCCESSSummary: 5 of 5 test jobs passed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
This PR is a fix for Issue: #12373.
The test needs to be fixed since there is a mismatch while handling counter wraparound in
test_sleep_autotest case.In the test timestamps in ticks are first converted to us and then while counting the time difference wraparound is handled (us and ticks fields are mismatched). The ticker wraparound case must be handled in the field of ticks, and then the ticks difference converted to us.
In the following line:
us_diff1 = (us_ts1 <= us_ts2) ? (us_ts2 - us_ts1) : (us_ticker_mask - us_ts1 + us_ts2 + 1);us_ts1- timestamp before sleep in us.us_ts2- timestamp after sleep in us.us_ticker_mask- max ticks count (depending on ticker width).So when wraparound is detected we have mismatched arithmetic operations (ticks and us).
Test results:
Impact of changes
Migration actions required
Documentation
Pull request type
Test results
Reviewers
@stevew817 @amq @fkjagodzinski