feat(usb_device): configurable bcdDevice, bMaxPower and remote-wakeup attribute - #796
Merged
Merged
Conversation
… attribute UsbDevice::Config gains bcd_device (default 0x0100, the previous hard-coded value), max_power_ma (default 100 mA, as before; clamped to 500) and remote_wakeup (default true, as before). Some hosts compare these against the device they expect: the Nintendo Switch is happier with a Pro Controller that reports bcdDevice 2.10 and 500 mA, which the esp-usb-ble-hid dongle could not express until now. An XInput-only device keeps reporting the Xbox 360 bcdDevice. The configuration descriptor's attribute and power bytes are patched in after TUD_CONFIG_DESCRIPTOR since the macro's arithmetic on a runtime value is a narrowing conversion inside the braced initializer. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
|
✅Static analysis result - no issues found! ✅ |
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Adds configurability for USB device/configuration descriptor fields that were previously hard-coded, enabling better host compatibility (e.g., Nintendo Switch expectations) without changing defaults for existing users.
Changes:
- Added
bcd_device,max_power_ma, andremote_wakeuptoespp::UsbDevice::Config. - Device descriptor now uses configurable
bcdDevicefor non–XInput-only devices. - Configuration descriptor
bmAttributesandbMaxPowerare patched post-TUD_CONFIG_DESCRIPTORto avoid narrowing warnings at runtime.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| components/usb_device/src/usb_device.cpp | Uses new config fields for bcdDevice, and patches config descriptor attributes/power bytes dynamically. |
| components/usb_device/include/usb_device.hpp | Extends UsbDevice::Config with new descriptor-related fields and documentation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…; round bMaxPower up The TUD_CONFIG_DESCRIPTOR macro cannot be expanded by cppcheck (it saw a 1-byte array patched at [7]/[8]) and hid which arguments were real: the 9-byte header is now spelled out field by field. bMaxPower (2 mA units) is clamped to 500 mA and rounded up so an odd request is never under-reported. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new field placement breaks positional aggregate initialization, and the public options are missing from both usage guides.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
…nt them bcd_device / max_power_ma / remote_wakeup now come last in UsbDevice::Config so positional aggregate initializers written against earlier releases keep their member positions. The README and doc/en/buses/usb_cdc.rst feature lists and usage snippets describe the three fields, the 500 mA clamp and the 2 mA round-up. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
…ntity; log_level stays last espp configs keep log_level as the final member and only promise named (designated) initializer compatibility, so bcd_device / max_power_ma / remote_wakeup sit with vid / pid / strings where they belong. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
espp::UsbDevice::Configgains three descriptor fields, all defaulting to the values that were hard-coded before, so existing users see no change:bcd_device0x0100bcdDevice(ignored for an XInput-only device, which keeps the Xbox 360 value)max_power_ma100bMaxPower(clamped to 500)remote_wakeuptruebmAttributesremote-wakeup bitWhy: some hosts compare these against the device they expect. The Nintendo Switch is happier with a Pro Controller that reports
bcdDevice2.10 and 500 mA (found by a downstream fork of esp-usb-ble-hid); the dongle could not express that throughUsbDeviceuntil now.Implementation note: the configuration descriptor's attribute and power bytes are written after
TUD_CONFIG_DESCRIPTORrather than passed to it — the macro's(_power_ma)/2on a runtime value is a narrowingint → uint8_tinside the braced initializer and fails under-Werror=narrowing.Built the
usb_deviceexample (esp32s3, IDF v6.1). Not hardware-tested.🤖 Generated with Claude Code
https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU