-
Notifications
You must be signed in to change notification settings - Fork 75
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
Make USB device config public #69
base: master
Are you sure you want to change the base?
Conversation
pub struct Config<'a> { | ||
/// USB device class | ||
pub device_class: u8, | ||
/// USB device subclass | ||
pub device_sub_class: u8, | ||
/// USB device protocol | ||
pub device_protocol: u8, | ||
/// USB control endpoint maximum packet size | ||
pub max_packet_size_0: u8, | ||
/// USB vendor ID | ||
pub vendor_id: u16, | ||
/// USB product ID | ||
pub product_id: u16, | ||
/// BCD encoded device release | ||
pub device_release: u16, | ||
/// Manufacturer string | ||
pub manufacturer: Option<&'a str>, | ||
/// Product string | ||
pub product: Option<&'a str>, | ||
/// Serial number | ||
pub serial_number: Option<&'a str>, | ||
/// Is device self-powered? | ||
pub self_powered: bool, | ||
/// Does device support remote wakeup? | ||
pub supports_remote_wakeup: bool, | ||
/// Is device composite with IADs? | ||
pub composite_with_iads: bool, | ||
/// Maximum power consumption of device | ||
pub max_power: u8, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As much as I like the builder semantics, making struct fields public is problematic . See https://rust-lang.github.io/api-guidelines/future-proofing.html#structs-have-private-fields-c-struct-private for a discussion on why this is problematic for a maintainability perspective.
Other than that, I like the builder config API
I was about to start tackling USB/IP when I discovered @Sawchord's https://github.com/Sawchord/usbip-device.
For
usbip list -r localhost
to be somewhat pretty, the USB/IP server should return basic info on the device, which duplicates the contents of the non-publicConfig
struct here. (It's not strictly necessary for this info to be correct, AFAIU.)Looking at UsbDeviceBuilder, in reality it's a ConfigBuilder, which would become quite redundant (candidate for deprecation?) if the config itself were made public with a fluent/builder interface. I'm gambling on increasing merge/publish potential by staying backwards compatible though :)
This would then allow re-use in the USB/IP driver (e.g. https://github.com/nickray/usbip-device/blob/7bab96f64072da82a552c94213b5dc98f2481c99/examples/twitching_mouse.rs#L16-L26)
There's a bit chicken+egg involved regarding DRY and the interface descriptors (which USB/IP also advertises); solving that seems it would require breaking changes which I don't want to propose.
P.S. Are there any/many useful cases for
Config<'a>
with non-'static
lifetimes?