usb: improved descriptor building API

The same API call allocates interfaces/endpoints/etc and writes their descriptors.
This means less API calls, and less possibility to screw things up.

DescriptorWriter is now private.

embassy-usb-hid/src/lib.rs

@@ -110,23 +110,13 @@ fn build<'d, D: Driver<'d>>(
     ));
 
     let len = config.report_descriptor.len();
-    let if_num = builder.alloc_interface_with_handler(control);
-    let ep_in = builder.alloc_interrupt_endpoint_in(config.max_packet_size, config.poll_ms);
-    let ep_out = if with_out_endpoint {
-        Some(builder.alloc_interrupt_endpoint_out(config.max_packet_size, config.poll_ms))
-    } else {
-        None
-    };
 
-    builder.config_descriptor.interface(
+    let mut func = builder.function(USB_CLASS_HID, USB_SUBCLASS_NONE, USB_PROTOCOL_NONE);
-        if_num,
+    let mut iface = func.interface(Some(control));
-        USB_CLASS_HID,
+    let mut alt = iface.alt_setting(USB_CLASS_HID, USB_SUBCLASS_NONE, USB_PROTOCOL_NONE);
-        USB_SUBCLASS_NONE,
-        USB_PROTOCOL_NONE,
-    );
 
     // HID descriptor
-    builder.config_descriptor.write(
+    alt.descriptor(
         HID_DESC_DESCTYPE_HID,
         &[
             // HID Class spec version
@@ -144,10 +134,12 @@ fn build<'d, D: Driver<'d>>(
         ],
     );
 
-    builder.config_descriptor.endpoint(ep_in.info());
+    let ep_in = alt.endpoint_interrupt_in(config.max_packet_size, config.poll_ms);
-    if let Some(ep_out) = &ep_out {
+    let ep_out = if with_out_endpoint {
-        builder.config_descriptor.endpoint(ep_out.info());
+        Some(alt.endpoint_interrupt_out(config.max_packet_size, config.poll_ms))
-    }
+    } else {
+        None
+    };
 
     (ep_out, ep_in, &state.out_report_offset)
 }

embassy-usb-serial/src/lib.rs

@@ -175,26 +175,15 @@ impl<'d, D: Driver<'d>> CdcAcmClass<'d, D> {
 
         assert!(builder.control_buf_len() >= 7);
 
-        let comm_if = builder.alloc_interface_with_handler(control);
+        let mut func = builder.function(USB_CLASS_CDC, CDC_SUBCLASS_ACM, CDC_PROTOCOL_NONE);
-        let comm_ep = builder.alloc_interrupt_endpoint_in(8, 255);
-        let data_if = builder.alloc_interface();
-        let read_ep = builder.alloc_bulk_endpoint_out(max_packet_size);
-        let write_ep = builder.alloc_bulk_endpoint_in(max_packet_size);
 
-        builder.config_descriptor.iad(
+        // Control interface
-            comm_if,
+        let mut iface = func.interface(Some(control));
-            2,
+        let comm_if = iface.interface_number();
-            USB_CLASS_CDC,
+        let data_if = u8::from(comm_if) + 1;
-            CDC_SUBCLASS_ACM,
+        let mut alt = iface.alt_setting(USB_CLASS_CDC, CDC_SUBCLASS_ACM, CDC_PROTOCOL_NONE);
-            CDC_PROTOCOL_NONE,
+
-        );
+        alt.descriptor(
-        builder.config_descriptor.interface(
-            comm_if,
-            USB_CLASS_CDC,
-            CDC_SUBCLASS_ACM,
-            CDC_PROTOCOL_NONE,
-        );
-        builder.config_descriptor.write(
             CS_INTERFACE,
             &[
                 CDC_TYPE_HEADER, // bDescriptorSubtype
@@ -202,14 +191,14 @@ impl<'d, D: Driver<'d>> CdcAcmClass<'d, D> {
                 0x01, // bcdCDC (1.10)
             ],
         );
-        builder.config_descriptor.write(
+        alt.descriptor(
             CS_INTERFACE,
             &[
                 CDC_TYPE_ACM, // bDescriptorSubtype
                 0x00,         // bmCapabilities
             ],
         );
-        builder.config_descriptor.write(
+        alt.descriptor(
             CS_INTERFACE,
             &[
                 CDC_TYPE_UNION, // bDescriptorSubtype
@@ -217,7 +206,7 @@ impl<'d, D: Driver<'d>> CdcAcmClass<'d, D> {
                 data_if.into(), // bSubordinateInterface
             ],
         );
-        builder.config_descriptor.write(
+        alt.descriptor(
             CS_INTERFACE,
             &[
                 CDC_TYPE_CALL_MANAGEMENT, // bDescriptorSubtype
@@ -225,13 +214,15 @@ impl<'d, D: Driver<'d>> CdcAcmClass<'d, D> {
                 data_if.into(),           // bDataInterface
             ],
         );
-        builder.config_descriptor.endpoint(comm_ep.info());
 
-        builder
+        let comm_ep = alt.endpoint_interrupt_in(8, 255);
-            .config_descriptor
+
-            .interface(data_if, USB_CLASS_CDC_DATA, 0x00, 0x00);
+        // Data interface
-        builder.config_descriptor.endpoint(write_ep.info());
+        let mut iface = func.interface(None);
-        builder.config_descriptor.endpoint(read_ep.info());
+        let data_if = iface.interface_number();
+        let mut alt = iface.alt_setting(USB_CLASS_CDC_DATA, 0x00, CDC_PROTOCOL_NONE);
+        let read_ep = alt.endpoint_bulk_out(max_packet_size);
+        let write_ep = alt.endpoint_bulk_in(max_packet_size);
 
         CdcAcmClass {
             _comm_ep: comm_ep,

embassy-usb/src/builder.rs

@@ -2,7 +2,7 @@ use heapless::Vec;
 
 use super::control::ControlHandler;
 use super::descriptor::{BosWriter, DescriptorWriter};
-use super::driver::{Driver, EndpointAllocError};
+use super::driver::{Driver, Endpoint};
 use super::types::*;
 use super::DeviceStateHandler;
 use super::UsbDevice;
@@ -117,7 +117,7 @@ impl<'a> Config<'a> {
     }
 }
 
-/// Used to build new [`UsbDevice`]s.
+/// [`UsbDevice`] builder.
 pub struct Builder<'d, D: Driver<'d>> {
     config: Config<'d>,
     handler: Option<&'d dyn DeviceStateHandler>,
@@ -128,10 +128,9 @@ pub struct Builder<'d, D: Driver<'d>> {
     next_interface_number: u8,
     next_string_index: u8,
 
-    // TODO make not pub?
+    device_descriptor: DescriptorWriter<'d>,
-    pub device_descriptor: DescriptorWriter<'d>,
+    config_descriptor: DescriptorWriter<'d>,
-    pub config_descriptor: DescriptorWriter<'d>,
+    bos_descriptor: BosWriter<'d>,
-    pub bos_descriptor: BosWriter<'d>,
 }
 
 impl<'d, D: Driver<'d>> Builder<'d, D> {
@@ -207,36 +206,12 @@ impl<'d, D: Driver<'d>> Builder<'d, D> {
         )
     }
 
-    /// Allocates a new interface number.
-    pub fn alloc_interface(&mut self) -> InterfaceNumber {
-        let number = self.next_interface_number;
-        self.next_interface_number += 1;
-
-        InterfaceNumber::new(number)
-    }
-
     /// Returns the size of the control request data buffer. Can be used by
     /// classes to validate the buffer is large enough for their needs.
     pub fn control_buf_len(&self) -> usize {
         self.control_buf.len()
     }
 
-    /// Allocates a new interface number, with a handler that will be called
-    /// for all the control requests directed to it.
-    pub fn alloc_interface_with_handler(
-        &mut self,
-        handler: &'d mut dyn ControlHandler,
-    ) -> InterfaceNumber {
-        let number = self.next_interface_number;
-        self.next_interface_number += 1;
-
-        if self.interfaces.push((number, handler)).is_err() {
-            panic!("max class count reached")
-        }
-
-        InterfaceNumber::new(number)
-    }
-
     /// Allocates a new string index.
     pub fn alloc_string(&mut self) -> StringIndex {
         let index = self.next_string_index;
@@ -245,146 +220,212 @@ impl<'d, D: Driver<'d>> Builder<'d, D> {
         StringIndex::new(index)
     }
 
-    /// Allocates an in endpoint.
+    /// Add an USB function.
     ///
-    /// This directly delegates to [`Driver::alloc_endpoint_in`], so see that method for details. In most
+    /// If [`Config::composite_with_iads`] is set, this will add an IAD descriptor
-    /// cases classes should call the endpoint type specific methods instead.
+    /// with the given class/subclass/protocol, associating all the child interfaces.
-    pub fn alloc_endpoint_in(
+    ///
+    /// If it's not set, no IAD descriptor is added.
+    pub fn function(
+        &mut self,
+        class: u8,
+        subclass: u8,
+        protocol: u8,
+    ) -> FunctionBuilder<'_, 'd, D> {
+        let iface_count_index = if self.config.composite_with_iads {
+            self.config_descriptor.iad(
+                InterfaceNumber::new(self.next_interface_number),
+                0,
+                class,
+                subclass,
+                protocol,
+            );
+
+            Some(self.config_descriptor.position() - 5)
+        } else {
+            None
+        };
+
+        FunctionBuilder {
+            builder: self,
+            iface_count_index,
+        }
+    }
+}
+
+/// Function builder.
+///
+/// A function is a logical grouping of interfaces that perform a given USB function.
+/// If [`Config::composite_with_iads`] is set, each function will have an IAD descriptor.
+/// If not, functions will not be visible as descriptors.
+pub struct FunctionBuilder<'a, 'd, D: Driver<'d>> {
+    builder: &'a mut Builder<'d, D>,
+    iface_count_index: Option<usize>,
+}
+
+impl<'a, 'd, D: Driver<'d>> FunctionBuilder<'a, 'd, D> {
+    /// Add an interface to the function.
+    ///
+    /// Interface numbers are guaranteed to be allocated consecutively, starting from 0.
+    pub fn interface(
+        &mut self,
+        handler: Option<&'d mut dyn ControlHandler>,
+    ) -> InterfaceBuilder<'_, 'd, D> {
+        if let Some(i) = self.iface_count_index {
+            self.builder.config_descriptor.buf[i] += 1;
+        }
+
+        let number = self.builder.next_interface_number;
+        self.builder.next_interface_number += 1;
+
+        if let Some(handler) = handler {
+            if self.builder.interfaces.push((number, handler)).is_err() {
+                panic!("max interface count reached")
+            }
+        }
+
+        InterfaceBuilder {
+            builder: self.builder,
+            interface_number: InterfaceNumber::new(number),
+            next_alt_setting_number: 0,
+        }
+    }
+}
+
+/// Interface builder.
+pub struct InterfaceBuilder<'a, 'd, D: Driver<'d>> {
+    builder: &'a mut Builder<'d, D>,
+    interface_number: InterfaceNumber,
+    next_alt_setting_number: u8,
+}
+
+impl<'a, 'd, D: Driver<'d>> InterfaceBuilder<'a, 'd, D> {
+    /// Get the interface number.
+    pub fn interface_number(&self) -> InterfaceNumber {
+        self.interface_number
+    }
+
+    /// Add an alternate setting to the interface and write its descriptor.
+    ///
+    /// Alternate setting numbers are guaranteed to be allocated consecutively, starting from 0.
+    ///
+    /// The first alternate setting, with number 0, is the default one.
+    pub fn alt_setting(
+        &mut self,
+        class: u8,
+        subclass: u8,
+        protocol: u8,
+    ) -> InterfaceAltBuilder<'_, 'd, D> {
+        let number = self.next_alt_setting_number;
+        self.next_alt_setting_number += 1;
+
+        self.builder.config_descriptor.interface_alt(
+            self.interface_number,
+            number,
+            class,
+            subclass,
+            protocol,
+            None,
+        );
+
+        InterfaceAltBuilder {
+            builder: self.builder,
+            interface_number: self.interface_number,
+            alt_setting_number: number,
+        }
+    }
+}
+
+/// Interface alternate setting builder.
+pub struct InterfaceAltBuilder<'a, 'd, D: Driver<'d>> {
+    builder: &'a mut Builder<'d, D>,
+    interface_number: InterfaceNumber,
+    alt_setting_number: u8,
+}
+
+impl<'a, 'd, D: Driver<'d>> InterfaceAltBuilder<'a, 'd, D> {
+    /// Get the interface number.
+    pub fn interface_number(&self) -> InterfaceNumber {
+        self.interface_number
+    }
+
+    /// Get the alternate setting number.
+    pub fn alt_setting_number(&self) -> u8 {
+        self.alt_setting_number
+    }
+
+    /// Add a custom descriptor to this alternate setting.
+    ///
+    /// Descriptors are written in the order builder functions are called. Note that some
+    /// classes care about the order.
+    pub fn descriptor(&mut self, descriptor_type: u8, descriptor: &[u8]) {
+        self.builder
+            .config_descriptor
+            .write(descriptor_type, descriptor)
+    }
+
+    fn endpoint_in(
         &mut self,
         ep_addr: Option<EndpointAddress>,
         ep_type: EndpointType,
         max_packet_size: u16,
         interval: u8,
-    ) -> Result<D::EndpointIn, EndpointAllocError> {
-        self.driver
-            .alloc_endpoint_in(ep_addr, ep_type, max_packet_size, interval)
-    }
-
-    /// Allocates an out endpoint.
-    ///
-    /// This directly delegates to [`Driver::alloc_endpoint_out`], so see that method for details. In most
-    /// cases classes should call the endpoint type specific methods instead.
-    pub fn alloc_endpoint_out(
-        &mut self,
-        ep_addr: Option<EndpointAddress>,
-        ep_type: EndpointType,
-        max_packet_size: u16,
-        interval: u8,
-    ) -> Result<D::EndpointOut, EndpointAllocError> {
-        self.driver
-            .alloc_endpoint_out(ep_addr, ep_type, max_packet_size, interval)
-    }
-
-    /// Allocates a control in endpoint.
-    ///
-    /// This crate implements the control state machine only for endpoint 0. If classes want to
-    /// support control requests in other endpoints, the state machine must be implemented manually.
-    /// This should rarely be needed by classes.
-    ///
-    /// # Arguments
-    ///
-    /// * `max_packet_size` - Maximum packet size in bytes. Must be one of 8, 16, 32 or 64.
-    ///
-    /// # Panics
-    ///
-    /// Panics if endpoint allocation fails, because running out of endpoints or memory is not
-    /// feasibly recoverable.
-    #[inline]
-    pub fn alloc_control_endpoint_in(&mut self, max_packet_size: u16) -> D::EndpointIn {
-        self.alloc_endpoint_in(None, EndpointType::Control, max_packet_size, 0)
-            .expect("alloc_ep failed")
-    }
-
-    /// Allocates a control out endpoint.
-    ///
-    /// This crate implements the control state machine only for endpoint 0. If classes want to
-    /// support control requests in other endpoints, the state machine must be implemented manually.
-    /// This should rarely be needed by classes.
-    ///
-    /// # Arguments
-    ///
-    /// * `max_packet_size` - Maximum packet size in bytes. Must be one of 8, 16, 32 or 64.
-    ///
-    /// # Panics
-    ///
-    /// Panics if endpoint allocation fails, because running out of endpoints or memory is not
-    /// feasibly recoverable.
-    #[inline]
-    pub fn alloc_control_pipe(&mut self, max_packet_size: u16) -> D::ControlPipe {
-        self.driver
-            .alloc_control_pipe(max_packet_size)
-            .expect("alloc_control_pipe failed")
-    }
-
-    /// Allocates a bulk in endpoint.
-    ///
-    /// # Arguments
-    ///
-    /// * `max_packet_size` - Maximum packet size in bytes. Must be one of 8, 16, 32 or 64.
-    ///
-    /// # Panics
-    ///
-    /// Panics if endpoint allocation fails, because running out of endpoints or memory is not
-    /// feasibly recoverable.
-    #[inline]
-    pub fn alloc_bulk_endpoint_in(&mut self, max_packet_size: u16) -> D::EndpointIn {
-        self.alloc_endpoint_in(None, EndpointType::Bulk, max_packet_size, 0)
-            .expect("alloc_ep failed")
-    }
-
-    /// Allocates a bulk out endpoint.
-    ///
-    /// # Arguments
-    ///
-    /// * `max_packet_size` - Maximum packet size in bytes. Must be one of 8, 16, 32 or 64.
-    ///
-    /// # Panics
-    ///
-    /// Panics if endpoint allocation fails, because running out of endpoints or memory is not
-    /// feasibly recoverable.
-    #[inline]
-    pub fn alloc_bulk_endpoint_out(&mut self, max_packet_size: u16) -> D::EndpointOut {
-        self.alloc_endpoint_out(None, EndpointType::Bulk, max_packet_size, 0)
-            .expect("alloc_ep failed")
-    }
-
-    /// Allocates a bulk in endpoint.
-    ///
-    /// # Arguments
-    ///
-    /// * `max_packet_size` - Maximum packet size in bytes. Cannot exceed 64 bytes.
-    ///
-    /// # Panics
-    ///
-    /// Panics if endpoint allocation fails, because running out of endpoints or memory is not
-    /// feasibly recoverable.
-    #[inline]
-    pub fn alloc_interrupt_endpoint_in(
-        &mut self,
-        max_packet_size: u16,
-        interval: u8,
     ) -> D::EndpointIn {
-        self.alloc_endpoint_in(None, EndpointType::Interrupt, max_packet_size, interval)
+        let ep = self
-            .expect("alloc_ep failed")
+            .builder
+            .driver
+            .alloc_endpoint_in(ep_addr, ep_type, max_packet_size, interval)
+            .expect("alloc_endpoint_in failed");
+
+        self.builder.config_descriptor.endpoint(ep.info());
+
+        ep
     }
 
-    /// Allocates a bulk in endpoint.
+    fn endpoint_out(
-    ///
-    /// # Arguments
-    ///
-    /// * `max_packet_size` - Maximum packet size in bytes. Cannot exceed 64 bytes.
-    ///
-    /// # Panics
-    ///
-    /// Panics if endpoint allocation fails, because running out of endpoints or memory is not
-    /// feasibly recoverable.
-    #[inline]
-    pub fn alloc_interrupt_endpoint_out(
         &mut self,
+        ep_addr: Option<EndpointAddress>,
+        ep_type: EndpointType,
         max_packet_size: u16,
         interval: u8,
     ) -> D::EndpointOut {
-        self.alloc_endpoint_out(None, EndpointType::Interrupt, max_packet_size, interval)
+        let ep = self
-            .expect("alloc_ep failed")
+            .builder
+            .driver
+            .alloc_endpoint_out(ep_addr, ep_type, max_packet_size, interval)
+            .expect("alloc_endpoint_out failed");
+
+        self.builder.config_descriptor.endpoint(ep.info());
+
+        ep
+    }
+
+    /// Allocate a BULK IN endpoint and write its descriptor.
+    ///
+    /// Descriptors are written in the order builder functions are called. Note that some
+    /// classes care about the order.
+    pub fn endpoint_bulk_in(&mut self, max_packet_size: u16) -> D::EndpointIn {
+        self.endpoint_in(None, EndpointType::Bulk, max_packet_size, 0)
+    }
+
+    /// Allocate a BULK OUT endpoint and write its descriptor.
+    ///
+    /// Descriptors are written in the order builder functions are called. Note that some
+    /// classes care about the order.
+    pub fn endpoint_bulk_out(&mut self, max_packet_size: u16) -> D::EndpointOut {
+        self.endpoint_out(None, EndpointType::Bulk, max_packet_size, 0)
+    }
+
+    /// Allocate a INTERRUPT IN endpoint and write its descriptor.
+    ///
+    /// Descriptors are written in the order builder functions are called. Note that some
+    /// classes care about the order.
+    pub fn endpoint_interrupt_in(&mut self, max_packet_size: u16, interval: u8) -> D::EndpointIn {
+        self.endpoint_in(None, EndpointType::Interrupt, max_packet_size, interval)
+    }
+
+    /// Allocate a INTERRUPT OUT endpoint and write its descriptor.
+    pub fn endpoint_interrupt_out(&mut self, max_packet_size: u16, interval: u8) -> D::EndpointOut {
+        self.endpoint_out(None, EndpointType::Interrupt, max_packet_size, interval)
     }
 }

embassy-usb/src/descriptor.rs

@@ -33,12 +33,11 @@ pub mod capability_type {
 }
 
 /// A writer for USB descriptors.
-pub struct DescriptorWriter<'a> {
+pub(crate) struct DescriptorWriter<'a> {
-    buf: &'a mut [u8],
+    pub buf: &'a mut [u8],
     position: usize,
     num_interfaces_mark: Option<usize>,
     num_endpoints_mark: Option<usize>,
-    write_iads: bool,
 }
 
 impl<'a> DescriptorWriter<'a> {
@@ -48,7 +47,6 @@ impl<'a> DescriptorWriter<'a> {
             position: 0,
             num_interfaces_mark: None,
             num_endpoints_mark: None,
-            write_iads: false,
         }
     }
 
@@ -106,8 +104,6 @@ impl<'a> DescriptorWriter<'a> {
     pub(crate) fn configuration(&mut self, config: &Config) {
         self.num_interfaces_mark = Some(self.position + 4);
 
-        self.write_iads = config.composite_with_iads;
-
         self.write(
             descriptor_type::CONFIGURATION,
             &[
@@ -160,10 +156,6 @@ impl<'a> DescriptorWriter<'a> {
         function_sub_class: u8,
         function_protocol: u8,
     ) {
-        if !self.write_iads {
-            return;
-        }
-
         self.write(
             descriptor_type::IAD,
             &[
@@ -177,33 +169,6 @@ impl<'a> DescriptorWriter<'a> {
         );
     }
 
-    /// Writes a interface descriptor.
-    ///
-    /// # Arguments
-    ///
-    /// * `number` - Interface number previously allocated with
-    ///   [`UsbDeviceBuilder::interface`](crate::bus::UsbDeviceBuilder::interface).
-    /// * `interface_class` - Class code assigned by USB.org. Use `0xff` for vendor-specific devices
-    ///   that do not conform to any class.
-    /// * `interface_sub_class` - Sub-class code. Depends on class.
-    /// * `interface_protocol` - Protocol code. Depends on class and sub-class.
-    pub fn interface(
-        &mut self,
-        number: InterfaceNumber,
-        interface_class: u8,
-        interface_sub_class: u8,
-        interface_protocol: u8,
-    ) {
-        self.interface_alt(
-            number,
-            DEFAULT_ALTERNATE_SETTING,
-            interface_class,
-            interface_sub_class,
-            interface_protocol,
-            None,
-        )
-    }
-
     /// Writes a interface descriptor with a specific alternate setting and
     /// interface string identifier.
     ///