Martin Bauer
4cb2d085ca
Authentication (01), one input policy for skip/mute/volume steps (04), the firmware's interleaved serial frames (05), podcast as an album kind (06), and eight smaller cleanups (07). Each says why, what was decided, the design, the tests and when it is done. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
76 lines
3.8 KiB
Markdown
76 lines
3.8 KiB
Markdown
# Task: stop the firmware garbling its own messages to the host
|
|
|
|
## Why
|
|
|
|
`sendMessageToHost` (`esp-firmware/src/Messages.h`) writes one frame as four separate
|
|
`Serial.write` calls: magic, type, size, payload. It runs on two FreeRTOS tasks at once:
|
|
|
|
- the Arduino `loop()` task, for touch, rotary encoder and button events
|
|
(`esp-firmware/src/main.cpp`);
|
|
- the RC522 reader's own task (`rc522_task` in `esp-firmware/lib/nfc/rc522.c`, priority
|
|
4), which calls `tagHandler` → `sendMessageToHost(MsgRfidTokenRead{...})` directly.
|
|
|
|
When a tag read coincides with a button or touch event, the two frames interleave on
|
|
the wire. The host's decoder (`python-backend/musicmouse/devices/wire.py`,
|
|
`FrameDecoder`) resynchronises on the magic token and drops what it cannot parse. So
|
|
the symptom is an occasionally **lost figure placement/removal or button press**, with a
|
|
"Discarding bad frame from firmware" warning in the backend log.
|
|
|
|
## What the owner decided
|
|
|
|
Fix it in whatever way keeps the firmware simplest. A mutex was the first idea. **Use
|
|
the queue below instead, unless it turns out not to work**, because it is simpler:
|
|
|
|
- With a queue, every frame and every log line is written from `loop()` alone, so no
|
|
lock is needed anywhere, including around the `Serial.println` log lines that could
|
|
otherwise land in the middle of a frame.
|
|
- It is exactly the pattern `main.cpp` already uses for the rotary encoder
|
|
(`eventQueueRotaryEncoder`, drained in `handleRotaryEncoder()` with
|
|
`xQueueReceive(..., 0)`).
|
|
|
|
A mutex would need wrapping every `Serial.write` / `Serial.print` in the firmware to be
|
|
complete. That is more code and easier to get wrong later.
|
|
|
|
## Design
|
|
|
|
In `esp-firmware/src/main.cpp`:
|
|
|
|
1. Create a FreeRTOS queue for tag reads in `setupRfidReader()`, **before**
|
|
`rc522_start`, so the callback can never run without it. Item type: the 5-byte tag id
|
|
(`MsgRfidTokenRead` itself is fine, it is a packed 5-byte struct). Depth: a handful
|
|
(4) is plenty; the reader polls every 125 ms.
|
|
2. `tagHandler` (runs on the RC522 task) only fills a `MsgRfidTokenRead`, all zeros
|
|
for "removed" exactly as now, and `xQueueSend(queue, &msg, 0)`. If the queue is full,
|
|
drop the read; the reader reports the state again on the next change. Nothing in
|
|
`tagHandler` touches `Serial` any more.
|
|
3. Add `handleRfid()`, called from `loop()` next to `handleRotaryEncoder()`. It drains
|
|
the queue with `xQueueReceive(..., 0)` and calls `sendMessageToHost` for each item.
|
|
4. Leave `Messages.h` alone. With a single writer, the four `Serial.write` calls are
|
|
fine as they are.
|
|
|
|
Do not change the wire format, message ids or the host. `python-backend/tests/test_wire.py`
|
|
parses `Messages.h` and pins the ids, and it must keep passing.
|
|
|
|
Out of scope: the touch thresholds, debouncing, anything in `lib/`.
|
|
|
|
## Checking it
|
|
|
|
There are no firmware unit tests that reach `main.cpp` (the `native` env only builds the
|
|
LED effects), and this machine may not have PlatformIO. So:
|
|
|
|
- It must build: `pio run -e esp32` in `esp-firmware/`. If PlatformIO is not
|
|
available, say so in the hand-off instead of claiming it builds.
|
|
- `pio run -e native` still builds `src/host_test.cpp`. That env excludes `main.cpp`, so
|
|
this only guards against accidental changes to shared headers.
|
|
- `scripts/check.sh backend` still passes (`test_wire.py`).
|
|
- Manual test on the device, with the backend running at `--log-level DEBUG`. Hold a
|
|
hand on an ear touch area while repeatedly placing and removing a figure for a
|
|
minute, and turn the knob while doing it. Expect every placement and removal to show
|
|
up as `RfidTokenRead`, and **no** "Discarding bad frame from firmware" warnings.
|
|
Before the fix, the same exercise should produce some.
|
|
|
|
## Done when
|
|
|
|
The firmware builds, the host tests pass, `tagHandler` contains no `Serial` call, and the
|
|
manual test shows no discarded frames.
|