arduino / arduino/ArduinoCore-samd

micros() is non-monotonic with -1ms glitches on Cortex cores from priority 0 or 1 ISRs

Open
#463 33 comments 2 reactions 0 assignees View on GitHub
Dominant language
C
Stars
502
Forks
740
PR merge metrics
No merged PRs in 30d

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).

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in cores/arduino/wiring.c, reading micros() alongside the SysTick priority configuration and the behavior described around PR #182. Reproduce the -1 ms non-monotonic result from priority 0 or 1 ISRs, then verify that the chosen resolution preserves accurate monotonic timestamps without regressing I2S support.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
embedded-iot
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.