feat(usb_device): Xbox 360 X-Input controller emulation - #785
Conversation
Add an XInputFunction to espp::UsbDevice so a device can present itself as a wired Xbox 360 controller (X-Input) over USB-OTG, recognized by a PC's XUSB driver. X-Input is a vendor-specific interface (0xFF/0x5D/0x01) with interrupt IN+OUT endpoints and a fixed 20-byte input / 8-byte rumble report format -- it is NOT HID, and TinyUSB's built-in vendor driver is bulk-only, so this adds a small custom TinyUSB application class driver (registered via the weak usbd_app_driver_get_cb) that opens the interrupt endpoints, primes the OUT read, and dispatches rumble/LED reports to an on_rumble callback. - include/xinput.hpp: dependency-free, host-testable helpers -- X-Input wire constants, a GamepadState that packs the 20-byte report, a Button flag enum, and the interface+XID+endpoints descriptor builder. - usb_device.hpp/.cpp: XInputFunction config, the interface/endpoint allocation branch, the device-descriptor identity/class override when X-Input is the only function (Xbox 360 VID/PID 0x045E:0x028E + 0xFF/0xFF/0xFF class, so XUSB binds), update_gamepad()/is_xinput_ready(), and the class driver. Also guard the HID descriptor branch with CFG_TUD_HID so the component builds without the HID class driver compiled in. - xinput_example/: an esp32s3 demo that enumerates as an Xbox 360 controller, sweeps sticks/triggers, steps face buttons, and logs rumble. Manager-off build. - test/xinput_host_test.cpp: host test of report() + the descriptor builder. Caveats (deferred to on-device testing): use X-Input as the ONLY function (the built-in vendor class also claims class 0xFF; XUSB only binds a known Xbox VID/PID). The VID/PID are Microsoft's, for emulation/testing only. GIP (Xbox One) is not implemented (it needs a crypto auth handshake). control_xfer_cb stalls X-Input's optional vendor control requests; revisit if a host needs them. Builds clean on ESP-IDF v6.1 (esp32s3): xinput_example.bin, 58% free. Host test passes. Not yet validated against a real PC/XUSB host or hardware. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
- build.yml: build components/usb_device/xinput_example (esp32s3, IDF_COMPONENT_MANAGER=0, matching the example's vendored-submodule setup), placed after usb_device/example. - Doxyfile: add include/xinput.hpp and xinput_example/main/xinput_example.cpp to the INPUT lists (alphabetical slots). - doc/en/buses/usb_cdc.rst: include inc/xinput.inc so the X-Input API renders on the usb_device docs page. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
|
✅Static analysis result - no issues found! ✅ |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical vendor-control handling and moderate TinyUSB driver-collision findings remain, along with documentation and example-discovery nits.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds Xbox 360 X-Input emulation to espp::UsbDevice through a custom TinyUSB driver, including reports, rumble handling, an example, tests, documentation, and CI integration.
Changes:
- Adds X-Input protocol helpers, descriptors, and public APIs.
- Implements interrupt IN/OUT transfers and device identity handling.
- Adds an ESP32-S3 example, host tests, documentation, and CI coverage.
File summaries
| File | Summary and final review notes |
|---|---|
doc/en/buses/usb_cdc.rst |
Adds generated X-Input API documentation. Nit (1 vote): Update overview, endpoint-budget, constraints, and example navigation sections. |
doc/Doxyfile |
Includes the new X-Input documentation inputs. |
components/usb_device/xinput_example/sdkconfig.defaults |
Configures the X-Input example. |
components/usb_device/xinput_example/main/xinput_example.cpp |
Adds the demonstration firmware. Nit (1 vote): Add the example to the manifest and provide a README. |
components/usb_device/xinput_example/main/CMakeLists.txt |
Configures the example application. |
components/usb_device/xinput_example/CMakeLists.txt |
Defines the example project. Nit (1 vote): Add xinput_example to the component manifest and provide a README. |
components/usb_device/test/xinput_host_test.cpp |
Tests report packing and descriptor generation. |
components/usb_device/src/usb_device.cpp |
Implements the custom X-Input driver and transfers. Critical (1 vote): Handle XUSB vendor control requests through the global TinyUSB vendor-request path. Moderate (2 votes): Prevent the built-in vendor driver from claiming the X-Input interface when CFG_TUD_VENDOR > 0. |
components/usb_device/README.md |
Documents X-Input usage and endpoint requirements. Nit (1 vote): Add the linked example README with setup instructions and the Microsoft-ID warning. |
components/usb_device/include/xinput.hpp |
Defines protocol constants, report serialization, and descriptors. |
components/usb_device/include/usb_device.hpp |
Adds X-Input configuration and public APIs. |
.github/workflows/build.yml |
Adds the X-Input example to CI. Nit (1 vote): Also list xinput_example in the component manifest. |
Review details
Suppressed comments (5)
.github/workflows/build.yml:352
- The new project is added to CI, but
components/usb_device/idf_component.yml:9-10still publishes onlyexampleunderexamples:. Unlike the secondary examples incomponents/mcp266/idf_component.yml:9-11andcomponents/canopen/idf_component.yml:9-11,xinput_exampletherefore will not be exposed as a Component Registry example even though the README links to it; add- path: xinput_exampleto the manifest.
- path: 'components/usb_device/xinput_example'
target: esp32s3
command: 'IDF_COMPONENT_MANAGER=0 idf.py build'
components/usb_device/README.md:29
- The README link now points users to
xinput_example/, but that new example directory has no README. Existing examples, including secondary examples such ascomponents/usb_device/example/README.mdandcomponents/canopen/can_bridge_example/README.md, provide build/flash instructions; without one, the XInput-specific native-USB setup and Microsoft-ID warning are not documented at the linked destination. Addxinput_example/README.md.
[`xinput_example`](xinput_example/). *These are Microsoft's IDs, for emulation /
testing of your own device only.*
components/usb_device/xinput_example/CMakeLists.txt:35
- Please add
xinput_exampletocomponents/usb_device/idf_component.ymland give it a README. Secondary examples are explicitly listed in manifests (for example,components/mcp266/idf_component.yml:9-11) and ship per-example READMEs; without that metadata this demo is built by CI but is not offered as a component-registry example.
project(xinput_example)
components/usb_device/xinput_example/main/xinput_example.cpp:9
- This introduces a second public example, but it has no example README and
components/usb_device/idf_component.ymlstill advertises onlyexample. Other multi-example components list each project in the manifest (for examplemcp266andcanopen), so registry users will not discover this demo or get its build/setup instructions. Addxinput_exampleto the manifest and provide its README.
// Xbox 360 (X-Input) controller emulation example.
//
// Presents the ESP32-S3 as a wired Xbox 360 controller over the native USB-OTG
// peripheral using espp::UsbDevice's XInput function. A PC's XUSB driver binds
// it (VID/PID default to Microsoft's 0x045E:0x028E), so it shows up as an Xbox
// 360 controller in the OS gamepad tester / games. This demo cycles the buttons
// and sweeps the sticks/triggers so you can see live input, and logs any rumble
// / LED reports the host sends back. The console/logs go to the separate
// built-in USB-Serial-JTAG.
doc/en/buses/usb_cdc.rst:215
- Adding only the generated
xinput.incleaves this user-facing page inconsistent: the overview/features and endpoint-budget table still omit XInput and its XInput-only/XUSB constraint. Please update those sections as well (and expose the demo in the page's example navigation); otherwise the feature is present only in the generated API, not in the main usage documentation.
.. include-build-file:: inc/xinput.inc
- Files reviewed: 12/12 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…bmit X-Input enumerated (Windows shows the Xbox 360 controller by VID/PID) but sent no input. Two root causes: 1. usbd_app_driver_get_cb() is a weak symbol that TinyUSB's usbd.c both defines AND calls in the same translation unit, so our strong override could be ignored at link time — the app class driver never registered, its open() never ran, and the interrupt endpoints were never opened. Force the linker to bind our strong definition with `-u usbd_app_driver_get_cb` (component CMakeLists). Verified in the ELF: usbd_app_driver_get_cb is now our global symbol returning driver_count=1 and &s_xinput_class_driver. 2. update_gamepad() submitted the interrupt-IN transfer from the caller's task with a bare busy-check + usbd_edpt_xfer(), which can race the USB task and wedge the endpoint. Claim the endpoint first (usbd_edpt_claim, releasing on failure) — the same cross-task pattern tud_hid_report() uses. Also add USB-Serial-JTAG diagnostics so the data path is observable on device: the class driver logs when it registers and when it opens (with the endpoint addresses), and update_gamepad logs a "reports flowing" heartbeat plus the reason on a not-ready / failed send. Builds clean on ESP-IDF v6.1 (esp32s3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
Address PR review on the X-Input driver: - is_xinput_ready()/update_gamepad() now use the endpoint the class driver actually OPENED (s_xinput_drv.ep_in, set in open() and cleared on bus reset) instead of the planned address from allocation. So if open() never ran (e.g. the app driver failed to register) they correctly report not-ready rather than pretending the endpoint exists. - Document that TinyUSB tries application class drivers BEFORE the built-in ones (usbd.c get_driver/process_set_config iterate app drivers first), so our X-Input driver claims the 0xFF/0x5D/0x01 interface before the built-in vendor (bulk) driver can — even when CFG_TUD_VENDOR>0. (The two review comments assume the built-in vendor driver wins the interface, and that a 0xC1 interface- recipient vendor request routes to the global tud_vendor_control_xfer_cb; per usbd.c both are handled by the owning app driver instead.) Builds clean on ESP-IDF v6.1 (esp32s3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
There was a problem hiding this comment.
🟡 Changes recommended
XUSB setup requests are currently stalled, with additional concurrency and TinyUSB compatibility defects.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (6)
Previously missed (5) — in code that hasn't changed since the last review.
components/usb_device/src/usb_device.cpp:667
- The PR specifies that X-Input must be the only USB function, but initialization still accepts mixed configurations; this branch then silently keeps the normal composite VID/PID, so XUSB will not bind although
initialize()succeeds. Rejectxinputcombined with CDC/vendor/HID usinginvalid_argument, or revise the documented constraint if non-XUSB custom-host operation is intentionally supported.
components/usb_device/src/usb_device.cpp:784 - The same full-speed interval bytes are emitted into the high-speed configuration built below. At high speed, values 4 and 8 mean 1 ms and 16 ms rather than the documented 4 ms and 8 ms, so the advertised ESP32-P4 path has different polling behavior. Pass high-speed encoded values (6 and 7 respectively), analogous to the HID conversion above.
components/usb_device/include/usb_device.hpp:173 - This describes every OUT report as 8 bytes, but the example correctly accepts 3-byte LED packets and the callback receives the actual transfer length. Document the two lengths separately so consumers do not reject valid LED reports based on this public contract.
components/usb_device/xinput_example/CMakeLists.txt:35 - This new secondary example is not listed in
components/usb_device/idf_component.yml'sexamples:block, so it will not be packaged/discovered with the published component. Addxinput_examplethere (as done for secondary examples incomponents/canopen/idf_component.yml:9-11) and provide the example README used by documentation.
doc/en/buses/usb_cdc.rst:215 - Adding only the generated API include leaves this documentation page's overview, feature list, basic usage, endpoint budget, and example navigation describing only CDC/vendor/HID. Add the XInput-only setup and reporting API, its IN/OUT budget, identity restriction, and a toctree entry for the new example so the public feature is actually discoverable here.
components/usb_device/src/usb_device.cpp:159
- XUSB vendor requests cannot reach this class callback: TinyUSB dispatches every vendor-type setup packet through the global
tud_vendor_control_xfer_cbfirst. In the XInput-only configurationCFG_TUD_VENDORis 0, so this file provides no global override and TinyUSB stalls those requests; Windows' XInput driver can then stop polling the IN endpoint. Handle the XInput vendor requests in a global callback that is present even without the built-in vendor class, while preserving the existing WebUSB routing.
// X-Input issues a few vendor/class control requests during init; none are
// needed for the interrupt data path, so leave them to be stalled (return
// false = "not handled by this driver"). Windows' XUSB driver tolerates this.
return false;
- Files reviewed: 13/13 changed files
- Comments generated: 3
- Review effort level: Balanced
…t -Wcomment - The X-Input class driver uses the usbd_class_driver_t `xfer_isr` member and the 5-arg usbd_edpt_xfer(..., is_isr) API, both TinyUSB >= 0.19. esp_tinyusb only requires tinyusb >= 0.17, so a managed build could resolve 0.17/0.18 and fail to compile. Pin `tinyusb: '>=0.19.0'` in the component manifest. (CI is unaffected: the manager-off build uses the vendored tinyusb 0.21.) - s_xinput_drv.ep_in/ep_out are written by open()/reset() on the TinyUSB task and read by update_gamepad()/is_xinput_ready() on the caller's task — a data race on plain uint8_t. Make them std::atomic<uint8_t>. - The host-test file documented its build command in a `//` comment with a trailing backslash, which trips -Wcomment under the documented -Werror. Move the command into a block comment. Verified: example builds clean on ESP-IDF v6.1 (esp32s3); the host test compiles under `c++ -std=c++20 -Wall -Wextra -Werror ...` as documented and passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
The interrupt-IN report path works (host polls and reads our reports — the "reports flowing" heartbeat keeps firing, which requires each prior transfer to complete), but Windows shows no input. The remaining suspect is XUSB's init control-request handshake, which we were stalling. - Log every control request to the X-Input interface (and any that reach the global vendor path) on the USB-Serial-JTAG console, so the XUSB handshake is observable and we can see exactly what it asks for and where it lands. - Respond to the interface's VENDOR control requests instead of stalling (zero-filled IN response of the requested length; ACK for OUT/no-data), a best-effort "don't stall the handshake" while we learn the real requests. Investigative; builds clean on ESP-IDF v6.1 (esp32s3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
Steam can send a rumble command but nothing appears on the console. Rumble is a host->device report on the interrupt-OUT endpoint, a separate path from the input reports we can see the host reading. Log every received OUT report RAW and unconditionally in handle_xinput_out(), so we can tell whether the OUT path receives anything at all, independent of how the app callback filters it (the example only logs data[0]==0x00 rumble / 0x01 LED reports). Investigative; builds clean on ESP-IDF v6.1 (esp32s3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
On-device the report send started failing with "usbd_edpt_xfer failed on ep 0x03"
— update_gamepad() was reading the wrong endpoint. A previous change had switched
it to read s_xinput_drv.ep_in (written by the class driver's open() on the TinyUSB
task); despite open() logging ep_in=0x81, the cross-task read produced a bad value
(0x03), breaking the data path that previously worked ("reports flowing on 0x81").
Use impl_->xinput_ep_in instead — the address fixed at initialize() and immutable
afterwards, so it is always the correct 0x81 and needs no cross-task
synchronization. tud_mounted() already implies the class driver opened this (only)
interface's endpoints, so readiness is still accurate. This also resolves the
data-race review comment: no app-task code reads the TinyUSB-task-owned
s_xinput_drv fields anymore, so they revert to plain uint8_t.
Builds clean on ESP-IDF v6.1 (esp32s3).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
Set the XID "unknown" vendor descriptor's byte [2] to 0x10 (from 0x00), matching a real wired Xbox 360 controller byte-for-byte, in case xusb22 keys off it. Low-risk descriptor-fidelity change; the host test still passes (it does not pin that byte). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
A USB capture shows every interrupt-IN report arriving on the wire with a spurious leading 0x01 (our correct "00 14 .." report shifted right by one, last byte truncated), which makes XUSB reject them (byte[0] != 0x00) — explaining "reads reports, shows no input". report() provably writes 0x00 at [0] and we xfer exactly 20 bytes, so log the bytes we hand to the stack to confirm the 0x01 is injected below us (esp_tinyusb / DWC2) rather than in our code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
…corruption) A USB capture proved the cause of "reads reports but shows no input": every interrupt-IN report reached the host with a spurious leading 0x01 (our correct "00 14 .." report shifted right by one), so XUSB rejected them (byte[0] != 0x00). A TX log confirmed our code hands "00 14 .." to the stack, so the 0x01 is injected below us — the ESP32-S3 DWC2 corrupting the IN stream because IN (0x81) and OUT (0x01) shared endpoint NUMBER 1. Allocate a separate OUT endpoint number (IN 0x81, OUT 0x02) so the two directions no longer share DWC2 endpoint state. interface_descriptor() now takes explicit IN and OUT endpoint addresses and patches both into the XID blob ([6]=IN, [13]=OUT). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
After separating the IN/OUT endpoints the controller enumerated and joy.cpl showed it, but no input registered anywhere. Cause: a debug change had made the class driver's control handler answer XUSB's vendor control requests — including GET_CAPABILITIES (bmReq 0xC1, bReq 0x01, wValue 0x0100, wLen 20) — with a zero-filled buffer. A zero capabilities report tells XUSB the controller has no controls, so it reads our input reports and ignores them. A real wired 360 controller and the known-working esp32s3 references leave these vendor requests unanswered and XUSB falls back to full default capabilities. Stop synthesizing responses; return true (handled, no data) to match. Keep the request logging for now. Builds clean on ESP-IDF v6.1 (esp32s3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
…tream Two USB captures proved the IN reports still arrive corrupted after separating the endpoints: every interrupt-IN report reaches the wire with the OUT endpoint NUMBER prepended (0x01 when OUT was ep1, 0x02 when OUT was ep2), shifting our correct "00 14 .." report one byte right — so XUSB rejects them (byte[0]!=0x00) and no input registers, even though the controller enumerates and joy.cpl shows it. The corruption byte tracking the OUT endpoint number, plus the known-working esp32s3-tinyusb-xinput reference which opens the OUT endpoint but never posts a read on it, point to the OUT read as the trigger on the ESP32-S3 DWC2. Stop priming the interrupt-OUT endpoint (in open() and xfer_cb). The endpoint stays declared and opened (a real 360 controller has one), we just never drive it. Consequence: host->device rumble/LED reports are no longer consumed. That is the same tradeoff the reference makes; getting input working is the priority, and a rumble path can be revisited once the DWC2 behavior is understood. Builds clean on ESP-IDF v6.1 (esp32s3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
…ruption) Three captures now show the interrupt-IN reports still arrive with the OUT endpoint NUMBER prepended (0x02), even after moving the OUT endpoint to a separate number and after we stopped posting OUT reads. The descriptor is verified correct (IN 0x81/32/int, OUT 0x02/32/int, class ff/5d/01) and our report bytes are correct (00 14 ..), so the byte is injected in the DWC2/esp_tinyusb IN data path, and it tracks the OUT endpoint number. Test whether an *active* interrupt-OUT endpoint is the trigger: keep the OUT endpoint in the descriptor (XUSB still sees a normal 2-endpoint 360 controller) but do NOT usbd_edpt_open() it, so the DWC2 never configures an OUT interrupt endpoint. If the IN prefix disappears, the dual active interrupt endpoints are the cause and we ship input-only (no rumble) — the reference is effectively that too. Builds clean on ESP-IDF v6.1 (esp32s3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
…tion cause) Root cause of the corrupted interrupt-IN reports, found from the captures: the prefix byte was never the OUT endpoint "leaking" — it was the byte in memory immediately BEFORE the report buffer. In Impl, xinput_report followed the three uint8_t fields (…, xinput_ep_out) with array alignment 1, so it landed at a misaligned address. The ESP32-S3 DWC2 reads the TX buffer by DMA and, for a misaligned buffer, reads from the aligned-down address — prepending the preceding byte (xinput_ep_out: 0x02, or 0x01 back when IN/OUT shared endpoint 1) to every report and dropping the last byte. XUSB then saw byte[0] != 0x00 and rejected all input, even though enumeration and joy.cpl worked and our TX log showed correct bytes. Fix: alignas(4) on xinput_report (and the OUT out_buf). With the buffer word aligned the DMA reads from the correct address and reports go out intact. This makes the earlier endpoint experiments unnecessary, so revert them: IN and OUT keep their separate numbers (harmless), the OUT endpoint is opened again, and OUT priming is restored so host->device rumble/LED reports are received. Builds clean on ESP-IDF v6.1 (esp32s3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
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
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
…nup, CDC) - control_xfer_cb: return false to STALL XUSB's vendor control requests instead of returning true without completing the transfer (which left it pending). A stall is unambiguous "not supported" — XUSB falls back to default capabilities, which the interrupt-IN input path needs; the comment now matches the behavior. - update_gamepad: follow the TinyUSB endpoint contract exactly — busy-check, then claim/xfer/release, releasing the claim after the transfer is queued on BOTH success and failure, so a missed completion can't leave the endpoint permanently claimed. (Also drops the temporary TX debug log.) - xinput_drv_open: if a later usbd_edpt_open() fails, close any endpoint already opened and reset ep_in/ep_out before returning 0, instead of leaving partial endpoint state. - CDC is now compiled conditionally (#if CFG_TUD_CDC>0): the tinyusb_cdc_acm.h include, kCdcPort, the CDC RX trampoline, the CDC descriptor + init, and the deinit are all guarded, with an early function_not_supported check when a CDC function is requested but CFG_TUD_CDC==0 (mirroring the vendor/HID checks). The xinput example now disables CDC entirely (leaner: 58% free vs 57%), removing the previous "must enable CDC even unused" workaround. Verified: xinput example builds clean with CDC off AND on; the CDC/vendor/HID descriptor paths still compile; host descriptor test passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
- CDC is now compiled conditionally, so the vendor/WebUSB enablement no longer lists the CDC config lines as required; added a note that CDC is opt-in (CONFIG_TINYUSB_CDC_ENABLED / CFG_TUD_CDC) and only needed for the CDC function. - New "Enabling X-Input (Xbox 360)" section: no CFG_TUD_* count (custom app class driver via -u usbd_app_driver_get_cb), the example's CDC-off sdkconfig, and the separate interrupt IN/OUT endpoints + word-aligned DMA buffers the ESP32-S3 DWC2 requires. TOC updated (also adds the previously-missing HID entry). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
There was a problem hiding this comment.
🟡 Changes recommended
The XID descriptor contains an incorrect compatibility-critical byte, with additional packaging and documentation gaps.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
components/usb_device/include/xinput.hpp:40
kReportOutSizedescribes every rumble/LED report as 8 bytes, but LED reports are 3 bytes (the new example already handles that at lines 35-36). Consumers using this public constant to validate callback payloads will discard valid LED reports. Split the two wire sizes or rename this to make it rumble-specific.
components/usb_device/src/usb_device.cpp:1551- This unconditional
ESP_LOGIruns in the TinyUSB task for every rumble/LED packet and bypassesConfig::log_level, so even the default WARN configuration performs formatting and console I/O on the USB callback path. Route diagnostics through the configured logger at DEBUG/rate-limited severity.
- Files reviewed: 14/14 changed files
- Comments generated: 3
- Review effort level: Balanced
| // is 0x10 on the retail controller; [6] = IN endpoint address, [7] = IN | ||
| // report size (0x14 = 20), [13] = OUT endpoint address, [14] = OUT report | ||
| // size (0x08 = 8). | ||
| 0x11, 0x21, 0x10, 0x01, 0x01, 0x25, |
| - path: 'components/usb_device/xinput_example' | ||
| target: esp32s3 | ||
| command: 'IDF_COMPONENT_MANAGER=0 idf.py build' |
|
|
||
| .. include-build-file:: inc/usb_device.inc | ||
| .. include-build-file:: inc/usb_cdc.inc | ||
| .. include-build-file:: inc/xinput.inc |
Adds Xbox 360 X-Input controller emulation to
espp::UsbDevice, so anESP32-S3 (also S2 / P4) can present itself as a wired Xbox 360 controller that a
PC's XUSB driver binds and games read as X-Input.
Why this needs a new class driver
X-Input is not HID. It's a vendor interface (
bInterfaceClass/SubClass/ Protocol = 0xFF/0x5D/0x01) with a vendor "XID" descriptor and interrupt IN +OUT endpoints. TinyUSB's built-in vendor class driver is bulk-only with a fixed
0xFF/0x00/0x00triple, so it can't express X-Input. This PR registers a smallapplication class driver via the weak
usbd_app_driver_get_cb(forced intothe link with
-u, since TinyUSB defines and calls that weak symbol in one TU),which claims only the X-Input interface (inert when the function is disabled),
opens the two interrupt endpoints, sends 20-byte input reports, and delivers
rumble/LED OUT reports to a callback.
What's added
include/xinput.hpp— dependency-free, host-testable: X-Input wire constants,a
Buttonflag enum,GamepadState(packs the 20-byte input report), and theinterface + XID + endpoints descriptor builder.
UsbDevice::Config::xinput(XInputFunction),update_gamepad(),is_xinput_ready(), and a rumble receive callback — idiomatic espp(non-throwing,
std::error_code), matching the existing CDC/Vendor/HID style.src/usb_device.cpp— the class driver, the allocation branch (1 interrupt INidentity/class override for the X-Input-only case.
xinput_example/— an esp32s3 demo that sweeps the sticks/triggers, stepsA/B/X/Y, and logs rumble. CDC/vendor/HID are all disabled in its sdkconfig.
test/xinput_host_test.cpp— host test ofGamepadState::report()+ thedescriptor builder.
build.yml) and the docs(Doxyfile +
usb_cdc.rst).Also, as a side effect of this work, CDC support in
usb_deviceis now compiledconditionally (
#if CFG_TUD_CDC > 0): an X-Input-only (or vendor-only) build nolonger forces the CDC class in. Existing CDC/vendor/HID configs are unchanged.
Testing against a computer
The example enumerates with Microsoft's Xbox 360 VID/PID (
0x045E:0x028E) anddevice class
0xFF/0xFF/0xFF— that identity is what makes a host's XUSB driverbind it, so it shows up as an Xbox 360 controller in Windows' "Set up USB game
controllers" (
joy.cpl) / any X-Input game, or under Linuxxpad. Flashxinput_exampleto an S3 and plug it into the USB-OTG port.Hardware bring-up (ESP32-S3 + Windows)
This was validated on real hardware with USB captures (USBPcap). Findings, all now
fixed in this PR:
controller (no driver warning),
xusb22binds, polls the interrupt-IN endpoint,and
joy.cpllists it. The interrupt-OUT endpoint receives the LED/rumblecommands XUSB sends on connect.
report reaching the wire with a spurious leading byte (our correct
00 14 …report shifted one byte right), so XUSB rejected them (
byte[0] != 0x00) and noinput registered. The cause was a misaligned DMA buffer: the report buffer
sat one byte after another struct field, and the ESP32-S3 DWC2 reads the TX
buffer by DMA — a misaligned buffer makes it read from the aligned-down address,
prepending the preceding byte. Fix:
alignas(4)on the report/OUT buffers. INand OUT also use separate endpoint numbers (IN
0x81, OUT0x02).GET_CAPABILITIES(and friends)are stalled (
control_xfer_cbreturns false); XUSB falls back to full defaultcapabilities, which is what the input path needs.
Status
xusb22binding, and the interrupt IN/OUT data path (the report-corruption root cause is
understood and fixed).
on; host
GamepadState::report()+ descriptor tests pass.joy.cplshow live stick andbutton movement after the alignment fix. The example still carries a little
diagnostic logging on the USB-Serial-JTAG console; it will be removed for the
final review once input is confirmed in-game.
cryptographic auth handshake (works on PC but blocked on real consoles).
🤖 Generated with Claude Code