-
-
Notifications
You must be signed in to change notification settings - Fork 729
Description
Whilst using micros() to timestamp hardware events on a SAMD21 processor, I've discovered that the function is broken if called from priority 0 or 1 ISRs. Specifically, from time to time, the result is 1ms behind, making the value returned non-monotonic and inaccurate.
Looking at the code, the micros() function has logic to determine if there is a pending SysTick ISR, and adds 1ms() if the SysTick counter has rolled, but the tick increment ISR has not been called. This works fine when the SysTick ISR has priority 0, as it is either pended (if another priority 0 ISR is running already), or runs to completion.
However, pull request #182, related to adding I2S support, reduced the SysTick ISR to priority 2 (from priority 0).
/cores/arduino/wiring.c
NVIC_SetPriority (SysTick_IRQn, (1 << __NVIC_PRIO_BITS) - 2); /* set Priority for Systick Interrupt (2nd lowest) */
This means that a priority 0 or 1 ISR (such as that generated by a GPIO interrupt) can preempt an already running SysTick ISR. Under these conditions the logic to add the required 1ms never activates (as the ISR is active not pending ).
If the SysTick ISR has already incremented the tick count, all is fine. If it hasn't, the result of micros() effectively loses 1ms, which makes the result inaccurate and non-monotonic. (Note, even if you checked whether the SysTick ISR is active, it isn't possible to determine whether the tick count has or hasn't been incremented from within micros())
I'm hitting the problem timestamping precision edges, and see the issue whenever an edge aligns with a roll of the SysTick. With 1PulsePerSecond edges I get the incorrect result a few times an hour, as the wander of the main clock leads to the SysTick roll aligning with the external edge.
Alternatively, I suspect that if you use a non-precision 1KHz+ input, you will see frequent 1ms backsteps.
I'm not aware of why it was felt necessary to reduce the priority of SysTick to get I2S to work, but I can confirm that restoring SysTick to priority 0 returns micros() to being monotonic and accurate from ISRs of priorities 0 & 1. I'd suggest that given the critical nature of the micros() function, SysTick should be restored to priority 0, and an alternative solution found for the I2S problem(s).