Correct SysTimer absolute time calculations #12326
Merged
Conversation
This file contains hidden or 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
`SysTimer::set_wake_time` incorrectly assumed that the `SysTimer`s tick count and the underlying HAL timer had the same zero base. This normally holds, at least approximately, in RTOS builds where the HAL timer starts from zero at the same time the SysTimer is initialised. But in bare metal builds, the HAL timer could be started some time before the SysTimer, giving a significant discrepancy. Beyond that, there's no requirement for HAL timers to start from zero in the spec. Record the HAL timer start time to get the conversion right.
Member
|
@kjbracey-arm, thank you for your changes. |
mprse
approved these changes
Jan 29, 2020
Contributor
mprse
left a comment
mprse
left a comment
There was a problem hiding this comment.
The fix looks good. Thanks for helping with this one.
Tests in the bare metal mode now pass! Results can be found below:
$ mbed test -m K64F -t GCC_ARM --app-config TESTS/configs/baremetal.json -n tests-mbed_drivers-lp_ticker -v
| target | platform_name | test suite | result | elapsed_time (sec) | copy_method |
|--------------|---------------|------------------------------|--------|--------------------|-------------|
| K64F-GCC_ARM | K64F | tests-mbed_drivers-lp_ticker | OK | 20.66 | default |
mbedgt: test suite results: 1 OK
mbedgt: test case report:
| target | platform_name | test suite | test case | passed | failed | result | elapsed_time (sec) |
|--------------|---------------|------------------------------|-------------------------------------------|--------|--------|--------|--------------------|
| K64F-GCC_ARM | K64F | tests-mbed_drivers-lp_ticker | Test attach for 0.001s and time measure | 1 | 0 | OK | 0.06 |
| K64F-GCC_ARM | K64F | tests-mbed_drivers-lp_ticker | Test attach for 0.01s and time measure | 1 | 0 | OK | 0.08 |
| K64F-GCC_ARM | K64F | tests-mbed_drivers-lp_ticker | Test attach for 0.1s and time measure | 1 | 0 | OK | 0.17 |
| K64F-GCC_ARM | K64F | tests-mbed_drivers-lp_ticker | Test attach for 0.5s and time measure | 1 | 0 | OK | 0.55 |
| K64F-GCC_ARM | K64F | tests-mbed_drivers-lp_ticker | Test attach_us for 100ms and time measure | 1 | 0 | OK | 0.16 |
| K64F-GCC_ARM | K64F | tests-mbed_drivers-lp_ticker | Test attach_us for 10ms and time measure | 1 | 0 | OK | 0.07 |
| K64F-GCC_ARM | K64F | tests-mbed_drivers-lp_ticker | Test attach_us for 1ms and time measure | 1 | 0 | OK | 0.08 |
| K64F-GCC_ARM | K64F | tests-mbed_drivers-lp_ticker | Test attach_us for 500ms and time measure | 1 | 0 | OK | 0.58 |
| K64F-GCC_ARM | K64F | tests-mbed_drivers-lp_ticker | Test detach | 1 | 0 | OK | 0.74 |
| K64F-GCC_ARM | K64F | tests-mbed_drivers-lp_ticker | Test multi call and time measure | 1 | 0 | OK | 1.06 |
| K64F-GCC_ARM | K64F | tests-mbed_drivers-lp_ticker | Test multi ticker | 1 | 0 | OK | 0.27 |
mbedgt: test case results: 11 OK
$ mbed test -m K64F -t GCC_ARM --app-config TESTS/configs/baremetal.json -n tests-mbed_hal-rtc -v
| target | platform_name | test suite | result | elapsed_time (sec) | copy_method |
|--------------|---------------|--------------------|--------|--------------------|-------------|
| K64F-GCC_ARM | K64F | tests-mbed_hal-rtc | OK | 56.87 | default |
mbedgt: test suite results: 1 OK
mbedgt: test case report:
| target | platform_name | test suite | test case | passed | failed | result | elapsed_time (sec) |
|--------------|---------------|--------------------|------------------|--------|--------|--------|--------------------|
| K64F-GCC_ARM | K64F | tests-mbed_hal-rtc | RTC - accuracy | 1 | 0 | OK | 9.73 |
| K64F-GCC_ARM | K64F | tests-mbed_hal-rtc | RTC - enabled | 1 | 0 | OK | 0.04 |
| K64F-GCC_ARM | K64F | tests-mbed_hal-rtc | RTC - glitch | 1 | 0 | OK | 3.65 |
| K64F-GCC_ARM | K64F | tests-mbed_hal-rtc | RTC - init | 1 | 0 | OK | 0.03 |
| K64F-GCC_ARM | K64F | tests-mbed_hal-rtc | RTC - persist | 1 | 0 | OK | 4.05 |
| K64F-GCC_ARM | K64F | tests-mbed_hal-rtc | RTC - range | 1 | 0 | OK | 16.04 |
| K64F-GCC_ARM | K64F | tests-mbed_hal-rtc | RTC - sleep | 1 | 0 | OK | 8.06 |
| K64F-GCC_ARM | K64F | tests-mbed_hal-rtc | RTC - write/read | 1 | 0 | OK | 0.06 |
mbedgt: test case results: 8 OK
$ mbed test -m NRF52840_DK -t GCC_ARM --app-config TESTS/configs/baremetal.json -n tests-mbed_drivers-lp_ticker -v
| target | platform_name | test suite | result | elapsed_time (sec) | copy_method |
|---------------------|---------------|------------------------------|--------|--------------------|-------------|
| NRF52840_DK-GCC_ARM | NRF52840_DK | tests-mbed_drivers-lp_ticker | OK | 24.65 | default |
mbedgt: test suite results: 1 OK
mbedgt: test case report:
| target | platform_name | test suite | test case | passed | failed | result | elapsed_time (sec) |
|---------------------|---------------|------------------------------|-------------------------------------------|--------|--------|--------|--------------------|
| NRF52840_DK-GCC_ARM | NRF52840_DK | tests-mbed_drivers-lp_ticker | Test attach for 0.001s and time measure | 1 | 0 | OK | 0.08 |
| NRF52840_DK-GCC_ARM | NRF52840_DK | tests-mbed_drivers-lp_ticker | Test attach for 0.01s and time measure | 1 | 0 | OK | 0.09 |
| NRF52840_DK-GCC_ARM | NRF52840_DK | tests-mbed_drivers-lp_ticker | Test attach for 0.1s and time measure | 1 | 0 | OK | 0.16 |
| NRF52840_DK-GCC_ARM | NRF52840_DK | tests-mbed_drivers-lp_ticker | Test attach for 0.5s and time measure | 1 | 0 | OK | 0.57 |
| NRF52840_DK-GCC_ARM | NRF52840_DK | tests-mbed_drivers-lp_ticker | Test attach_us for 100ms and time measure | 1 | 0 | OK | 0.16 |
| NRF52840_DK-GCC_ARM | NRF52840_DK | tests-mbed_drivers-lp_ticker | Test attach_us for 10ms and time measure | 1 | 0 | OK | 0.07 |
| NRF52840_DK-GCC_ARM | NRF52840_DK | tests-mbed_drivers-lp_ticker | Test attach_us for 1ms and time measure | 1 | 0 | OK | 0.06 |
| NRF52840_DK-GCC_ARM | NRF52840_DK | tests-mbed_drivers-lp_ticker | Test attach_us for 500ms and time measure | 1 | 0 | OK | 0.57 |
| NRF52840_DK-GCC_ARM | NRF52840_DK | tests-mbed_drivers-lp_ticker | Test detach | 1 | 0 | OK | 0.74 |
| NRF52840_DK-GCC_ARM | NRF52840_DK | tests-mbed_drivers-lp_ticker | Test multi call and time measure | 1 | 0 | OK | 1.06 |
| NRF52840_DK-GCC_ARM | NRF52840_DK | tests-mbed_drivers-lp_ticker | Test multi ticker | 1 | 0 | OK | 0.28 |
mbedgt: test case results: 11 OK
Approved.
Contributor
Author
|
CI started |
Test run: SUCCESSSummary: 11 of 11 test jobs passed |
|
This PR does not contain release version label after merging. |
Contributor
|
I've fixed the version: Set to 6.0.0-alpha-2 |
0xc0170
added
release-version: 6.0.0-alpha-2
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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
SysTimer::set_wake_timeincorrectly assumed that theSysTimers tick count and the underlying HAL timer had the same zero base. This normally holds, at least approximately, in RTOS builds where the HAL timer starts from zero at the same time the SysTimer is initialised.But in bare metal builds, the HAL timer could be started some time before the SysTimer, giving a significant discrepancy.
Beyond that, there's no requirement for HAL timers to start from zero in the spec.
Record the HAL timer start time to get the conversion right.
Fixes #12313
Impact of changes
Migration actions required
Documentation
None
Pull request type
Test results
Reviewers
@mprse