Skip to content

feat(usb_device): ODrive-compatible USB device example (ASCII + native/Fibre + HID) - #725

Merged
finger563 merged 32 commits into
mainfrom
feat/odrive-usb-native
Aug 18, 2026
Merged

feat(usb_device): ODrive-compatible USB device example (ASCII + native/Fibre + HID)#725
finger563 merged 32 commits into
mainfrom
feat/odrive-usb-native

Conversation

@finger563

@finger563 finger563 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Description

Turns the usb_device example into a complete ODrive-compatible composite USB device, exercising three interfaces from one simulated motor state — the same split a real ODrive uses:

Interface Protocol Client tooling
CDC-ACM serial espp::OdriveAscii (text) any terminal, the hosted Web Serial console
vendor (0xFF, WebUSB) espp::OdriveNative (Fibre binary) reference fibre/odrivetool over USB, the hosted WebUSB control panel
HID animated gamepad (hid-rp) OS gamepad stack, the hosted WebHID visualizer

Also included:

  • odrive_usb_probe.py — a USB hardware-test probe that drives the genuine reference fibre client (auto-clones ODrive fw-v0.5.1) against the flashed board: endpoint-0 tree download, typed reads, write-then-read assertions.
  • HARDWARE_TEST.md — step-by-step validation guide for all three interfaces (probe, ASCII terminal, WebUSB panel, WebHID visualizer), including single-USB-connector board caveats.
  • sdkconfig.defaults pins esp32s3 (native USB-OTG is not on classic ESP32) and enables the CDC/vendor/HID classes.
  • Post-merge review fixes for the hosted web tools (they ship to docs/apps/ from main): WebUSB timeout/desync recovery via device.reset() (uncancelable-transfer fix), propagated selectAlternateInterface failures, strict motor-value codecs (no partial parses / wraps), and a productName XSS fix in the HID visualizer.

Verification

  • esp32s3 example builds clean (freshly re-verified after merging main).
  • Hardware-validated on a flashed board: native-protocol-over-WebUSB confirmed end-to-end via the control panel; HID gamepad enumerates and streams reports.
  • odrive_native golden/host tests and the real-fibre interop gate cover the protocol path (both green on main).

Follow-up (tracked, not in this PR): real odrivetool/pyusb auto-discovery additionally needs the vendor interface to advertise class 0x00/subclass 0x01, which requires a small custom TinyUSB class driver; WebUSB works as-is with class 0xFF.

🤖 Generated with Claude Code

finger563 and others added 12 commits August 16, 2026 21:47
…i example

Add espp::UsbCdc, a thin idiomatic wrapper around ESP-IDF's esp_tinyusb
managed component that presents a dedicated native USB CDC-ACM interface
on the ESP32-S3/-S2/-P4 USB-OTG peripheral with a configurable VID/PID and
manufacturer/product/serial strings, separate from the log console.

The example wires espp::UsbCdc RX -> espp::OdriveAscii::process_bytes ->
espp::UsbCdc::write so the device enumerates as an ODrive-like serial port
while the log console stays on USB-Serial-JTAG.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…evice

Generalize the usb_device component from a single hard-coded CDC-ACM
transport into espp::UsbDevice, which assembles a native USB device from a
set of selectable functions (CDC and/or vendor-specific) with interface
numbers, endpoint addresses and string indices allocated sequentially and
checked against the ESP32-S3 USB-OTG endpoint budget.

- Add a vendor-specific interface (bInterfaceClass 0xFF, bulk IN + bulk OUT)
  carrying a raw byte stream, plus WebUSB + MS OS 2.0 descriptors (BOS, URL
  descriptor, MS-OS-2.0 set) so browsers/Windows bind driverlessly.
- Vendor class enabled via CONFIG_TINYUSB_VENDOR_COUNT>0 (CFG_TUD_VENDOR);
  all tud_vendor_* paths are #if-guarded so CDC-only builds still link.
- Keep espp::UsbCdc as a thin CDC-only preset over UsbDevice (back-compat).
- Reserve HID/MSC extension points and document the endpoint budget table.
- Update example to a composite CDC + Vendor/WebUSB device feeding one
  OdriveAscii; docs (README, rst, Doxyfile) updated. esp32s3 build passes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…erver

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…raming

Add the real-tool interop gate for components/odrive_native, mirroring how
components/rtps is gated against real FastDDS/ROS 2: the genuine reference fibre
client (pure-python legacy fibre from odriverobotics/ODrive @ fw-v0.5.1) connects
to a host build of the odrive_native device shim over a PTY serial loopback,
downloads endpoint 0, enumerates the tree, and reads/writes endpoints.

- detail/odrive_native_stream.hpp: UART stream framing (odrive_crc8, stream_frame,
  StreamDeframer) verified byte-for-byte against the fw-v0.5.1 reference.
- interop/: device shim (PTY, detail/-only, plain c++), real fibre client driver,
  run_interop.sh + run.sh runner, README, .gitignore for the fetched client/venv.
- test/odrive_native_stream_test.cpp + pc/tests/odrive_native_golden.cpp: golden
  wire-format tests (CRC8/CRC16 goldens, exact frame bytes, deframe/packet
  round-trips); pc/CMakeLists.txt gives odrive_native_* targets the include dir.
- .github/workflows/odrive_native_interop.yml: PASS/FAIL-gated CI.

Fix (found by this harness): the endpoint canary json_crc is
calc_crc16(json, init=PROTOCOL_VERSION=1), NOT the 0x1337 packet-CRC init -- this
matches the fw-v0.5.1 firmware (endpoints_template.j2) and the reference client
(discovery.py). Without it the real client's endpoint reads are all rejected.
Corrected the core, host test, and PROTOCOL.md accordingly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rive-usb-native

# Conflicts:
#	.github/workflows/build.yml
#	doc/Doxyfile
…dor, ASCII on CDC

Wire the usb_device composite example to present a real ODrive-style protocol
split from one simulated motor state:
- CDC interface  -> espp::OdriveAscii  (text; terminal / Web Serial console)
- vendor interface (0xFF/WebUSB) -> espp::OdriveNative (the Fibre binary protocol
  that odrivetool / the fibre library auto-discover over USB)

Previously the vendor interface carried ASCII too (a shortcut); the vendor
interface is where the native protocol belongs. Adds a USB hardware probe
(odrive_usb_probe.py, the reference-fibre USB-backend sibling of the serial
interop client) and HARDWARE_TEST.md. Builds for esp32s3.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The example uses native USB-OTG (S3/S2/P4 only); pinning the target makes a bare 'idf.py build' target esp32s3 instead of defaulting to esp32 and failing.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The odrive-ref/ clone is a git-ignored interop-harness artifact that doesn't exist on a fresh checkout, so the hard-coded --fibre-path was dead. The probe now auto-detects known clone locations, adds --clone to fetch it, and prints a clear clone command; HARDWARE_TEST.md updated to match.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…eview fixes

Implement the HID function using TinyUSB's HID class driver: store the
application-supplied report descriptor, provide the tud_hid_* weak-callback
overrides, allocate the interrupt IN (+ optional OUT) endpoint and interface,
append TUD_HID_DESCRIPTOR to the config descriptor, include HID in the
endpoint-budget accounting, and add write_hid_report()/is_hid_ready(). All
tud_hid_* usage is gated behind CFG_TUD_HID so HID-less builds still link.

Exercise it in the example as a composite CDC + vendor + HID gamepad: build the
report descriptor from espp::GamepadInputReport (hid-rp) and animate axes +
buttons, sending input reports at ~10 Hz.

Example-manifest cleanup: move esp_tinyusb into the usb_device component's
idf_component.yml, delete the example main manifest, and resolve espp deps via
EXTRA_COMPONENT_DIRS (no override_path).

PR #720 review fixes: device descriptor uses MISC/IAD only when CDC is enabled
(else 0x00/0x00/0x00); validate WebUSB URL length fits a uint8_t; clear s_device
before driver teardown; allocation-free CDC/vendor RX via preallocated buffers;
clarify landing_page_url / url_scheme=255 docs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ath)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t/odrive-usb-native

# Conflicts:
#	components/usb_device/example/CMakeLists.txt
#	components/usb_device/example/main/CMakeLists.txt
#	components/usb_device/example/main/idf_component.yml
#	components/usb_device/example/main/usb_cdc_example.cpp
… test

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown

✅Static analysis result - no issues found! ✅

@finger563
finger563 marked this pull request as ready for review August 18, 2026 03:51
Copilot AI lite review requested due to automatic review settings August 18, 2026 03:51
Sync manifest with feat/usb-cdc-transport: registry tags allow only
[A-Za-z0-9_] (no hyphens), and the code uses the esp_tinyusb 2.x API.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Integrates a composite USB device example that exposes ODrive-compatible dual-protocol behavior (ASCII over CDC + legacy Fibre/native over vendor/WebUSB), while adding the underlying usb_device and odrive_native components plus host/interop validation and docs.

Changes:

  • Add components/usb_device composable USB-device wrapper (CDC + vendor/WebUSB + HID) and a flashed hardware example + probe script.
  • Add components/odrive_native legacy Fibre/native protocol server with host-buildable “wire core”, goldens, and a real fibre-client serial-loopback interop harness + CI workflow.
  • Wire docs + Doxygen inputs and add component upload/build workflow entries.

Reviewed changes

Copilot reviewed 49 out of 49 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pc/tests/odrive_native_golden.cpp Adds host golden wire-format tests for ODrive-native framing/CRC/roundtrip.
pc/CMakeLists.txt Updates PC test harness to include odrive_native headers for golden test builds.
doc/en/motor_control/odrive_native_example.md Adds docs include for the odrive_native example README.
doc/en/motor_control/odrive_native.rst Adds Sphinx page for the odrive_native component.
doc/en/motor_control/index.rst Links odrive_native docs from motor_control index.
doc/en/buses/usb_cdc_example.md Adds docs include for the usb_device example README.
doc/en/buses/usb_cdc.rst Adds Sphinx page for usb_device / usb_cdc component docs.
doc/en/buses/index.rst Links USB CDC/device docs from buses index.
doc/Doxyfile Adds usb_device + odrive_native headers/examples to Doxygen inputs.
components/usb_device/src/usb_device.cpp Implements composable TinyUSB device (CDC/vendor/WebUSB/HID) and descriptor building.
components/usb_device/src/usb_cdc.cpp Implements UsbCdc as a CDC-only preset over UsbDevice.
components/usb_device/include/usb_device.hpp Public API for UsbDevice configuration and I/O.
components/usb_device/include/usb_cdc.hpp Public API for UsbCdc back-compat transport wrapper.
components/usb_device/idf_component.yml Declares usb_device component metadata and dependencies.
components/usb_device/example/sdkconfig.defaults.esp32s3 Pins example console to USB-Serial-JTAG on esp32s3.
components/usb_device/example/sdkconfig.defaults Pins target to esp32s3 and enables TinyUSB CDC/vendor/HID configs.
components/usb_device/example/odrive_usb_probe.py Adds hardware probe script using reference fibre over USB.
components/usb_device/example/main/usb_cdc_example.cpp Implements composite ODrive-style example: CDC=ASCII, vendor=Fibre/native, plus HID.
components/usb_device/example/main/CMakeLists.txt Registers example main component requirements.
components/usb_device/example/README.md Documentation for building/flashing/using the usb_device example.
components/usb_device/example/HARDWARE_TEST.md Hardware validation steps for dual-protocol USB example.
components/usb_device/example/CMakeLists.txt Example project CMake enabling component manager and limiting scanned components.
components/usb_device/example/.gitignore Ignores probe venv / cloned reference repo.
components/usb_device/README.md Component README describing UsbDevice features, endpoint budget, and usage.
components/usb_device/CMakeLists.txt Registers usb_device IDF component build metadata.
components/odrive_native/test/odrive_native_stream_test.cpp Adds host-buildable tests for UART stream framing/goldens.
components/odrive_native/test/odrive_native_host_test.cpp Adds host-buildable unit tests for the native protocol core.
components/odrive_native/interop/run_interop.sh Adds serial-loopback interop harness using real reference fibre client.
components/odrive_native/interop/run.sh Adds thin entrypoint wrapper for interop harness.
components/odrive_native/interop/odrive_native_interop_device.cpp Adds host shim device serving native protocol over PTY/serial.
components/odrive_native/interop/odrive_fibre_client.py Adds client script driving the reference fibre library against the shim.
components/odrive_native/interop/README.md Documents interop harness purpose and usage.
components/odrive_native/interop/.gitignore Ignores interop venv and reference clone.
components/odrive_native/include/odrive_native.hpp Adds BaseComponent wrapper around host-buildable native protocol core.
components/odrive_native/include/detail/odrive_native_stream.hpp Adds host-buildable UART stream framing implementation.
components/odrive_native/include/detail/odrive_native_core.hpp Adds host-buildable protocol wire core (CRC, JSON, dispatch).
components/odrive_native/idf_component.yml Declares odrive_native component metadata and dependencies.
components/odrive_native/example/sdkconfig.defaults.esp32s3 Sets console config for esp32s3 for the example.
components/odrive_native/example/sdkconfig.defaults Sets common task stack sizing for the example.
components/odrive_native/example/main/odrive_native_example.cpp Adds a scripted on-target example demonstrating packet I/O.
components/odrive_native/example/main/CMakeLists.txt Registers example main component build metadata.
components/odrive_native/example/README.md Documents building/running the odrive_native example.
components/odrive_native/example/CMakeLists.txt Example project CMake for odrive_native usage.
components/odrive_native/README.md Component README describing odrive_native capabilities and API.
components/odrive_native/PROTOCOL.md Adds authoritative protocol spec for the legacy Fibre/native endpoint protocol.
components/odrive_native/CMakeLists.txt Registers odrive_native IDF component build metadata.
.github/workflows/upload_components.yml Adds usb_device and odrive_native to component upload workflow list.
.github/workflows/odrive_native_interop.yml Adds CI workflow running the real fibre serial-loopback interop harness.
.github/workflows/build.yml Adds usb_device and odrive_native examples to the build matrix.
Suppressed comments (2)

components/usb_device/src/usb_device.cpp:1

  • handle_cdc_rx() can spin forever if cdc_rx_buf_ is size 0 (e.g. Config::CdcFunction::rx_chunk_size == 0), because rx_size and buf.size() are both 0 and the do { ... } while (rx_size == buf.size()) condition never becomes false. This also passes a potentially-null pointer to tinyusb_cdcacm_read(). Recommend validating rx_chunk_size >= 1 during initialize() (fail with invalid_argument) and/or adding an early-return guard if cdc_rx_buf_.empty().
    components/usb_device/src/usb_device.cpp:1
  • The vendor RX callback ignores buffer/bufsize and instead drains via tud_vendor_read() inside handle_vendor_rx(). That loses per-transfer boundaries and can split (or coalesce) host transfers into arbitrary rx_chunk_size chunks, which is incompatible with protocols that rely on “one USB bulk transfer == one packet” semantics (notably the ODrive legacy native/Fibre-over-USB usage described in this PR). Recommend dispatching the received buffer as a single callback payload (preserving transfer boundaries) or adding an explicit “packet mode” option that buffers until bufsize is fully delivered before calling user code.

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

Comment thread components/usb_device/example/README.md Outdated
Comment thread components/odrive_native/include/detail/odrive_native_stream.hpp
Comment thread components/odrive_native/include/detail/odrive_native_core.hpp
…orrect README

- odrive_native: STL insert in stream test + reject accessor-less
  endpoints (mirrors #721; clears the 6 cppcheck findings on this branch).
- example README: describe the actual wiring — CDC->OdriveAscii,
  vendor->OdriveNative (Fibre), plus a HID gamepad — instead of the stale
  'both interfaces -> OdriveAscii' text (Copilot review).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@finger563 finger563 self-assigned this Aug 18, 2026
@finger563
finger563 requested a balanced review from Copilot August 18, 2026 04:20
@finger563 finger563 added enhancement New feature or request odrive usb device labels Aug 18, 2026
finger563 and others added 2 commits August 17, 2026 23:26
…e bit

Mirror #721: unknown endpoints return no response (per PROTOCOL.md) rather
than an ACK-only; adds a regression test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…721)

The #725 copy of the stream framer predated the guard; a packet larger than
kStreamMaxPacket (127) would truncate the single-byte length field into a
malformed frame. Refuse it. Addresses PR #725 review (stream.hpp:76).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (4)

components/usb_device/src/usb_device.cpp:1

  • For the ODrive native/Fibre use-case, USB transfer boundaries are semantically important (one bulk OUT transfer == one packet). This callback currently ignores buffer/bufsize and drains via tud_vendor_read(), which can split or coalesce payload across reads and break packet-per-transfer expectations. A concrete fix is to dispatch std::span<const uint8_t>(buffer, bufsize) directly to the configured vendor receive callback (or provide an explicit 'stream' vs 'datagram' mode, using the callback parameters for the datagram case).
    components/usb_device/src/usb_device.cpp:1
  • If rx_chunk_size is configured as 0, buf.size() is 0 and this loop becomes an infinite loop (rx_size stays 0 and rx_size == buf.size() remains true). Since this runs in the TinyUSB task context, it can hang USB processing. Consider rejecting rx_chunk_size == 0 in initialize() (set ec = invalid_argument and return false) and/or add an early if (buf.empty()) return; guard before the read loop.
    doc/en/buses/usb_cdc.rst:1
  • This page is named usb_cdc.rst but documents the broader UsbDevice component (CDC + vendor/WebUSB + HID). That mismatch can be confusing in navigation and URLs. Consider renaming the doc page to something like usb_device.rst (and adjusting doc/en/buses/index.rst / link targets), and reserving usb_cdc docs for the CDC-only preset if desired.
    components/odrive_native/include/detail/odrive_native_stream.hpp:76
  • The framing format requires len < 128 (and the comments mention <= kStreamMaxPacket), but stream_frame() does not enforce it. This makes it easy for callers to accidentally emit invalid frames. Consider adding an explicit check (e.g., return empty / truncate / assert) when packet.size() > kStreamMaxPacket so invalid frames can’t be produced silently.
inline std::vector<uint8_t> stream_frame(std::span<const uint8_t> packet) {
  std::vector<uint8_t> out;
  const uint8_t len = static_cast<uint8_t>(packet.size());
  out.reserve(packet.size() + 5);
  out.push_back(kStreamSync);
  out.push_back(len);
  const uint8_t header[2] = {kStreamSync, len};
  out.push_back(odrive_crc8(std::span<const uint8_t>(header, 2)));
  out.insert(out.end(), packet.begin(), packet.end());
  const uint16_t c = odrive_crc16(packet);
  out.push_back(static_cast<uint8_t>((c >> 8) & 0xff)); // big-endian: high byte
  out.push_back(static_cast<uint8_t>(c & 0xff));        //             low byte
  return out;
}

Comment thread components/odrive_native/include/detail/odrive_native_core.hpp
Comment thread components/odrive_native/include/detail/odrive_native_core.hpp
finger563 and others added 3 commits August 17, 2026 23:40
…onent)

The #725 branch carried a stale odrive_native snapshot that predated several
#721 review fixes: missing <algorithm>/<bit>/<type_traits> includes, the
std::endian little-endian check, the ep_size response cap, and the O(n)
deframer read-cursor rewrite (it still had the quadratic front-erase loop).
Bring the whole component to #721's committed version so the two PRs ship
identical source; also adds the espp_odrive Python client. Host tests +
esp32s3 usb example build clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… PR branches

Bring this integration branch's component copies up to the re-review fixes
landed on the source PRs (strict/typed-setter parsing + case-insensitive
bool in odrive_ascii; atomic instance routing, WebUSB wIndex guard, FIFO
drain, retry errc, zero-copy vendor RX, board console + esp_tinyusb >=2.0
in usb_device). The usb_device example (this PR's delta) is untouched.
esp32s3 integration example builds clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread components/odrive_native/python/tests/test_odrive.py Fixed
…yle)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… var)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 65 out of 66 changed files in this pull request and generated 2 comments.

Suppressed comments (15)

components/usb_device/README.md:182

  • This description is stale: the example routes CDC to OdriveAscii, vendor/WebUSB to OdriveNative, and also enables HID. Saying both interfaces feed one ASCII server contradicts the implementation and the PR's central integration behavior.
    doc/Doxyfile:367
  • These additions violate the repository requirement that Doxyfile INPUT entries remain alphabetical. Place the odrive_native headers after odrive_ascii, and move the usb_device headers to the u section (with usb_cdc.hpp before usb_device.hpp).
    components/usb_device/example/odrive_usb_probe.py:115
  • This probe cannot discover the device produced by this PR. The configured TinyUSB vendor descriptor advertises interface class 0xFF, while the PR description explicitly notes that the legacy Fibre/pyusb discovery path requires class 0x00/subclass 0x01. Consequently find_any(path="usb") times out before any of the assertions run. Either add the custom class driver/descriptor needed by Fibre discovery or change this probe so it does not claim to validate that path.
    components/usb_device/example/HARDWARE_TEST.md:75
  • The referenced control panel does not exist anywhere in the repository (components/odrive_ascii/web is absent), so users cannot perform this verification step. Add the WebUSB panel to the PR or replace this with a valid hosted/repository path.
    components/usb_device/example/HARDWARE_TEST.md:84
  • The referenced hid_visualizer.html does not exist in the repository, so this hardware-test instruction cannot be followed. Add the visualizer or point this step at an existing tool.
    components/usb_device/web/board_console.html:402
  • While paused, incoming serial data is appended to pausedBuffer without any limit. A continuously logging board can therefore grow browser memory indefinitely even though the visible DOM is capped. Bound this buffer (with an explicit truncation indication) or discard paused data.
    components/odrive_native/python/espp_odrive/device.py:120
  • Device.get() calls this method directly, but get_value() does not enforce can_read. A write-only property therefore sends an invalid read and eventually fails with an incidental short-buffer/timeout error, unlike attribute access which correctly reports that it is write-only. Enforce access here so every read path behaves consistently.
    doc/Doxyfile:155
  • The new Doxygen example entries violate the repository requirement that EXAMPLE_PATH remain alphabetical: odrive_native must follow odrive_ascii, while usb_device belongs in the u section after tt21100.

This issue also appears on line 364 of the same file.
.github/workflows/build.yml:216

  • The CI matrix is required to stay alphabetical. odrive_native should immediately follow odrive_ascii, while usb_device belongs after tt21100 and before vl53l; the current placement makes future component tracking error-prone.
        - path: 'components/usb_device/example'
          target: esp32s3
        - path: 'components/odrive_native/example'
          target: esp32

components/usb_device/README.md:40

  • The generated table of contents is stale: it omits the new “Enabling the HID class” section and links to “Extending with HID / MSC,” but the actual heading is now “Extending with MSC.” Refresh the markdown TOC so its links match the document.

This issue also appears on line 178 of the same file.
components/usb_device/example/README.md:98

  • hid_visualizer.html is not present anywhere in this repository, so readers cannot follow this instruction. Add the missing visualizer or replace the reference with a valid URL/tool.
    components/odrive_native/README.md:72
  • The component is now wired to a concrete USB stack by components/usb_device/example, so “handled in a later phase” is outdated within this PR. Point readers to that integration example while retaining the component’s transport-agnostic distinction.
- This component implements the **properties** (primitive get/set) surface of the
  legacy protocol; functions / endpoint refs are not implemented yet.
- Wiring to a concrete USB device stack is a later phase; this component is
  purely the protocol server.

doc/en/motor_control/odrive_native.rst:61

  • The USB integration is included in this PR, so saying it is deferred to a later phase is no longer accurate. Refer to the usb_device composite example instead.
    components/usb_device/example/README.md:93
  • The PR description says legacy odrivetool/pyusb discovery does not recognize the current class-0xFF interface, so this usage text promises behavior the image does not provide. Document WebUSB as the supported native-protocol transport for this version and state that legacy Fibre discovery requires the planned class 0x00/subclass 0x01 driver.
    components/usb_device/example/HARDWARE_TEST.md:31
  • This section presents the reference Fibre USB probe as the real gate and later says it should auto-discover the board, but the PR description explicitly states that discovery requires interface class 0x00/subclass 0x01; this image exposes class 0xFF. As written, the documented expected result is unattainable. Reframe this as a known limitation until the custom TinyUSB class driver lands.

Comment thread components/usb_device/web/board_console.html Outdated
Comment thread components/usb_device/src/usb_device.cpp
finger563 and others added 10 commits August 18, 2026 10:07
# Conflicts:
#	.github/workflows/upload_components.yml
#	doc/en/buses/index.rst
…ches #720)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…y, strict codecs, XSS

- control panel: WebUSB transfers are uncancelable, so the old timeout
  clearHalt and the 50ms drain race left an abandoned transferIn pending
  that could consume a later response and permanently desync the link.
  Replace both with recoverLink(): device.reset() aborts every pending
  transfer and restores clean endpoint state (fall back to disconnect if
  the reset itself fails). Timeout errors are tagged and trigger recovery.
- control panel + webusb console: a failed selectAlternateInterface now
  propagates and fails the connect instead of reporting Connected with
  endpoints from an unselected alternate.
- control panel: strict type codecs — integers must fully parse and fit
  the target type's range (no '1abc', no '1.9' truncation, no uint8-300
  wrap; int64/uint64 BigInt range-checked), floats must fully parse and
  fit float32 (Math.fround overflow check). Typos now surface an error
  instead of commanding the motor with garbage.
- hid visualizer: device productName is attacker-controlled; build the
  device-info rows with DOM nodes + textContent instead of interpolating
  into innerHTML (XSS).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rict+bounded flash offsets)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts:
#	.github/workflows/build.yml
#	components/odrive_native/python/README.md
#	components/usb_device/example/CMakeLists.txt
#	components/usb_device/example/README.md
#	components/usb_device/example/main/CMakeLists.txt
#	components/usb_device/example/main/usb_cdc_example.cpp
#	components/usb_device/example/sdkconfig.defaults
#	doc/Doxyfile
#	pc/tests/odrive_native_golden.cpp
main (#721) removed the odrive_native include-dir special case from
pc/CMakeLists.txt; this branch's older history re-introduced it through the
merge. Match main.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@finger563 finger563 changed the title feat(usb_device): ODrive-compatible USB example (native/Fibre on vendor, ASCII on CDC) feat(usb_device): ODrive-compatible USB device example (ASCII + native/Fibre + HID) Aug 18, 2026
@finger563
finger563 requested a balanced review from Copilot August 18, 2026 21:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (5)

components/usb_device/example/HARDWARE_TEST.md:9

  • The intro says the device presents 'two interfaces', but this PR’s example also enumerates a HID gamepad interface (and the file later has a HID section). Update the intro to state three interfaces (CDC + vendor + HID) to avoid confusing test readers.
This example makes an ESP32-S3 (or -S2/-P4) enumerate as an **ODrive-compatible
USB device** on its native USB-OTG peripheral, presenting two interfaces from one
simulated motor state:

components/odrive_ascii/web/odrive_control_panel.html:838

  • After device.reset(), WebUSB devices typically return to an unconfigured state and previously claimed interfaces/endpoints may no longer be valid. Logging 'link resynchronized' without re-selecting configuration / re-claiming the interface (or re-running the connect sequence) can leave subsequent transfers failing in a hard-to-diagnose way. Consider making recoverLink() explicitly re-run the same configuration + claimInterface + selectAlternateInterface setup (and refresh epIn/epOut), or fall back to a forced disconnect + reconnect flow.
    async function recoverLink(reason) {
      if (!device) return;
      logLine("err", "Recovering link (" + reason + "): resetting device...");
      try {
        await device.reset();
        logLine("sys", "Device reset complete; link resynchronized.");
      } catch (e) {
        logLine("err", "Device reset failed (" + e.message + "); disconnecting.");
        manualDisconnect = false;
        await safeClose();
      }
    }

components/usb_device/example/odrive_usb_probe.py:32

  • The probe auto-clones dependencies from a remote Git tag, which is a supply-chain risk because tags can be retargeted. To make the probe more tamper-resistant and reproducible, consider pinning to a specific commit hash (or verifying the cloned commit matches an expected SHA) and describing the pin in the log output/help text.
FIBRE_REPO = "https://github.com/odriverobotics/ODrive.git"
FIBRE_TAG = "fw-v0.5.1"
FIBRE_SUBPATH = os.path.join("Firmware", "fibre", "python")

components/usb_device/example/odrive_usb_probe.py:70

  • The probe auto-clones dependencies from a remote Git tag, which is a supply-chain risk because tags can be retargeted. To make the probe more tamper-resistant and reproducible, consider pinning to a specific commit hash (or verifying the cloned commit matches an expected SHA) and describing the pin in the log output/help text.
def clone_fibre():
    """Shallow-clone the reference fibre into ./odrive-ref next to this script."""
    here = os.path.dirname(os.path.abspath(__file__))
    dest = os.path.join(here, "odrive-ref")
    log("cloning reference fibre: %s @ %s -> %s" % (FIBRE_REPO, FIBRE_TAG, dest))
    subprocess.check_call([
        "git", "clone", "--depth", "1", "--branch", FIBRE_TAG,
        "--filter=blob:none", "--sparse", FIBRE_REPO, dest,
    ])

components/usb_device/example/main/usb_cdc_example.cpp:46

  • This translation unit now directly uses std::atomic (and later uses std::sin/std::cos). To avoid relying on transitive includes from project headers, ensure the file explicitly includes the corresponding standard headers (e.g., <atomic> and <cmath>) in its include list.
    std::atomic<float> vbus{24.0f};
    std::atomic<float> position{0.0f};
    std::atomic<float> velocity{0.0f};
    std::atomic<float> torque{0.0f};
    std::atomic<float> vel_limit{20.0f};
    std::atomic<uint32_t> error{0};
    std::atomic<uint64_t> serial{0xA1B2C3D4E5ULL};

@finger563
finger563 merged commit 181dd9d into main Aug 18, 2026
142 of 143 checks passed
@finger563
finger563 deleted the feat/odrive-usb-native branch August 18, 2026 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants