Bug: Event race condition

[hchargois]

I've noticed that button presses are sometimes "skipped", i.e. I press a button and no BUTTON_UP event arrives to the face's loop function. I've also noticed that the higher the requested tick_frequency, the more often buttons presses are skipped.

I've created this PR to demonstrate the issue, with a simple face that runs at a 64 Hz tick frequency, and counts up on each press of the alarm button, or at least it should. When running in the simulator, for 10 presses of the button, the counter only goes up by around 3 to 5, so more than half of the button presses are skipped.

What I think happens, is that:

• the button press makes the button interrupt handler and callback run
• the global event.event_type is set to the correct BUTTON event
• but, the face's loop function is not called immediately, and before it is called...
• a tick happens, the tick handler sets the event.event_type to TICK, "overwriting" the BUTTON event into oblivion
• finally the face's loop is called with the TICK event

I've added a naive fix in which the tick handler avoids overwriting the event if it's not empty. This seems to fix the counter face: 10 button presses now reliably increment the counter 10 times.

I don't expect this fix to be the best way to fix the issue, so I don't expect this PR to be merged, I just wanted to demonstrate the bug and give weight to the race condition theory above with a simple fix.

I imagine a proper fix may be to add a queue of pending events and call the face's loop multiple times in a row if there are multiple pending events. Maybe the tick events also need to be handled differently than the other ones: if we are falling behind tick events, we don't want to replay them all, we usually want only the "latest" tick event.

I should also say that I don't have the actual board yet, so these observations are made with the simulator. But since I think this bug is in movement.c and not in the watch library, I expect the same bug to be exhibited by the hardware board. Unless there's something different with how interrupts are handled.

[matheusmoreira]

I have also noticed this problem on hardware but it's especially noticeable in the emulator.

I imagine a proper fix may be to add a queue of pending events and call the face's loop multiple times in a row if there are multiple pending events. Maybe the tick events also need to be handled differently than the other ones: if we are falling behind tick events, we don't want to replay them all, we usually want only the "latest" tick event.

Completely agree. A circular buffer seems like a good way to implement this queue.

Buffering events will add latency which could impact user experience. The target is a microcontroller running on bare metal: can we make any guarantees regarding event handling times?