Make common tickers test more strict

[c1728p9]

Description

The ticker specification requires that the ticker interrupt fires only
when the ticker times increments to or past the value set by
ticker_set_interrupt. The test for this, ticker_interrupt_test,
only asserted that the interrupt to fired within +-TICKER_DELTA of the
match value.

This patch removes TICKER_DELTA tolerance so the test will fail if the
interrupt does not fire at the exact time specified.

Pull request type

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

[cmonr]

@mbed-os-test @kjbracey-arm @mprse Mind taking a look?

[0xc0170]

I'll review this soon.

@mprse Can you review please?

[0xc0170]

Why are we removing the delta (making it more strict) ? Does this change fit all targets?

[jamesbeyond]

Why are we removing the delta (making it more strict) ? Does this change fit all targets?

I got same question want to ask

[mprse]

I agree that form of the test which is proposed by @c1728p9 is closer to the requirements, then current version. It should be okay for boards which uses for Ticker one counter and CC registers to generate interrupts (e.g. NRF5x). But we have also boards like K64F which uses 2 counters. One for counting time and another for interrupt generation. I checked that interrupt test case fails on K64F with the fix and passes on NRF51_DK.

BTW I checked the implementation for K64F and it looks like there is a bug:

mbed-os/targets/TARGET_Freescale/TARGET_MCUXpresso_MCUS/TARGET_MCU_K64F/us_ticker.c

Line 118 in 232543a

timestamp >= now_ticks ? timestamp - now_ticks : (uint32_t)((uint64_t) timestamp + 0xFFFFFFFF - now_ticks);

should be:
timestamp >= now_ticks ? timestamp - now_ticks : (uint32_t)((uint64_t) timestamp + 0xFFFFFFFF - now_ticks + 1);

But even with this + 1 the test fails (with fix).

[cmonr]

@c1728p9 @mprse @jamesbeyond @0xc0170 Anything needed to keep this PR moving?

[mprse]

Anything needed to keep this PR moving?

I think we should run morph and check the results.

[cmonr]

Will run since CI is idle, but the PR still needs to be reviewed.

/morph build

[mbed-ci]

Build : SUCCESS

Build number : 3428
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7598/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

[mbed-ci]

Exporter Build : SUCCESS

Build number : 3050
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7598/

[mbed-ci]

Test : FAILURE

Build number : 3219
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7598/3219

[cmonr]

Interesting. Might have been too strict.

Please take a look at the errors.

@ARMmbed/mbed-os-test @kjbracey-arm @mprse Please take a look when y'all have a chance.

[c1728p9]

Closing this for now since it makes tests too strict for some platforms to pass, similar to #8129.

[jeromecoutant]

Hi
In some netsocket tests, a MBED_EXTENDED_TESTS macro exists.

Maybe you can keep some extra tests or this delta time definition within a new macro
in order to not loose better test implementation ?