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.

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

Fix timing of EOMtheEMSrunner #501

Merged
merged 4 commits into from
Jun 14, 2024

Conversation

marcoaccame
Copy link
Contributor

@marcoaccame marcoaccame commented Jun 13, 2024

This PR fixes false reporting of execution time overflow emitted by object EOMtheEMSrunner that we have observed when the system is highly stressed.

The reason of the false reporting was due to a race condition inside the RTOS function osal_system_abstime_get() which may return non-increasing time between two consecutive calls, thus generating negative delta time.

In doing so, I have also added support to a proper monitoring of min, max, average duration of the RX, DO, and TX phases of the runner. Now this feature is disabled. Ideally, it could be useful to let the xml file that specifies the duration RX, DO, and TX phases also specify whether or not to activate this monitoring and the frequency of its reporting.

Some more about the RTOS improvement.

In osal, we measure system time in units of $us$ (but also the $ns$ are available). We achieve that by adding two contributions. The tick time of the RTOS that is typically at 1 $ms$ granularity and is incremented by the SysTick_Handler() gives the coarse resolution. The content of the STCUR register that counts down from its reload value to zero gives the fine resolution.
The combination of coarse and fine values are done inside the SVC_Handler() that has a higher priority than the SysTick_Handler(). In rare cases it may happen that the SVC_Handler() is called to get the system time but in the meantime the STCUR register goes to zero and set the SysTick_Handler() pending. In this case it is correct to add +1 to the tick count. Failure to do that results in a system time that can be wrong of 1 $ms$ so delta times can be wring by +1000 or -1000 $us$.

This PR fixed that.

Mergeability

I have tested the new feature on a dedicated setup with a nasty test harness and partly on iRonCub robot running at 1kHz reporting rate, so I believe that we can safely merge.

Associated PRs

We also have new binaries for ems, mc4plus, mc2plus, amc:

…among svc, systick and other irq handlers

now consecutive calls are guaranteed to have incremented time values
…avereage phase duration (eoerror_value_SYS_ctrloop_rxphasemax, eoerror_value_SYS_ctrloop_rxphasemin, eoerror_value_SYS_ctrloop_rxphaseaverage etc for do and tx)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant