Rework SetupPacket - move from driver to embassy-usb-host by bugadani · Pull Request #5882 · embassy-rs/embassy

bugadani · 2026-04-17T15:00:58Z

RequestType was implemented using bitflags which is not the right tool for the job for defining bitfields. This PR removes the dependency and hand-implements the type with proper one-of-many semantics on RequestType fields.

Copilot

Pull request overview

Reworks USB host RequestType away from bitflags into a structured representation (Direction + ControlType + Recipient) and switches SetupPacket serialization to an explicit 8-byte wire encoding, removing the bitflags dependency from embassy-usb-driver.

Changes:

Replace RequestType bitflags with RequestType { direction, control_type, recipient } plus to_bits/from_bits and helper constructors.
Replace unsafe SetupPacket::as_bytes() usage with SetupPacket::to_bytes() at call sites.
Update host stack code to construct request types via device_to_host/host_to_device and remove bitflags dependency in embassy-usb-driver.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
embassy-usb-synopsys-otg/src/host.rs	Switch control transfers to pass serialized SETUP bytes via `SetupPacket::to_bytes()`.
embassy-usb-host/src/lib.rs	Update SETUP parsing to use new `RequestType::from_bits`.
embassy-usb-host/src/control.rs	Update control request construction to new `RequestType` API with `ControlType`/`Recipient`.
embassy-usb-host/src/class/uac/mod.rs	Update UAC class control transfers to new `RequestType` API.
embassy-usb-host/src/class/hub.rs	Update hub class control transfers to new `RequestType` API.
embassy-usb-driver/src/host.rs	Implement new `Recipient`, `ControlType`, `RequestType`, and `SetupPacket::to_bytes()` wire serialization.
embassy-usb-driver/Cargo.toml	Remove `bitflags` dependency from `embassy-usb-driver`.
embassy-stm32/src/usb/usb_host.rs	Switch STM32 control SETUP stage writes to `SetupPacket::to_bytes()`.
embassy-rp/src/usb/host.rs	Update RP host setup write to use `RequestType::to_bits()` instead of `bits()`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-17T15:06:13Z

+/// Recipient of a USB control request.
+///
+/// This is the 5-bit Recipient sub-field of `bmRequestType`
+/// (USB 2.0 spec Table 9-2, bits 4..0). The discriminant of each variant
+/// matches the on-wire value.
+#[repr(u8)]
+#[derive(Copy, Clone, Eq, PartialEq, Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub enum Recipient {
+    /// The request is intended for the entire device.
+    Device = 0,
+    /// The request is intended for an interface.
+    Interface = 1,
+    /// The request is intended for an endpoint.
+    Endpoint = 2,
+    /// The recipient of the request is unspecified.
+    Other = 3,
+    /// Any reserved recipient value (4..=31).
+    Reserved = 4,
 }


Recipient docs say each variant’s discriminant matches the on-wire bmRequestType recipient value, but Recipient::Reserved = 4 is used to represent any reserved recipient value (4..=31). from_bits() collapses all those values into Reserved, and to_bits() will re-encode them as 0b00100, so round-tripping is lossy and the discriminant claim is not accurate. Consider either (a) adjusting the docs to explicitly call out this lossiness / that Reserved encodes as 4, or (b) representing recipient as a raw 5-bit newtype (with constants for the standard recipients) so reserved values can be preserved.

IDK if we should add 20-some reserved variants just for this

Dirbaio · 2026-04-17T16:01:16Z

-    pub fn as_bytes(&self) -> &[u8] {
-        // Safe because we know that the size of SetupPacket is 8 bytes.
-        unsafe { core::slice::from_raw_parts(self as *const _ as *const u8, core::mem::size_of::<Self>()) }
+    /// Serialize this SETUP packet to its 8-byte wire format.


do you think it's possible to move SetupPacket out of embassy-usb-driver? ie make the driver work with just bytes.

The usb-host crate can have the nicer public api with parsing/unparsing.

Less api surface means less potential for breaking changes.

Sure why not

I guess it's a bit weird that driver implementations need to bit-fiddle with the setup packet bytes, though 😅

What if we did something in the middle? Keep SetupPacket in the driver, but let the trait method take &[u8; 8]? That way implementations could use SetupPacket to wrap/parse what they need out of the bytes, but it would be a utility instead of an integral part of the API.

The SetupPacket would still be part of the embassy-usb-driver's API surface even as a utility right? The breakage potential is still the same I think.

I do wonder if a embassy-usb-util/embassy-usb-common library that both embassy-usb and embassy-usb-host could use might make sense here. A whole library for just one struct feels silly though 😅 but it would solve the API surface problem and the bit-fiddling problem.

It's not like setup packets will suddenly change anyway, but it'd be possible to add v2, v3, ... versions of utilities until a breaking release could clear out the accumulated garbage. In the end it's not a big deal.

Dominaezzz · 2026-04-18T08:47:40Z

        // Now get the full string with the correct length
        let packet = SetupPacket {
-            request_type: RequestType::IN | RequestType::TYPE_STANDARD | RequestType::RECIPIENT_DEVICE,
+            request_type: RequestType::device_to_host(ControlType::Standard, Recipient::Device),


It's a bit odd to see device_to_host and also Recipient::Device in the same line. Makes me wonder who the real recipient is. Though I'm not sure how to improve this since in is a keyword.

TBH I find the names better than "request_in/request_out", although from a host perspective in/out is less bad than from a device's perspective :) I can rename these into RequestType::r#in/out if that's better (or request_in/request_out though a bit redundant), but I'm not sure if it is better.

Maybe control_in/control_out could work?

Oh actually, what about RequestType::new(Direction::In, ControlType::Standard, Recipient::Device) ?

What if we drop the constructor completely and construct the struct directly? It's a bit wordy, but there's no confusion about parameter positions.

xoviat · 2026-04-18T11:20:44Z

bender run

Copilot AI review requested due to automatic review settings April 17, 2026 15:00

Copilot started reviewing on behalf of bugadani April 17, 2026 15:01 View session

Rework RequestType

582bfb9

bugadani force-pushed the usb-host-request-type branch from b283907 to 582bfb9 Compare April 17, 2026 15:05

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Dirbaio reviewed Apr 17, 2026

View reviewed changes

bugadani changed the title ~~Rework RequestType~~ Rework SetupPacket - move from driver to embassy-usb-host Apr 17, 2026

Shrink driver API surface

b605f59

bugadani force-pushed the usb-host-request-type branch from 0e9ec5a to b605f59 Compare April 17, 2026 16:53

leftger added the e-usb-host Issues for USB Host label Apr 18, 2026

Dominaezzz reviewed Apr 18, 2026

View reviewed changes

Remove RequestType constructors

7a2a5b6

bugadani force-pushed the usb-host-request-type branch from 1916cff to 7a2a5b6 Compare April 18, 2026 09:09

Clean up a bit

6923845

bugadani force-pushed the usb-host-request-type branch from d07b01b to 6923845 Compare April 18, 2026 09:15

xoviat assigned leftger Apr 18, 2026

Merge branch 'main' into usb-host-request-type

50e36e7

leftger added this pull request to the merge queue Apr 19, 2026

Merged via the queue into embassy-rs:main with commit bc7b6af Apr 19, 2026
7 checks passed

bugadani deleted the usb-host-request-type branch April 19, 2026 07:00

Conversation

bugadani commented Apr 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bugadani Apr 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xoviat commented Apr 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bugadani Apr 18, 2026 •

edited

Loading