Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow custom bcd usb version #3599

Merged
merged 7 commits into from
Dec 2, 2024

Conversation

LegitCamper
Copy link
Contributor

Allows a custom bcdusb version like 1.1 (0x011) but leaves the default to 2.0/2.1

config.device_sub_class, // bDeviceSubClass
config.device_protocol, // bDeviceProtocol
config.max_packet_size_0, // bMaxPacketSize0
(config.bcd_usb >> 8) as u8, // bcdUSB
Copy link
Member

Choose a reason for hiding this comment

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

the 0x10 above is also part of bcd_usb, it's the lower byte. for example USB 2.1 is 0x0210.

The config default should also be 0x0210 instead of 0x02

@Dirbaio
Copy link
Member

Dirbaio commented Dec 2, 2024

what's the use case for this? in what cases do you want to force the USB version down to 2.0 or 1.x?

Would these use cases still be satisfied if the API was a "usb version" enum instead of a freeform u16? IMO that'd be more user-friendly because it'd prevent invalid values.

Also, do we need to disable some features that weren't available in older USBs?

@LegitCamper
Copy link
Contributor Author

what's the use case for this? in what cases do you want to force the USB version down to 2.0 or 1.x?

I think in many cases where you are trying to emulate a specific device. When using 2.1 the host assumes you are using a bos_descriptor since that is what 2.1 is. By forcing the use of 2.0 you are disabling unused features. Maybe the same is true for 1.x, but I'm not sure what other changes might be required for that use case.

Would these use cases still be satisfied if the API was a "usb version" enum instead of a freeform u16? IMO that'd be more user-friendly because it'd prevent invalid values.

I think so, I can easily create one for 2.1, and 2.0 and others can include others when needed.

Also, do we need to disable some features that weren't available in older USBs?

In my testing, no everything seems to work as is.

/// Device BCD USB version.
///
/// Default: `0x0210` ("2.1")
pub bcd_usb: BcdUsbVersion,
Copy link
Member

Choose a reason for hiding this comment

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

I'd name this just UsbVersion.

"BCD" is a reference to Binary Coded Decimal, ie how it's coded in binary. if it's an enum the user no longer cares about that.

Copy link
Member

@Dirbaio Dirbaio left a comment

Choose a reason for hiding this comment

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

thanks! 🚀

@Dirbaio Dirbaio enabled auto-merge December 2, 2024 23:13
@Dirbaio Dirbaio added this pull request to the merge queue Dec 2, 2024
Merged via the queue into embassy-rs:main with commit 5fabf7d Dec 2, 2024
7 checks passed
@LegitCamper LegitCamper mentioned this pull request Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants