Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

embassy-usb modifications #1509

Closed
wants to merge 2 commits into from
+64 −16
Closed

embassy-usb modifications #1509

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc02c7e
feat(embassy-usb): Add ability to get and set HID protocol version
pcbzzz May 29, 2023
56da5a2
fix(embassy-usb): Make BOS descriptor optional
pcbzzz May 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions embassy-usb/src/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ pub struct Builder<'d, D: Driver<'d>> {

device_descriptor: DescriptorWriter<'d>,
config_descriptor: DescriptorWriter<'d>,
bos_descriptor: BosWriter<'d>,
bos_descriptor: Option<BosWriter<'d>>,

#[cfg(feature = "msos-descriptor")]
msos_descriptor: MsOsDescriptorWriter<'d>,
Expand All @@ -148,7 +148,7 @@ impl<'d, D: Driver<'d>> Builder<'d, D> {
config: Config<'d>,
device_descriptor_buf: &'d mut [u8],
config_descriptor_buf: &'d mut [u8],
bos_descriptor_buf: &'d mut [u8],
bos_descriptor_buf: Option<&'d mut [u8]>,
#[cfg(feature = "msos-descriptor")] msos_descriptor_buf: &'d mut [u8],
control_buf: &'d mut [u8],
) -> Self {
Expand All @@ -170,11 +170,13 @@ impl<'d, D: Driver<'d>> Builder<'d, D> {

let mut device_descriptor = DescriptorWriter::new(device_descriptor_buf);
let mut config_descriptor = DescriptorWriter::new(config_descriptor_buf);
let mut bos_descriptor = BosWriter::new(DescriptorWriter::new(bos_descriptor_buf));
let mut bos_descriptor = bos_descriptor_buf.map(|buf| BosWriter::new(DescriptorWriter::new(buf)));

device_descriptor.device(&config);
config_descriptor.configuration(&config);
bos_descriptor.bos();
if let Some(descriptor) = bos_descriptor.as_mut() {
descriptor.bos();
}

Builder {
driver,
Expand All @@ -199,12 +201,16 @@ impl<'d, D: Driver<'d>> Builder<'d, D> {
let msos_descriptor = self.msos_descriptor.build(&mut self.bos_descriptor);

self.config_descriptor.end_configuration();
self.bos_descriptor.end_bos();
if let Some(descriptor) = self.bos_descriptor.as_mut() {
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 used: {}", self.bos_descriptor.writer.position());
if let Some(descriptor) = self.bos_descriptor.as_ref() {
info!("USB: bos_descriptor used: {}", descriptor.writer.position());
}
#[cfg(feature = "msos-descriptor")]
info!("USB: msos_descriptor used: {}", msos_descriptor.len());
info!("USB: control_buf size: {}", self.control_buf.len());
Expand All @@ -215,7 +221,8 @@ impl<'d, D: Driver<'d>> Builder<'d, D> {
self.handlers,
self.device_descriptor.into_buf(),
self.config_descriptor.into_buf(),
self.bos_descriptor.writer.into_buf(),
self.bos_descriptor
.map(|descriptor| descriptor.writer.into_buf() as &[u8]),
self.interfaces,
self.control_buf,
#[cfg(feature = "msos-descriptor")]
Expand Down
44 changes: 40 additions & 4 deletions embassy-usb/src/class/hid.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! USB HID (Human Interface Device) class implementation.

use core::mem::MaybeUninit;
use core::mem::{self, MaybeUninit};
use core::ops::Range;
use core::sync::atomic::{AtomicUsize, Ordering};

Expand Down Expand Up @@ -369,8 +369,30 @@ impl<'d, D: Driver<'d>, const N: usize> HidReader<'d, D, N> {
}
}

/// HID protocol
#[repr(u8)]
#[derive(Copy, Clone, Eq, PartialEq, Debug)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
pub enum Protocol {
/// Boot protocol, custom reports are ignored
Boot = 0,
/// Custom reports
Report = 1,
}

/// Handler for HID-related control requests.
pub trait RequestHandler {
/// Gets the current HID protocol
fn get_protocol(&self) -> Option<Protocol> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None here means "return 1", it has no special meaning. I think it'd make more sense to make this method not return an Option, and make the default impl return 1.

None
}

/// Set the current HID protocol
fn set_protocol(&self, protocol: Protocol) -> OutResponse {
let _ = protocol;
OutResponse::Rejected
}

/// Reads the value of report `id` into `buf` returning the size.
///
/// Returns `None` if `id` is invalid or no data is available.
Expand Down Expand Up @@ -478,7 +500,15 @@ impl<'d> Handler for Control<'d> {
_ => Some(OutResponse::Rejected),
},
HID_REQ_SET_PROTOCOL => {
if req.value == 1 {
if let Some(handler) = self.request_handler {
let protocol = if req.value <= 1 {
unsafe { mem::transmute(req.value as u8) }
} else {
Protocol::Report
};

Some(handler.set_protocol(protocol))
} else if req.value == 1 {
Some(OutResponse::Accepted)
} else {
warn!("HID Boot Protocol is unsupported.");
Expand Down Expand Up @@ -535,8 +565,14 @@ impl<'d> Handler for Control<'d> {
}
}
HID_REQ_GET_PROTOCOL => {
// UNSUPPORTED: Boot Protocol
buf[0] = 1;
if let Some(handler) = self.request_handler {
let protocol = handler.get_protocol().unwrap_or(Protocol::Report);
buf[0] = protocol as u8;
} else {
// UNSUPPORTED: Boot Protocol
buf[0] = 1;
}

Some(InResponse::Accepted(&buf[0..1]))
}
_ => Some(InResponse::Rejected),
Expand Down
15 changes: 10 additions & 5 deletions embassy-usb/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,9 @@ pub struct UsbBufferReport {
/// Number of config descriptor bytes used
pub config_descriptor_used: usize,
/// Number of bos descriptor bytes used
pub bos_descriptor_used: usize,
///
/// Will be `None` if "bos_descriptor_buf" was not supplied to builder
pub bos_descriptor_used: Option<usize>,
/// Number of msos descriptor bytes used
///
/// Will be `None` if the "msos-descriptor" feature is not active.
Expand All @@ -196,7 +198,7 @@ struct Inner<'d, D: Driver<'d>> {
config: Config<'d>,
device_descriptor: &'d [u8],
config_descriptor: &'d [u8],
bos_descriptor: &'d [u8],
bos_descriptor: Option<&'d [u8]>,

device_state: UsbDeviceState,
suspended: bool,
Expand Down Expand Up @@ -224,7 +226,7 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> {
handlers: Vec<&'d mut dyn Handler, MAX_HANDLER_COUNT>,
device_descriptor: &'d [u8],
config_descriptor: &'d [u8],
bos_descriptor: &'d [u8],
bos_descriptor: Option<&'d [u8]>,
interfaces: Vec<Interface, MAX_INTERFACE_COUNT>,
control_buf: &'d mut [u8],
#[cfg(feature = "msos-descriptor")] msos_descriptor: crate::msos::MsOsDescriptorSet<'d>,
Expand Down Expand Up @@ -269,7 +271,7 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> {
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(),
bos_descriptor_used: self.inner.bos_descriptor.map(|descriptor| descriptor.len()),
msos_descriptor_used: mdu,
control_buffer_size: self.control_buf.len(),
}
Expand Down Expand Up @@ -726,7 +728,10 @@ impl<'d, D: Driver<'d>> Inner<'d, D> {
let (dtype, index) = req.descriptor_type_index();

match dtype {
descriptor_type::BOS => InResponse::Accepted(self.bos_descriptor),
descriptor_type::BOS => match self.bos_descriptor {
Some(descriptor) => InResponse::Accepted(descriptor),
None => InResponse::Rejected,
},
descriptor_type::DEVICE => InResponse::Accepted(self.device_descriptor),
descriptor_type::CONFIGURATION => InResponse::Accepted(self.config_descriptor),
descriptor_type::STRING => {
Expand Down
Loading