-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add USBHost v2 #4032
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.
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
Add USBHost v2 #4032
Conversation
…isochronuous channels
Use stat_tx and stat_rx instead of nak() flag
Should only be included for usb_v4 peripherals.
Move retarget_channel from UsbHostDriver to UsbChannel trait implementation.
It's already implemented in mod.rs
Compiles and runs ok on a rp2040 board.
println!("Found device with speed = {:?}", speed); | ||
|
||
let enum_info = usbhost.enumerate_root_bare(speed, 1).await.unwrap(); | ||
let mut kbd = KbdHandler::try_register(&usbhost, &enum_info) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would I go about supporting the keyboard being unplugged and plugged in again?
Might be worth it to add that to the example.
EDIT:
It seems to me that KbdHandler::wait_for_event should return when the device is disconnected so the driver can be cleaned up. Right now it awaiting on that just hangs forever.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is HandlerEvent::HandlerDisconnected
but it's not triggered on disconnect. I don't know if it's a bug or it's for something else.
I've managed to get re-plugging devices to partially work. Seems channels get exhausted after some time (probably due to lack of drop_channel). All in all it's reliable only the first time the device is plugged in.
#![no_std]
#![no_main]
use defmt::*;
use embassy_executor::Spawner;
use embassy_rp::{bind_interrupts, peripherals, usb};
use embassy_usb_driver::host::{UsbHostDriver, DeviceEvent};
use embassy_usb::host::UsbHostBusExt;
use embassy_usb::handlers::{UsbHostHandler, kbd};
use embassy_futures::select::{select, Either};
use embassy_usb_driver::Speed;
use {defmt_rtt as _, panic_probe as _};
bind_interrupts!(struct Irqs {
USBCTRL_IRQ => usb::host::InterruptHandler<peripherals::USB>;
});
#[embassy_executor::main]
async fn main(_spawner: Spawner) -> ! {
let p = embassy_rp::init(Default::default());
let mut usbhost = usb::host::Driver::new(p.USB, Irqs);
loop {
match usbhost.wait_for_device_event().await {
DeviceEvent::Connected(speed) => {
handle_device(&mut usbhost, speed).await;
}
DeviceEvent::Disconnected => {
warn!("Disconnected when there's no device?");
}
}
};
}
async fn handle_device(
usbhost: &mut embassy_rp::usb::host::Driver<'_, peripherals::USB>,
speed: Speed) {
let enum_info = usbhost.enumerate_root_bare(speed, 1).await.unwrap();
info!("enum_info.device_desc = {}", enum_info.device_desc);
let mut kbd = kbd::KbdHandler::try_register(usbhost, &enum_info).await.expect("Couldn't register keyboard");
handle_kbd_events(usbhost, &mut kbd).await;
}
async fn handle_kbd_events(
usbhost: &mut embassy_rp::usb::host::Driver<'_, peripherals::USB>,
kbd: &mut kbd::KbdHandler<embassy_rp::usb::host::Driver<'_, peripherals::USB>>){
loop {
let result = select(usbhost.wait_for_device_event(), kbd.wait_for_event()).await;
match result {
Either::First(dev_event) => match dev_event {
DeviceEvent::Connected(_) => {
defmt::panic!("More than one device?");
}
DeviceEvent::Disconnected => {
info!("Disconnected!");
break;
}
}
Either::Second(kbd_event) => {
info!("keyboard event = {}", kbd_event);
}
}
};
}
embassy-usb/src/host/mod.rs
Outdated
/// Request the underlying bytes for a descriptor of a specific type. | ||
/// bytes.len() determines how many bytes are read at maximum. | ||
/// This can be used for descriptors of varying length, which are parsed by the caller. | ||
async fn request_descriptor_bytes<T: USBDescriptor>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can't use "T: USBDescriptor" as USBDescriptor trait requires the descriptor to be of a fixed size.
I'll try preparing a patch that addresses takes DESC_TYPE as another another argument.
I'm not sure if it would be better to have USBDescriptor not to enforce fixed size (I don't know how/if this can be accomplished). Maybe rename it to something like FixedSizeUSBDescriptor.
From what I understand(correct me if I'm wrong) HID Report descriptors are also USB descriptors but they aren't fixed size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we're kind of deprecating the USBDescriptor trait (as it was originally used) as it's only useful for fetching the headers of descriptors. Since we are restricted on memory buffer the typical flow of a descriptor request is fetch header -> check if static buffer can support descriptor -> fetch buffer as size of descriptor
. For HID specifically you fetch the entire ConfigurationDescriptor
and then use the RawDescriptorIterator
as shown here 39c9d16
Could definitely rename it, maybe USBDescriptorHeader
or something similar would be more apt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Report descriptors don't even contain their own length(it's in corresponding HID descriptor). It's more akin to a string.
So I'm not sure it would be useful for that.
I don't have better idea for the name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm it seems like the length of the report should be in the wItemLength
in HIDDescriptor
(EndpointDescriptor eqv.). So yeah USBDescriptorHeader
wouldn't apply for ReportDescriptors
use crate::host::descriptor::{ConfigurationDescriptor, DeviceDescriptor, USBDescriptor}; | ||
use crate::host::ControlChannelExt; | ||
|
||
pub mod hub; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the name alone this could mean two things. Either host hub driver or implementation of a device hub.
I can imagine both being useful. One could make a device that shows as a hub with two devices connected. Heck pio on rp2040 can be used as USB host so it would be possible to make a hub based on that eg a rp2040 based USB keyboard with a port for mice. Or split keyboard that is connected together via USB (and one that actually works as a true USB port albeit slow).
I think it would be great to make it immediately obvious that this is host hub driver.
What do You folks think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The handlers
module is current for host drivers but we can move it to host. Also I think it would be interesting to have an emulated hub for supporting multiple virtual devices, but that seems better for another PR.
const SET_PROTOCOL: u8 = 0x0B; | ||
const BOOT_PROTOCOL: u16 = 0x0000; | ||
if let Err(err) = control_channel | ||
.class_request_out(SET_PROTOCOL, BOOT_PROTOCOL, iface.interface_number as u16, &[]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit unfortunate as according to the HID spec the simplified, boot protocol is optional and the full report protocol is mandatory. So we should probably just use the report protocol for everything. I'd think we want to support something like game pads/joysticks anyway and the boot protocol is defined for keyboard and mice only.
I'll trying to write report protocol support atm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this was just made from an example of a previous keyboard driver to get something that works for a few devices we tested, not necessarily fully spec-compliant.
If report protocol can help increase compatibility then we should definitely incorporate that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been tinkering with hid last few days. I've came to conclusion it would deserve a crate of it's own. From what I see Bluetooth reuses the same report descriptors and parsing those is not insignificant amount of code.
I'd think we would be better served by a different example than a keyboard. I haven't checked but it seems to me that mass storage class would should be simple, useful and short. I might get around to writing it later.
I think it would be good to have another example that relies on vendor_id and product_id. So that we get the driver choice right. I don't have idea of a device that would not have a class, Not be a device that should have been a class device. Would have a simple and short driver. Would be useful to embassy users. Anyone has an idea? If we don't get any we can make a dummy driver.
0.000431 DEBUG Detecting device └─ usb_host_keyboard::____embassy_main_task::{async_fn#0} @ src/bin/usb_host_keyboard.rs:25 Found device with speed = Low 0.161293 ERROR panicked at src/bin/usb_host_keyboard.rs:36:65: called `Result::unwrap()` on an `Err` value: ChannelError(Stall)
USBDescriptor trait requires the descriptor to be of a fixed size and this function is meant for getting descriptors of variable size.
I've fixed some bugs: |
@lynxD you can PR into the author's branch on their fork by the way. If merged, the changes will show up here automatically. |
Fixes for usb-asynch
// FIXME: Fallible, propegate errors? | ||
let iface = InterfaceDescriptor::try_from_bytes(remaining_buf).ok()?; | ||
self.offset += iface.len as usize + iface.buffer.len(); | ||
self.index += 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a bug here. It assumes that there is one interface descriptor per interface.
A DS4 gamepad gives me following configuration descriptor decoded with (http://eleccelerator.com/usbdescreqparser/):
0x09, // bLength
0x02, // bDescriptorType (Configuration)
0xE1, 0x00, // wTotalLength 225
0x04, // bNumInterfaces 4
0x01, // bConfigurationValue
0x00, // iConfiguration (String Index)
0xC0, // bmAttributes Self Powered
0xFA, // bMaxPower 500mA
0x09, // bLength
0x04, // bDescriptorType (Interface)
0x00, // bInterfaceNumber 0
0x00, // bAlternateSetting
0x00, // bNumEndpoints 0
0x01, // bInterfaceClass (Audio)
0x01, // bInterfaceSubClass (Audio Control)
0x00, // bInterfaceProtocol
0x00, // iInterface (String Index)
0x0A, // bLength
0x24, // bDescriptorType (See Next Line)
0x01, // bDescriptorSubtype (CS_INTERFACE -> HEADER)
0x00, 0x01, // bcdADC 1.00
0x47, 0x00, // wTotalLength 71
0x02, // binCollection 0x02
0x01, // baInterfaceNr 1
0x02, // baInterfaceNr 2
0x0C, // bLength
0x24, // bDescriptorType (See Next Line)
0x02, // bDescriptorSubtype (CS_INTERFACE -> INPUT_TERMINAL)
0x01, // bTerminalID
0x01, 0x01, // wTerminalType (USB Streaming)
0x06, // bAssocTerminal
0x02, // bNrChannels 2
0x03, 0x00, // wChannelConfig (Left and Right Front)
0x00, // iChannelNames
0x00, // iTerminal
0x0A, // bLength
0x24, // bDescriptorType (See Next Line)
0x06, // bDescriptorSubtype (CS_INTERFACE -> FEATURE_UNIT)
0x02, // bUnitID
0x01, // bSourceID
0x01, // bControlSize 1
0x03, 0x00, // bmaControls[0] (Mute,Volume)
0x00, 0x00, // bmaControls[1] (None)
0x09, // bLength
0x24, // bDescriptorType (See Next Line)
0x03, // bDescriptorSubtype (CS_INTERFACE -> OUTPUT_TERMINAL)
0x03, // bTerminalID
0x02, 0x04, // wTerminalType (Headset)
0x04, // bAssocTerminal
0x02, // bSourceID
0x00, // iTerminal
0x0C, // bLength
0x24, // bDescriptorType (See Next Line)
0x02, // bDescriptorSubtype (CS_INTERFACE -> INPUT_TERMINAL)
0x04, // bTerminalID
0x02, 0x04, // wTerminalType (Headset)
0x03, // bAssocTerminal
0x01, // bNrChannels 1
0x00, 0x00, // wChannelConfig
0x00, // iChannelNames
0x00, // iTerminal
0x09, // bLength
0x24, // bDescriptorType (See Next Line)
0x06, // bDescriptorSubtype (CS_INTERFACE -> FEATURE_UNIT)
0x05, // bUnitID
0x04, // bSourceID
0x01, // bControlSize 1
0x03, 0x00, // bmaControls[0] (Mute,Volume)
0x00, // iFeature
0x09, // bLength
0x24, // bDescriptorType (See Next Line)
0x03, // bDescriptorSubtype (CS_INTERFACE -> OUTPUT_TERMINAL)
0x06, // bTerminalID
0x01, 0x01, // wTerminalType (USB Streaming)
0x01, // bAssocTerminal
0x05, // bSourceID
0x00, // iTerminal
0x09, // bLength
0x04, // bDescriptorType (Interface)
0x01, // bInterfaceNumber 1
0x00, // bAlternateSetting
0x00, // bNumEndpoints 0
0x01, // bInterfaceClass (Audio)
0x02, // bInterfaceSubClass (Audio Streaming)
0x00, // bInterfaceProtocol
0x00, // iInterface (String Index)
0x09, // bLength
0x04, // bDescriptorType (Interface)
0x01, // bInterfaceNumber 1
0x01, // bAlternateSetting
0x01, // bNumEndpoints 1
0x01, // bInterfaceClass (Audio)
0x02, // bInterfaceSubClass (Audio Streaming)
0x00, // bInterfaceProtocol
0x00, // iInterface (String Index)
0x07, // bLength
0x24, // bDescriptorType (See Next Line)
0x01, // bDescriptorSubtype (CS_INTERFACE -> AS_GENERAL)
0x01, // bTerminalLink
0x01, // bDelay 1
0x01, 0x00, // wFormatTag (PCM)
0x0B, // bLength
0x24, // bDescriptorType (See Next Line)
0x02, // bDescriptorSubtype (CS_INTERFACE -> FORMAT_TYPE)
0x01, // bFormatType 1
0x02, // bNrChannels (Stereo)
0x02, // bSubFrameSize 2
0x10, // bBitResolution 16
0x01, // bSamFreqType 1
0x00, 0x7D, 0x00, // tSamFreq[1] 32000 Hz
0x09, // bLength
0x05, // bDescriptorType (See Next Line)
0x01, // bEndpointAddress (OUT/H2D)
0x09, // bmAttributes (Isochronous, Adaptive, Data EP)
0x84, 0x00, // wMaxPacketSize 132
0x01, // bInterval 1 (unit depends on device speed)
0x00, // bRefresh
0x00, // bSyncAddress
0x07, // bLength
0x25, // bDescriptorType (See Next Line)
0x01, // bDescriptorSubtype (CS_ENDPOINT -> EP_GENERAL)
0x00, // bmAttributes (None)
0x00, // bLockDelayUnits
0x00, 0x00, // wLockDelay 0
0x09, // bLength
0x04, // bDescriptorType (Interface)
0x02, // bInterfaceNumber 2
0x00, // bAlternateSetting
0x00, // bNumEndpoints 0
0x01, // bInterfaceClass (Audio)
0x02, // bInterfaceSubClass (Audio Streaming)
0x00, // bInterfaceProtocol
0x00, // iInterface (String Index)
0x09, // bLength
0x04, // bDescriptorType (Interface)
0x02, // bInterfaceNumber 2
0x01, // bAlternateSetting
0x01, // bNumEndpoints 1
0x01, // bInterfaceClass (Audio)
0x02, // bInterfaceSubClass (Audio Streaming)
0x00, // bInterfaceProtocol
0x00, // iInterface (String Index)
0x07, // bLength
0x24, // bDescriptorType (See Next Line)
0x01, // bDescriptorSubtype (CS_INTERFACE -> AS_GENERAL)
0x06, // bTerminalLink
0x01, // bDelay 1
0x01, 0x00, // wFormatTag (PCM)
0x0B, // bLength
0x24, // bDescriptorType (See Next Line)
0x02, // bDescriptorSubtype (CS_INTERFACE -> FORMAT_TYPE)
0x01, // bFormatType 1
0x01, // bNrChannels (Mono)
0x02, // bSubFrameSize 2
0x10, // bBitResolution 16
0x01, // bSamFreqType 1
0x80, 0x3E, 0x00, // tSamFreq[1] 16000 Hz
0x09, // bLength
0x05, // bDescriptorType (See Next Line)
0x82, // bEndpointAddress (IN/D2H)
0x05, // bmAttributes (Isochronous, Async, Data EP)
0x22, 0x00, // wMaxPacketSize 34
0x01, // bInterval 1 (unit depends on device speed)
0x00, // bRefresh
0x00, // bSyncAddress
0x07, // bLength
0x25, // bDescriptorType (See Next Line)
0x01, // bDescriptorSubtype (CS_ENDPOINT -> EP_GENERAL)
0x00, // bmAttributes (None)
0x00, // bLockDelayUnits
0x00, 0x00, // wLockDelay 0
0x09, // bLength
0x04, // bDescriptorType (Interface)
0x03, // bInterfaceNumber 3
0x00, // bAlternateSetting
0x02, // bNumEndpoints 2
0x03, // bInterfaceClass
0x00, // bInterfaceSubClass
0x00, // bInterfaceProtocol
0x00, // iInterface (String Index)
0x09, // bLength
0x21, // bDescriptorType (HID)
0x11, 0x01, // bcdHID 1.11
0x00, // bCountryCode
0x01, // bNumDescriptors
0x22, // bDescriptorType[0] (HID)
0xFB, 0x01, // wDescriptorLength[0] 507
0x07, // bLength
0x05, // bDescriptorType (Endpoint)
0x84, // bEndpointAddress (IN/D2H)
0x03, // bmAttributes (Interrupt)
0x40, 0x00, // wMaxPacketSize 64
0x05, // bInterval 5 (unit depends on device speed)
0x07, // bLength
0x05, // bDescriptorType (Endpoint)
0x03, // bEndpointAddress (OUT/H2D)
0x03, // bmAttributes (Interrupt)
0x40, 0x00, // wMaxPacketSize 64
0x05, // bInterval 5 (unit depends on device speed)
// 225 bytes
Notice how there are multiple descriptors with bInterfaceNumber
= 1.
This iterator would never get to interface with bInterfaceNumber
= 3 (the hid class one).
I'll fix it so that the user will get all of them. At minimum this needs to be mentioned in doc string so the user knows to expect more then num_interfaces
members.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm yeah seems like we're not accounting for bAlternateSetting
variations, we could try to parse all descriptors (should account for most edge-cases) but it would be a bit slower. Would definitely be good to mention in docs as reminder of this behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made a fix to this and some other problems:
i404788#2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@i404788 kind reminder, Could you take a look?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@i404788 Ping
/// Errors returned by [`ChannelOut::write`] and [`ChannelIn::read`] | ||
#[derive(Copy, Clone, Eq, PartialEq, Debug)] | ||
#[cfg_attr(feature = "defmt", derive(defmt::Format))] | ||
pub enum ChannelError { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I wrong or what we call channel is called pipe in USB spec?
If so I'd think we should align ourselves with the spec.
From what I gather this doesn't cover ULPI-based hosts in e.g. stm32, does it? |
Follow up to #3307 (see for discussion)
This add
UsbHostDriver
&UsbChannel
traits allowing HAL's to implement Usb Host support.Example handlers are in
embassy-usb/src/handlers/
, and there is additionally a UAC handler made by @AlexCharlton available.Changes since #3307:
UsbChannel
s now have aset_timeout
to enable configurable timeoutsPending tasks:
set_timeout
set_timeout