From e5728a440fe382e7abf3a9d5389f249bdc669163 Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Thu, 2 Jul 2020 18:07:16 +0200 Subject: [PATCH 01/15] first sketch: pre-impl set_address to make ws work for mac os users (works; not ported, not cleaned up) --- advanced/common/usb/src/lib.rs | 70 ++++++++++++++++++--- advanced/firmware/src/bin/usb-2-solution.rs | 40 +++++++++--- advanced/firmware/src/bin/usb-4.rs | 1 - 3 files changed, 90 insertions(+), 21 deletions(-) diff --git a/advanced/common/usb/src/lib.rs b/advanced/common/usb/src/lib.rs index 3b455b3..4b8ff2e 100644 --- a/advanced/common/usb/src/lib.rs +++ b/advanced/common/usb/src/lib.rs @@ -4,7 +4,6 @@ #![deny(warnings)] #![no_std] -#[cfg(TODO)] use core::num::NonZeroU8; /// Standard USB request @@ -21,7 +20,6 @@ pub enum Request { /// SET_ADDRESS // see section 9.4.6 of the USB specification - #[cfg(TODO)] SetAddress { /// New device address, in the range `1..=127` address: Option, @@ -43,14 +41,48 @@ impl Request { /// Returns `Err` if the SETUP data doesn't match a supported standard request // see section 9.4 of the USB specification; in particular tables 9-3, 9-4 and 9-5 pub fn parse( - _bmrequesttype: u8, - _brequest: u8, - _wvalue: u16, - _windex: u16, - _wlength: u16, + bmrequesttype: u8, + brequest: u8, + wvalue: u16, + windex: u16, + wlength: u16, ) -> Result { - // FIXME - Err(()) + // see table 9-4 (USB specification) + const SET_ADDRESS: u8 = 5; + const GET_DESCRIPTOR: u8 = 6; // todo deleteme + + // TODO löschen und durch instructions ersetzen + if bmrequesttype == 0b10000000 && brequest == GET_DESCRIPTOR { + // see table 9-5 + const DEVICE: u8 = 1; + + let desc_ty = (wvalue >> 8) as u8; + let desc_index = wvalue as u8; + let langid = windex; + + if desc_ty == DEVICE && desc_index == 0 && langid == 0 { + Ok(Request::GetDescriptor { + descriptor: Descriptor::Device, + length: wlength, + }) + } else { + Err(()) + } + } else if bmrequesttype == 0b00000000 && brequest == SET_ADDRESS { + + // Set the device address for all future accesses. + // Needed to successfully init when using Apple devices. + if wvalue < 128 && windex == 0 && wlength == 0 { + Ok(Request::SetAddress { + address: NonZeroU8::new(wvalue as u8), + }) + } else { + Err(()) + } + } else { + Err(()) + } + } } @@ -69,9 +101,27 @@ pub enum Descriptor { // there are even more descriptor types but we don't need to support them } +/// Device address assigned by the host; will be in the range 1..=127 +pub type Address = NonZeroU8; +/// The state of the USB device +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum DeviceState { + /// The default state + Default, + /// The address-ed state + Address(Address), + /// The configured state + Configured { + /// The address of the device + address: Address, + /// The configuration value + value: NonZeroU8, + }, +} + + #[cfg(test)] mod tests { - #[cfg(TODO)] use core::num::NonZeroU8; use crate::{Descriptor, Request}; diff --git a/advanced/firmware/src/bin/usb-2-solution.rs b/advanced/firmware/src/bin/usb-2-solution.rs index 4cec5dc..c49d430 100644 --- a/advanced/firmware/src/bin/usb-2-solution.rs +++ b/advanced/firmware/src/bin/usb-2-solution.rs @@ -6,11 +6,12 @@ use dk::{ usbd::{self, Event}, }; use panic_log as _; // panic handler -use usb::{Descriptor, Request}; +use usb::{Descriptor, Request, DeviceState}; #[rtic::app(device = dk)] const APP: () = { struct Resources { + state: DeviceState, usbd: USBD, } @@ -20,20 +21,24 @@ const APP: () = { usbd::init(board.power, &board.usbd); - init::LateResources { usbd: board.usbd } + init::LateResources { + usbd: board.usbd, + state: DeviceState::Default, + } } - #[task(binds = USBD, resources = [usbd])] + #[task(binds = USBD, resources = [state, usbd])] fn main(cx: main::Context) { let usbd = cx.resources.usbd; + let state = cx.resources.state; // used to store the device address sent by the host while let Some(event) = usbd::next_event(usbd) { - on_event(usbd, event) + on_event(usbd, state, event) } } }; -fn on_event(usbd: &USBD, event: Event) { +fn on_event(usbd: &USBD, state: &mut DeviceState, event: Event) { log::info!("USB: {:?} @ {:?}", event, dk::uptime()); match event { @@ -62,16 +67,31 @@ fn on_event(usbd: &USBD, event: Event) { wvalue ); - if let Ok(Request::GetDescriptor { descriptor, length }) = - Request::parse(bmrequesttype, brequest, wvalue, windex, wlength) - { - match descriptor { - Descriptor::Device => { + // todo handle less indentedly? + if let Ok(request) = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength) { + match request { + Request::GetDescriptor { descriptor, length } + if descriptor == Descriptor::Device => + { log::info!("GET_DESCRIPTOR Device [length={}]", length); log::info!("Goal reached; move to the next section"); dk::exit() } + Request::SetAddress { address } => { + log::info!("SETUP: device state is {:?}", state); + + // todo hide this in a helper? + // todo check for state configured -> err + if let Some(address) = address { + log::info!("SETUP: assigning address {}", address); + *state = DeviceState::Address(address); + } else { + log::info!("SETUP: address was None; assigning Default"); + *state = DeviceState::Default; + } + } + _ => unreachable!(), // we don't handle any other Requests } } else { unreachable!() // don't care about this for now diff --git a/advanced/firmware/src/bin/usb-4.rs b/advanced/firmware/src/bin/usb-4.rs index f79983d..8aa68f9 100644 --- a/advanced/firmware/src/bin/usb-4.rs +++ b/advanced/firmware/src/bin/usb-4.rs @@ -103,7 +103,6 @@ fn ep0setup(usbd: &USBD, ep0in: &mut Ep0In, _state: &mut usb2::State) -> Result< // TODO Configuration descriptor // Descriptor::Configuration => todo!(), }, - // TODO // Request::SetAddress { .. } => todo!(), // Request::SetConfiguration { .. } => todo!(), From 351cce7c16d57738373eef187c0d4077b29289b9 Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Thu, 2 Jul 2020 18:23:41 +0200 Subject: [PATCH 02/15] usb-2-solution.rs: cleanup, move addr setting into helper --- advanced/common/usb/src/lib.rs | 8 +-- advanced/firmware/src/bin/usb-2-solution.rs | 54 ++++++++++----------- 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/advanced/common/usb/src/lib.rs b/advanced/common/usb/src/lib.rs index 4b8ff2e..466e09a 100644 --- a/advanced/common/usb/src/lib.rs +++ b/advanced/common/usb/src/lib.rs @@ -6,6 +6,9 @@ use core::num::NonZeroU8; +/// Device address assigned by the host; will be in the range 1..=127 +pub type Address = NonZeroU8; + /// Standard USB request #[derive(Clone, Copy, Debug, PartialEq)] pub enum Request { @@ -22,7 +25,7 @@ pub enum Request { // see section 9.4.6 of the USB specification SetAddress { /// New device address, in the range `1..=127` - address: Option, + address: Option
, }, /// SET_CONFIGURATION @@ -101,8 +104,6 @@ pub enum Descriptor { // there are even more descriptor types but we don't need to support them } -/// Device address assigned by the host; will be in the range 1..=127 -pub type Address = NonZeroU8; /// The state of the USB device #[derive(Clone, Copy, Debug, PartialEq)] pub enum DeviceState { @@ -122,6 +123,7 @@ pub enum DeviceState { #[cfg(test)] mod tests { + #[cfg(TODO)] use core::num::NonZeroU8; use crate::{Descriptor, Request}; diff --git a/advanced/firmware/src/bin/usb-2-solution.rs b/advanced/firmware/src/bin/usb-2-solution.rs index c49d430..9132adc 100644 --- a/advanced/firmware/src/bin/usb-2-solution.rs +++ b/advanced/firmware/src/bin/usb-2-solution.rs @@ -6,7 +6,7 @@ use dk::{ usbd::{self, Event}, }; use panic_log as _; // panic handler -use usb::{Descriptor, Request, DeviceState}; +use usb::{Address, Descriptor, DeviceState, Request}; #[rtic::app(device = dk)] const APP: () = { @@ -67,35 +67,35 @@ fn on_event(usbd: &USBD, state: &mut DeviceState, event: Event) { wvalue ); - // todo handle less indentedly? - if let Ok(request) = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength) { - match request { - Request::GetDescriptor { descriptor, length } - if descriptor == Descriptor::Device => - { - log::info!("GET_DESCRIPTOR Device [length={}]", length); + let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength) + .expect("Error parsing request"); + match request { + Request::GetDescriptor { descriptor, length } + if descriptor == Descriptor::Device => + { + log::info!("GET_DESCRIPTOR Device [length={}]", length); - log::info!("Goal reached; move to the next section"); - dk::exit() - } - Request::SetAddress { address } => { - log::info!("SETUP: device state is {:?}", state); - - // todo hide this in a helper? - // todo check for state configured -> err - if let Some(address) = address { - log::info!("SETUP: assigning address {}", address); - *state = DeviceState::Address(address); - } else { - log::info!("SETUP: address was None; assigning Default"); - *state = DeviceState::Default; - } - } - _ => unreachable!(), // we don't handle any other Requests + log::info!("Goal reached; move to the next section"); + dk::exit() } - } else { - unreachable!() // don't care about this for now + Request::SetAddress { address } => set_device_address(state, address), + _ => unreachable!(), // we don't handle any other Requests } } } } + +fn set_device_address(state: &mut DeviceState, address: Option
) { + match state { + DeviceState::Default | DeviceState::Address(..) => { + if let Some(address) = address { + log::info!("SETUP: assigning device address {}", address); + *state = DeviceState::Address(address); + } else { + log::info!("SETUP: address was None; assigning Default"); + *state = DeviceState::Default; + } + } + DeviceState::Configured { .. } => panic!(), // unspecified behavior + } +} From a72886059f7790f0c855f16b8ef4d6b5ed522621 Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Thu, 2 Jul 2020 18:32:29 +0200 Subject: [PATCH 03/15] =?UTF-8?q?advanced/common/usb/lib.rs:=20cleanup?= =?UTF-8?q?=E2=80=93=20rm=20GET=5FDESCRIPTOR=20handling,=20suggest=20outli?= =?UTF-8?q?ne?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- advanced/common/usb/src/lib.rs | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/advanced/common/usb/src/lib.rs b/advanced/common/usb/src/lib.rs index 466e09a..716fec5 100644 --- a/advanced/common/usb/src/lib.rs +++ b/advanced/common/usb/src/lib.rs @@ -52,27 +52,17 @@ impl Request { ) -> Result { // see table 9-4 (USB specification) const SET_ADDRESS: u8 = 5; - const GET_DESCRIPTOR: u8 = 6; // todo deleteme - // TODO löschen und durch instructions ersetzen - if bmrequesttype == 0b10000000 && brequest == GET_DESCRIPTOR { - // see table 9-5 - const DEVICE: u8 = 1; + /* TODO implement another branch handling GET_DESCRIPTOR requests: */ - let desc_ty = (wvalue >> 8) as u8; - let desc_index = wvalue as u8; - let langid = windex; - - if desc_ty == DEVICE && desc_index == 0 && langid == 0 { - Ok(Request::GetDescriptor { - descriptor: Descriptor::Device, - length: wlength, - }) - } else { - Err(()) - } - } else if bmrequesttype == 0b00000000 && brequest == SET_ADDRESS { + /* 1. get descriptor type and descriptor index from wValue */ + /* 2. confirm that the descriptor + - is of type DEVICE and + - has descriptor index 0 (i.e. it is the first implemented descriptor for this type) and + - has wIndex 0 (i.e. no language ID since it's not a string descriptor) + */ + if bmrequesttype == 0b00000000 && brequest == SET_ADDRESS { // Set the device address for all future accesses. // Needed to successfully init when using Apple devices. if wvalue < 128 && windex == 0 && wlength == 0 { @@ -85,7 +75,6 @@ impl Request { } else { Err(()) } - } } From 2b3e9a73b7d05776d596d736e530eb742f206d08 Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Fri, 3 Jul 2020 12:04:05 +0200 Subject: [PATCH 04/15] respond to comments: ignore set_address for now --- advanced/firmware/src/bin/usb-2-solution.rs | 32 ++++++--------------- advanced/firmware/src/bin/usb-2.rs | 28 ++++++++++-------- 2 files changed, 25 insertions(+), 35 deletions(-) diff --git a/advanced/firmware/src/bin/usb-2-solution.rs b/advanced/firmware/src/bin/usb-2-solution.rs index 9132adc..8494098 100644 --- a/advanced/firmware/src/bin/usb-2-solution.rs +++ b/advanced/firmware/src/bin/usb-2-solution.rs @@ -6,12 +6,11 @@ use dk::{ usbd::{self, Event}, }; use panic_log as _; // panic handler -use usb::{Address, Descriptor, DeviceState, Request}; +use usb::{Descriptor, Request}; #[rtic::app(device = dk)] const APP: () = { struct Resources { - state: DeviceState, usbd: USBD, } @@ -23,22 +22,20 @@ const APP: () = { init::LateResources { usbd: board.usbd, - state: DeviceState::Default, } } - #[task(binds = USBD, resources = [state, usbd])] + #[task(binds = USBD, resources = [usbd])] fn main(cx: main::Context) { let usbd = cx.resources.usbd; - let state = cx.resources.state; // used to store the device address sent by the host while let Some(event) = usbd::next_event(usbd) { - on_event(usbd, state, event) + on_event(usbd, event) } } }; -fn on_event(usbd: &USBD, state: &mut DeviceState, event: Event) { +fn on_event(usbd: &USBD, event: Event) { log::info!("USB: {:?} @ {:?}", event, dk::uptime()); match event { @@ -78,24 +75,13 @@ fn on_event(usbd: &USBD, state: &mut DeviceState, event: Event) { log::info!("Goal reached; move to the next section"); dk::exit() } - Request::SetAddress { address } => set_device_address(state, address), + Request::SetAddress { .. } => { + // On Mac OS you'll get this request before the GET_DESCRIPTOR request so we + // need to catch it here. We'll properly handle this request later + // but for now it's OK to do nothing. + }, _ => unreachable!(), // we don't handle any other Requests } } } } - -fn set_device_address(state: &mut DeviceState, address: Option
) { - match state { - DeviceState::Default | DeviceState::Address(..) => { - if let Some(address) = address { - log::info!("SETUP: assigning device address {}", address); - *state = DeviceState::Address(address); - } else { - log::info!("SETUP: address was None; assigning Default"); - *state = DeviceState::Default; - } - } - DeviceState::Configured { .. } => panic!(), // unspecified behavior - } -} diff --git a/advanced/firmware/src/bin/usb-2.rs b/advanced/firmware/src/bin/usb-2.rs index b63310a..36cf873 100644 --- a/advanced/firmware/src/bin/usb-2.rs +++ b/advanced/firmware/src/bin/usb-2.rs @@ -60,20 +60,24 @@ fn on_event(_usbd: &USBD, event: Event) { wvalue ); - // FIXME modify `advanced/common/usb` to make this work - if let Ok(Request::GetDescriptor { descriptor, length }) = - Request::parse(bmrequesttype, brequest, wvalue, windex, wlength) - { - match descriptor { - Descriptor::Device => { - log::info!("GET_DESCRIPTOR Device [length={}]", length); + // TODO modify `advanced/common/usb` to make this work + let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength) + .expect("Error parsing request"); + match request { + Request::GetDescriptor { descriptor, length } + if descriptor == Descriptor::Device => + { + log::info!("GET_DESCRIPTOR Device [length={}]", length); - log::info!("Goal reached; move to the next section"); - dk::exit() - } + log::info!("Goal reached; move to the next section"); + dk::exit() } - } else { - unreachable!() // don't care about this for now + Request::SetAddress { .. } => { + // On Mac OS you'll get this request before the GET_DESCRIPTOR request so we + // need to catch it here. We'll properly handle this request later + // but for now it's OK to do nothing. + }, + _ => unreachable!(), // we don't handle any other Requests } } } From 08884f1718770908fd6562c8949c7e22fb40f8df Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Fri, 3 Jul 2020 12:16:59 +0200 Subject: [PATCH 05/15] aaaaand remove DeviceState too --- advanced/common/usb/src/lib.rs | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/advanced/common/usb/src/lib.rs b/advanced/common/usb/src/lib.rs index 716fec5..55d2ba2 100644 --- a/advanced/common/usb/src/lib.rs +++ b/advanced/common/usb/src/lib.rs @@ -62,6 +62,7 @@ impl Request { - has descriptor index 0 (i.e. it is the first implemented descriptor for this type) and - has wIndex 0 (i.e. no language ID since it's not a string descriptor) */ + if bmrequesttype == 0b00000000 && brequest == SET_ADDRESS { // Set the device address for all future accesses. // Needed to successfully init when using Apple devices. @@ -93,23 +94,6 @@ pub enum Descriptor { // there are even more descriptor types but we don't need to support them } -/// The state of the USB device -#[derive(Clone, Copy, Debug, PartialEq)] -pub enum DeviceState { - /// The default state - Default, - /// The address-ed state - Address(Address), - /// The configured state - Configured { - /// The address of the device - address: Address, - /// The configuration value - value: NonZeroU8, - }, -} - - #[cfg(test)] mod tests { #[cfg(TODO)] From 4bd350c9480e416c0df6de488d6815b7a3db8c16 Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Fri, 3 Jul 2020 12:22:58 +0200 Subject: [PATCH 06/15] usb/src/lib.rs: make set_address() run by default, move upwards --- advanced/common/usb/src/lib.rs | 36 ++++++++++++++++------------------ 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/advanced/common/usb/src/lib.rs b/advanced/common/usb/src/lib.rs index 55d2ba2..c66da16 100644 --- a/advanced/common/usb/src/lib.rs +++ b/advanced/common/usb/src/lib.rs @@ -96,7 +96,6 @@ pub enum Descriptor { #[cfg(test)] mod tests { - #[cfg(TODO)] use core::num::NonZeroU8; use crate::{Descriptor, Request}; @@ -121,24 +120,6 @@ mod tests { // ^^^^ } - #[cfg(TODO)] - #[test] - fn get_descriptor_configuration() { - // OK: GET_DESCRIPTOR Configuration 0 [length=9] - assert_eq!( - Request::parse(0b1000_0000, 0x06, 0x02_00, 0, 9), - Ok(Request::GetDescriptor { - descriptor: Descriptor::Configuration { index: 0 }, - length: 9 - }) - ); - - // has language ID but shouldn't - assert!(Request::parse(0b1000_0000, 0x06, 0x02_00, 1033, 9).is_err()); - // ^^^^ - } - - #[cfg(TODO)] #[test] fn set_address() { // OK: SET_ADDRESS 16 @@ -168,6 +149,23 @@ mod tests { // ^ } + #[cfg(TODO)] + #[test] + fn get_descriptor_configuration() { + // OK: GET_DESCRIPTOR Configuration 0 [length=9] + assert_eq!( + Request::parse(0b1000_0000, 0x06, 0x02_00, 0, 9), + Ok(Request::GetDescriptor { + descriptor: Descriptor::Configuration { index: 0 }, + length: 9 + }) + ); + + // has language ID but shouldn't + assert!(Request::parse(0b1000_0000, 0x06, 0x02_00, 1033, 9).is_err()); + // ^^^^ + } + #[cfg(TODO)] #[test] fn set_configuration() { From f2cc43b55fc3eb91fc7c6e34cc3a68284d6d89c6 Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Fri, 3 Jul 2020 14:49:54 +0200 Subject: [PATCH 07/15] re review. point to usb spec for wvalue etc --- advanced/common/usb/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/advanced/common/usb/src/lib.rs b/advanced/common/usb/src/lib.rs index c66da16..498a54d 100644 --- a/advanced/common/usb/src/lib.rs +++ b/advanced/common/usb/src/lib.rs @@ -65,7 +65,9 @@ impl Request { if bmrequesttype == 0b00000000 && brequest == SET_ADDRESS { // Set the device address for all future accesses. - // Needed to successfully init when using Apple devices. + // (Needed to successfully init when conected to Apple devices) + // Section 9.4.6 Set Address of the USB specification explains which values for wvalue, + // windex and wlength are valid. if wvalue < 128 && windex == 0 && wlength == 0 { Ok(Request::SetAddress { address: NonZeroU8::new(wvalue as u8), From c3d680e76df32382daf4d50ab91294bfc895dbfd Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Fri, 3 Jul 2020 15:15:30 +0200 Subject: [PATCH 08/15] re coments: move TODO instructions in usb-2 --- advanced/firmware/src/bin/usb-2.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/advanced/firmware/src/bin/usb-2.rs b/advanced/firmware/src/bin/usb-2.rs index 36cf873..8c5f95a 100644 --- a/advanced/firmware/src/bin/usb-2.rs +++ b/advanced/firmware/src/bin/usb-2.rs @@ -60,13 +60,15 @@ fn on_event(_usbd: &USBD, event: Event) { wvalue ); - // TODO modify `advanced/common/usb` to make this work let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength) .expect("Error parsing request"); match request { Request::GetDescriptor { descriptor, length } if descriptor == Descriptor::Device => { + // TODO modify `Request::parse()` in `advanced/common/usb/lib.rs` + // so that this branch is reached + log::info!("GET_DESCRIPTOR Device [length={}]", length); log::info!("Goal reached; move to the next section"); From 44d3bfe32a609c89d3edcc29f7b7239e24abcbe0 Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Fri, 3 Jul 2020 15:25:01 +0200 Subject: [PATCH 09/15] get-descriptor-device.rs: add SET_ADDRESS handling; comments to solution --- .../common/usb/src/get-descriptor-device.rs | 64 ++++++++++++------- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/advanced/common/usb/src/get-descriptor-device.rs b/advanced/common/usb/src/get-descriptor-device.rs index 36fb245..fd1c210 100644 --- a/advanced/common/usb/src/get-descriptor-device.rs +++ b/advanced/common/usb/src/get-descriptor-device.rs @@ -5,9 +5,11 @@ #![deny(warnings)] #![no_std] -#[cfg(TODO)] use core::num::NonZeroU8; +/// Device address assigned by the host; will be in the range 1..=127 +pub type Address = NonZeroU8; + /// Standard USB request #[derive(Clone, Copy, Debug, PartialEq)] pub enum Request { @@ -22,10 +24,9 @@ pub enum Request { /// SET_ADDRESS // see section 9.4.6 of the USB specification - #[cfg(TODO)] SetAddress { /// New device address, in the range `1..=127` - address: Option, + address: Option
, }, /// SET_CONFIGURATION @@ -50,17 +51,24 @@ impl Request { windex: u16, wlength: u16, ) -> Result { - // see table 9-4 + // see table 9-4 (USB specification) + const SET_ADDRESS: u8 = 5; const GET_DESCRIPTOR: u8 = 6; + if bmrequesttype == 0b10000000 && brequest == GET_DESCRIPTOR { // see table 9-5 const DEVICE: u8 = 1; + // 1. get descriptor type and descriptor index from wValue let desc_ty = (wvalue >> 8) as u8; let desc_index = wvalue as u8; let langid = windex; + // 2. confirm that the descriptor + // - is of type DEVICE and + // - has descriptor index 0 (i.e. it is the first implemented descriptor for this type) and + // - has wIndex 0 (i.e. no language ID since it's not a string descriptor) if desc_ty == DEVICE && desc_index == 0 && langid == 0 { Ok(Request::GetDescriptor { descriptor: Descriptor::Device, @@ -69,6 +77,18 @@ impl Request { } else { Err(()) } + } else if bmrequesttype == 0b00000000 && brequest == SET_ADDRESS { + // Set the device address for all future accesses. + // (Needed to successfully init when conected to Apple devices) + // Section 9.4.6 Set Address of the USB specification explains which values for wvalue, + // windex and wlength are valid. + if wvalue < 128 && windex == 0 && wlength == 0 { + Ok(Request::SetAddress { + address: NonZeroU8::new(wvalue as u8), + }) + } else { + Err(()) + } } else { Err(()) } @@ -92,7 +112,6 @@ pub enum Descriptor { #[cfg(test)] mod tests { - #[cfg(TODO)] use core::num::NonZeroU8; use crate::{Descriptor, Request}; @@ -117,24 +136,6 @@ mod tests { // ^^^^ } - #[cfg(TODO)] - #[test] - fn get_descriptor_configuration() { - // OK: GET_DESCRIPTOR Configuration 0 [length=9] - assert_eq!( - Request::parse(0b1000_0000, 0x06, 0x02_00, 0, 9), - Ok(Request::GetDescriptor { - descriptor: Descriptor::Configuration { index: 0 }, - length: 9 - }) - ); - - // has language ID but shouldn't - assert!(Request::parse(0b1000_0000, 0x06, 0x02_00, 1033, 9).is_err()); - // ^^^^ - } - - #[cfg(TODO)] #[test] fn set_address() { // OK: SET_ADDRESS 16 @@ -164,6 +165,23 @@ mod tests { // ^ } + #[cfg(TODO)] + #[test] + fn get_descriptor_configuration() { + // OK: GET_DESCRIPTOR Configuration 0 [length=9] + assert_eq!( + Request::parse(0b1000_0000, 0x06, 0x02_00, 0, 9), + Ok(Request::GetDescriptor { + descriptor: Descriptor::Configuration { index: 0 }, + length: 9 + }) + ); + + // has language ID but shouldn't + assert!(Request::parse(0b1000_0000, 0x06, 0x02_00, 1033, 9).is_err()); + // ^^^^ + } + #[cfg(TODO)] #[test] fn set_configuration() { From bffed51a77a1a8ba22fb3546c11bac2036437bbb Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Fri, 3 Jul 2020 15:25:29 +0200 Subject: [PATCH 10/15] lib.rs: unify instruction comment formatting --- advanced/common/usb/src/lib.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/advanced/common/usb/src/lib.rs b/advanced/common/usb/src/lib.rs index 498a54d..2685091 100644 --- a/advanced/common/usb/src/lib.rs +++ b/advanced/common/usb/src/lib.rs @@ -53,15 +53,14 @@ impl Request { // see table 9-4 (USB specification) const SET_ADDRESS: u8 = 5; - /* TODO implement another branch handling GET_DESCRIPTOR requests: */ + // TODO implement another branch handling GET_DESCRIPTOR requests: - /* 1. get descriptor type and descriptor index from wValue */ + // 1. get descriptor type and descriptor index from wValue - /* 2. confirm that the descriptor - - is of type DEVICE and - - has descriptor index 0 (i.e. it is the first implemented descriptor for this type) and - - has wIndex 0 (i.e. no language ID since it's not a string descriptor) - */ + // 2. confirm that the descriptor + // - is of type DEVICE and + // - has descriptor index 0 (i.e. it is the first implemented descriptor for this type) and + // - has wIndex 0 (i.e. no language ID since it's not a string descriptor) if bmrequesttype == 0b00000000 && brequest == SET_ADDRESS { // Set the device address for all future accesses. From bb7ea8f6850eb0847dd8d656b27baa0674bf96d2 Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Fri, 3 Jul 2020 15:47:14 +0200 Subject: [PATCH 11/15] usb-3, sb-3-solution.rs, get-descriptor-configuration.rs: handle SET_ADDRESS --- .../usb/src/get-descriptor-configuration.rs | 58 +++++++++++------- advanced/firmware/src/bin/usb-3-solution.rs | 60 ++++++++++--------- advanced/firmware/src/bin/usb-3.rs | 34 ++++++----- 3 files changed, 88 insertions(+), 64 deletions(-) diff --git a/advanced/common/usb/src/get-descriptor-configuration.rs b/advanced/common/usb/src/get-descriptor-configuration.rs index 388de88..3d531b6 100644 --- a/advanced/common/usb/src/get-descriptor-configuration.rs +++ b/advanced/common/usb/src/get-descriptor-configuration.rs @@ -1,13 +1,15 @@ //! Some USB 2.0 data types -// NOTE this is a solution to exercise `usb-2` +// NOTE this is a solution to exercise `usb-3` #![deny(missing_docs)] #![deny(warnings)] #![no_std] -#[cfg(TODO)] use core::num::NonZeroU8; +/// Device address assigned by the host; will be in the range 1..=127 +pub type Address = NonZeroU8; + /// Standard USB request #[derive(Clone, Copy, Debug, PartialEq)] pub enum Request { @@ -22,10 +24,9 @@ pub enum Request { /// SET_ADDRESS // see section 9.4.6 of the USB specification - #[cfg(TODO)] SetAddress { /// New device address, in the range `1..=127` - address: Option, + address: Option
, }, /// SET_CONFIGURATION @@ -50,7 +51,8 @@ impl Request { windex: u16, wlength: u16, ) -> Result { - // see table 9-4 + // see table 9-4 (USB specification) + const SET_ADDRESS: u8 = 5; const GET_DESCRIPTOR: u8 = 6; if bmrequesttype == 0b10000000 && brequest == GET_DESCRIPTOR { @@ -75,6 +77,18 @@ impl Request { } else { Err(()) } + } else if bmrequesttype == 0b00000000 && brequest == SET_ADDRESS { + // Set the device address for all future accesses. + // (Needed to successfully init when conected to Apple devices) + // Section 9.4.6 Set Address of the USB specification explains which values for wvalue, + // windex and wlength are valid. + if wvalue < 128 && windex == 0 && wlength == 0 { + Ok(Request::SetAddress { + address: NonZeroU8::new(wvalue as u8), + }) + } else { + Err(()) + } } else { Err(()) } @@ -97,7 +111,6 @@ pub enum Descriptor { #[cfg(test)] mod tests { - #[cfg(TODO)] use core::num::NonZeroU8; use crate::{Descriptor, Request}; @@ -122,23 +135,6 @@ mod tests { // ^^^^ } - #[test] - fn get_descriptor_configuration() { - // OK: GET_DESCRIPTOR Configuration 0 [length=9] - assert_eq!( - Request::parse(0b1000_0000, 0x06, 0x02_00, 0, 9), - Ok(Request::GetDescriptor { - descriptor: Descriptor::Configuration { index: 0 }, - length: 9 - }) - ); - - // has language ID but shouldn't - assert!(Request::parse(0b1000_0000, 0x06, 0x02_00, 1033, 9).is_err()); - // ^^^^ - } - - #[cfg(TODO)] #[test] fn set_address() { // OK: SET_ADDRESS 16 @@ -168,6 +164,22 @@ mod tests { // ^ } + #[test] + fn get_descriptor_configuration() { + // OK: GET_DESCRIPTOR Configuration 0 [length=9] + assert_eq!( + Request::parse(0b1000_0000, 0x06, 0x02_00, 0, 9), + Ok(Request::GetDescriptor { + descriptor: Descriptor::Configuration { index: 0 }, + length: 9 + }) + ); + + // has language ID but shouldn't + assert!(Request::parse(0b1000_0000, 0x06, 0x02_00, 1033, 9).is_err()); + // ^^^^ + } + #[cfg(TODO)] #[test] fn set_configuration() { diff --git a/advanced/firmware/src/bin/usb-3-solution.rs b/advanced/firmware/src/bin/usb-3-solution.rs index fbc405f..2afd70f 100644 --- a/advanced/firmware/src/bin/usb-3-solution.rs +++ b/advanced/firmware/src/bin/usb-3-solution.rs @@ -64,35 +64,41 @@ fn on_event(usbd: &USBD, ep0in: &mut Ep0In, event: Event) { wvalue ); - if let Ok(Request::GetDescriptor { descriptor, length }) = - Request::parse(bmrequesttype, brequest, wvalue, windex, wlength) - { - match descriptor { - Descriptor::Device => { - log::info!("GET_DESCRIPTOR Device [length={}]", length); + let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength) + .expect("Error parsing request"); + match request { + Request::GetDescriptor { descriptor, length } + if descriptor == Descriptor::Device => + { + log::info!("GET_DESCRIPTOR Device [length={}]", length); - let desc = usb2::device::Descriptor { - bDeviceClass: 0, - bDeviceProtocol: 0, - bDeviceSubClass: 0, - bMaxPacketSize0: usb2::device::bMaxPacketSize0::B64, - bNumConfigurations: core::num::NonZeroU8::new(1).unwrap(), - bcdDevice: 0x01_00, // 1.00 - iManufacturer: None, - iProduct: None, - iSerialNumber: None, - idProduct: consts::PID, - idVendor: consts::VID, - }; - let desc_bytes = desc.bytes(); - let resp = - &desc_bytes[..core::cmp::min(desc_bytes.len(), usize::from(length))]; - ep0in.start(&resp, usbd); - } + let desc = usb2::device::Descriptor { + bDeviceClass: 0, + bDeviceProtocol: 0, + bDeviceSubClass: 0, + bMaxPacketSize0: usb2::device::bMaxPacketSize0::B64, + bNumConfigurations: core::num::NonZeroU8::new(1).unwrap(), + bcdDevice: 0x01_00, // 1.00 + iManufacturer: None, + iProduct: None, + iSerialNumber: None, + idProduct: consts::PID, + idVendor: consts::VID, + }; + let desc_bytes = desc.bytes(); + let resp = + &desc_bytes[..core::cmp::min(desc_bytes.len(), usize::from(length))]; + ep0in.start(&resp, usbd); + } + Request::SetAddress { .. } => { + // On Mac OS you'll get this request before the GET_DESCRIPTOR request so we + // need to catch it here. We'll properly handle this request later + // but for now it's OK to do nothing. + }, + _ => { + log::error!("unknown request (goal achieved if GET_DESCRIPTOR Device was handled)"); + dk::exit() } - } else { - log::error!("unknown request (goal achieved if GET_DESCRIPTOR Device was handled)"); - dk::exit() } } } diff --git a/advanced/firmware/src/bin/usb-3.rs b/advanced/firmware/src/bin/usb-3.rs index a68b0bc..f2f0471 100644 --- a/advanced/firmware/src/bin/usb-3.rs +++ b/advanced/firmware/src/bin/usb-3.rs @@ -64,22 +64,28 @@ fn on_event(usbd: &USBD, ep0in: &mut Ep0In, event: Event) { wvalue ); - if let Ok(Request::GetDescriptor { descriptor, length }) = - Request::parse(bmrequesttype, brequest, wvalue, windex, wlength) - { - match descriptor { - Descriptor::Device => { - log::info!("GET_DESCRIPTOR Device [length={}]", length); + let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength) + .expect("Error parsing request"); + match request { + Request::GetDescriptor { descriptor, length } + if descriptor == Descriptor::Device => + { + log::info!("GET_DESCRIPTOR Device [length={}]", length); - // FIXME send back a valid device descriptor, truncated to `length` bytes - // let desc = usb2::device::Descriptor { .. }; - let resp = []; - ep0in.start(&resp, usbd); - } + // TODO send back a valid device descriptor, truncated to `length` bytes + // let desc = usb2::device::Descriptor { .. }; + let resp = []; + ep0in.start(&resp, usbd); + } + Request::SetAddress { .. } => { + // On Mac OS you'll get this request before the GET_DESCRIPTOR request so we + // need to catch it here. We'll properly handle this request later + // but for now it's OK to do nothing. + }, + _ => { + log::error!("unknown request (goal achieved if GET_DESCRIPTOR Device was handled)"); + dk::exit() } - } else { - log::error!("unknown request (goal achieved if GET_DESCRIPTOR Device was handled)"); - dk::exit() } } } From 88d713d65fbac81ab74d8299f9df99d9218de38a Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Fri, 3 Jul 2020 16:27:48 +0200 Subject: [PATCH 12/15] usb-4.rs: unify --- advanced/firmware/src/bin/usb-4.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/advanced/firmware/src/bin/usb-4.rs b/advanced/firmware/src/bin/usb-4.rs index 8aa68f9..b99f05f 100644 --- a/advanced/firmware/src/bin/usb-4.rs +++ b/advanced/firmware/src/bin/usb-4.rs @@ -77,10 +77,11 @@ fn ep0setup(usbd: &USBD, ep0in: &mut Ep0In, _state: &mut usb2::State) -> Result< wvalue ); - let req = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength)?; - log::info!("{:?}", req); + let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength) + .expect("Error parsing request"); + log::info!("EP0: {:?}", request); - match req { + match request { Request::GetDescriptor { descriptor, length } => match descriptor { Descriptor::Device => { let desc = usb2::device::Descriptor { @@ -100,11 +101,17 @@ fn ep0setup(usbd: &USBD, ep0in: &mut Ep0In, _state: &mut usb2::State) -> Result< let _ = ep0in.start(&bytes[..core::cmp::min(bytes.len(), length.into())], usbd); } - // TODO Configuration descriptor - // Descriptor::Configuration => todo!(), + // TODO implement Configuration descriptor + // Descriptor::Configuration { .. } => todo!(), }, - // TODO - // Request::SetAddress { .. } => todo!(), + Request::SetAddress { .. } => { + // On Mac OS you'll get this request before the GET_DESCRIPTOR request so we + // need to catch it here. + + // TODO: handle this request properly now. + todo!() + } + // TODO handle SET_CONFIGURATION request // Request::SetConfiguration { .. } => todo!(), } From c48dd306f45d2b8f7a2c7c777963f1e7e418029f Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Fri, 3 Jul 2020 16:28:16 +0200 Subject: [PATCH 13/15] usb-3-solution.rs, usb-2.rs: rm trailing commata --- advanced/firmware/src/bin/usb-2.rs | 2 +- advanced/firmware/src/bin/usb-3-solution.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/advanced/firmware/src/bin/usb-2.rs b/advanced/firmware/src/bin/usb-2.rs index 8c5f95a..abcf6df 100644 --- a/advanced/firmware/src/bin/usb-2.rs +++ b/advanced/firmware/src/bin/usb-2.rs @@ -78,7 +78,7 @@ fn on_event(_usbd: &USBD, event: Event) { // On Mac OS you'll get this request before the GET_DESCRIPTOR request so we // need to catch it here. We'll properly handle this request later // but for now it's OK to do nothing. - }, + } _ => unreachable!(), // we don't handle any other Requests } } diff --git a/advanced/firmware/src/bin/usb-3-solution.rs b/advanced/firmware/src/bin/usb-3-solution.rs index 2afd70f..6739de4 100644 --- a/advanced/firmware/src/bin/usb-3-solution.rs +++ b/advanced/firmware/src/bin/usb-3-solution.rs @@ -94,7 +94,7 @@ fn on_event(usbd: &USBD, ep0in: &mut Ep0In, event: Event) { // On Mac OS you'll get this request before the GET_DESCRIPTOR request so we // need to catch it here. We'll properly handle this request later // but for now it's OK to do nothing. - }, + } _ => { log::error!("unknown request (goal achieved if GET_DESCRIPTOR Device was handled)"); dk::exit() From 83f92ab02b163a4b1c8aded7dd9067c8093ee810 Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Fri, 3 Jul 2020 16:53:54 +0200 Subject: [PATCH 14/15] update text in advanced/README.md --- advanced/README.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/advanced/README.md b/advanced/README.md index ae62584..e187bf0 100644 --- a/advanced/README.md +++ b/advanced/README.md @@ -256,9 +256,11 @@ So that's what we'll do here. In the `advanced/common/usb` folder you'll find st To sum up the work to do here: -1. write a SETUP data parser in `advanced/common/usb`. You only need to handle the GET_DESCRIPTOR request and make the `get_descriptor_device` test pass for now. +1. write a SETUP data parser in `advanced/common/usb`. You only need to handle the GET_DESCRIPTOR request and make the `get_descriptor_device` test pass for now. Note that the parser already handles SET_ADDRESS requests. -2. modify `usb-1` to read (USBD registers) and parse the SETUP data when the EPSETUP event is received. +2. modify `usb-1` to read (USBD registers) and parse the SETUP data when the EPSETUP event is received. + +> Note: If you're using a Mac, you need to catch `SetAddress` requests returned by the parser as these are sent before the first GetDescriptor request. You can handle them by doing nothing. 3. when you have successfully received a GET_DESCRIPTOR request for a Device descriptor you are done and can move to the next section. @@ -269,6 +271,7 @@ If you are logging like the `usb-2` starter code does then you should see an out ``` console INFO:usb_2 -- USB: UsbReset @ 438.842772ms INFO:usb_2 -- USB: UsbEp0Setup @ 514.984128ms +... INFO:usb_2 -- SETUP: bmrequesttype: 128, brequest: 6, wlength: 64, windex: 0, wvalue: 256 INFO:usb_2 -- GET_DESCRIPTOR Device [length=64] INFO:usb_2 -- Goal reached; move to the next section From cc0d8440240c5749a3640abe1e65aef3192da981 Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Fri, 3 Jul 2020 17:00:01 +0200 Subject: [PATCH 15/15] run cargo fmt --- advanced/firmware/src/bin/usb-2-solution.rs | 6 ++---- advanced/firmware/src/bin/usb-3-solution.rs | 7 ++++--- advanced/firmware/src/bin/usb-3.rs | 6 ++++-- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/advanced/firmware/src/bin/usb-2-solution.rs b/advanced/firmware/src/bin/usb-2-solution.rs index 8494098..a49c948 100644 --- a/advanced/firmware/src/bin/usb-2-solution.rs +++ b/advanced/firmware/src/bin/usb-2-solution.rs @@ -20,9 +20,7 @@ const APP: () = { usbd::init(board.power, &board.usbd); - init::LateResources { - usbd: board.usbd, - } + init::LateResources { usbd: board.usbd } } #[task(binds = USBD, resources = [usbd])] @@ -79,7 +77,7 @@ fn on_event(usbd: &USBD, event: Event) { // On Mac OS you'll get this request before the GET_DESCRIPTOR request so we // need to catch it here. We'll properly handle this request later // but for now it's OK to do nothing. - }, + } _ => unreachable!(), // we don't handle any other Requests } } diff --git a/advanced/firmware/src/bin/usb-3-solution.rs b/advanced/firmware/src/bin/usb-3-solution.rs index 6739de4..31b4ab4 100644 --- a/advanced/firmware/src/bin/usb-3-solution.rs +++ b/advanced/firmware/src/bin/usb-3-solution.rs @@ -86,8 +86,7 @@ fn on_event(usbd: &USBD, ep0in: &mut Ep0In, event: Event) { idVendor: consts::VID, }; let desc_bytes = desc.bytes(); - let resp = - &desc_bytes[..core::cmp::min(desc_bytes.len(), usize::from(length))]; + let resp = &desc_bytes[..core::cmp::min(desc_bytes.len(), usize::from(length))]; ep0in.start(&resp, usbd); } Request::SetAddress { .. } => { @@ -96,7 +95,9 @@ fn on_event(usbd: &USBD, ep0in: &mut Ep0In, event: Event) { // but for now it's OK to do nothing. } _ => { - log::error!("unknown request (goal achieved if GET_DESCRIPTOR Device was handled)"); + log::error!( + "unknown request (goal achieved if GET_DESCRIPTOR Device was handled)" + ); dk::exit() } } diff --git a/advanced/firmware/src/bin/usb-3.rs b/advanced/firmware/src/bin/usb-3.rs index f2f0471..9a43ccb 100644 --- a/advanced/firmware/src/bin/usb-3.rs +++ b/advanced/firmware/src/bin/usb-3.rs @@ -81,9 +81,11 @@ fn on_event(usbd: &USBD, ep0in: &mut Ep0In, event: Event) { // On Mac OS you'll get this request before the GET_DESCRIPTOR request so we // need to catch it here. We'll properly handle this request later // but for now it's OK to do nothing. - }, + } _ => { - log::error!("unknown request (goal achieved if GET_DESCRIPTOR Device was handled)"); + log::error!( + "unknown request (goal achieved if GET_DESCRIPTOR Device was handled)" + ); dk::exit() } }