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 +++++++++++++------------------------------ 1 file changed, 39 insertions(+), 95 deletions(-) (limited to 'embassy-usb/src') 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()); -- cgit