From 9fdce6ae1936001a35ecba9df7a2f1383e01cee0 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sat, 5 Sep 2026 08:30:58 -0500 Subject: [PATCH 1/5] 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) --- components/usb_device/include/usb_device.hpp | 24 ++++++ components/usb_device/src/usb_device.cpp | 91 +++++++++++++++++--- 2 files changed, 102 insertions(+), 13 deletions(-) diff --git a/components/usb_device/include/usb_device.hpp b/components/usb_device/include/usb_device.hpp index 105035a5a..7b44971b4 100644 --- a/components/usb_device/include/usb_device.hpp +++ b/components/usb_device/include/usb_device.hpp @@ -64,6 +64,10 @@ class UsbDevice : public BaseComponent { */ using receive_callback_fn = std::function data)>; + /// @brief Callback for a device lifecycle event (mount / unmount). Invoked in + /// the TinyUSB device-task context. + using event_callback_fn = std::function; + /** * @brief CDC-ACM (virtual serial port) function. * @@ -287,6 +291,19 @@ class UsbDevice : public BaseComponent { /// @brief Set or replace the vendor receive callback (nullptr to detach). void set_vendor_receive_callback(const receive_callback_fn &cb); + /// @brief Register a callback invoked when the device is mounted (the host has + /// configured it). Runs in the TinyUSB device-task context; nullptr + /// detaches. esp_tinyusb owns the raw tud_mount_cb, so applications + /// should register here rather than defining that callback themselves. + void set_mount_callback(const event_callback_fn &cb); + + /// @brief Register a callback invoked when the device is unmounted (detached / + /// re-enumerated / suspended). The component clears the vendor + CDC TX + /// FIFOs before invoking it. Runs in the TinyUSB device-task context; + /// nullptr detaches. Register here instead of defining tud_umount_cb + /// (esp_tinyusb already defines it). + void set_unmount_callback(const event_callback_fn &cb); + /// @brief Whether initialize() has completed successfully. bool is_initialized() const; @@ -311,6 +328,11 @@ class UsbDevice : public BaseComponent { /// @param bufsize Number of bytes at @p buffer (0 when @p buffer is null). void handle_vendor_rx(const uint8_t *buffer = nullptr, size_t bufsize = 0); + /// @brief Internal: mount / unmount handling driven by esp_tinyusb's event_cb + /// (clears the TX FIFOs on unmount, then invokes the app callback). + void handle_usb_mount(); + void handle_usb_unmount(); + /// @brief Internal: pointer to the BOS descriptor bytes (nullptr if none). const uint8_t *bos_descriptor() const; @@ -341,6 +363,8 @@ class UsbDevice : public BaseComponent { std::mutex cb_mutex_; receive_callback_fn on_cdc_receive_; receive_callback_fn on_vendor_receive_; + event_callback_fn on_mount_; + event_callback_fn on_unmount_; // Preallocated RX scratch buffers (sized in initialize()) so the TinyUSB-task // RX handlers stay allocation-free (no heap churn on the hot path). diff --git a/components/usb_device/src/usb_device.cpp b/components/usb_device/src/usb_device.cpp index df0ee821a..77d59f662 100644 --- a/components/usb_device/src/usb_device.cpp +++ b/components/usb_device/src/usb_device.cpp @@ -209,19 +209,21 @@ bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, #endif // CFG_TUD_VENDOR > 0 -// Device unmount: drop any bytes still queued in the TX FIFOs. A host that goes -// away (cable pull / re-enumeration / suspend) leaves its unread backlog in the -// software FIFO; clearing it here means the next host to mount starts from an -// empty pipe and cannot mis-parse a stale frame as the reply to its first -// command. (An abrupt tab close does NOT unmount, so it does not reach here -- -// that path relies on the streaming producer's own backpressure handling.) -void tud_umount_cb(void) { -#if (CFG_TUD_VENDOR > 0) - tud_vendor_write_clear(); -#endif -#if (CFG_TUD_CDC > 0) - tud_cdc_n_write_clear(kCdcPort); -#endif +// NOTE: the TinyUSB device lifecycle callbacks (tud_mount_cb / tud_umount_cb / +// tud_suspend_cb / tud_resume_cb) are defined by esp_tinyusb itself, which +// forwards them to the tinyusb_config_t::event_cb we register in initialize(). +// Do NOT define tud_umount_cb here -- it would be a duplicate symbol. The +// unmount TX-FIFO clear + the app mount/unmount hooks live in the handlers +// below, driven by this event callback. +// cppcheck-suppress constParameterCallback // signature must match tinyusb_event_cb_t +extern "C" void espp_usb_device_event_cb(tinyusb_event_t *event, void *arg) { + auto *dev = static_cast(arg); + if (!dev || !event) + return; + if (event->id == TINYUSB_EVENT_ATTACHED) + dev->handle_usb_mount(); + else if (event->id == TINYUSB_EVENT_DETACHED) + dev->handle_usb_unmount(); } #if (CFG_TUD_HID > 0) @@ -289,6 +291,7 @@ const uint8_t *UsbDevice::hid_report_descriptor() const { // --------------------------------------------------------------------------- void UsbDevice::handle_cdc_rx() { +#if (CFG_TUD_CDC > 0) receive_callback_fn cb; { std::scoped_lock lk(cb_mutex_); @@ -310,6 +313,7 @@ void UsbDevice::handle_cdc_rx() { if (rx_size > 0 && cb) cb(std::span(buf.data(), rx_size)); } while (rx_size == buf.size()); +#endif } void UsbDevice::handle_vendor_rx(const uint8_t *buffer, size_t bufsize) { @@ -834,6 +838,11 @@ bool UsbDevice::initialize(std::error_code &ec) { tusb_cfg.descriptor.qualifier = &impl_->qualifier_desc; #endif + // Route esp_tinyusb's device lifecycle events (mount / unmount) to us so we + // can clear the TX FIFOs on unmount and invoke any app-registered callbacks. + tusb_cfg.event_cb = espp_usb_device_event_cb; + tusb_cfg.event_arg = this; + // Register before installing so the BOS / vendor callbacks can find us. // Claim the singleton slot ATOMICALLY: the null check at the top of // initialize() is only a fast-fail, so two threads (or two instances) that @@ -1143,18 +1152,68 @@ void UsbDevice::set_vendor_receive_callback(const receive_callback_fn &cb) { on_vendor_receive_ = cb; } +void UsbDevice::set_mount_callback(const event_callback_fn &cb) { + std::scoped_lock lk(cb_mutex_); + on_mount_ = cb; +} + +void UsbDevice::set_unmount_callback(const event_callback_fn &cb) { + std::scoped_lock lk(cb_mutex_); + on_unmount_ = cb; +} + +void UsbDevice::handle_usb_mount() { + event_callback_fn cb; + { + std::scoped_lock lk(cb_mutex_); + cb = on_mount_; + } + if (cb) + cb(); // runs in the TinyUSB task context +} + +void UsbDevice::handle_usb_unmount() { + // Drop any bytes still queued in the TX FIFOs so the next host to mount starts + // from an empty pipe (a departed host's unread backlog otherwise lingers in + // the software FIFO and can be mis-parsed as a reply to the next host's first + // command). +#if (CFG_TUD_VENDOR > 0) + if (config_.vendor) + tud_vendor_write_clear(); +#endif +#if (CFG_TUD_CDC > 0) + if (config_.cdc) + tud_cdc_n_write_clear(kCdcPort); +#endif + event_callback_fn cb; + { + std::scoped_lock lk(cb_mutex_); + cb = on_unmount_; + } + if (cb) + cb(); // runs in the TinyUSB task context +} + bool UsbDevice::is_initialized() const { return initialized_; } bool UsbDevice::is_cdc_connected() const { +#if (CFG_TUD_CDC > 0) if (!initialized_ || !config_.cdc) return false; return tud_cdc_n_connected(kCdcPort); +#else + return false; +#endif } bool UsbDevice::is_vendor_connected() const { +#if (CFG_TUD_VENDOR > 0) if (!initialized_ || !config_.vendor) return false; return tud_mounted(); +#else + return false; +#endif } size_t UsbDevice::vendor_write_available() const { @@ -1168,9 +1227,13 @@ size_t UsbDevice::vendor_write_available() const { } size_t UsbDevice::cdc_write_available() const { +#if (CFG_TUD_CDC > 0) if (!initialized_ || !config_.cdc || !tud_mounted()) return 0; return tud_cdc_n_write_available(kCdcPort); +#else + return 0; +#endif } void UsbDevice::vendor_write_clear() { @@ -1181,8 +1244,10 @@ void UsbDevice::vendor_write_clear() { } void UsbDevice::cdc_write_clear() { +#if (CFG_TUD_CDC > 0) if (initialized_ && config_.cdc) tud_cdc_n_write_clear(kCdcPort); +#endif } bool UsbDevice::is_hid_ready() const { From c5f8d0882c0504462371f8803f1c50e60236caa3 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sat, 5 Sep 2026 08:56:28 -0500 Subject: [PATCH 2/5] 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) --- components/usb_device/src/usb_device.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/components/usb_device/src/usb_device.cpp b/components/usb_device/src/usb_device.cpp index 77d59f662..cdaaab53e 100644 --- a/components/usb_device/src/usb_device.cpp +++ b/components/usb_device/src/usb_device.cpp @@ -217,6 +217,11 @@ bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, // below, driven by this event callback. // cppcheck-suppress constParameterCallback // signature must match tinyusb_event_cb_t extern "C" void espp_usb_device_event_cb(tinyusb_event_t *event, void *arg) { + // Runs in the TinyUSB device-task context: record it so a mount/unmount + // callback that calls write_cdc()/write_vendor() takes the non-blocking + // fail-fast TX path instead of vTaskDelay()-ing inside the TinyUSB task + // (which would deadlock USB servicing). + note_tinyusb_task(); auto *dev = static_cast(arg); if (!dev || !event) return; From c465e541e7b3c3e4dfd2202c788832d6f7647590 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sat, 5 Sep 2026 09:09:13 -0500 Subject: [PATCH 3/5] 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) --- components/usb_device/README.md | 15 +++++++++++++++ components/usb_device/include/usb_device.hpp | 6 +++--- components/usb_device/src/usb_device.cpp | 9 +++++++-- doc/en/buses/usb_cdc.rst | 7 +++++++ 4 files changed, 32 insertions(+), 5 deletions(-) diff --git a/components/usb_device/README.md b/components/usb_device/README.md index cc62be534..e50c57490 100644 --- a/components/usb_device/README.md +++ b/components/usb_device/README.md @@ -104,6 +104,16 @@ Key methods: - `bool write_hid_report(uint8_t report_id, std::span report, ...)` — send a HID input report on the HID interrupt IN endpoint. - `void set_cdc_receive_callback(...)` / `void set_vendor_receive_callback(...)`. +- `void set_mount_callback(...)` / `void set_unmount_callback(...)` — register + device mount / unmount handlers. `esp_tinyusb` owns the raw `tud_mount_cb` / + `tud_umount_cb`, so register here instead of defining those yourself (which + would be a duplicate symbol). On unmount the component first clears the vendor + + CDC TX FIFOs — so a departed host's queued backlog is not delivered to the + next host that mounts — then invokes your callback. +- `size_t vendor_write_available() const` / `size_t cdc_write_available() const` + and `void vendor_write_clear()` / `void cdc_write_clear()` — TX-FIFO free space + and flush helpers (skip/defer or drop a streaming frame when the host stops + draining). - `bool is_cdc_connected() const` / `bool is_vendor_connected() const` / `bool is_hid_ready() const`. @@ -192,3 +202,8 @@ the USB-Serial-JTAG peripheral. - Only one `espp::UsbDevice` / `espp::UsbCdc` instance may exist at a time. - The receive callbacks run in the TinyUSB device task; keep them short and non-blocking. +- The TinyUSB device lifecycle callbacks (`tud_mount_cb` / `tud_umount_cb` / + `tud_suspend_cb` / `tud_resume_cb`) are owned by `esp_tinyusb`. Register mount + / unmount handlers via `set_mount_callback()` / `set_unmount_callback()` + rather than defining those callbacks yourself. The mount / unmount handlers + also run in the TinyUSB device task. diff --git a/components/usb_device/include/usb_device.hpp b/components/usb_device/include/usb_device.hpp index 7b44971b4..1277a073c 100644 --- a/components/usb_device/include/usb_device.hpp +++ b/components/usb_device/include/usb_device.hpp @@ -298,9 +298,9 @@ class UsbDevice : public BaseComponent { void set_mount_callback(const event_callback_fn &cb); /// @brief Register a callback invoked when the device is unmounted (detached / - /// re-enumerated / suspended). The component clears the vendor + CDC TX - /// FIFOs before invoking it. Runs in the TinyUSB device-task context; - /// nullptr detaches. Register here instead of defining tud_umount_cb + /// re-enumerated). The component clears the vendor + CDC TX FIFOs before + /// invoking it. Runs in the TinyUSB device-task context; nullptr + /// detaches. Register here instead of defining tud_umount_cb /// (esp_tinyusb already defines it). void set_unmount_callback(const event_callback_fn &cb); diff --git a/components/usb_device/src/usb_device.cpp b/components/usb_device/src/usb_device.cpp index cdaaab53e..aa9e82ea7 100644 --- a/components/usb_device/src/usb_device.cpp +++ b/components/usb_device/src/usb_device.cpp @@ -222,7 +222,11 @@ extern "C" void espp_usb_device_event_cb(tinyusb_event_t *event, void *arg) { // fail-fast TX path instead of vTaskDelay()-ing inside the TinyUSB task // (which would deadlock USB servicing). note_tinyusb_task(); - auto *dev = static_cast(arg); + // Load the teardown-guarded singleton (not event_arg): a destructor that has + // atomically detached the instance during teardown then yields nullptr here, + // matching the other tud_*_cb trampolines. + (void)arg; + auto *dev = s_device.load(); if (!dev || !event) return; if (event->id == TINYUSB_EVENT_ATTACHED) @@ -845,8 +849,9 @@ bool UsbDevice::initialize(std::error_code &ec) { // Route esp_tinyusb's device lifecycle events (mount / unmount) to us so we // can clear the TX FIFOs on unmount and invoke any app-registered callbacks. + // The callback loads the teardown-guarded s_device singleton itself, so no + // event_arg is needed. tusb_cfg.event_cb = espp_usb_device_event_cb; - tusb_cfg.event_arg = this; // Register before installing so the BOS / vendor callbacks can find us. // Claim the singleton slot ATOMICALLY: the null check at the top of diff --git a/doc/en/buses/usb_cdc.rst b/doc/en/buses/usb_cdc.rst index dfbfe55cd..5d4ff3295 100644 --- a/doc/en/buses/usb_cdc.rst +++ b/doc/en/buses/usb_cdc.rst @@ -189,6 +189,13 @@ Notes (the TinyUSB stack and the BOS / vendor control callbacks are global). - The receive callbacks run in the TinyUSB device task; keep them short and non-blocking. It is safe to call the matching ``write_*()`` from within them. +- The TinyUSB device lifecycle callbacks (``tud_mount_cb`` / ``tud_umount_cb`` / + ``tud_suspend_cb`` / ``tud_resume_cb``) are owned by ``esp_tinyusb``. Register + mount / unmount handlers via ``set_mount_callback()`` / ``set_unmount_callback()`` + rather than defining those callbacks yourself (which would be a duplicate + symbol). On unmount the component clears the vendor + CDC TX FIFOs — so a + departed host's queued backlog is not delivered to the next host that mounts — + before invoking your callback; both handlers run in the TinyUSB device task. - The WebUSB landing-page URL is configured *without* a scheme; the scheme is encoded separately via ``VendorFunction::url_scheme`` (0 = http, 1 = https). From 1d4a60f24f0cda52783c7db480b68f66a0a78636 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Sep 2026 15:15:33 +0000 Subject: [PATCH 4/5] fix(usb_device): make handle_usb_mount/unmount private with friend trampoline Co-authored-by: finger563 <213467+finger563@users.noreply.github.com> --- components/usb_device/include/usb_device.hpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/components/usb_device/include/usb_device.hpp b/components/usb_device/include/usb_device.hpp index 1277a073c..fec45741b 100644 --- a/components/usb_device/include/usb_device.hpp +++ b/components/usb_device/include/usb_device.hpp @@ -12,6 +12,7 @@ #include #include "base_component.hpp" +#include "tinyusb.h" // for tinyusb_event_t (esp_tinyusb is already a REQUIRES dependency) namespace espp { @@ -328,11 +329,6 @@ class UsbDevice : public BaseComponent { /// @param bufsize Number of bytes at @p buffer (0 when @p buffer is null). void handle_vendor_rx(const uint8_t *buffer = nullptr, size_t bufsize = 0); - /// @brief Internal: mount / unmount handling driven by esp_tinyusb's event_cb - /// (clears the TX FIFOs on unmount, then invokes the app callback). - void handle_usb_mount(); - void handle_usb_unmount(); - /// @brief Internal: pointer to the BOS descriptor bytes (nullptr if none). const uint8_t *bos_descriptor() const; @@ -354,6 +350,15 @@ class UsbDevice : public BaseComponent { static UsbDevice *instance(); private: + // Trampoline registered as tinyusb_config_t::event_cb; routes + // TINYUSB_EVENT_ATTACHED/DETACHED to the private handlers below. + friend void espp_usb_device_event_cb(tinyusb_event_t *event, void *arg); + + /// @brief Internal: mount / unmount handling driven by esp_tinyusb's event_cb + /// (clears the TX FIFOs on unmount, then invokes the app callback). + void handle_usb_mount(); + void handle_usb_unmount(); + struct Impl; // holds TinyUSB descriptors, kept alive for driver lifetime std::unique_ptr impl_; From 00e5d35613b8fa6d14e27f07eba712687ffde6c5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Sep 2026 17:43:57 +0000 Subject: [PATCH 5/5] fix(usb_device): forward-declare espp_usb_device_event_cb before friending it Co-authored-by: finger563 <213467+finger563@users.noreply.github.com> --- components/usb_device/include/usb_device.hpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/components/usb_device/include/usb_device.hpp b/components/usb_device/include/usb_device.hpp index fec45741b..7ee5483c2 100644 --- a/components/usb_device/include/usb_device.hpp +++ b/components/usb_device/include/usb_device.hpp @@ -57,6 +57,13 @@ namespace espp { * \section usb_device_ex1 UsbDevice (composite CDC + Vendor/WebUSB) Example * \snippet usb_cdc_example.cpp usb_cdc_example */ + +// Forward-declare the extern "C" trampoline (defined in usb_device.cpp, inside +// `namespace espp`) so the in-class friend declaration below refers to this +// existing C-linkage declaration instead of introducing a conflicting +// C++-linkage espp::espp_usb_device_event_cb. +extern "C" void espp_usb_device_event_cb(tinyusb_event_t *event, void *arg); + class UsbDevice : public BaseComponent { public: /**