review comments

10 files changed, 78 insertions, 114 deletions

diff --git a/embassy-usb/src/class/hid.rs b/embassy-usb/src/class/hid.rs
index b9830baeb..6723afbbc 100644
--- a/embassy-usb/src/class/hid.rs
+++ b/embassy-usb/src/class/hid.rs
@@ -8,8 +8,6 @@ use core::sync::atomic::{AtomicUsize, Ordering};
 use ssmarshal::serialize;
 #[cfg(feature = "usbd-hid")]
 use usbd_hid::descriptor::AsInputReport;
-#[cfg(feature = "usbd-hid")]
-use usbd_hid::hid_class::HidProtocolMode;
 
 use crate::control::{InResponse, OutResponse, Recipient, Request, RequestType};
 use crate::driver::{Driver, Endpoint, EndpointError, EndpointIn, EndpointOut};
@@ -18,13 +16,6 @@ use crate::{Builder, Handler};
 
 const USB_CLASS_HID: u8 = 0x03;
 
-const USB_SUBCLASS_REPORT_ONLY: u8 = 0x00;
-const USB_SUBCLASS_BOOT_OR_REPORT: u8 = 0x01;
-
-const USB_PROTOCOL_NONE: u8 = 0x00;
-const USB_PROTOCOL_KEYBOARD: u8 = 0x01;
-const USB_PROTOCOL_MOUSE: u8 = 0x02;
-
 // HID
 const HID_DESC_DESCTYPE_HID: u8 = 0x21;
 const HID_DESC_DESCTYPE_HID_REPORT: u8 = 0x22;
@@ -40,7 +31,6 @@ const HID_REQ_SET_PROTOCOL: u8 = 0x0b;
 
 /// Get/Set Protocol mapping
 /// See (7.2.5 and 7.2.6): <https://www.usb.org/sites/default/files/hid1_11.pdf>
-#[cfg(not(feature = "usbd-hid"))]
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 #[repr(u8)]
@@ -51,7 +41,6 @@ pub enum HidProtocolMode {
     Report = 1,
 }
 
-#[cfg(not(feature = "usbd-hid"))]
 impl From<u8> for HidProtocolMode {
     fn from(mode: u8) -> HidProtocolMode {
         if mode == HidProtocolMode::Boot as u8 {
@@ -62,6 +51,30 @@ impl From<u8> for HidProtocolMode {
     }
 }
 
+/// USB HID interface subclass values.
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+#[repr(u8)]
+pub enum HidSubclass {
+    /// Only report mode is supported.
+    ReportOnly = 0,
+    /// Both boot and report mode are supported.
+    ReportOrBoot = 1,
+}
+
+/// USB HID protocol values.
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+#[repr(u8)]
+pub enum HidBootProtocol {
+    /// No specific boot protocol.
+    None = 0,
+    /// Boot protocol keyboard.
+    Keyboard = 1,
+    /// Boot protocol mouse.
+    Mouse = 2,
+}
+
 /// Configuration for the HID class.
 pub struct Config<'d> {
     /// HID report descriptor.
@@ -79,6 +92,12 @@ pub struct Config<'d> {
 
     /// Max packet size for both the IN and OUT endpoints.
     pub max_packet_size: u16,
+
+    /// The HID subclass of this interface
+    pub hid_subclass: HidSubclass,
+
+    /// The HID boot protocol of this interface
+    pub hid_boot_protocol: HidBootProtocol,
 }
 
 /// Report ID
@@ -137,15 +156,18 @@ fn build<'d, D: Driver<'d>>(
     state: &'d mut State<'d>,
     config: Config<'d>,
     with_out_endpoint: bool,
-    usb_subclass: u8,
-    usb_protocol: u8,
 ) -> (Option<D::EndpointOut>, D::EndpointIn, &'d AtomicUsize) {
     let len = config.report_descriptor.len();
 
-    let mut func = builder.function(USB_CLASS_HID, usb_subclass, usb_protocol);
+    let mut func = builder.function(USB_CLASS_HID, config.hid_subclass as u8, config.hid_boot_protocol as u8);
     let mut iface = func.interface();
     let if_num = iface.interface_number();
-    let mut alt = iface.alt_setting(USB_CLASS_HID, usb_subclass, usb_protocol, None);
+    let mut alt = iface.alt_setting(
+        USB_CLASS_HID,
+        config.hid_subclass as u8,
+        config.hid_boot_protocol as u8,
+        None,
+    );
 
     // HID descriptor
     alt.descriptor(
@@ -193,42 +215,7 @@ impl<'d, D: Driver<'d>, const READ_N: usize, const WRITE_N: usize> HidReaderWrit
     /// HID reports, consider using [`HidWriter::new`] instead, which allocates an IN endpoint only.
     ///
     pub fn new(builder: &mut Builder<'d, D>, state: &'d mut State<'d>, config: Config<'d>) -> Self {
-        HidReaderWriter::_new(builder, state, config, USB_SUBCLASS_REPORT_ONLY, USB_PROTOCOL_NONE)
-    }
-
-    /// Creates a new `HidReaderWriter` for a HID Mouse, with support for the BOOT protocol mode.
-    ///
-    /// This will allocate one IN and one OUT endpoints. If you only need writing (sending)
-    /// HID reports, consider using [`HidWriter::new`] instead, which allocates an IN endpoint only.
-    ///
-    pub fn new_mouse(builder: &mut Builder<'d, D>, state: &'d mut State<'d>, config: Config<'d>) -> Self {
-        HidReaderWriter::_new(builder, state, config, USB_SUBCLASS_BOOT_OR_REPORT, USB_PROTOCOL_MOUSE)
-    }
-
-    /// Creates a new `HidReaderWriter` for a HID Keyboard, with support for the BOOT protocol mode.
-    ///
-    /// This will allocate one IN and one OUT endpoints. If you only need writing (sending)
-    /// HID reports, consider using [`HidWriter::new`] instead, which allocates an IN endpoint only.
-    ///
-    pub fn new_keyboard(builder: &mut Builder<'d, D>, state: &'d mut State<'d>, config: Config<'d>) -> Self {
-        HidReaderWriter::_new(
-            builder,
-            state,
-            config,
-            USB_SUBCLASS_BOOT_OR_REPORT,
-            USB_PROTOCOL_KEYBOARD,
-        )
-    }
-
-    /// Private helper function to create a new `HidReaderWriter`.
-    fn _new(
-        builder: &mut Builder<'d, D>,
-        state: &'d mut State<'d>,
-        config: Config<'d>,
-        usb_subclass: u8,
-        usb_protocol: u8,
-    ) -> Self {
-        let (ep_out, ep_in, offset) = build(builder, state, config, true, usb_subclass, usb_protocol);
+        let (ep_out, ep_in, offset) = build(builder, state, config, true);
 
         Self {
             reader: HidReader {
@@ -317,50 +304,7 @@ impl<'d, D: Driver<'d>, const N: usize> HidWriter<'d, D, N> {
     /// of CPU on the device & bandwidth on the bus. A value of 10 is reasonable for
     /// high performance uses, and a value of 255 is good for best-effort usecases.
     pub fn new(builder: &mut Builder<'d, D>, state: &'d mut State<'d>, config: Config<'d>) -> Self {
-        HidWriter::_new(builder, state, config, USB_SUBCLASS_REPORT_ONLY, USB_PROTOCOL_NONE)
-    }
-
-    /// Creates a new `HidWriter` for a HID Mouse, with support for the BOOT protocol mode.
-    ///
-    /// This will allocate one IN endpoint only, so the host won't be able to send
-    /// reports to us. If you need that, consider using [`HidReaderWriter::new`] instead.
-    ///
-    /// poll_ms configures how frequently the host should poll for reading/writing
-    /// HID reports. A lower value means better throughput & latency, at the expense
-    /// of CPU on the device & bandwidth on the bus. A value of 10 is reasonable for
-    /// high performance uses, and a value of 255 is good for best-effort usecases.
-    pub fn new_mouse(builder: &mut Builder<'d, D>, state: &'d mut State<'d>, config: Config<'d>) -> Self {
-        HidWriter::_new(builder, state, config, USB_SUBCLASS_BOOT_OR_REPORT, USB_PROTOCOL_MOUSE)
-    }
-
-    /// Creates a new `HidWriter` for a HID Keyboard, with support for the BOOT protocol mode.
-    ///
-    /// This will allocate one IN endpoint only, so the host won't be able to send
-    /// reports to us. If you need that, consider using [`HidReaderWriter::new`] instead.
-    ///
-    /// poll_ms configures how frequently the host should poll for reading/writing
-    /// HID reports. A lower value means better throughput & latency, at the expense
-    /// of CPU on the device & bandwidth on the bus. A value of 10 is reasonable for
-    /// high performance uses, and a value of 255 is good for best-effort usecases.
-    pub fn new_keyboard(builder: &mut Builder<'d, D>, state: &'d mut State<'d>, config: Config<'d>) -> Self {
-        HidWriter::_new(
-            builder,
-            state,
-            config,
-            USB_SUBCLASS_BOOT_OR_REPORT,
-            USB_PROTOCOL_KEYBOARD,
-        )
-    }
-
-    /// Private helper function to create a new `HidWriter`.
-    pub fn _new(
-        builder: &mut Builder<'d, D>,
-        state: &'d mut State<'d>,
-        config: Config<'d>,
-        usb_subclass: u8,
-        usb_protocol: u8,
-    ) -> Self {
-        let (ep_out, ep_in, _offset) = build(builder, state, config, false, usb_subclass, usb_protocol);
+        let (ep_out, ep_in, _offset) = build(builder, state, config, false);
 
         assert!(ep_out.is_none());
 
diff --git a/examples/nrf52840/Cargo.toml b/examples/nrf52840/Cargo.toml
index 9a1fc080e..452e83b7e 100644
--- a/examples/nrf52840/Cargo.toml
+++ b/examples/nrf52840/Cargo.toml
@@ -28,7 +28,7 @@ cortex-m-rt = "0.7.0"
 panic-probe = { version = "1.0.0", features = ["print-defmt"] }
 rand = { version = "0.9.0", default-features = false }
 embedded-storage = "0.3.1"
-usbd-hid = { version = "0.8.1", features = ["defmt"] }
+usbd-hid = "0.8.1"
 serde = { version = "1.0.136", default-features = false }
 embedded-hal = { version = "1.0" }
 embedded-hal-async = { version = "1.0" }
diff --git a/examples/nrf52840/src/bin/usb_hid_keyboard.rs b/examples/nrf52840/src/bin/usb_hid_keyboard.rs
index a4931099a..8649d5667 100644
--- a/examples/nrf52840/src/bin/usb_hid_keyboard.rs
+++ b/examples/nrf52840/src/bin/usb_hid_keyboard.rs
@@ -13,11 +13,12 @@ use embassy_nrf::usb::Driver;
 use embassy_nrf::{bind_interrupts, pac, peripherals, usb};
 use embassy_sync::blocking_mutex::raw::CriticalSectionRawMutex;
 use embassy_sync::signal::Signal;
-use embassy_usb::class::hid::{HidReaderWriter, ReportId, RequestHandler, State};
+use embassy_usb::class::hid::{
+    HidBootProtocol, HidProtocolMode, HidReaderWriter, HidSubclass, ReportId, RequestHandler, State,
+};
 use embassy_usb::control::OutResponse;
 use embassy_usb::{Builder, Config, Handler};
 use usbd_hid::descriptor::{KeyboardReport, SerializedDescriptor};
-use usbd_hid::hid_class::HidProtocolMode;
 use {defmt_rtt as _, panic_probe as _};
 
 bind_interrupts!(struct Irqs {
@@ -81,8 +82,10 @@ async fn main(_spawner: Spawner) {
         request_handler: None,
         poll_ms: 60,
         max_packet_size: 64,
+        hid_subclass: HidSubclass::ReportOrBoot,
+        hid_boot_protocol: HidBootProtocol::Keyboard,
     };
-    let hid = HidReaderWriter::<_, 1, 8>::new_keyboard(&mut builder, &mut state, config);
+    let hid = HidReaderWriter::<_, 1, 8>::new(&mut builder, &mut state, config);
 
     // Build the builder.
     let mut usb = builder.build();
diff --git a/examples/nrf52840/src/bin/usb_hid_mouse.rs b/examples/nrf52840/src/bin/usb_hid_mouse.rs
index 6ec8a2d33..4baf2e814 100644
--- a/examples/nrf52840/src/bin/usb_hid_mouse.rs
+++ b/examples/nrf52840/src/bin/usb_hid_mouse.rs
@@ -10,11 +10,12 @@ use embassy_nrf::usb::vbus_detect::HardwareVbusDetect;
 use embassy_nrf::usb::Driver;
 use embassy_nrf::{bind_interrupts, pac, peripherals, usb};
 use embassy_time::Timer;
-use embassy_usb::class::hid::{HidWriter, ReportId, RequestHandler, State};
+use embassy_usb::class::hid::{
+    HidBootProtocol, HidProtocolMode, HidSubclass, HidWriter, ReportId, RequestHandler, State,
+};
 use embassy_usb::control::OutResponse;
 use embassy_usb::{Builder, Config};
 use usbd_hid::descriptor::{MouseReport, SerializedDescriptor};
-use usbd_hid::hid_class::HidProtocolMode;
 use {defmt_rtt as _, panic_probe as _};
 
 bind_interrupts!(struct Irqs {
@@ -72,9 +73,11 @@ async fn main(_spawner: Spawner) {
         request_handler: Some(&mut request_handler),
         poll_ms: 60,
         max_packet_size: 8,
+        hid_subclass: HidSubclass::ReportOrBoot,
+        hid_boot_protocol: HidBootProtocol::Mouse,
     };
 
-    let mut writer = HidWriter::<_, 5>::new_mouse(&mut builder, &mut state, config);
+    let mut writer = HidWriter::<_, 5>::new(&mut builder, &mut state, config);
 
     // Build the builder.
     let mut usb = builder.build();
diff --git a/examples/rp/src/bin/usb_hid_keyboard.rs b/examples/rp/src/bin/usb_hid_keyboard.rs
index 8658da6b5..fa78d0c2e 100644
--- a/examples/rp/src/bin/usb_hid_keyboard.rs
+++ b/examples/rp/src/bin/usb_hid_keyboard.rs
@@ -10,7 +10,7 @@ use embassy_rp::bind_interrupts;
 use embassy_rp::gpio::{Input, Pull};
 use embassy_rp::peripherals::USB;
 use embassy_rp::usb::{Driver, InterruptHandler};
-use embassy_usb::class::hid::{HidReaderWriter, ReportId, RequestHandler, State};
+use embassy_usb::class::hid::{HidBootProtocol, HidReaderWriter, HidSubclass, ReportId, RequestHandler, State};
 use embassy_usb::control::OutResponse;
 use embassy_usb::{Builder, Config, Handler};
 use usbd_hid::descriptor::{KeyboardReport, SerializedDescriptor};
@@ -67,8 +67,10 @@ async fn main(_spawner: Spawner) {
         request_handler: None,
         poll_ms: 60,
         max_packet_size: 64,
+        hid_subclass: HidSubclass::ReportOrBoot,
+        hid_boot_protocol: HidBootProtocol::Keyboard,
     };
-    let hid = HidReaderWriter::<_, 1, 8>::new_keyboard(&mut builder, &mut state, config);
+    let hid = HidReaderWriter::<_, 1, 8>::new(&mut builder, &mut state, config);
 
     // Build the builder.
     let mut usb = builder.build();
diff --git a/examples/rp/src/bin/usb_hid_mouse.rs b/examples/rp/src/bin/usb_hid_mouse.rs
index 4d8fc354e..100e6048a 100755
--- a/examples/rp/src/bin/usb_hid_mouse.rs
+++ b/examples/rp/src/bin/usb_hid_mouse.rs
@@ -11,7 +11,7 @@ use embassy_rp::clocks::RoscRng;
 use embassy_rp::peripherals::USB;
 use embassy_rp::usb::{Driver, InterruptHandler};
 use embassy_time::Timer;
-use embassy_usb::class::hid::{HidReaderWriter, ReportId, RequestHandler, State};
+use embassy_usb::class::hid::{HidBootProtocol, HidReaderWriter, HidSubclass, ReportId, RequestHandler, State};
 use embassy_usb::control::OutResponse;
 use embassy_usb::{Builder, Config, Handler};
 use rand::Rng;
@@ -69,8 +69,10 @@ async fn main(_spawner: Spawner) {
         request_handler: None,
         poll_ms: 60,
         max_packet_size: 64,
+        hid_subclass: HidSubclass::ReportOrBoot,
+        hid_boot_protocol: HidBootProtocol::Mouse,
     };
-    let hid = HidReaderWriter::<_, 1, 8>::new_keyboard(&mut builder, &mut state, config);
+    let hid = HidReaderWriter::<_, 1, 8>::new(&mut builder, &mut state, config);
 
     // Build the builder.
     let mut usb = builder.build();
diff --git a/examples/rp235x/src/bin/usb_hid_keyboard.rs b/examples/rp235x/src/bin/usb_hid_keyboard.rs
index fa9eaa863..3203176cb 100644
--- a/examples/rp235x/src/bin/usb_hid_keyboard.rs
+++ b/examples/rp235x/src/bin/usb_hid_keyboard.rs
@@ -10,7 +10,9 @@ use embassy_rp::bind_interrupts;
 use embassy_rp::gpio::{Input, Pull};
 use embassy_rp::peripherals::USB;
 use embassy_rp::usb::{Driver as UsbDriver, InterruptHandler};
-use embassy_usb::class::hid::{HidReaderWriter, ReportId, RequestHandler, State as HidState};
+use embassy_usb::class::hid::{
+    HidBootProtocol, HidReaderWriter, HidSubclass, ReportId, RequestHandler, State as HidState,
+};
 use embassy_usb::control::OutResponse;
 use embassy_usb::{Builder, Config, Handler};
 use usbd_hid::descriptor::{KeyboardReport, SerializedDescriptor};
@@ -67,8 +69,10 @@ async fn main(_spawner: Spawner) {
         request_handler: None,
         poll_ms: 60,
         max_packet_size: 64,
+        hid_subclass: HidSubclass::ReportOrBoot,
+        hid_boot_protocol: HidBootProtocol::Keyboard,
     };
-    let hid = HidReaderWriter::<_, 1, 8>::new_keyboard(&mut builder, &mut state, config);
+    let hid = HidReaderWriter::<_, 1, 8>::new(&mut builder, &mut state, config);
 
     // Build the builder.
     let mut usb = builder.build();
diff --git a/examples/stm32f4/src/bin/usb_hid_keyboard.rs b/examples/stm32f4/src/bin/usb_hid_keyboard.rs
index 6ddfba83a..740fbcaef 100644
--- a/examples/stm32f4/src/bin/usb_hid_keyboard.rs
+++ b/examples/stm32f4/src/bin/usb_hid_keyboard.rs
@@ -11,7 +11,7 @@ use embassy_stm32::gpio::Pull;
 use embassy_stm32::time::Hertz;
 use embassy_stm32::usb::Driver;
 use embassy_stm32::{bind_interrupts, peripherals, usb, Config};
-use embassy_usb::class::hid::{HidReaderWriter, ReportId, RequestHandler, State};
+use embassy_usb::class::hid::{HidBootProtocol, HidReaderWriter, HidSubclass, ReportId, RequestHandler, State};
 use embassy_usb::control::OutResponse;
 use embassy_usb::{Builder, Handler};
 use usbd_hid::descriptor::{KeyboardReport, SerializedDescriptor};
@@ -105,9 +105,11 @@ async fn main(_spawner: Spawner) {
         request_handler: None,
         poll_ms: 60,
         max_packet_size: 8,
+        hid_subclass: HidSubclass::ReportOrBoot,
+        hid_boot_protocol: HidBootProtocol::Keyboard,
     };
 
-    let hid = HidReaderWriter::<_, 1, 8>::new_keyboard(&mut builder, &mut state, config);
+    let hid = HidReaderWriter::<_, 1, 8>::new(&mut builder, &mut state, config);
 
     // Build the builder.
     let mut usb = builder.build();
diff --git a/examples/stm32f4/src/bin/usb_hid_mouse.rs b/examples/stm32f4/src/bin/usb_hid_mouse.rs
index 8d035d0d5..09af204c4 100644
--- a/examples/stm32f4/src/bin/usb_hid_mouse.rs
+++ b/examples/stm32f4/src/bin/usb_hid_mouse.rs
@@ -8,7 +8,7 @@ use embassy_stm32::time::Hertz;
 use embassy_stm32::usb::Driver;
 use embassy_stm32::{bind_interrupts, peripherals, usb, Config};
 use embassy_time::Timer;
-use embassy_usb::class::hid::{HidWriter, ReportId, RequestHandler, State};
+use embassy_usb::class::hid::{HidBootProtocol, HidSubclass, HidWriter, ReportId, RequestHandler, State};
 use embassy_usb::control::OutResponse;
 use embassy_usb::Builder;
 use usbd_hid::descriptor::{MouseReport, SerializedDescriptor};
@@ -95,9 +95,11 @@ async fn main(_spawner: Spawner) {
         request_handler: Some(&mut request_handler),
         poll_ms: 60,
         max_packet_size: 8,
+        hid_subclass: HidSubclass::ReportOrBoot,
+        hid_boot_protocol: HidBootProtocol::Mouse,
     };
 
-    let mut writer = HidWriter::<_, 5>::new_mouse(&mut builder, &mut state, config);
+    let mut writer = HidWriter::<_, 5>::new(&mut builder, &mut state, config);
 
     // Build the builder.
     let mut usb = builder.build();
diff --git a/examples/stm32l5/src/bin/usb_hid_mouse.rs b/examples/stm32l5/src/bin/usb_hid_mouse.rs
index 6f9200548..30dbd2698 100644
--- a/examples/stm32l5/src/bin/usb_hid_mouse.rs
+++ b/examples/stm32l5/src/bin/usb_hid_mouse.rs
@@ -7,7 +7,7 @@ use embassy_futures::join::join;
 use embassy_stm32::usb::Driver;
 use embassy_stm32::{bind_interrupts, peripherals, usb, Config};
 use embassy_time::Timer;
-use embassy_usb::class::hid::{HidWriter, ReportId, RequestHandler, State};
+use embassy_usb::class::hid::{HidBootProtocol, HidSubclass, HidWriter, ReportId, RequestHandler, State};
 use embassy_usb::control::OutResponse;
 use embassy_usb::Builder;
 use usbd_hid::descriptor::{MouseReport, SerializedDescriptor};
@@ -77,9 +77,11 @@ async fn main(_spawner: Spawner) {
         request_handler: Some(&mut request_handler),
         poll_ms: 60,
         max_packet_size: 8,
+        hid_subclass: HidSubclass::ReportOrBoot,
+        hid_boot_protocol: HidBootProtocol::Mouse,
     };
 
-    let mut writer = HidWriter::<_, 5>::new_mouse(&mut builder, &mut state, config);
+    let mut writer = HidWriter::<_, 5>::new(&mut builder, &mut state, config);
 
     // Build the builder.
     let mut usb = builder.build();