From 5c8218b8750bed3f4bef7973e250aa830d8c2fe3 Mon Sep 17 00:00:00 2001 From: matteo Date: Tue, 30 Sep 2025 18:38:43 +0200 Subject: review comments --- embassy-usb/src/class/hid.rs | 134 ++++++++------------------ examples/nrf52840/Cargo.toml | 2 +- examples/nrf52840/src/bin/usb_hid_keyboard.rs | 9 +- examples/nrf52840/src/bin/usb_hid_mouse.rs | 9 +- examples/rp/src/bin/usb_hid_keyboard.rs | 6 +- examples/rp/src/bin/usb_hid_mouse.rs | 6 +- examples/rp235x/src/bin/usb_hid_keyboard.rs | 8 +- examples/stm32f4/src/bin/usb_hid_keyboard.rs | 6 +- examples/stm32f4/src/bin/usb_hid_mouse.rs | 6 +- examples/stm32l5/src/bin/usb_hid_mouse.rs | 6 +- 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): -#[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 for HidProtocolMode { fn from(mode: u8) -> HidProtocolMode { if mode == HidProtocolMode::Boot as u8 { @@ -62,6 +51,30 @@ impl From 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::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(); -- cgit