feat(usb_device): TX FIFO space queries + clear; clear stale TX on unmount - #774
Merged
Conversation
…mount Adds primitives for backpressure-aware streaming and clean reconnects: - vendor_write_available() / cdc_write_available(): free space (bytes) in the TX FIFO, so a streaming producer can skip/defer a frame instead of building it and having write_vendor()/write_cdc() drop it when the host stops draining. - vendor_write_clear() / cdc_write_clear(): drop any queued-but-unsent TX bytes, for when the host goes away so a stale backlog is not delivered to the next host. - tud_umount_cb(): clears both TX FIFOs on unmount (cable pull / re-enumeration / suspend), so the next host to mount starts from an empty pipe and cannot mis-parse a stale frame as the reply to its first command. All thin wrappers over TinyUSB's tud_*_write_available / tud_*_write_clear (already used internally by write_vendor/write_cdc), guarded like the existing getters. Note: an abrupt browser-tab close does NOT unmount, so that path still relies on the streaming producer's own backpressure (e.g. the bldc_haptics telemetry auto-pause) rather than tud_umount_cb. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
finger563
added a commit
that referenced
this pull request
Sep 5, 2026
…on auto-pause (#775) * fix(bldc_haptics web): drain stale RX before the connect handshake Re-applies the RX-drain that missed the #773 squash-merge. On an abrupt tab close the device's vendor TX FIFO is not cleared, so a reconnecting page could read leftover telemetry / reply frames and mis-handle them ("first Connect returns immediately, second works"). initializeDevice() now flushes the pipe (set a `draining` flag, reset the parser, let the RX pump discard for ~150 ms) before the GET_INFO handshake; dispatchFrame drops frames while draining. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(bldc_haptics): clear the vendor TX backlog when telemetry auto-pauses When the telemetry stall guard fires (host stopped draining), also drop the queued-but-unsent telemetry via usb.vendor_write_clear() (added in #774). An abrupt tab close does not unmount the device, so the FIFO is not cleared for us; clearing it here means a reconnecting host reads a clean stream instead of a stale backlog it might mis-parse as the reply to its first command. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Adds TX FIFO “available” and “clear” helpers to espp::UsbDevice and ensures TX FIFOs are cleared on USB unmount to avoid delivering stale queued data after reconnects.
Changes:
- Add
vendor_write_available()/cdc_write_available()to query free TX FIFO space. - Add
vendor_write_clear()/cdc_write_clear()to discard queued TX bytes. - Clear vendor/CDC TX FIFOs from
tud_umount_cb()on device unmount.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| components/usb_device/src/usb_device.cpp | Implements FIFO-availability/clear APIs and clears TX FIFOs on unmount. |
| components/usb_device/include/usb_device.hpp | Exposes the new FIFO-availability/clear APIs with usage docs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1170
to
+1174
| size_t UsbDevice::cdc_write_available() const { | ||
| if (!initialized_ || !config_.cdc || !tud_mounted()) | ||
| return 0; | ||
| return tud_cdc_n_write_available(kCdcPort); | ||
| } |
Comment on lines
+1183
to
+1186
| void UsbDevice::cdc_write_clear() { | ||
| if (initialized_ && config_.cdc) | ||
| tud_cdc_n_write_clear(kCdcPort); | ||
| } |
finger563
added a commit
that referenced
this pull request
Sep 5, 2026
…add mount/unmount hooks (#779) * fix(usb_device): don't define tud_umount_cb (esp_tinyusb owns it); add mount/unmount hooks #774 added a tud_umount_cb definition to usb_device.cpp, but esp_tinyusb already defines it (tinyusb.c) -> "multiple definition of tud_umount_cb", breaking every manager-off USB example link (bldc_haptics, can_bridge, coredump, mcp266, ota, usb_device). esp_tinyusb owns the TinyUSB device lifecycle callbacks and forwards them to a tinyusb_config_t::event_cb, so: - remove usb_device's tud_umount_cb definition; - register an event_cb (event_arg = this) and route ATTACHED/DETACHED to handle_usb_mount()/handle_usb_unmount(); - handle_usb_unmount() clears the vendor + CDC TX FIFOs (the behavior #774 intended) then invokes an app callback; - expose set_mount_callback()/set_unmount_callback() so applications register their mount/unmount handlers via UsbDevice instead of defining tud_*_cb (which would also collide with esp_tinyusb). Also (requested): add CFG_TUD_CDC / CFG_TUD_VENDOR guards around the remaining CDC/vendor method bodies (is_cdc_connected, is_vendor_connected, cdc_write_available, cdc_write_clear, handle_cdc_rx), returning a safe default when the interface is disabled -- matching write_vendor / vendor_write_* which were already guarded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(usb_device): note TinyUSB-task context in the lifecycle event callback espp_usb_device_event_cb runs in the TinyUSB device task; call note_tinyusb_task() first (like the other tud_*_cb callbacks) so a mount/unmount callback that writes via write_cdc()/write_vendor() takes the non-blocking fail-fast TX path rather than vTaskDelay()-ing inside the TinyUSB task and deadlocking USB servicing. Addresses the review note on #779. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(usb_device): load s_device in the event cb (teardown-safe); doc the lifecycle API Follow-ups on #779: - espp_usb_device_event_cb now loads the teardown-guarded s_device singleton instead of using event_arg, so a destructor that has atomically detached the instance yields nullptr here (matching the other tud_*_cb trampolines); dropped the now-unused tusb_cfg.event_arg. - set_unmount_callback docs no longer claim "suspended" (only ATTACHED/DETACHED are routed to mount/unmount). - documented the new set_mount_callback/set_unmount_callback + vendor/cdc write_available/write_clear helpers and the automatic unmount TX-FIFO clear in the component README (Key methods, Notes) and the usb_cdc RST (Notes). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(usb_device): make handle_usb_mount/unmount private with friend trampoline Co-authored-by: finger563 <213467+finger563@users.noreply.github.com> * fix(usb_device): forward-declare espp_usb_device_event_cb before friending it Co-authored-by: finger563 <213467+finger563@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: finger563 <213467+finger563@users.noreply.github.com>
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.
Adds TX-FIFO primitives to
espp::UsbDevicefor backpressure-aware streaming and clean reconnects — motivated by the bldc_haptics telemetry issue (#773) where the device kept streaming into a full FIFO after the host stopped draining.New API
vendor_write_available()/cdc_write_available()— bytes of free space currently in the TX FIFO (0 if not mounted / no interface). Lets a streaming producer check space before building a frame and skip/defer it, instead of building it and havingwrite_vendor()/write_cdc()drop it. Point-in-time hint; stable with a single serialized writer.vendor_write_clear()/cdc_write_clear()— discard queued-but-unsent TX bytes, so a stale backlog left by a departed host isn't delivered to the next one.Behavior change
tud_umount_cb()now clears both TX FIFOs on unmount (cable pull / re-enumeration / suspend), so the next host to mount starts from an empty pipe and can't mis-parse a stale frame as the reply to its first command.All are thin wrappers over TinyUSB functions the component already uses internally (
tud_vendor_write_available/tud_cdc_n_write_availablein the write paths,tud_vendor_write_clearin the vendor control handler), guarded the same way as the existingis_*_connected()getters. cppcheck-clean.🤖 Generated with Claude Code