feat(usb_device): composable native-USB device (CDC + vendor/WebUSB, esp_tinyusb)

[finger563]

Summary

New usb_device component: espp::UsbDevice, a composable native-USB device on the ESP32-S3/-S2/-P4 USB-OTG peripheral, wrapping the managed espressif/esp_tinyusb.

• Assembles a device from selectable functions (Config has optional cdc / vendor slots; hid/msc reserved) rather than hard-coding one class. Interface numbers, endpoint addresses and string indices are allocated sequentially, with an endpoint-budget check (errors via std::error_code if exceeded — ~5 IN / 5 OUT on the S3).
• CDC-ACM function with a configurable VID/PID (advertise your own identifiers on a link separate from the log console).
• Vendor function (class 0xFF) with WebUSB: BOS + MS-OS-2.0/WinUSB (driverless on Windows) + a configurable landing-page URL; bulk IN/OUT carrying a raw byte stream.
• espp::UsbCdc kept as a thin CDC-only preset. CFG_TUD_VENDOR enabled via CONFIG_TINYUSB_VENDOR_COUNT; all tud_vendor_* gated so CDC-only builds still link.
• HID/MSC are documented extension points (endpoint-budget table incl. which class combos fit).

Verification

esp32s3 example (composite CDC + vendor) builds and links (nm-verified vendor/WebUSB symbols). README + docs + CI wiring included.

🤖 Generated with Claude Code

[github-actions]

✅Static analysis result - no issues found! ✅

[Copilot]

Pull request overview

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.

Adds a new usb_device component providing a composable native USB device for ESP32-S3/-S2/-P4 (CDC-ACM + optional vendor/WebUSB), plus documentation, an example project, and CI wiring.

Changes:

• Introduces espp::UsbDevice with descriptor building, endpoint budgeting, CDC + vendor/WebUSB support, and a CDC-only espp::UsbCdc preset.
• Adds an ESP-IDF example showcasing composite CDC + vendor/WebUSB wired to OdriveAscii.
• Integrates component docs/Doxygen inputs and updates CI to build and publish the new component.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
doc/en/buses/usb_cdc_example.md	Includes the usb_device example README into the docs site.
doc/en/buses/usb_cdc.rst	Adds user-facing documentation for UsbDevice/UsbCdc usage and constraints.
doc/en/buses/index.rst	Adds the new USB CDC/Device page to the buses index.
doc/Doxyfile	Adds new headers and example source to Doxygen input paths.
components/usb_device/src/usb_device.cpp	Implements UsbDevice, descriptor construction, callbacks, and IO paths.
components/usb_device/src/usb_cdc.cpp	Implements UsbCdc as a CDC-only thin wrapper over UsbDevice.
components/usb_device/include/usb_device.hpp	Public API for composable USB device + function configs.
components/usb_device/include/usb_cdc.hpp	Public API for the CDC-only preset transport.
components/usb_device/idf_component.yml	Registers the new component for the IDF component manager.
components/usb_device/example/sdkconfig.defaults.esp32s3	Example target-specific console configuration.
components/usb_device/example/sdkconfig.defaults	Enables TinyUSB CDC and vendor class in the example.
components/usb_device/example/main/usb_cdc_example.cpp	Example app demonstrating composite CDC + vendor/WebUSB.
components/usb_device/example/main/idf_component.yml	Example dependency overrides to use in-repo espp components.
components/usb_device/example/main/CMakeLists.txt	Registers the example “main” component and dependencies.
components/usb_device/example/README.md	Example documentation and usage instructions.
components/usb_device/example/CMakeLists.txt	Example project configuration (component selection + dirs).
components/usb_device/README.md	Component README (features, API, endpoint budgeting, notes).
components/usb_device/CMakeLists.txt	Component build registration (include/src/requirements).
.github/workflows/upload_components.yml	Adds usb_device to the upload/publish workflow.
.github/workflows/build.yml	Adds usb_device example to the CI build matrix.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

Suppressed comments (5)

components/usb_device/src/usb_device.cpp:69

• There is a race window where TinyUSB task callbacks can still observe s_device == this while teardown is in progress (until after tinyusb_driver_uninstall() returns). Clearing s_device before beginning deinit/uninstall (and ideally preventing further callback dispatch) reduces the chance of a callback calling into an object that is being destructed.

UsbDevice::~UsbDevice() {
  if (initialized_) {
    if (config_.cdc)
      tinyusb_cdcacm_deinit(kCdcPort);
    tinyusb_driver_uninstall();
    if (s_device == this)
      s_device = nullptr;
    initialized_ = false;
  }
}

components/usb_device/src/usb_device.cpp:138

• The vendor control handler responds solely based on bRequest (and for MS OS 2.0 only wIndex == 7). For WebUSB and MS OS 2.0, the spec-defined fields (notably wIndex/wValue) should be validated so you only respond to the exact expected requests (e.g., WebUSB GET_URL and a specific URL index). Without this, unrelated vendor requests using the same vendor code can be incorrectly answered, and hosts may observe non-compliant behavior.

  switch (request->bmRequestType_bit.type) {
  case TUSB_REQ_TYPE_VENDOR:
    if (request->bRequest == vendor.webusb_vendor_code) {
      // Return the WebUSB landing-page URL descriptor.
      uint8_t len = 0;
      const uint8_t *url = s_device->webusb_url_descriptor(len);
      if (!url)
        return false;
      return tud_control_xfer(rhport, request, (void *)(uintptr_t)url, len);
    }
    if (request->bRequest == vendor.ms_os_vendor_code && request->wIndex == 7) {
      // Return the MS OS 2.0 descriptor set.
      uint16_t total_len = 0;
      const uint8_t *ms = s_device->ms_os_20_descriptor(total_len);
      if (!ms)
        return false;
      return tud_control_xfer(rhport, request, (void *)(uintptr_t)ms, total_len);
    }
    return false;

components/usb_device/src/usb_device.cpp:377

• The WebUSB URL descriptor length is stored into a uint8_t and can overflow if landing_page_url.size() is large (producing an invalid descriptor). Consider validating landing_page_url length during initialize() and failing with a clear std::error_code (e.g., invalid_argument) when it can’t fit in a single descriptor.

    // WebUSB URL descriptor: bLength, bDescriptorType(3), bScheme, url...
    impl_->webusb_url_desc.clear();
    impl_->webusb_url_desc.push_back(static_cast<uint8_t>(3 + v.landing_page_url.size()));
    impl_->webusb_url_desc.push_back(3); // WEBUSB URL descriptor type
    impl_->webusb_url_desc.push_back(v.url_scheme);
    impl_->webusb_url_desc.insert(impl_->webusb_url_desc.end(), v.landing_page_url.begin(),
                                  v.landing_page_url.end());

components/usb_device/src/usb_device.cpp:204

• Both RX handlers allocate a new std::vector buffer on every callback invocation. Since these callbacks run in the TinyUSB device task and can be high-frequency, repeated heap allocation can add latency and fragmentation. Consider keeping a reusable buffer as a member (or per-function buffer in Impl) sized to rx_chunk_size, and resizing only when the configured chunk size changes.

void UsbDevice::handle_cdc_rx() {
  receive_callback_fn cb;
  {
    std::scoped_lock lk(cb_mutex_);
    cb = on_cdc_receive_;
  }
  if (!cb || !config_.cdc)
    return;
  std::vector<uint8_t> buf(config_.cdc->rx_chunk_size);
  size_t rx_size = 0;
  do {
    rx_size = 0;
    esp_err_t err = tinyusb_cdcacm_read(kCdcPort, buf.data(), buf.size(), &rx_size);
    if (err != ESP_OK) {
      logger_.error("CDC read error: {}", esp_err_to_name(err));
      break;
    }
    if (rx_size > 0)
      cb(std::span<const uint8_t>(buf.data(), rx_size));
  } while (rx_size == buf.size());
}

components/usb_device/include/usb_device.hpp:97

• The url_scheme doc comment claims 255 = URL includes its own scheme, but the WebUSB URL descriptor scheme field is typically restricted to the defined scheme codes (commonly 0=http, 1=https) and does not embed an arbitrary scheme string. If you intend to support nonstandard values, it would be good to document the expected host behavior; otherwise, constrain/validate url_scheme and update the comment to match the spec.

    uint8_t url_scheme{1};         /**< 0 = http, 1 = https, 255 = URL includes its own scheme. */
    uint8_t webusb_vendor_code{1}; /**< bRequest used for the WebUSB URL control request. */
    uint8_t ms_os_vendor_code{
        2}; /**< bRequest used for the MS OS 2.0 descriptor control request. */

[Copilot]

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.

Suppressed comments (6)

components/usb_device/src/usb_device.cpp:458

• When HID is enabled, report_len can be 0 (empty report descriptor) and poll_interval_ms can be 0, both of which can produce invalid/undesirable HID descriptors at enumeration time. Recommended: during initialize(), reject HID configs with an empty report_descriptor and enforce poll_interval_ms in the valid USB range (1..255), returning std::errc::invalid_argument on failure.

  if (config_.hid) {
    const uint16_t report_len = static_cast<uint16_t>(impl_->hid_report_desc.size());
    const uint8_t poll = config_.hid->poll_interval_ms;
    // Interrupt endpoints are full-speed (<=64 byte packets) even on high-speed
    // parts; a 64-byte endpoint buffer comfortably fits the gamepad report.
    constexpr uint8_t kHidEpSize = 64;

components/usb_device/src/usb_device.cpp:16

• s_device is accessed from TinyUSB callbacks (TinyUSB task context) and is also written from initialize()/destructor, but it is a plain pointer with no synchronization. On dual-core targets this is a data race (undefined behavior) even if the intent is just to avoid UAF. Recommended: make s_device a std::atomic<espp::UsbDevice*> (or an IDF/FreeRTOS-safe atomic), use atomic loads in callbacks, and atomic stores during init/deinit.

espp::UsbDevice *s_device = nullptr;

components/usb_device/src/usb_device.cpp:71

• s_device is accessed from TinyUSB callbacks (TinyUSB task context) and is also written from initialize()/destructor, but it is a plain pointer with no synchronization. On dual-core targets this is a data race (undefined behavior) even if the intent is just to avoid UAF. Recommended: make s_device a std::atomic<espp::UsbDevice*> (or an IDF/FreeRTOS-safe atomic), use atomic loads in callbacks, and atomic stores during init/deinit.

    if (s_device == this)
      s_device = nullptr;

components/usb_device/src/usb_device.cpp:691

• s_device is accessed from TinyUSB callbacks (TinyUSB task context) and is also written from initialize()/destructor, but it is a plain pointer with no synchronization. On dual-core targets this is a data race (undefined behavior) even if the intent is just to avoid UAF. Recommended: make s_device a std::atomic<espp::UsbDevice*> (or an IDF/FreeRTOS-safe atomic), use atomic loads in callbacks, and atomic stores during init/deinit.

  s_device = this;

components/usb_device/src/usb_device.cpp:142

• Control transfers should respect request->wLength (host-requested length). The current implementation always uses len / total_len as the transfer length, which can be larger than wLength for some hosts and requests. Recommended: clamp the transmitted length to min(descriptor_len, request->wLength) for both WebUSB URL and MS OS 2.0 descriptor responses. Also consider validating the URL request's wIndex (landing-page index) to avoid responding with the wrong descriptor if the host queries a different index.

    if (request->bRequest == vendor.webusb_vendor_code) {
      // Return the WebUSB landing-page URL descriptor.
      uint8_t len = 0;
      const uint8_t *url = s_device->webusb_url_descriptor(len);
      if (!url)
        return false;
      return tud_control_xfer(rhport, request, (void *)(uintptr_t)url, len);
    }
    if (request->bRequest == vendor.ms_os_vendor_code && request->wIndex == 7) {
      // Return the MS OS 2.0 descriptor set.
      uint16_t total_len = 0;
      const uint8_t *ms = s_device->ms_os_20_descriptor(total_len);
      if (!ms)
        return false;
      return tud_control_xfer(rhport, request, (void *)(uintptr_t)ms, total_len);
    }

components/usb_device/src/usb_device.cpp:437

• The configuration descriptor advertises REMOTE_WAKEUP. If the device does not actually support/implement remote wakeup behavior, advertising this capability can be misleading to hosts. Recommended: either (a) switch the attribute to omit remote wakeup unless explicitly enabled by configuration, or (b) document/ensure the stack supports remote wakeup end-to-end for this device.

        TUD_CONFIG_DESCRIPTOR(1, itf_count, 0, total_len, TUSB_DESC_CONFIG_ATT_REMOTE_WAKEUP, 100),

[Copilot]

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.

Suppressed comments (13)

components/usb_device/include/usb_device.hpp:104

• The default WebUSB landing page cannot communicate with this vendor function: board_console.html only detects and calls navigator.serial and explicitly directs users to a USB-Serial-JTAG/UART port. Thus clicking the advertised WebUSB landing page cannot open the vendor bulk endpoints as documented. Either add a navigator.usb transport for this VID/PID/interface or point the default at an actual WebUSB console.

    std::string landing_page_url{"esp-cpp.github.io/espp/apps/board_console.html"};

components/usb_device/src/usb_device.cpp:498

• An enabled HID function can reach this cast with the default empty report descriptor, or with a descriptor larger than 65535 bytes. Both produce an invalid/truncated HID descriptor while initialize() still succeeds; poll_interval_ms == 0 likewise emits an invalid interrupt endpoint. Reject these configurations before appending the descriptor.

    const uint16_t report_len = static_cast<uint16_t>(impl_->hid_report_desc.size());
    const uint8_t poll = config_.hid->poll_interval_ms;

components/usb_device/web/board_console.html:402

• Pausing bypasses the DOM backlog limit and appends all incoming serial data to one unbounded string. At the supported high baud rates, leaving the monitor paused can consume memory indefinitely, and repeated string concatenation becomes increasingly expensive until the tab stalls. Apply a bounded backlog (dropping the oldest data) just as the live monitor does.

      if (paused && kind === "rx") { pausedBuffer += text; return; }

components/usb_device/idf_component.yml:25

• This component is documented and implemented only for ESP32-S2/S3/P4 USB-OTG targets, but the registry manifest currently advertises it for every ESP-IDF target. Add a target constraint so unsupported projects fail dependency resolution clearly rather than attempting to compile target-specific TinyUSB code.

  espressif/esp_tinyusb: '>=2.0'

components/usb_device/web/README.md:69

• The vendored esptool-js bundle is Apache-2.0 code, but this repository only contains its MIT root license and the bundle contains no Apache license text; a link and attribution in this README do not include the license copy required for redistribution. Add the upstream Apache-2.0 license (and any upstream NOTICE, if present) as third-party licensing material alongside the bundle.

The flashing feature is powered by **esptool-js**
(<https://github.com/espressif/esptool-js>), © Espressif Systems, licensed under
the **Apache License 2.0**. To keep this app free of any runtime third-party CDN
dependency, esptool-js is **vendored same-origin**: the published bundled build
is committed here verbatim as `esptool-bundle.js` and imported by

doc/Doxyfile:154

• New EXAMPLE_PATH entries must remain alphabetically ordered. usb_device is currently between odrive_ascii and pca9535; move it after tt21100 and before vl53l.

  $(PROJECT_PATH)/components/usb_device/example/main/usb_cdc_example.cpp \

doc/Doxyfile:364

• New Doxygen INPUT entries must remain alphabetically ordered. Move this component after tt21100 and before utils/vl53l, and order its headers as usb_cdc.hpp then usb_device.hpp.

  $(PROJECT_PATH)/components/usb_device/include/usb_device.hpp \
  $(PROJECT_PATH)/components/usb_device/include/usb_cdc.hpp \

.github/workflows/build.yml:214

• The CI example matrix is maintained alphabetically. Move the usb_device entry after tt21100 and before vl53l rather than placing it between odrive_ascii and pca9535.

        - path: 'components/usb_device/example'
          target: esp32s3

components/usb_device/example/README.md:32

• The example now enables and initializes HID, but this copyable configuration block omits CONFIG_TINYUSB_HID_COUNT=1. A user following it will build without CFG_TUD_HID and the example will fail initialization with function_not_supported. Document all three required classes here.

The example's `sdkconfig.defaults` enables both the CDC and vendor classes:

components/usb_device/src/usb_device.cpp:854

• The public method accepts an arbitrary-size span, but this narrowing cast silently wraps lengths above 65535; TinyUSB then queues only the truncated length while this method may return success. Reject oversized reports with value_too_large before calling tud_hid_report.

  if (!tud_hid_report(report_id, report.data(), static_cast<uint16_t>(report.size()))) {

components/usb_device/src/usb_device.cpp:536

• url_scheme is copied directly into the WebUSB descriptor even though the public contract permits only 0, 1, or 255. Any other configurable value creates a malformed URL descriptor while initialize() reports success. Validate the enum value before constructing the descriptor.

    impl_->webusb_url_desc.push_back(static_cast<uint8_t>(3 + v.landing_page_url.size()));
    impl_->webusb_url_desc.push_back(3); // WEBUSB URL descriptor type
    impl_->webusb_url_desc.push_back(v.url_scheme);

components/usb_device/src/usb_device.cpp:200

• has_out_endpoint=true advertises a HID interrupt-OUT endpoint, but every OUT report delivered here is silently discarded and there is no HID receive callback in the public API. Consumers enabling it for output reports will see successful USB transfers with no application-visible data. Add an output-report callback and dispatch to it, or remove/reject the option until supported.

void tud_hid_set_report_cb(uint8_t instance, uint8_t report_id, hid_report_type_t report_type,
                           uint8_t const *buffer, uint16_t bufsize) {
  (void)instance;
  (void)report_id;
  (void)report_type;

components/usb_device/README.md:40

• The generated table of contents does not match the headings below: it omits “Enabling the HID class” and links to a nonexistent “Extending with HID / MSC” heading. Update these entries so both sections are reachable.

  - [Enabling the vendor / WebUSB class](#enabling-the-vendor--webusb-class)
  - [Endpoint budget (ESP32-S3 USB-OTG)](#endpoint-budget-esp32-s3-usb-otg)
  - [Extending with HID / MSC](#extending-with-hid--msc)
  - [Example](#example)

[Copilot]

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated no new comments.

Suppressed comments (7)

components/usb_device/example/README.md:32

• The example firmware also enables/uses HID (CONFIG_TINYUSB_HID_COUNT=1 in sdkconfig.defaults, and the example code adds a HID gamepad function), but the README only mentions CDC+vendor. Update this section to include HID enablement (and any relevant usage note) so the documented requirements match the shipped example.

The example's `sdkconfig.defaults` enables both the CDC and vendor classes:

components/usb_device/include/usb_device.hpp:104

• The PR description and docs refer to a default "WebUSB console" landing page, but the default URL points to board_console.html, which (per its own README) is a Web Serial tool intended for USB-Serial-JTAG/UART ports and explicitly advises against connecting to a firmware-created CDC port. Either (a) provide a WebUSB-based landing page that actually talks to the vendor/WebUSB interface, or (b) adjust the PR/docs wording and defaults so the advertised landing page behavior matches what the hosted app supports.

     *        instead *include* its own scheme (e.g. "http://..."). Defaults to
     *        the espp docs-hosted board console + ESP flasher (scheme-less,
     *        https), a general-purpose Web Serial monitor and esptool-js flasher.
     * @note The descriptor length (3 + URL bytes) must fit a uint8_t, so the URL
     *       is limited to 252 bytes; `initialize()` rejects a longer URL.
     */
    std::string landing_page_url{"esp-cpp.github.io/espp/apps/board_console.html"};

components/usb_device/src/usb_device.cpp:526

• When HID is enabled, poll_interval_ms can currently be set to 0, which would generate an invalid HID interrupt endpoint bInterval at full speed and can break enumeration on some hosts. Consider validating config_.hid->poll_interval_ms >= 1 during initialize() (similar to the existing rx_chunk_size checks) and fail with std::errc::invalid_argument if it’s 0.

  const uint8_t hid_poll_ms = config_.hid ? config_.hid->poll_interval_ms : 0;
  // Full-speed configuration: 64-byte bulk endpoints, bInterval in ms frames.
  build_config_desc(impl_->config_desc, 64, hid_poll_ms);

components/usb_device/src/usb_device.cpp:55

• The LANGID entry is typically represented as a uint16_t array (e.g. {0x0409}) and passed via the pointer table. Storing it as uint8_t[2] and casting through const char* works in some implementations but risks alignment/interpretation issues depending on how the IDF/TinyUSB string-descriptor helper reads index 0. Prefer storing LANGID as std::array<uint16_t, 1>{0x0409} (or equivalent) to match common TinyUSB expectations and avoid potential unaligned access.

  // Owning strings + the pointer table TinyUSB reads (index 0 is the LANGID).
  std::array<uint8_t, 2> langid{{0x09, 0x04}};
  std::vector<std::string> owned_strings;
  std::vector<const char *> strings;

components/usb_device/src/usb_device.cpp:150

• For the WebUSB GET_URL request, wValue is the URL index (commonly 1). The handler currently ignores wValue and responds for any index as long as wIndex==2. To better match the WebUSB request semantics and avoid surprising behavior if additional URLs are added later, validate request->wValue (e.g., only answer index 1 and return false for others).

    if (request->bRequest == vendor.webusb_vendor_code && request->wIndex == 2) {
      // Return the WebUSB landing-page URL descriptor.
      uint8_t len = 0;
      const uint8_t *url = dev->webusb_url_descriptor(len);
      if (!url)
        return false;
      return tud_control_xfer(rhport, request, (void *)(uintptr_t)url, len);
    }

components/usb_device/idf_component.yml:5

• Using the git:// protocol is commonly blocked in CI/corporate networks and is generally deprecated in favor of HTTPS. Switching this to an https://github.com/... URL improves reliability of dependency fetching in restricted environments.

repository: "git://github.com/esp-cpp/espp.git"

components/usb_device/web/board_console.html:655

• FileReader.readAsBinaryString() is deprecated and may be removed or behave inconsistently in future browsers. Prefer reading as ArrayBuffer (and then passing a Uint8Array if esptool-js supports it, or converting explicitly if it requires a binary string) to keep the flasher robust long-term.

    function readFileAsBinaryString(file) {
      return new Promise((resolve, reject) => {
        const r = new FileReader();
        r.onload = () => resolve(r.result);
        r.onerror = () => reject(r.error || new Error("file read failed"));
        r.readAsBinaryString(file);
      });
    }

[Copilot]

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated 1 comment.

Suppressed comments (5)

components/usb_device/src/usb_device.cpp:535

• poll_interval_ms is written directly into the HID interrupt endpoint bInterval. At full-speed, bInterval=0 is invalid per USB spec (and values must fit 1..255). Add validation when HID is enabled to reject poll_interval_ms == 0 (and ideally clamp/reject values > 255) to avoid emitting an invalid configuration descriptor that can break enumeration.

  const uint8_t hid_poll_ms = config_.hid ? config_.hid->poll_interval_ms : 0;
  // Full-speed configuration: 64-byte bulk endpoints, bInterval in ms frames.
  build_config_desc(impl_->config_desc, 64, hid_poll_ms);

components/usb_device/web/board_console.html:614

• This uses innerHTML with string interpolation for offset, which can lead to DOM injection if offset ever contains quotes/markup (even if currently only set by this page, it’s easy for future refactors to pass user-controlled strings). Prefer constructing the DOM nodes and assigning input.value via the property (or otherwise escaping) instead of injecting HTML.

    function addFileRow(offset) {
      const id = "row-" + (rowSeq++);
      const row = document.createElement("div");
      row.className = "file-row"; row.id = id;
      row.innerHTML = `
        <div class="offset"><input type="text" class="mono offset-in" value="${offset || "0x0"}" placeholder="0x0" spellcheck="false"></div>
        <div class="file"><input type="file" class="file-in" accept=".bin,application/octet-stream"></div>
        <button class="tiny danger rm" title="Remove">&times;</button>
        <div class="progress-wrap"><div class="progress-bar"></div></div>`;

components/usb_device/web/board_console.html:657

• FileReader.readAsBinaryString() is deprecated in web standards and may eventually regress. Prefer readAsArrayBuffer() and pass a Uint8Array/ArrayBuffer onward (or adapt it to the format esptool-js expects) to keep the flasher future-proof.

    function readFileAsBinaryString(file) {
      return new Promise((resolve, reject) => {
        const r = new FileReader();
        r.onload = () => resolve(r.result);
        r.onerror = () => reject(r.error || new Error("file read failed"));
        r.readAsBinaryString(file);
      });
    }

components/usb_device/example/README.md:32

• The example project’s sdkconfig.defaults also enables HID (CONFIG_TINYUSB_HID_COUNT=1), but this snippet/docs only mention CDC + vendor. Update the README snippet/text to include HID (or clarify HID is optional) so the docs match the actual example configuration.

The example's `sdkconfig.defaults` enables both the CDC and vendor classes:

components/usb_device/include/usb_device.hpp:159

• The PR description says Config has cdc/vendor slots and that hid is reserved, but the code/docs include a fully defined HidFunction and HID enablement. Update the PR description (or adjust the stated scope) so it matches the implementation being introduced.

    std::optional<CdcFunction> cdc{};       /**< Enable a CDC-ACM function. */
    std::optional<VendorFunction> vendor{}; /**< Enable a vendor-specific / WebUSB function. */
    std::optional<HidFunction> hid{};       /**< Enable a HID function. */
    std::optional<MscFunction> msc{};       /**< (Future) enable an MSC function. */

[Copilot]

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated 1 comment.

Suppressed comments (5)

components/usb_device/src/usb_device.cpp:351

• HID interrupt endpoints require a non-zero bInterval, but poll_interval_ms can currently be set to 0 (it’s a uint8_t with no validation). That would emit an invalid interrupt endpoint descriptor at full-speed. Add validation here (or clamp) to require poll_interval_ms >= 1 when HID is enabled.

    if (config_.hid->report_descriptor.empty()) {
      // A default-constructed HidFunction has no report descriptor; proceeding
      // would emit a HID interface with wDescriptorLength == 0 (and a null
      // report callback) -- an invalid HID interface that "succeeds" here and
      // then confuses the host. Reject it as an invalid configuration.
      logger_.error("HID function enabled but report_descriptor is empty; supply the HID "
                    "report descriptor bytes (e.g. built with the hid-rp component).");
      ec = std::make_error_code(std::errc::invalid_argument);
      return false;
    }

components/usb_device/src/usb_device.cpp:883

• When the device is not mounted / host not connected, tud_vendor_write() may return 0; the current code reports that as no_buffer_space and logs 'TX buffer full', which is misleading and makes it hard for callers to distinguish disconnect vs backpressure. Consider checking tud_mounted() (or the appropriate vendor-connection predicate) up front and returning std::errc::not_connected before attempting to write.

  if (!initialized_ || !config_.vendor) {
    ec = std::make_error_code(std::errc::not_connected);
    return false;
  }
  size_t offset = 0;
  while (offset < data.size()) {
    uint32_t queued = tud_vendor_write(data.data() + offset, data.size() - offset);
    tud_vendor_write_flush();
    if (queued == 0) {
      logger_.warn_rate_limited("Vendor TX buffer full, dropping {} bytes", data.size() - offset);
      ec = std::make_error_code(std::errc::no_buffer_space);
      break;
    }
    offset += queued;
  }

components/usb_device/src/usb_device.cpp:64

• Returning an std::atomic<UsbDevice*> via implicit conversion obscures the fact that this is an atomic load (and makes it easier to miss memory-ordering intent). Prefer return s_device.load(); (optionally with an explicit memory order) for clarity/consistency with the rest of the file’s atomic usage.

UsbDevice *UsbDevice::instance() { return s_device; }

doc/en/buses/usb_cdc.rst:6

• This page is named usb_cdc.rst and is linked as usb_cdc in the buses index, but the title/content primarily describes the broader UsbDevice component (CDC + vendor/WebUSB + HID). To reduce reader confusion, consider either: (a) renaming the page to something like usb_device (and updating doc/en/buses/index.rst + component manifest doc URL), or (b) adjusting the title/intro to clearly state it covers both UsbDevice and the UsbCdc preset.

USB Device Component
====================

components/usb_device/web/board_console.html:657

• FileReader.readAsBinaryString() is deprecated and can be a portability risk long-term. Consider switching to readAsArrayBuffer() and passing a Uint8Array/ArrayBuffer-compatible payload to esptool-js (or converting as required by the library) to avoid relying on deprecated browser behavior.

    function readFileAsBinaryString(file) {
      return new Promise((resolve, reject) => {
        const r = new FileReader();
        r.onload = () => resolve(r.result);
        r.onerror = () => reject(r.error || new Error("file read failed"));
        r.readAsBinaryString(file);
      });
    }