Skip to content

Fix/serial framing led and tud task - #11

Open
alsocat wants to merge 3 commits into
ZimengXiong:mainfrom
alsocat:fix/serial-framing-led-and-tud-task
Open

Fix/serial framing led and tud task#11
alsocat wants to merge 3 commits into
ZimengXiong:mainfrom
alsocat:fix/serial-framing-led-and-tud-task

Conversation

@alsocat

@alsocat alsocat commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Three bugs found while debugging a device on macOS, plus a docs correction.
Each was reproduced on hardware. The host-side fixes have regression tests
that fail without them.

Helper silently dropped fingerprint touches

serve_port() treated whatever readline() returned at its 0.2 s timeout as a
complete line, so an event split across two reads was HMAC-verified as a
fragment and rejected. In a captured log this lost 2 of 20 touches, visible
only as bad event mac on stderr.

A USB suspend can also cut a write off mid-event. Those bytes are lost rather
than delayed, so the remains arrive concatenated with the next event:

EV cad95bbe78f4e8deEV 80c32bde151003d04c191db753f590ed 15 1 1 86fb5bf8…

That parses as too many fields, so a second valid touch is dropped too — and
with no log line at all, because the field count is checked before the MAC.

Fix: buffer until a newline arrives, and resynchronise on the last EV marker
so a truncated leading fragment cannot take the intact event behind it down.
The discarded fragment is now reported instead of failing silently.

Aura LED could stick on after a result

set_aura() cached the requested colour even when the sensor had not
acknowledged the command, and skipped sending anything when the cache already
matched. One dropped command therefore desynchronised cache from hardware
permanently: every later request for that colour became a no-op, so the aura
stayed on whatever it had been doing.

show_result() made that reachable in ordinary use — it started a two-cycle
flash program, then sent a steady colour 350 ms later while that program was
still running, so the sensor discarded it.

Fix: cache the colour only when the command is acknowledged, and build the
result blink from steady commands so no program is left running.

tud_task() was serviced from two contexts

usb_ccid_start() uses TINYUSB_DEFAULT_CONFIG(), which populates .task in
esp_tinyusb 2.2.1 — so tinyusb_driver_install() starts its own task whose
entire job is calling tud_task(). app_main() called it again in a 1 ms
loop, leaving two contexts racing the same event queue.

app_main() now returns once the subsystems are started; config_console and
touch_hid already run as their own tasks.

README wiring

The wiring section named pins 6/7 for TX/RX and pin 1 for the interrupt. Both
firmwares use GPIO43 (TX), GPIO44 (RX), GPIO2 (INT) — and the CLI's own sensor
error message already tells users to check GPIO44/43, so the README
contradicted the tool.

Verification

  • idf.py build clean, no new warnings
  • pytest tests/ passes: 23 tests, 3 new

Not addressed

The device still becomes unreachable until physically replugged after the host
suspends — enumerated, host at full power, but CDC/HID/CCID all silent. The
tud_task() change does not fix this; the fault persists with it applied
and the cause is not yet known. It reproduces quickly without sleeping by
running idf.py flash against the running application. Worth a separate
issue rather than a guess bundled in here.

alsocat and others added 3 commits August 12, 2026 17:16
serve_port() treated whatever readline() returned at its 0.2 s timeout as a
complete line, so an event split across two reads was HMAC-verified as a
fragment and rejected. In a captured log this silently discarded 2 of 20
touches, visible only as "bad event mac" on stderr.

A USB suspend can also cut a write off mid-event. Those bytes are lost rather
than delayed, so the remains arrive concatenated with the next event and parse
as too many fields -- dropping a second, valid touch as well, and this time
with no log line at all because the field count is checked before the MAC.

Buffer until a newline arrives, and resynchronise on the last "EV " marker so a
truncated leading fragment cannot take the intact event behind it down with it.
The discarded fragment is now reported instead of failing silently.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
usb_ccid_start() configures TinyUSB with TINYUSB_DEFAULT_CONFIG(), which in
esp_tinyusb 2.2.1 populates .task -- so tinyusb_driver_install() starts its own
task whose entire job is calling tud_task(). app_main() then called it again in
a 1 ms loop, leaving two contexts racing the same event queue.

app_main() now returns once the subsystems are started. config_console and
touch_hid already run as their own tasks, so nothing depended on the main task
staying alive, and usb_ccid_task() had no other caller.

This is not a fix for the device becoming unreachable after the host suspends.
That fault persists with this change applied and its cause is not yet known.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The wiring section named pins 6 and 7 for TX/RX and pin 1 for the interrupt.
Both firmwares use GPIO43 (TX), GPIO44 (RX) and GPIO2 (INT) -- and the CLI's
own sensor error message already directs users to check GPIO44/43, so the
README contradicted the tool.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@alsocat
alsocat force-pushed the fix/serial-framing-led-and-tud-task branch from 74fd1df to d851c98 Compare August 15, 2026 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant