diff --git a/src/control_pipe.rs b/src/control_pipe.rs index c3d2a56..1dd66ca 100644 --- a/src/control_pipe.rs +++ b/src/control_pipe.rs @@ -129,18 +129,19 @@ impl ControlPipe<'_, B> { None } - pub fn handle_out(&mut self) -> Option { + pub fn handle_out(&mut self) -> Result> { match self.state { ControlState::DataOut(req) => { let i = self.i; let count = match self.ep_out.read(&mut self.buf[i..]) { Ok(count) => count, - Err(UsbError::WouldBlock) => return None, - Err(_) => { + Err(UsbError::WouldBlock) => return Ok(None), + Err(_err) => { // Failed to read or buffer overflow (overflow is only possible if the host // sends more data than it indicated in the SETUP request) + usb_debug!("Failed EP0 read: {:?}", _err); self.set_error(); - return None; + return Ok(None); } }; @@ -154,7 +155,7 @@ impl ControlPipe<'_, B> { if self.i >= self.len { usb_debug!("Request OUT complete: {:?}", req); self.state = ControlState::CompleteOut; - return Some(req); + return Ok(Some(req)); } } // The host may terminate a DATA stage early by sending a zero-length status packet @@ -167,7 +168,7 @@ impl ControlPipe<'_, B> { "Control transfer completed. Current state: {:?}", self.state ); - let _ = self.ep_out.read(&mut []); + self.ep_out.read(&mut [])?; self.state = ControlState::Idle; } _ => { @@ -176,28 +177,23 @@ impl ControlPipe<'_, B> { "Discarding EP0 data due to unexpected state. Current state: {:?}", self.state ); - let _ = self.ep_out.read(&mut []); + self.ep_out.read(&mut [])?; // Unexpected OUT packet self.set_error() } } - None + Ok(None) } - pub fn handle_in_complete(&mut self) -> bool { + pub fn handle_in_complete(&mut self) -> Result { match self.state { ControlState::DataIn => { - self.write_in_chunk(); + self.write_in_chunk()?; } ControlState::DataInZlp => { - if self.ep_in.write(&[]).is_err() { - // There isn't much we can do if the write fails, except to wait for another - // poll or for the host to resend the request. - return false; - } - + self.ep_in.write(&[])?; usb_trace!("wrote EP0: ZLP"); self.state = ControlState::DataInLast; } @@ -207,7 +203,7 @@ impl ControlPipe<'_, B> { } ControlState::StatusIn => { self.state = ControlState::Idle; - return true; + return Ok(true); } ControlState::Idle => { // If we received a message on EP0 while sending the last portion of an IN @@ -221,23 +217,14 @@ impl ControlPipe<'_, B> { } }; - false + Ok(false) } - fn write_in_chunk(&mut self) { + fn write_in_chunk(&mut self) -> Result<()> { let count = min(self.len - self.i, self.ep_in.max_packet_size() as usize); let buffer = self.static_in_buf.unwrap_or(&self.buf); - let count = match self.ep_in.write(&buffer[self.i..(self.i + count)]) { - Ok(c) => c, - // There isn't much we can do if the write fails, except to wait for another poll or for - // the host to resend the request. - Err(_err) => { - usb_debug!("Failed to write EP0: {:?}", _err); - return; - } - }; - + let count = self.ep_in.write(&buffer[self.i..(self.i + count)])?; usb_trace!("wrote EP0: {:?}", &buffer[self.i..(self.i + count)]); self.i += count; @@ -251,6 +238,8 @@ impl ControlPipe<'_, B> { ControlState::DataInLast }; } + + Ok(()) } pub fn accept_out(&mut self) -> Result<()> { @@ -262,7 +251,7 @@ impl ControlPipe<'_, B> { } }; - let _ = self.ep_in.write(&[]); + self.ep_in.write(&[])?; self.state = ControlState::StatusIn; Ok(()) } @@ -304,7 +293,7 @@ impl ControlPipe<'_, B> { self.len = min(data_len, req.length as usize); self.i = 0; self.state = ControlState::DataIn; - self.write_in_chunk(); + self.write_in_chunk()?; Ok(()) } diff --git a/src/device.rs b/src/device.rs index a2f24f9..e5b49df 100644 --- a/src/device.rs +++ b/src/device.rs @@ -182,6 +182,7 @@ impl UsbDevice<'_, B> { } _ => { self.bus.resume(); + self.device_state = self .suspended_device_state .expect("Unknown state before suspend"); @@ -213,17 +214,30 @@ impl UsbDevice<'_, B> { let req = if (ep_setup & 1) != 0 { self.control.handle_setup() } else if (ep_out & 1) != 0 { - self.control.handle_out() + match self.control.handle_out() { + Ok(req) => req, + Err(_err) => { + // TODO: Propagate error out of `poll()` + usb_debug!("Failed to handle EP0: {:?}", _err); + None + } + } } else { None }; match req { Some(req) if req.direction == UsbDirection::In => { - self.control_in(classes, req) + if let Err(_err) = self.control_in(classes, req) { + // TODO: Propagate error out of `poll()` + usb_debug!("Failed to handle input control request: {:?}", _err); + } } Some(req) if req.direction == UsbDirection::Out => { - self.control_out(classes, req) + if let Err(_err) = self.control_out(classes, req) { + // TODO: Propagate error out of `poll()` + usb_debug!("Failed to handle output control request: {:?}", _err); + } } None if ((ep_in_complete & 1) != 0) => { @@ -232,7 +246,17 @@ impl UsbDevice<'_, B> { // phases of the control transfer. If we just got a SETUP packet or // an OUT token, we can safely ignore the IN-COMPLETE indication and // continue with the next transfer. - let completed = self.control.handle_in_complete(); + let completed = match self.control.handle_in_complete() { + Ok(completed) => completed, + Err(_err) => { + // TODO: Propagate this out of `poll()` + usb_debug!( + "Failed to process control-input complete: {:?}", + _err + ); + false + } + }; if !B::QUIRK_SET_ADDRESS_BEFORE_STATUS && completed @@ -310,14 +334,14 @@ impl UsbDevice<'_, B> { false } - fn control_in(&mut self, classes: &mut ClassList<'_, B>, req: control::Request) { + fn control_in(&mut self, classes: &mut ClassList<'_, B>, req: control::Request) -> Result<()> { use crate::control::{Recipient, Request}; for cls in classes.iter_mut() { cls.control_in(ControlIn::new(&mut self.control, &req)); if !self.control.waiting_for_response() { - return; + return Ok(()); } } @@ -334,14 +358,14 @@ impl UsbDevice<'_, B> { 0x0000 }; - let _ = xfer.accept_with(&status.to_le_bytes()); + xfer.accept_with(&status.to_le_bytes())?; } (Recipient::Interface, Request::GET_STATUS) => { usb_trace!("Processing Interface::GetStatus"); let status: u16 = 0x0000; - let _ = xfer.accept_with(&status.to_le_bytes()); + xfer.accept_with(&status.to_le_bytes())?; } (Recipient::Endpoint, Request::GET_STATUS) => { @@ -354,12 +378,12 @@ impl UsbDevice<'_, B> { 0x0000 }; - let _ = xfer.accept_with(&status.to_le_bytes()); + xfer.accept_with(&status.to_le_bytes())?; } (Recipient::Device, Request::GET_DESCRIPTOR) => { usb_trace!("Processing Device::GetDescriptor"); - UsbDevice::get_descriptor(&self.config, classes, xfer) + UsbDevice::get_descriptor(&self.config, classes, xfer)?; } (Recipient::Device, Request::GET_CONFIGURATION) => { @@ -369,15 +393,14 @@ impl UsbDevice<'_, B> { _ => CONFIGURATION_NONE, }; - let _ = xfer.accept_with(&config.to_le_bytes()); + xfer.accept_with(&config.to_le_bytes())?; } (Recipient::Interface, Request::GET_INTERFACE) => { usb_trace!("Processing Interface::GetInterface"); // Reject interface numbers bigger than 255 if req.index > core::u8::MAX.into() { - let _ = xfer.reject(); - return; + return xfer.reject(); } // Ask class implementations, whether they know the alternate setting @@ -385,33 +408,34 @@ impl UsbDevice<'_, B> { for cls in classes { if let Some(setting) = cls.get_alt_setting(InterfaceNumber(req.index as u8)) { - let _ = xfer.accept_with(&setting.to_le_bytes()); - return; + return xfer.accept_with(&setting.to_le_bytes()); } } // If no class returned an alternate setting, return the default value - let _ = xfer.accept_with(&DEFAULT_ALTERNATE_SETTING.to_le_bytes()); + xfer.accept_with(&DEFAULT_ALTERNATE_SETTING.to_le_bytes())?; } - _ => (), + _ => {} }; } if self.control.waiting_for_response() { usb_debug!("Rejecting control transfer because we were waiting for a response"); - let _ = self.control.reject(); + self.control.reject()?; } + + Ok(()) } - fn control_out(&mut self, classes: &mut ClassList<'_, B>, req: control::Request) { + fn control_out(&mut self, classes: &mut ClassList<'_, B>, req: control::Request) -> Result<()> { use crate::control::{Recipient, Request}; for cls in classes.iter_mut() { cls.control_out(ControlOut::new(&mut self.control, &req)); if !self.control.waiting_for_response() { - return; + return Ok(()); } } @@ -430,14 +454,14 @@ impl UsbDevice<'_, B> { ) => { usb_debug!("Remote wakeup disabled"); self.remote_wakeup_enabled = false; - let _ = xfer.accept(); + xfer.accept()?; } (Recipient::Endpoint, Request::CLEAR_FEATURE, Request::FEATURE_ENDPOINT_HALT) => { usb_debug!("EP{} halt removed", req.index & 0x8f); self.bus .set_stalled(((req.index as u8) & 0x8f).into(), false); - let _ = xfer.accept(); + xfer.accept()?; } ( @@ -447,14 +471,14 @@ impl UsbDevice<'_, B> { ) => { usb_debug!("Remote wakeup enabled"); self.remote_wakeup_enabled = true; - let _ = xfer.accept(); + xfer.accept()?; } (Recipient::Endpoint, Request::SET_FEATURE, Request::FEATURE_ENDPOINT_HALT) => { usb_debug!("EP{} halted", req.index & 0x8f); self.bus .set_stalled(((req.index as u8) & 0x8f).into(), true); - let _ = xfer.accept(); + xfer.accept()?; } (Recipient::Device, Request::SET_ADDRESS, 1..=127) => { @@ -465,24 +489,24 @@ impl UsbDevice<'_, B> { } else { self.pending_address = req.value as u8; } - let _ = xfer.accept(); + xfer.accept()?; } (Recipient::Device, Request::SET_CONFIGURATION, CONFIGURATION_VALUE_U16) => { usb_debug!("Device configured"); self.device_state = UsbDeviceState::Configured; - let _ = xfer.accept(); + xfer.accept()?; } (Recipient::Device, Request::SET_CONFIGURATION, CONFIGURATION_NONE_U16) => { usb_debug!("Device deconfigured"); match self.device_state { UsbDeviceState::Default => { - let _ = xfer.accept(); + xfer.accept()?; } _ => { self.device_state = UsbDeviceState::Addressed; - let _ = xfer.accept(); + xfer.accept()?; } } } @@ -490,43 +514,49 @@ impl UsbDevice<'_, B> { (Recipient::Interface, Request::SET_INTERFACE, alt_setting) => { // Reject interface numbers and alt settings bigger than 255 if req.index > core::u8::MAX.into() || alt_setting > core::u8::MAX.into() { - let _ = xfer.reject(); - return; + xfer.reject()?; + return Ok(()); } // Ask class implementations, whether they accept the alternate interface setting. for cls in classes { if cls.set_alt_setting(InterfaceNumber(req.index as u8), alt_setting as u8) { - let _ = xfer.accept(); - return; + xfer.accept()?; + return Ok(()); } } // Default behaviour, if no class implementation accepted the alternate setting. if alt_setting == DEFAULT_ALTERNATE_SETTING_U16 { usb_debug!("Accepting unused alternate settings"); - let _ = xfer.accept(); + xfer.accept()?; } else { usb_debug!("Rejecting unused alternate settings"); - let _ = xfer.reject(); + xfer.reject()?; } } _ => { - let _ = xfer.reject(); - return; + xfer.reject()?; + return Ok(()); } } } if self.control.waiting_for_response() { usb_debug!("Rejecting control transfer due to waiting response"); - let _ = self.control.reject(); + self.control.reject()?; } + + Ok(()) } - fn get_descriptor(config: &Config, classes: &mut ClassList<'_, B>, xfer: ControlIn) { + fn get_descriptor( + config: &Config, + classes: &mut ClassList<'_, B>, + xfer: ControlIn, + ) -> Result<()> { let req = *xfer.request(); let (dtype, index) = req.descriptor_type_index(); @@ -534,12 +564,14 @@ impl UsbDevice<'_, B> { fn accept_writer( xfer: ControlIn, f: impl FnOnce(&mut DescriptorWriter) -> Result<()>, - ) { - let _ = xfer.accept(|buf| { + ) -> Result<()> { + xfer.accept(|buf| { let mut writer = DescriptorWriter::new(buf); f(&mut writer)?; Ok(writer.position()) - }); + })?; + + Ok(()) } match dtype { @@ -554,9 +586,9 @@ impl UsbDevice<'_, B> { bw.end_bos(); Ok(()) - }), + })?, - descriptor_type::DEVICE => accept_writer(xfer, |w| w.device(config)), + descriptor_type::DEVICE => accept_writer(xfer, |w| w.device(config))?, descriptor_type::CONFIGURATION => accept_writer(xfer, |w| { w.configuration(config)?; @@ -569,7 +601,7 @@ impl UsbDevice<'_, B> { w.end_configuration(); Ok(()) - }), + })?, descriptor_type::STRING => match index { // first STRING Request @@ -587,7 +619,7 @@ impl UsbDevice<'_, B> { descriptor_type::STRING, &lang_id_bytes[..config.string_descriptors.len() * 2], ) - }) + })?; } // rest STRING Requests @@ -602,8 +634,8 @@ impl UsbDevice<'_, B> { .iter() .find(|lang| lang.id == lang_id) else { - xfer.reject().ok(); - return; + xfer.reject()?; + return Ok(()); }; match index { @@ -622,17 +654,19 @@ impl UsbDevice<'_, B> { }; if let Some(string_descriptor) = string { - accept_writer(xfer, |w| w.string(string_descriptor)); + accept_writer(xfer, |w| w.string(string_descriptor))?; } else { - let _ = xfer.reject(); + xfer.reject()?; } } }, _ => { - let _ = xfer.reject(); + xfer.reject()?; } - } + }; + + Ok(()) } fn reset(&mut self, classes: &mut ClassList<'_, B>) { diff --git a/src/macros.rs b/src/macros.rs index e6edd32..ff96a6d 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -1,7 +1,7 @@ #[cfg(all(feature = "log", not(feature = "defmt")))] macro_rules! usb_log { (trace, $($arg:expr),*) => { log::trace!($($arg),*) }; - (debug, $($arg:expr),*) => { log::trace!($($arg),*) }; + (debug, $($arg:expr),*) => { log::debug!($($arg),*) }; } #[cfg(feature = "defmt")]