Fix/serial framing led and tud task - #11
Open
alsocat wants to merge 3 commits into
Open
Conversation
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
force-pushed
the
fix/serial-framing-led-and-tud-task
branch
from
August 15, 2026 05:15
74fd1df to
d851c98
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 whateverreadline()returned at its 0.2 s timeout as acomplete 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 macon 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:
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
EVmarkerso 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 notacknowledged 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-cycleflash 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()usesTINYUSB_DEFAULT_CONFIG(), which populates.taskinesp_tinyusb 2.2.1 — so
tinyusb_driver_install()starts its own task whoseentire job is calling
tud_task().app_main()called it again in a 1 msloop, leaving two contexts racing the same event queue.
app_main()now returns once the subsystems are started;config_consoleandtouch_hidalready 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 buildclean, no new warningspytest tests/passes: 23 tests, 3 newNot 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 appliedand the cause is not yet known. It reproduces quickly without sleeping by
running
idf.py flashagainst the running application. Worth a separateissue rather than a guess bundled in here.