RTOS: SysTimer: Fix test timing issues

[fkjagodzinski]

Description

Use a busy loop with non-blocking Semaphore::wait(0) calls instead of a
single Semaphore::wait(osWaitForever) to improve time measurement
accuracy.

This should resolve #9400.

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[x] Test update
[ ] Breaking change

Reviewers

@c1728p9 @jeromecoutant

[ciarmcom]

@fkjagodzinski, thank you for your changes.
@c1728p9 @ARMmbed/mbed-os-core @ARMmbed/mbed-os-test @ARMmbed/mbed-os-maintainers please review.

[deepikabhavnani]

Use a busy loop with non-blocking Semaphore::wait(0) calls instead of a single Semaphore::wait(osWaitForever) to improve time measurement accuracy.

Please share some background information on this, how will it improve time measurement?
I believe we are not saying that code below(changed) takes less time then sem_slots = st.sem_wait(osWaitForever);

  while (sem_slots != 1) {
        sem_slots = st.sem_wait(0);
    }

[jeromecoutant]

Test is 100% OK with all STM32L0 and STM32L4 targets now!

[0xc0170]

@deepikabhavnani good question !

Can we have the answer captured in the commit msg (why are we changing this code - both are valid, arenth they)

[fkjagodzinski]

Please share some background information on this, how will it improve time measurement?

@deepikabhavnani, @0xc0170 This patch prevents the board from entering sleep or deepsleep modes while waiting for the semaphore. The difference can be seen with this code (which is just a simplified version of the code from the test).

#ifndef MBED_TICKLESS
#error Tickless mode not supported for this target.
#endif

#if !DEVICE_LPTICKER
#error Current SysTimer implementation requires lp ticker support.
#endif

#ifndef MBED_CPU_STATS_ENABLED
#error Please set MBED_CPU_STATS_ENABLED to see the sleep stats
#endif

#include "mbed.h"
extern "C" {
#include "rtx_lib.h"
}
#include "rtos/TARGET_CORTEX/SysTimer.h"

#define TEST_TICKS 42UL

const us_timestamp_t DELAY_US = 1000000ULL * TEST_TICKS / OS_TICK_FREQ;

// Override the handler() -- the SysTick interrupt must not be set as pending by the test code.
class SysTimerTest: public rtos::internal::SysTimer {
private:
    Semaphore _sem;
    virtual void handler()
    {
        core_util_critical_section_enter();
        _increment_tick();
        core_util_critical_section_exit();
        _sem.release();
    }

public:
    SysTimerTest() :
        SysTimer(), _sem(0, 1)
    {
    }

    SysTimerTest(const ticker_data_t *data) :
        SysTimer(data), _sem(0, 1)
    {
    }

    virtual ~SysTimerTest()
    {
    }

    int32_t sem_wait(uint32_t millisec)
    {
        return _sem.wait(millisec);
    }
};

int main()
{
    SysTimerTest st;
    st.schedule_tick(TEST_TICKS);
    us_timestamp_t t1 = st.get_time();
    int32_t sem_slots = 0;

#ifdef SEM_WAIT_FOREVER
#define WAIT_METHOD "osWaitForever"
    sem_slots = st.sem_wait(osWaitForever);
#else
#define WAIT_METHOD "busy_loop"
    while (sem_slots != 1) {
        sem_slots = st.sem_wait(0);
    }
#endif

    us_timestamp_t t2 = st.get_time();

    us_timestamp_t t_idle = mbed_time_idle();
    us_timestamp_t t_sleep = mbed_time_sleep();
    us_timestamp_t t_deepsleep = mbed_time_deepsleep();

    printf("%15s%15s\r\n", "wait_method = ", WAIT_METHOD);
    printf("%15s%15llu\r\n", "t_idle = ", t_idle);
    printf("%15s%15llu\r\n", "t_sleep = ", t_sleep);
    printf("%15s%15llu\r\n", "t_deepsleep = ", t_deepsleep);
    printf("%15s%15llu\r\n\r\n", "t2 - t1 = ", t2 - t1);
}

mbed compile -t GCC_ARM -m NUCLEO_L476RG -f --sterm -DMBED_CPU_STATS_ENABLED -DSEM_WAIT_FOREVER

gives:

 wait_method =   osWaitForever
      t_idle =           44220
     t_sleep =             122
 t_deepsleep =           44098
     t2 - t1 =           44586

mbed compile -t GCC_ARM -m NUCLEO_L476RG -f --sterm -DMBED_CPU_STATS_ENABLED

gives:

 wait_method =       busy_loop
      t_idle =               0
     t_sleep =               0
 t_deepsleep =               0
     t2 - t1 =           42083

[deepikabhavnani]

Thanks for explanation @fkjagodzinski. Looks good to me 👍

It will be good to add below line to the commit message
"Looping in Semaphore::wait(0) prevents the board from entering sleep or deepsleep modes while waiting for the semaphore. By skipping the overhead wakeup time, we get more accurate timings"

[fkjagodzinski]

I've added a short comment in the code and updated the commit message as suggested.

[cmonr]

CI started

[mbed-ci]

Test run: SUCCESS

Summary: 9 of 9 test jobs passed
Build number : 1
Build artifacts