Skip to content

tests: lib: timespec_util: support tick period not multiples of 100ms #92255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

cfriedt
Copy link
Member

@cfriedt cfriedt commented Jun 26, 2025

If the system tick period is not a multiple of 100ms, then K_MSEC(100) does not correspond exactly to 100000000 ns after going through tick conversion.

Convert 100ms to ticks and then to ns to account for this within the testsuite.

Fixes #92158

Testing Done:

west build -p auto -b nrf52_bsim -t run tests/lib/timespec_util -T libraries.timespec_utils.picolibc
...
d_00: @00:00:00.000000  *** Booting Zephyr OS build v4.1.0-6916-g38cd1da16c1f ***
d_00: @00:00:00.000000  Running TESTSUITE timeutil_api
d_00: @00:00:00.000000  ===================================================================
d_00: @00:00:00.000000  CONFIG_TIMEOUT_64BIT=y
d_00: @00:00:00.000000  K_TICK_MAX: 9223372036854775807
d_00: @00:00:00.000000  minimum timeout: {0, 30517}
d_00: @00:00:00.000000  maximum timeout: {9223372036, 854745291}
d_00: @00:00:00.000000  START - test_timespec_add
d_00: @00:00:00.000000   PASS - test_timespec_add in 0.000 seconds
d_00: @00:00:00.000000  ===================================================================
d_00: @00:00:00.000000  START - test_timespec_compare
d_00: @00:00:00.000000   PASS - test_timespec_compare in 0.000 seconds
d_00: @00:00:00.000000  ===================================================================
d_00: @00:00:00.000000  START - test_timespec_equal
d_00: @00:00:00.000000   PASS - test_timespec_equal in 0.000 seconds
d_00: @00:00:00.000000  ===================================================================
d_00: @00:00:00.000000  START - test_timespec_from_timeout
d_00: @00:00:00.000000   PASS - test_timespec_from_timeout in 0.000 seconds
d_00: @00:00:00.000000  ===================================================================
d_00: @00:00:00.000000  START - test_timespec_is_valid
d_00: @00:00:00.000000   PASS - test_timespec_is_valid in 0.000 seconds
d_00: @00:00:00.000000  ===================================================================
d_00: @00:00:00.000000  START - test_timespec_negate
d_00: @00:00:00.000000   PASS - test_timespec_negate in 0.000 seconds
d_00: @00:00:00.000000  ===================================================================
d_00: @00:00:00.000000  START - test_timespec_normalize
d_00: @00:00:00.000000   PASS - test_timespec_normalize in 0.000 seconds
d_00: @00:00:00.000000  ===================================================================
d_00: @00:00:00.000000  START - test_timespec_sub
d_00: @00:00:00.000000   PASS - test_timespec_sub in 0.000 seconds
d_00: @00:00:00.000000  ===================================================================
d_00: @00:00:00.000000  START - test_timespec_to_timeout
d_00: @00:00:00.000000   PASS - test_timespec_to_timeout in 0.000 seconds
d_00: @00:00:00.000000  ===================================================================
d_00: @00:00:00.000000  TESTSUITE timeutil_api succeeded
d_00: @00:00:00.000000  
d_00: @00:00:00.000000  ------ TESTSUITE SUMMARY START ------
d_00: @00:00:00.000000  
d_00: @00:00:00.000000  SUITE PASS - 100.00% [timeutil_api]: pass = 9, fail = 0, skip = 0, total = 9 duration = 0.000 seconds
d_00: @00:00:00.000000   - PASS - [timeutil_api.test_timespec_add] duration = 0.000 seconds
d_00: @00:00:00.000000   - PASS - [timeutil_api.test_timespec_compare] duration = 0.000 seconds
d_00: @00:00:00.000000   - PASS - [timeutil_api.test_timespec_equal] duration = 0.000 seconds
d_00: @00:00:00.000000   - PASS - [timeutil_api.test_timespec_from_timeout] duration = 0.000 seconds
d_00: @00:00:00.000000   - PASS - [timeutil_api.test_timespec_is_valid] duration = 0.000 seconds
d_00: @00:00:00.000000   - PASS - [timeutil_api.test_timespec_negate] duration = 0.000 seconds
d_00: @00:00:00.000000   - PASS - [timeutil_api.test_timespec_normalize] duration = 0.000 seconds
d_00: @00:00:00.000000   - PASS - [timeutil_api.test_timespec_sub] duration = 0.000 seconds
d_00: @00:00:00.000000   - PASS - [timeutil_api.test_timespec_to_timeout] duration = 0.000 seconds
d_00: @00:00:00.000000  
d_00: @00:00:00.000000  ------ TESTSUITE SUMMARY END ------
d_00: @00:00:00.000000  
d_00: @00:00:00.000000  ===================================================================
d_00: @00:00:00.000000  PROJECT EXECUTION SUCCESSFUL

If the system tick period is not a multiple of 100ms, then K_MSEC(100)
does not correspond exactly to 100000000 ns after going through tick
conversion.

Convert 100ms to ticks and then to ns to account for this within the
testsuite.

Signed-off-by: Chris Friedt <cfriedt@tenstorrent.com>
@github-actions github-actions bot added the size: XS A PR changing only a single line of code label Jun 26, 2025
Copy link

Copy link
Member

@aescolar aescolar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into a fix @cfriedt ,
But doesn't this problem apply to all values in this array in general?
The first column is a value in ticks. So it will have whatever resolution the tick has.
The bug report provided a reproduction with one particular platform in which the rounding was off for the 100ms value, but depending on CONFIG_SYS_CLOCK_TICKS_PER_SEC any other of this values may round out badly, or?
Meaning shouldn't this change be applied to all values here?

@cfriedt
Copy link
Member Author

cfriedt commented Jun 27, 2025

Sure, I can probably do the conversion within the macro to cover other cases.

@aescolar
Copy link
Member

aescolar commented Jul 2, 2025

Sure, I can probably do the conversion within the macro to cover other cases.

@cfriedt do you think you will have the time to do this change sometime soon?
Otherwise we could just merge this PR as is. It will likely fail with other platforms but at least it unblocks Nordic ones..

@aescolar aescolar dismissed their stale review July 2, 2025 08:09

At least it fixes the issue happening for nordic devices.

@cfriedt
Copy link
Member Author

cfriedt commented Jul 3, 2025

@aescolar - I think I can probably make an improved version tomorrow with hopefully much better test cases defined. There were quite a few corner cases that I wanted to cover and also possibly some implementation improvements

@cfriedt
Copy link
Member Author

cfriedt commented Jul 4, 2025

@aescolar - maybe it's fine to merge this one for now. tick rate / time domain problems always seem to have corner cases.

For reference, my alternative is here
https://github.com/zephyrproject-rtos/zephyr/pull/92709/files

@cfriedt
Copy link
Member Author

cfriedt commented Jul 6, 2025

Closing this in favour of #92709

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: XS A PR changing only a single line of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests/lib/timespec_util Assumes system tick divides 100ms evenly and fails in platforms where it doesn't
2 participants