Add new read_setup() for control endpoints

CHANGELOG.md

@@ -9,11 +9,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ### Added
 
+* Test case that hard-resets the device and measures the time it takes to re-enumerate.
 * `DummyUsbBus` without functionality to allow examples that actually compile (but not run).
 * Extended `UsbRev` enum with variants for USB 1.0 and 1.1.
 
 ### Changed
 
+* [breaking] Bus API now has separate `read()` and `read_setup()` methods for OUT and SETUP data.
 * [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.
 * Don't require UsbBus to be Sync. If a UsbBus is not Sync, it can still be used to make a UsbDevice, but that UsbDevice will not be Sync (ensuring soundness).

src/bus.rs

@@ -81,8 +81,8 @@ pub trait UsbBus: Sized {
     /// Implementations may also return other errors if applicable.
     fn write(&self, ep_addr: EndpointAddress, buf: &[u8]) -> Result<usize>;
 
-    /// Reads a single packet of data from the specified endpoint and returns the actual length of
-    /// the packet.
+    /// Reads a single packet of OUT data from the specified endpoint and returns the length of the
+    /// packet.
     ///
     /// This should also clear any NAK flags and prepare the endpoint to receive the next packet.
     ///
@@ -97,10 +97,32 @@ pub trait UsbBus: Sized {
     ///   fit in `buf`. This is generally an error in the class implementation, because the class
     ///   should use a buffer that is large enough for the `max_packet_size` it specified when
     ///   allocating the endpoint.
+    /// * [`InvalidState`](crate::UsbError::InvalidState) - The received packet is a SETUP
+    ///   transaction, and needs to be read through [`read_setup()`](UsbBus::read_setup()) instead.
     ///
     /// Implementations may also return other errors if applicable.
     fn read(&self, ep_addr: EndpointAddress, buf: &mut [u8]) -> Result<usize>;
 
+    /// Reads a packet of SETUP data from the specified endpoint, returns the length of the packet
+    ///
+    /// This is a distinct method from [`read()`](UsbBus::read()) because the USB spec states that
+    /// the function (device) must accept the SETUP transaction, which in some implementations can
+    /// result in a SETUP transaction overwriting data from an earlier OUT transaction.  Separate
+    /// read methods allow the control pipe implementation to detect this overwriting.
+    ///
+    /// # Errors
+    ///
+    /// * [`InvalidEndpoint`](crate::UsbError::InvalidEndpoint) - The `ep_addr` does not point to a
+    ///   valid endpoint that was previously allocated with [`UsbBus::alloc_ep`].
+    /// * [`WouldBlock`](crate::UsbError::WouldBlock) - There is no SETUP packet to be read. Note
+    ///   that this is different from a received zero-length packet, which is valid in USB. A
+    ///   zero-length packet will return `Ok(0)`.
+    /// * [`BufferOverflow`](crate::UsbError::BufferOverflow) - The received packet is too long to
+    ///   fit in `buf`. This is generally an error in the class implementation, because the class
+    ///   should use a buffer that is large enough for the `max_packet_size` it specified when
+    ///   allocating the endpoint.
+    fn read_setup(&self, ep_addr: EndpointAddress, buf: &mut [u8]) -> Result<usize>;
+
     /// Sets or clears the STALL condition for an endpoint. If the endpoint is an OUT endpoint, it
     /// should be prepared to receive data again.
     fn set_stalled(&self, ep_addr: EndpointAddress, stalled: bool);

src/class.rs

@@ -8,8 +8,7 @@ use crate::{Result, UsbError};
 
 /// A trait for implementing USB classes.
 ///
-/// All methods are optional callbacks that will be called by
-/// [UsbBus::poll](crate::bus::UsbBus::poll)
+/// All methods are optional callbacks that will be called by [UsbBus::poll]
 pub trait UsbClass<B: UsbBus> {
     /// Called when a GET_DESCRIPTOR request is received for a configuration descriptor. When
     /// called, the implementation should write its interface, endpoint and any extra class

src/control_pipe.rs

@@ -65,7 +65,7 @@ impl<B: UsbBus> ControlPipe<'_, B> {
     }
 
     pub fn handle_setup(&mut self) -> Option<Request> {
-        let count = match self.ep_out.read(&mut self.buf[..]) {
+        let count = match self.ep_out.read_setup(&mut self.buf[..]) {
             Ok(count) => {
                 usb_trace!("Read {} bytes on EP0-OUT: {:?}", count, &self.buf[..count]);
                 count
@@ -165,7 +165,15 @@ impl<B: UsbBus> ControlPipe<'_, B> {
                     "Control transfer completed. Current state: {:?}",
                     self.state
                 );
-                self.ep_out.read(&mut [])?;
+                match self.ep_out.read(&mut []) {
+                    Ok(_) => {}
+                    Err(UsbError::InvalidState) => {
+                        // Host sent a new SETUP transaction, which may have overwritten the ZLP
+                    }
+                    Err(err) => {
+                        return Err(err);
+                    }
+                }
                 self.state = ControlState::Idle;
             }
             _ => {

src/dummy.rs

@@ -56,6 +56,14 @@ impl UsbBus for DummyUsbBus {
         unimplemented!()
     }
 
+    fn read_setup(
+        &self,
+        ep_addr: crate::class_prelude::EndpointAddress,
+        buf: &mut [u8],
+    ) -> crate::Result<usize> {
+        unimplemented!()
+    }
+
     fn reset(&self) {
         unimplemented!()
     }

src/endpoint.rs

@@ -1,5 +1,5 @@
 use crate::bus::UsbBus;
-use crate::{Result, UsbDirection};
+use crate::{Result, UsbDirection, UsbError};
 use core::marker::PhantomData;
 use portable_atomic::{AtomicPtr, Ordering};
 
@@ -197,9 +197,10 @@ impl<B: UsbBus> Endpoint<'_, B, In> {
 }
 
 impl<B: UsbBus> Endpoint<'_, B, Out> {
-    /// Reads a single packet of data from the specified endpoint and returns the actual length of
-    /// the packet. The buffer should be large enough to fit at least as many bytes as the
-    /// `max_packet_size` specified when allocating the endpoint.
+    /// Reads a single packet of data and returns the length of the packet
+    ///
+    /// The buffer should be large enough to fit at least as many bytes as the `max_packet_size`
+    /// specified when allocating the endpoint.
     ///
     /// # Errors
     ///
@@ -211,9 +212,37 @@ impl<B: UsbBus> Endpoint<'_, B, Out> {
     ///   USB. A zero-length packet will return `Ok(0)`.
     /// * [`BufferOverflow`](crate::UsbError::BufferOverflow) - The received packet is too long to
     ///   fit in `data`. This is generally an error in the class implementation.
+    /// * [`InvalidState`](crate::UsbError::InvalidState) - The received packet is a SETUP
+    ///   transaction, and needs to be read through [`read_setup()`](Endpoint::read_setup())
+    ///   instead.
     pub fn read(&self, data: &mut [u8]) -> Result<usize> {
         self.bus().read(self.address, data)
     }
+
+    /// Reads a single packet of SETUP data and returns the length of the packet
+    ///
+    /// The buffer should be large enough to fit at least as many bytes as the `max_packet_size`
+    /// specified when allocating the endpoint.  See [`UsbBus::read_setup()`] for rationale for two
+    /// distinct read methods.
+    ///
+    /// # Errors
+    ///
+    /// Note: USB bus implementation errors are directly passed through, so be prepared to handle
+    /// other errors as well.
+    ///
+    /// * [`WouldBlock`](crate::UsbError::WouldBlock) - There is no packet to be read. Note that
+    ///   this is different from a received zero-length packet, which is valid and significant in
+    ///   USB. A zero-length packet will return `Ok(0)`.
+    /// * [`BufferOverflow`](crate::UsbError::BufferOverflow) - The received packet is too long to
+    ///   fit in `data`. This is generally an error in the class implementation.
+    pub fn read_setup(&self, data: &mut [u8]) -> Result<usize> {
+        // SETUP transactions can only occur on control endpoints
+        if self.ep_type != EndpointType::Control {
+            Err(UsbError::InvalidEndpoint)
+        } else {
+            self.bus().read_setup(self.address, data)
+        }
+    }
 }
 
 /// Type-safe endpoint address.

src/lib.rs

@@ -258,6 +258,10 @@ fn _ensure_sync() {
             Err(UsbError::InvalidEndpoint)
         }
 
+        fn read_setup(&self, _ep_addr: EndpointAddress, _buf: &mut [u8]) -> Result<usize> {
+            Err(UsbError::InvalidEndpoint)
+        }
+
         fn set_stalled(&self, _ep_addr: EndpointAddress, _stalled: bool) {}
         fn is_stalled(&self, _ep_addr: EndpointAddress) -> bool {
             false

src/test_class.rs

@@ -5,6 +5,7 @@ use crate::device::{StringDescriptors, UsbDevice, UsbDeviceBuilder, UsbVidPid};
 use crate::Result;
 use core::cell::UnsafeCell;
 use core::cmp;
+use core::marker::PhantomData;
 
 #[cfg(feature = "test-class-high-speed")]
 mod sizes {
@@ -24,9 +25,14 @@ mod sizes {
 
 static mut CONTROL_BUFFER: UnsafeCell<[u8; 256]> = UnsafeCell::new([0; 256]);
 
+pub trait HardwareSupport {
+    /// Hard reset the test device
+    fn hard_reset() -> !;
+}
+
 /// 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> {
+pub struct TestClass<'a, B: UsbBus, H: HardwareSupport> {
     custom_string: StringIndex,
     interface_string: StringIndex,
     iface: InterfaceNumber,
@@ -45,6 +51,7 @@ pub struct TestClass<'a, B: UsbBus> {
     expect_bulk_out: bool,
     expect_interrupt_in_complete: bool,
     expect_interrupt_out: bool,
+    hardware: PhantomData<H>,
 }
 
 pub const VID: u16 = 0x16c0;
@@ -60,13 +67,14 @@ pub const REQ_READ_BUFFER: u8 = 2;
 pub const REQ_WRITE_BUFFER: u8 = 3;
 pub const REQ_SET_BENCH_ENABLED: u8 = 4;
 pub const REQ_READ_LONG_DATA: u8 = 5;
+pub const REQ_HARD_RESET: u8 = 6;
 pub const REQ_UNKNOWN: u8 = 42;
 
 pub const LONG_DATA: &[u8] = &[0x17; 257];
 
-impl<B: UsbBus> TestClass<'_, B> {
+impl<B: UsbBus, H: HardwareSupport> TestClass<'_, B, H> {
     /// Creates a new TestClass.
-    pub fn new(alloc: &UsbBusAllocator<B>) -> TestClass<'_, B> {
+    pub fn new(alloc: &UsbBusAllocator<B>) -> TestClass<'_, B, H> {
         TestClass {
             custom_string: alloc.string(),
             interface_string: alloc.string(),
@@ -91,6 +99,7 @@ impl<B: UsbBus> TestClass<'_, B> {
             expect_bulk_out: false,
             expect_interrupt_in_complete: false,
             expect_interrupt_out: false,
+            hardware: PhantomData,
         }
     }
 
@@ -214,7 +223,7 @@ impl<B: UsbBus> TestClass<'_, B> {
     }
 }
 
-impl<B: UsbBus> UsbClass<B> for TestClass<'_, B> {
+impl<B: UsbBus, H: HardwareSupport> UsbClass<B> for TestClass<'_, B, H> {
     fn reset(&mut self) {
         self.len = 0;
         self.i = 0;
@@ -335,6 +344,9 @@ impl<B: UsbBus> UsbClass<B> for TestClass<'_, B> {
                 xfer.accept()
                     .expect("control_out REQ_SET_BENCH_ENABLED failed");
             }
+            REQ_HARD_RESET => {
+                H::hard_reset();
+            }
             _ => xfer.reject().expect("control_out reject failed"),
         }
     }