Mbed-os: New ticker tests are not adapted to variable-width tickers.

Created on 27 Mar 2018 · 8Comments · Source: ARMmbed/mbed-os

Description

Type: Bug

@0xc0170 What we talked about earlier.
@LMESTM I guess you're having the same issues.

Bug

Target
EFM32

Toolchain:
GCC_ARM

mbed-os sha:
458e726b1

Expected behavior
Unit tests work with all tickers corresponding to the new requirements (width between 16 bits and 32 bits, frequency between 250kHz and 8MHz).

Actual behavior
At various stages in the unit test (https://github.com/ARMmbed/mbed-os/blob/458e726b1e2c32fb2ea6238771190d6fdd78e18d/TESTS/mbed_hal/common_tickers/main.cpp) there are steps that don't account for getting a potential overflow on a fast-running short-width timer.

Examples:

https://github.com/ARMmbed/mbed-os/blob/458e726b1e2c32fb2ea6238771190d6fdd78e18d/TESTS/mbed_hal/common_tickers/main.cpp#L52 - When the ticker overflows during counting, the count will be wildly off. Neither this counting function or the dependant test account for potential overflows.
https://github.com/ARMmbed/mbed-os/blob/458e726b1e2c32fb2ea6238771190d6fdd78e18d/TESTS/mbed_hal/common_tickers/main.cpp#L106 - Operation is illegal according to the specification, since you might be trying to set an interrupt for a timestamp that is greater than the maximum timer value.
https://github.com/ARMmbed/mbed-os/blob/458e726b1e2c32fb2ea6238771190d6fdd78e18d/TESTS/mbed_hal/common_tickers/main.cpp#L114 - Big risk for this loop to loop forever with a non-32-bit ticker.
https://github.com/ARMmbed/mbed-os/blob/458e726b1e2c32fb2ea6238771190d6fdd78e18d/TESTS/mbed_hal/common_tickers/main.cpp#L385 - Gets used for both us_ticker and lp_ticker tests, but is only adapted for the lp_ticker case. When using a us_ticker you want a larger backoff time, since it is running way faster.

closed_in_jira mirrored bug

Source

stevew817

Most helpful comment

@mprse @jamesbeyond could you split the test fixes to separate PR please.

bulislaw on 27 Mar 2018

👍2

All 8 comments

Indeed - same issues faced, now also with sleep tests, for instance here:
https://github.com/ARMmbed/mbed-os/blob/feature-hal-spec-sleep/TESTS/mbed_hal/sleep/main.cpp#L87

LMESTM on 27 Mar 2018

Agree also, seems that some tests need few update to ticker_info_t with frequency other than 1MHz and counter different than 32.

jeromecoutant on 27 Mar 2018

This test assumes that each test case (if needed) is executed before overflow occurs. overflow_protect() function should protect against overflow.

I also had some issues with overflows and had to modify this first test version. Final test version is provided here PR https://github.com/ARMmbed/mbed-os/pull/5629 (https://github.com/ARMmbed/mbed-os/pull/5629/commits/80c55e47f85aaeb8137367dbd962cb47b86e4bf6).

Please check if this test version works.