Skip to content
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.

Sign up for GitHub

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

Jump to bottom

tests: kernel: timer: timer_behavior: Tweak expected std deviation #74939

Merged
merged 2 commits into from
Jul 9, 2024
Merged

tests: kernel: timer: timer_behavior: Tweak expected std deviation #74939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
72bae8f
tests: kernel: timer: timer_behavior: Tweak expected std deviation
nordic-krch Jun 25, 2024
5e700e6
tests: kernel: timer: behavior: pytest: Adjust max stddev
nordic-krch Jun 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion tests/kernel/timer/timer_behavior/pytest/test_timer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ def do_analysis(test, stats, stats_count, config, sys_clock_hw_cycles_per_sec):
max_bound = (test_period + period_max_drift * test_period +
expected_period_drift) / 1_000_000

cyc_us = 1000000 / sys_clock_hw_cycles_per_sec
Copy link
Contributor

Choose a reason for hiding this comment

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

Naming whine: many platforms have cycle rates > 1MHz, making this value a zero. Giving it a name that looks and sounds like "cycles per microsecond" is likely to end up in a divide-by-zero as the code evolves. It's used as a clamp, so e.g. "min_cyc = ...; /* Minimum possible cycles that can be measured */' might be better?

max_stddev = int(config['TIMER_TEST_MAX_STDDEV']) / 1_000_000
# Max STDDEV cannot be lower than clock single cycle
max_stddev = max(cyc_us, max_stddev)

max_drift_ppm = int(config['TIMER_EXTERNAL_TEST_MAX_DRIFT_PPM'])
time_diff = stats['total_time'] - seconds - expected_total_drift
Expand All @@ -62,13 +65,13 @@ def do_analysis(test, stats, stats_count, config, sys_clock_hw_cycles_per_sec):
f', "min_bound_us":{min_bound * 1_000_000:.6f}'
f', "max_bound_us":{max_bound * 1_000_000:.6f}'
f', "expected_period_cycles":{expected_period:.0f}'
f', "MAX_STD_DEV":{max_stddev:.6f}'
f', "sys_clock_hw_cycles_per_sec":{sys_clock_hw_cycles_per_sec}, ' +
', '.join(['"CONFIG_{}":{}'.format(k, str(config[k]).rstrip()) for k in [
'SYS_CLOCK_HW_CYCLES_PER_SEC',
'SYS_CLOCK_TICKS_PER_SEC',
'TIMER_TEST_PERIOD',
'TIMER_TEST_SAMPLES',
'TIMER_TEST_MAX_STDDEV',
'TIMER_EXTERNAL_TEST_PERIOD_MAX_DRIFT_PPM',
'TIMER_EXTERNAL_TEST_MAX_DRIFT_PPM'
]
Expand Down
9 changes: 6 additions & 3 deletions tests/kernel/timer/timer_behavior/src/jitter_drift.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,8 @@ static void do_test_using(void (*sample_collection_fn)(void), const char *mechan
* CONFIG_TIMER_TEST_SAMPLES;
double time_diff_us = actual_time_us - expected_time_us
- expected_time_drift_us;
/* If max stddev is lower than a single clock cycle then round it up. */
uint32_t max_stddev = MAX(k_cyc_to_us_ceil32(1), CONFIG_TIMER_TEST_MAX_STDDEV);

TC_PRINT("timer clock rate %d, kernel tick rate %d\n",
sys_clock_hw_cycles_per_sec(), CONFIG_SYS_CLOCK_TICKS_PER_SEC);
Expand Down Expand Up @@ -283,7 +285,7 @@ static void do_test_using(void (*sample_collection_fn)(void), const char *mechan
", \"CONFIG_SYS_CLOCK_TICKS_PER_SEC\":%d"
", \"CONFIG_TIMER_TEST_PERIOD\":%d"
", \"CONFIG_TIMER_TEST_SAMPLES\":%d"
", \"CONFIG_TIMER_TEST_MAX_STDDEV\":%d"
", \"MAX STD DEV\":%d"
Copy link
Member

Choose a reason for hiding this comment

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

same recording output change should be in pytest/test_timer.py as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

Choose a reason for hiding this comment

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

why the record name is different now ?

"}\n",
mechanism,
CONFIG_TIMER_TEST_SAMPLES - periodic_rollovers, periodic_rollovers,
Expand All @@ -304,7 +306,7 @@ static void do_test_using(void (*sample_collection_fn)(void), const char *mechan
CONFIG_SYS_CLOCK_TICKS_PER_SEC,
CONFIG_TIMER_TEST_PERIOD,
CONFIG_TIMER_TEST_SAMPLES,
CONFIG_TIMER_TEST_MAX_STDDEV
max_stddev
);

/* Validate the maximum/minimum timer period is off by no more than 10% */
Expand All @@ -323,10 +325,11 @@ static void do_test_using(void (*sample_collection_fn)(void), const char *mechan
"Longest timer period too long (off by more than expected %d%)",
CONFIG_TIMER_TEST_PERIOD_MAX_DRIFT_PERCENT);


/* Validate the timer deviation (precision/jitter of the timer) is within a configurable
* bound
*/
zassert_true(stddev_us < (double)CONFIG_TIMER_TEST_MAX_STDDEV,
zassert_true(stddev_us < (double)max_stddev,
"Standard deviation (in microseconds) outside expected bound");

/* Validate the timer drift (accuracy over time) is within a configurable bound */
Expand Down
Loading