-
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
Constructing the control pipe from a user-provided buffer #144
Conversation
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.
Thank you! Could you have a look at the CI?
Fixed CI and reverted this to a draft - I'd like to release before we make breaking API changes here. |
Perhaps it would be nice to have a check that the buffer is at least as large as max_packet_size_0. diff --git a/src/device_builder.rs b/src/device_builder.rs
index c32c03d..d582737 100644
--- a/src/device_builder.rs
+++ b/src/device_builder.rs
@@ -33,6 +33,8 @@ pub enum BuilderError {
InvalidPacketSize,
/// Configuration specifies higher USB power draw than allowed
PowerTooHigh,
+ /// Control buffer is smaller than max_packet_size_0
+ SmallControlBuffer,
}
/// Provides basic string descriptors about the device, including the manufacturer, product name,
@@ -110,8 +112,12 @@ impl<'a, B: UsbBus> UsbDeviceBuilder<'a, B> {
}
/// Creates the [`UsbDevice`] instance with the configuration in this builder.
- pub fn build(self) -> UsbDevice<'a, B> {
- UsbDevice::build(self.alloc, self.config, self.control_buffer)
+ pub fn build(self) -> Result<UsbDevice<'a, B>, BuilderError> {
+ if self.control_buffer.len() < self.config.max_packet_size_0 as usize {
+ Err(BuilderError::SmallControlBuffer)
+ } else {
+ Ok(UsbDevice::build(self.alloc, self.config, self.control_buffer))
+ }
}
builder_fields! {
diff --git a/src/test_class.rs b/src/test_class.rs
index 70dbb9c..e08736a 100644
--- a/src/test_class.rs
+++ b/src/test_class.rs
@@ -96,7 +96,7 @@ impl<B: UsbBus> TestClass<'_, B> {
/// Convenience method to create a UsbDevice that is configured correctly for TestClass.
pub fn make_device<'a>(&self, usb_bus: &'a UsbBusAllocator<B>) -> UsbDevice<'a, B> {
- self.make_device_builder(usb_bus).build()
+ self.make_device_builder(usb_bus).build().unwrap()
}
/// Convenience method to create a UsbDeviceBuilder that is configured correctly for TestClass. Or just a diff --git a/src/device_builder.rs b/src/device_builder.rs
index c32c03d..02320e3 100644
--- a/src/device_builder.rs
+++ b/src/device_builder.rs
@@ -111,6 +111,9 @@ impl<'a, B: UsbBus> UsbDeviceBuilder<'a, B> {
/// Creates the [`UsbDevice`] instance with the configuration in this builder.
pub fn build(self) -> UsbDevice<'a, B> {
+ if self.control_buffer.len() < self.config.max_packet_size_0 as usize {
+ panic!("control_buffer is smaller than max_packet_size_0");
+ }
UsbDevice::build(self.alloc, self.config, self.control_buffer)
}
|
@vitalyvb Great idea on checking the control buffer against the max packet size. I've added these checks to the builder. |
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.
Looks good. Thanks!
This PR updates the
UsbDevice
to use a buffer for the control pipe that is provided by the user. This fixes #31.This is a breaking API change so would require an eventual 0.4.0 release, which would be a good time to additionally propagate errors outwards etc. in the API.
This closes #62 by making control buffers confiugrable by the user.