Merge pull request #144 from rust-embedded-community/feature/control-buffer-args

Constructing the control pipe from a user-provided buffer

Author: ryan-summers

.github/workflows/ci.yml

@@ -31,5 +31,5 @@ jobs:
         run: sudo apt-get install -y libusb-1.0.0-dev
 
       - run: cargo check --all-targets
-      - run: cargo check --features control-buffer-256
       - run: cargo check --features defmt
+      - run: cargo check --features log

.github/workflows/rustfmt.yml

@@ -17,5 +17,5 @@ jobs:
       - run: cargo fmt --all -- --check
       - run: cargo clippy --all-features
       - run: cargo clippy --features defmt
-      - run: cargo clippy --features control-buffer-256
+      - run: cargo clippy --features log
       - run: cargo clippy

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
+### Changed
+* [breaking] The control pipe is now provided in the `UsbDeviceBuilder` API to allow for user-provided control
+pipes. This makes it so that control pipes have configurable sizing.
+
 ## [0.3.2] - 2024-03-06
 
 ### Added

Cargo.toml

@@ -20,11 +20,6 @@ rusb = "0.9.1"
 rand = "0.8.5"
 
 [features]
-# Use a 256 byte buffer for control transfers instead of 128.
-control-buffer-256 = []
-
-# Enable logging and tracing via the `log` crate
-log = ["dep:log"]
 
 # Use larger endpoint buffers for highspeed operation (default fullspeed)
 #

src/control_pipe.rs

@@ -20,31 +20,28 @@ enum ControlState {
     Error,
 }
 
-// Maximum length of control transfer data stage in bytes. 128 bytes by default. You can define the
-// feature "control-buffer-256" to make it 256 bytes if you have larger control transfers.
-#[cfg(not(feature = "control-buffer-256"))]
-const CONTROL_BUF_LEN: usize = 128;
-#[cfg(feature = "control-buffer-256")]
-const CONTROL_BUF_LEN: usize = 256;
-
 /// Buffers and parses USB control transfers.
 pub struct ControlPipe<'a, B: UsbBus> {
     ep_out: EndpointOut<'a, B>,
     ep_in: EndpointIn<'a, B>,
     state: ControlState,
-    buf: [u8; CONTROL_BUF_LEN],
+    buf: &'a mut [u8],
     static_in_buf: Option<&'static [u8]>,
     i: usize,
     len: usize,
 }
 
 impl<B: UsbBus> ControlPipe<'_, B> {
-    pub fn new<'a>(ep_out: EndpointOut<'a, B>, ep_in: EndpointIn<'a, B>) -> ControlPipe<'a, B> {
+    pub fn new<'a>(
+        buf: &'a mut [u8],
+        ep_out: EndpointOut<'a, B>,
+        ep_in: EndpointIn<'a, B>,
+    ) -> ControlPipe<'a, B> {
         ControlPipe {
             ep_out,
             ep_in,
             state: ControlState::Idle,
-            buf: [0; CONTROL_BUF_LEN],
+            buf,
             static_in_buf: None,
             i: 0,
             len: 0,

src/device.rs

@@ -82,7 +82,11 @@ pub const DEFAULT_ALTERNATE_SETTING: u8 = 0;
 type ClassList<'a, B> = [&'a mut dyn UsbClass<B>];
 
 impl<B: UsbBus> UsbDevice<'_, B> {
-    pub(crate) fn build<'a>(alloc: &'a UsbBusAllocator<B>, config: Config<'a>) -> UsbDevice<'a, B> {
+    pub(crate) fn build<'a>(
+        alloc: &'a UsbBusAllocator<B>,
+        config: Config<'a>,
+        control_buffer: &'a mut [u8],
+    ) -> UsbDevice<'a, B> {
         let control_out = alloc
             .alloc(
                 Some(0x00.into()),
@@ -106,7 +110,7 @@ impl<B: UsbBus> UsbDevice<'_, B> {
         UsbDevice {
             bus,
             config,
-            control: ControlPipe::new(control_out, control_in),
+            control: ControlPipe::new(control_buffer, control_out, control_in),
             device_state: UsbDeviceState::Default,
             remote_wakeup_enabled: false,
             self_powered: false,

src/device_builder.rs

@@ -8,6 +8,7 @@ pub struct UsbVidPid(pub u16, pub u16);
 /// Used to build new [`UsbDevice`]s.
 pub struct UsbDeviceBuilder<'a, B: UsbBus> {
     alloc: &'a UsbBusAllocator<B>,
+    control_buffer: &'a mut [u8],
     config: Config<'a>,
 }
 
@@ -32,6 +33,8 @@ pub enum BuilderError {
     InvalidPacketSize,
     /// Configuration specifies higher USB power draw than allowed
     PowerTooHigh,
+    /// The provided control buffer is too small for the provided maximum packet size.
+    ControlBufferTooSmall,
 }
 
 /// Provides basic string descriptors about the device, including the manufacturer, product name,
@@ -82,9 +85,14 @@ impl<'a> StringDescriptors<'a> {
 
 impl<'a, B: UsbBus> UsbDeviceBuilder<'a, B> {
     /// Creates a builder for constructing a new [`UsbDevice`].
-    pub fn new(alloc: &'a UsbBusAllocator<B>, vid_pid: UsbVidPid) -> UsbDeviceBuilder<'a, B> {
+    pub fn new(
+        alloc: &'a UsbBusAllocator<B>,
+        vid_pid: UsbVidPid,
+        control_buffer: &'a mut [u8],
+    ) -> UsbDeviceBuilder<'a, B> {
         UsbDeviceBuilder {
             alloc,
+            control_buffer,
             config: Config {
                 device_class: 0x00,
                 device_sub_class: 0x00,
@@ -104,8 +112,16 @@ 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)
+    pub fn build(self) -> Result<UsbDevice<'a, B>, BuilderError> {
+        if self.control_buffer.len() < self.config.max_packet_size_0 as usize {
+            return Err(BuilderError::ControlBufferTooSmall);
+        }
+
+        Ok(UsbDevice::build(
+            self.alloc,
+            self.config,
+            self.control_buffer,
+        ))
     }
 
     builder_fields! {
@@ -188,6 +204,10 @@ impl<'a, B: UsbBus> UsbDeviceBuilder<'a, B> {
             _ => return Err(BuilderError::InvalidPacketSize),
         }
 
+        if self.control_buffer.len() < max_packet_size_0 as usize {
+            return Err(BuilderError::ControlBufferTooSmall);
+        }
+
         self.config.max_packet_size_0 = max_packet_size_0;
         Ok(self)
     }

src/lib.rs

@@ -134,9 +134,12 @@ pub mod endpoint;
 /// To implement USB support for your own project, the required code is usually as follows:
 ///
 /// ``` ignore
+/// use core::cell::UnsafeCell;
 /// use usb_device::prelude::*;
 /// use usb_serial; // example class crate (not included)
 ///
+/// static mut CONTROL_BUFFER: UnsafeCell<[u8; 128]> = UnsafeCell::new([0; 128]);
+///
 /// // Create the device-specific USB peripheral driver. The exact name and arguments are device
 /// // specific, so check the documentation for your device driver crate.
 /// let usb_bus = device_specific_usb::UsbBus::new(...);
@@ -151,12 +154,12 @@ pub mod endpoint;
 /// // pair. Additional builder arguments can specify parameters such as device class code or
 /// // product name. If using an existing class, remember to check the class crate documentation
 /// // for correct values.
-/// let mut usb_dev = UsbDeviceBuilder::new(&usb_bus, UsbVidPid(0x5824, 0x27dd))
+/// let mut usb_dev = UsbDeviceBuilder::new(&usb_bus, UsbVidPid(0x5824, 0x27dd), unsafe { CONTROL_BUFFER.get_mut() })
 ///     .strings(&[StringDescriptors::new(LangID::EN)
 ///         .product("Serial port")])
 ///         .expect("Failed to set strings")
 ///     .device_class(usb_serial::DEVICE_CLASS)
-///     .build();
+///     .build().unwrap();
 ///
 /// // At this point the USB peripheral is enabled and a connected host will attempt to enumerate
 /// // it.

src/test_class.rs

@@ -1,9 +1,9 @@
 #![allow(missing_docs)]
 
 use crate::class_prelude::*;
-use crate::descriptor::lang_id::LangID;
 use crate::device::{StringDescriptors, UsbDevice, UsbDeviceBuilder, UsbVidPid};
 use crate::Result;
+use core::cell::UnsafeCell;
 use core::cmp;
 
 #[cfg(feature = "test-class-high-speed")]
@@ -22,6 +22,8 @@ mod sizes {
     pub const INTERRUPT_ENDPOINT: u16 = 31;
 }
 
+static mut CONTROL_BUFFER: UnsafeCell<[u8; 256]> = UnsafeCell::new([0; 256]);
+
 /// Test USB class for testing USB driver implementations. Supports various endpoint types and
 /// requests for testing USB peripheral drivers on actual hardware.
 pub struct TestClass<'a, B: UsbBus> {
@@ -94,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.
@@ -112,14 +114,16 @@ impl<B: UsbBus> TestClass<'_, B> {
         &self,
         usb_bus: &'a UsbBusAllocator<B>,
     ) -> UsbDeviceBuilder<'a, B> {
-        UsbDeviceBuilder::new(usb_bus, UsbVidPid(VID, PID))
-            .strings(&[StringDescriptors::default()
-                .manufacturer(MANUFACTURER)
-                .product(PRODUCT)
-                .serial_number(SERIAL_NUMBER)])
-            .unwrap()
-            .max_packet_size_0(sizes::CONTROL_ENDPOINT)
-            .unwrap()
+        UsbDeviceBuilder::new(usb_bus, UsbVidPid(VID, PID), unsafe {
+            CONTROL_BUFFER.get_mut()
+        })
+        .strings(&[StringDescriptors::default()
+            .manufacturer(MANUFACTURER)
+            .product(PRODUCT)
+            .serial_number(SERIAL_NUMBER)])
+        .unwrap()
+        .max_packet_size_0(sizes::CONTROL_ENDPOINT)
+        .unwrap()
     }
 
     /// Must be called after polling the UsbDevice.