arduino / arduino/ArduinoCore-renesas
`Arduino_LED_Matrix.h`'s timer has a data race (missing `volatile`) which breaks animations
- Dominant language
- C
- Stars
- 193
- Forks
- 112
- PR merge metrics
- No merged PRs in 30d
Description
The `Arduino_LED_Matrix` class uses a `FspTimer` in order to advance its animations. The timer has this callback installed:
https://github.com/arduino/ArduinoCore-renesas/blob/main/libraries/Arduino_LED_Matrix/src/Arduino_LED_Matrix.h#L345
The callback touches a bunch of data fields of the `Arduino_LED_Matrix` object (that installed the callback). The object is indeed passed as the context argument (last parameter of https://github.com/arduino/ArduinoCore-renesas/blob/main/libraries/Arduino_LED_Matrix/src/Arduino_LED_Matrix.h#L182 ).
**The problem is that these fields are not marked `volatile`.**
`FspTimer` uses ISR to manage the timer, and any non-local data touched by ISR callbacks must be marked `volatile`, as per https://www.arduino.cc/reference/cs/language/functions/external-interrupts/attachinterrupt/
> Typically global variables are used to pass data between an ISR and the main program. To make sure variables shared between an ISR and the main program are updated correctly, declare them as `volatile`.
Since the mark is not there, there's a data race. I am able to make animations block or behave erratically by simply changing the currently running animation. Most data members need `volatile`:
```
int _currentFrame = 0; // missing, touched by next(), called by the callback
uint32_t _frameHolder[3];
uint32_t* _frames; // missing, touched by next()
uint32_t _framesCount; // missing, touched by next()
uint32_t _interval = 0; // missing, touched by the callback
uint32_t _lastInterval = 0; // missing, touched by the callback
bool _loop = false; // missing, touched by next()
FspTimer _ledTimer;
bool _sequenceDone = false; // missing, touched by next()
voidFuncPtr _callBack; // missing, touched by next()
```
Contributor guide
No contributing guide indexed for this repository
Research direction
Read Arduino_LED_Matrix.h around the timer setup at line 182 and callback at line 345, then inspect the listed animation fields shared with FspTimer. Done means animation changes no longer block or behave erratically when the timer callback is active.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- embedded-iot
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100