From a6cef4baf220409bb20d81af538f2c507ec4c4c9 Mon Sep 17 00:00:00 2001 From: James Munns Date: Mon, 27 Mar 2023 14:19:00 +0200 Subject: [PATCH 1/3] Add logging and interface for debugging buffer usage --- embassy-usb/src/builder.rs | 14 ++++++++++++++ embassy-usb/src/class/hid.rs | 3 +++ embassy-usb/src/lib.rs | 37 ++++++++++++++++++++++++++++++++++++ embassy-usb/src/msos.rs | 5 +++++ 4 files changed, 59 insertions(+) diff --git a/embassy-usb/src/builder.rs b/embassy-usb/src/builder.rs index 305dfa02..6649cd5b 100644 --- a/embassy-usb/src/builder.rs +++ b/embassy-usb/src/builder.rs @@ -201,6 +201,20 @@ impl<'d, D: Driver<'d>> Builder<'d, D> { self.config_descriptor.end_configuration(); self.bos_descriptor.end_bos(); + info!("USB: device_descriptor used: {}", self.device_descriptor.position()); + info!("USB: config_descriptor used: {}", self.config_descriptor.position()); + info!("USB: bos_descriptor_buf used: {}", self.bos_descriptor.writer.position()); + #[cfg(feature = "msos-descriptor")] + info!("USB: device_descriptor used: {}", msos_descriptor.len()); + if self.control_buf.len() != self.config.max_packet_size_0.into() { + warn!( + "Mismatch in control buf and max packet size! buf len: {}, max ep0 size: {}", + self.control_buf.len(), + self.config.max_packet_size_0, + ); + } + info!("USB: device_descriptor used: {}", self.config_descriptor.position()); + UsbDevice::build( self.driver, self.config, diff --git a/embassy-usb/src/class/hid.rs b/embassy-usb/src/class/hid.rs index 974268c6..59740342 100644 --- a/embassy-usb/src/class/hid.rs +++ b/embassy-usb/src/class/hid.rs @@ -458,6 +458,9 @@ impl<'d> Handler for Control<'d> { return None; } + // TODO(AJM): This uses a defmt-specific formatter that causes use of the `log` + // feature to fail to build + #[cfg(feature = "defmt")] trace!("HID control_out {:?} {=[u8]:x}", req, data); match req.request { HID_REQ_SET_IDLE => { diff --git a/embassy-usb/src/lib.rs b/embassy-usb/src/lib.rs index bfeccd5f..3016b81c 100644 --- a/embassy-usb/src/lib.rs +++ b/embassy-usb/src/lib.rs @@ -165,6 +165,25 @@ struct Interface { num_alt_settings: u8, } +/// A report of the used size of the runtime allocated buffers +#[derive(PartialEq, Eq, Copy, Clone, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub struct UsbBufferReport { + /// Number of device descriptor bytes used + pub device_descriptor_used: usize, + /// Number of config descriptor bytes used + pub config_descriptor_used: usize, + /// Number of bos descriptor bytes used + pub bos_descriptor_used: usize, + /// Number of msos descriptor bytes used + /// + /// Will be `None` if the "msos-descriptor" feature is not active. + /// Otherwise will return Some(bytes). + pub msos_descriptor_used: Option, + /// Size of the control buffer + pub control_buffer_size: usize, +} + /// Main struct for the USB device stack. pub struct UsbDevice<'d, D: Driver<'d>> { control_buf: &'d mut [u8], @@ -239,6 +258,24 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> { } } + /// Returns a report of the consumed buffers + /// + /// Useful for tuning buffer sizes for actual usage + pub fn buffer_usage(&self) -> UsbBufferReport { + #[cfg(not(feature = "msos-descriptor"))] + let mdu = None; + #[cfg(feature = "msos-descriptor")] + let mdu = Some(self.inner.msos_descriptor.len()); + + UsbBufferReport { + device_descriptor_used: self.inner.device_descriptor.len(), + config_descriptor_used: self.inner.config_descriptor.len(), + bos_descriptor_used: self.inner.bos_descriptor.len(), + msos_descriptor_used: mdu, + control_buffer_size: self.control_buf.len(), + } + } + /// Runs the `UsbDevice` forever. /// /// This future may leave the bus in an invalid state if it is dropped. diff --git a/embassy-usb/src/msos.rs b/embassy-usb/src/msos.rs index b1e0335e..218d9931 100644 --- a/embassy-usb/src/msos.rs +++ b/embassy-usb/src/msos.rs @@ -32,6 +32,11 @@ impl<'d> MsOsDescriptorSet<'d> { pub fn is_empty(&self) -> bool { self.descriptor.is_empty() } + + /// Returns the length of the descriptor field + pub fn len(&self) -> usize { + self.descriptor.len() + } } /// Writes a Microsoft OS 2.0 Descriptor set into a buffer. From a77fdefd7c16cb6077d4f27ec5094e52457fcde5 Mon Sep 17 00:00:00 2001 From: James Munns Date: Mon, 27 Mar 2023 15:37:12 +0200 Subject: [PATCH 2/3] Correct copy/paste errors --- embassy-usb/src/builder.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/embassy-usb/src/builder.rs b/embassy-usb/src/builder.rs index 6649cd5b..cad1ecda 100644 --- a/embassy-usb/src/builder.rs +++ b/embassy-usb/src/builder.rs @@ -201,19 +201,21 @@ impl<'d, D: Driver<'d>> Builder<'d, D> { self.config_descriptor.end_configuration(); self.bos_descriptor.end_bos(); + // Log the number of allocator bytes actually used in descriptor buffers info!("USB: device_descriptor used: {}", self.device_descriptor.position()); info!("USB: config_descriptor used: {}", self.config_descriptor.position()); - info!("USB: bos_descriptor_buf used: {}", self.bos_descriptor.writer.position()); + info!("USB: bos_descriptor used: {}", self.bos_descriptor.writer.position()); #[cfg(feature = "msos-descriptor")] - info!("USB: device_descriptor used: {}", msos_descriptor.len()); + info!("USB: msos_descriptor used: {}", msos_descriptor.len()); if self.control_buf.len() != self.config.max_packet_size_0.into() { warn!( - "Mismatch in control buf and max packet size! buf len: {}, max ep0 size: {}", + "USB: Mismatch in control buf and max packet size! buf len: {}, max ep0 size: {}", self.control_buf.len(), self.config.max_packet_size_0, ); + } else { + info!("USB: control_buf size: {}", self.control_buf.len()); } - info!("USB: device_descriptor used: {}", self.config_descriptor.position()); UsbDevice::build( self.driver, From 20aa86d63e6967989ab4d70a638cfe8a0a62a2ca Mon Sep 17 00:00:00 2001 From: James Munns Date: Mon, 27 Mar 2023 18:21:11 +0200 Subject: [PATCH 3/3] Address review comments --- embassy-usb/src/builder.rs | 10 +--------- embassy-usb/src/class/hid.rs | 4 ++-- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/embassy-usb/src/builder.rs b/embassy-usb/src/builder.rs index cad1ecda..6b68bcd7 100644 --- a/embassy-usb/src/builder.rs +++ b/embassy-usb/src/builder.rs @@ -207,15 +207,7 @@ impl<'d, D: Driver<'d>> Builder<'d, D> { info!("USB: bos_descriptor used: {}", self.bos_descriptor.writer.position()); #[cfg(feature = "msos-descriptor")] info!("USB: msos_descriptor used: {}", msos_descriptor.len()); - if self.control_buf.len() != self.config.max_packet_size_0.into() { - warn!( - "USB: Mismatch in control buf and max packet size! buf len: {}, max ep0 size: {}", - self.control_buf.len(), - self.config.max_packet_size_0, - ); - } else { - info!("USB: control_buf size: {}", self.control_buf.len()); - } + info!("USB: control_buf size: {}", self.control_buf.len()); UsbDevice::build( self.driver, diff --git a/embassy-usb/src/class/hid.rs b/embassy-usb/src/class/hid.rs index 59740342..03e4c1db 100644 --- a/embassy-usb/src/class/hid.rs +++ b/embassy-usb/src/class/hid.rs @@ -458,8 +458,8 @@ impl<'d> Handler for Control<'d> { return None; } - // TODO(AJM): This uses a defmt-specific formatter that causes use of the `log` - // feature to fail to build + // This uses a defmt-specific formatter that causes use of the `log` + // feature to fail to build, so leave it defmt-specific for now. #[cfg(feature = "defmt")] trace!("HID control_out {:?} {=[u8]:x}", req, data); match req.request {