Implement proper MSC error handling

embassy-usb/src/class/msc/subclass/scsi/block_device.rs

@@ -1,19 +1,42 @@
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub enum BlockDeviceError {
-    // TODO
+    /// Block device is not present and cannot be accessed.
+    ///
+    /// SCSI NOT READY 3Ah/00h MEDIUM NOT PRESENT
+    MediumNotPresent,
+    /// Logical Block Address is out of range
+    ///
+    /// SCSI ILLEGAL REQUEST 21h/00h LOGICAL BLOCK ADDRESS OUT OF RANGE
+    LbaOutOfRange,
+    /// Unrecoverable hardware error
+    ///
+    /// SCSI HARDWARE ERROR 00h/00h NO ADDITIONAL SENSE INFORMATION
+    HardwareError,
+    /// SCSI MEDIUM ERROR 11h/00h UNRECOVERED READ ERROR
+    ReadError,
+    /// SCSI MEDIUM ERROR 0Ch/00h WRITE ERROR
+    WriteError,
+    /// SCSI MEDIUM ERROR 51h/00h ERASE FAILURE
+    EraseError,
 }
 
 pub trait BlockDevice {
+    /// Called for periodic `TEST UNIT READY` SCSI requests.
+    ///
+    /// Should return error if device is not ready (i.e. [BlockDeviceError::MediumRemoved] if SD card is not present).
+    fn status(&self) -> Result<(), BlockDeviceError>;
+
     /// The number of bytes per block. This determines the size of the buffer passed
-    /// to read/write functions
+    /// to read/write functions.
-    fn block_size(&self) -> usize;
+    fn block_size(&self) -> Result<usize, BlockDeviceError>;
 
     /// Number of blocks in device (max LBA index)
-    fn num_blocks(&self) -> u32;
+    fn max_lba(&self) -> Result<u32, BlockDeviceError>;
 
     /// Read the block indicated by `lba` into the provided buffer
-    fn read_block(&self, lba: u32, block: &mut [u8]) -> Result<(), BlockDeviceError>;
+    async fn read_block(&self, lba: u32, block: &mut [u8]) -> Result<(), BlockDeviceError>;
 
     /// Write the `block` buffer to the block indicated by `lba`
-    fn write_block(&mut self, lba: u32, block: &[u8]) -> Result<(), BlockDeviceError>;
+    async fn write_block(&mut self, lba: u32, block: &[u8]) -> Result<(), BlockDeviceError>;
 }

embassy-usb/src/class/msc/subclass/scsi/commands/request_sense.rs

@@ -1,5 +1,6 @@
 use super::control::Control;
 use crate::class::msc::subclass::scsi::enums::{ResponseCode, SenseKey};
+use crate::packed::BE;
 use crate::packed_struct;
 
 packed_struct! {
@@ -20,16 +21,36 @@ impl RequestSenseCommand<[u8; RequestSenseCommand::SIZE]> {
 }
 
 packed_struct! {
-    pub struct RequestSenseResponse<8> {
+    pub struct RequestSenseResponse<20> {
         #[offset = 0, size = 7]
         response_code: ResponseCode,
-        #[offset = 1*8+0, size = 4]
+        #[offset = 1*8+0, size = 8]
+        segment_number: u8,
+        #[offset = 2*8+0, size = 4]
         sense_key: SenseKey,
-        #[offset = 2*8+0, size = 8]
+        #[offset = 3*8+0, size = 4*8]
-        additional_sense_code: u8,
+        information: BE<u32>,
-        #[offset = 3*8+0, size = 8]
-        additional_sense_code_qualifier: u8,
         #[offset = 7*8+0, size = 8]
         additional_sense_length: u8,
+        #[offset = 8*8+0, size = 4*8]
+        command_specific_information: BE<u32>,
+        #[offset = 12*8+0, size = 8]
+        additional_sense_code: u8,
+        #[offset = 13*8+0, size = 8]
+        additional_sense_code_qualifier: u8,
+        #[offset = 14*8+0, size = 8]
+        field_replaceable_unit_code: u8,
+        #[offset = 15*8+0, size = 3]
+        bit_pointer: u8,
+        #[offset = 15*8+3, size = 1]
+        bpv: bool,
+        /// Command/Data
+        #[offset = 15*8+6, size = 1]
+        cd: bool,
+        /// Sense Key Specific Valid
+        #[offset = 15*8+7, size = 1]
+        sksv: bool,
+        #[offset = 16*8+0, size = 2*8]
+        field_pointer: BE<u16>,
     }
 }

embassy-usb/src/class/msc/subclass/scsi/enums.rs

@@ -199,12 +199,18 @@ pub enum AdditionalSenseCode {
     InvalidFieldInCdb,
     /// ASC 0x0, ASCQ: 0x0 - NO ADDITIONAL SENSE INFORMATION
     NoAdditionalSenseInformation,
+    /// ASC 0x11, ASCQ: 0x0 - UNRECOVERED READ ERROR
+    UnrecoveredReadError,
     /// ASC 0xC, ASCQ: 0x0 - WRITE ERROR
     WriteError,
     /// ASC 0x51, ASCQ: 0x0 - ERASE FAILURE
     EraseFailure,
     /// ASC 0x21, ASCQ: 0x0 - LOGICAL BLOCK ADDRESS OUT OF RANGE
     LogicalBlockAddressOutOfRange,
+    /// ASC 0x3A, ASCQ: 0x0 - MEDIUM NOT PRESENT
+    MediumNotPresent,
+    /// ASC 0x25, ASCQ: 0x00 - LOGICAL UNIT NOT SUPPORTED
+    LogicalUnitNotSupported,
 }
 
 impl AdditionalSenseCode {
@@ -215,9 +221,12 @@ impl AdditionalSenseCode {
             AdditionalSenseCode::InvalidPacketSize => 100,
             AdditionalSenseCode::InvalidFieldInCdb => 36,
             AdditionalSenseCode::NoAdditionalSenseInformation => 0,
+            AdditionalSenseCode::UnrecoveredReadError => 0x11,
             AdditionalSenseCode::WriteError => 12,
             AdditionalSenseCode::EraseFailure => 81,
             AdditionalSenseCode::LogicalBlockAddressOutOfRange => 33,
+            AdditionalSenseCode::MediumNotPresent => 0x3A,
+            AdditionalSenseCode::LogicalUnitNotSupported => 0x25,
         }
     }
 
@@ -228,9 +237,12 @@ impl AdditionalSenseCode {
             AdditionalSenseCode::InvalidPacketSize => 1,
             AdditionalSenseCode::InvalidFieldInCdb => 0,
             AdditionalSenseCode::NoAdditionalSenseInformation => 0,
+            AdditionalSenseCode::UnrecoveredReadError => 0,
             AdditionalSenseCode::WriteError => 0,
             AdditionalSenseCode::EraseFailure => 0,
             AdditionalSenseCode::LogicalBlockAddressOutOfRange => 0,
+            AdditionalSenseCode::MediumNotPresent => 0,
+            AdditionalSenseCode::LogicalUnitNotSupported => 0,
         }
     }
 }

embassy-usb/src/class/msc/subclass/scsi/mod.rs

@@ -5,77 +5,259 @@ pub mod enums;
 
 use core::mem::MaybeUninit;
 
-use self::block_device::BlockDevice;
+use self::block_device::{BlockDevice, BlockDeviceError};
+use self::enums::AdditionalSenseCode;
 use crate::class::msc::subclass::scsi::commands::{
-    CachingModePage, InquiryCommand, InquiryResponse, ModeParameterHeader6, ModeSense6Command, PageCode,
+    InquiryCommand, InquiryResponse, ModeParameterHeader6, ModeSense6Command, PreventAllowMediumRemoval, Read10Command,
-    PreventAllowMediumRemoval, Read10Command, ReadCapacity10Command, ReadCapacity10Response,
+    ReadCapacity10Command, ReadCapacity10Response, ReadFormatCapacitiesCommand, ReadFormatCapacitiesResponse,
-    ReadFormatCapacitiesCommand, ReadFormatCapacitiesResponse, RequestSenseCommand, RequestSenseResponse,
+    RequestSenseCommand, RequestSenseResponse, TestUnitReadyCommand, Write10Command,
-    TestUnitReadyCommand, Write10Command,
 };
 use crate::class::msc::subclass::scsi::enums::{
     PeripheralDeviceType, PeripheralQualifier, ResponseCode, ResponseDataFormat, SenseKey, SpcVersion,
     TargetPortGroupSupport,
 };
 use crate::class::msc::transport::{self, CommandSetHandler};
+use crate::class::msc::MscSubclass;
+
+/// Stores information (errors) about last operation.
+///
+/// Sent on `RequestSenseCommand` as `RequestSenseResponse`.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub struct SenseData {
+    /// Sense key
+    key: SenseKey,
+    /// Additional Sense Code
+    asc: AdditionalSenseCode,
+}
 
 pub struct Scsi<B: BlockDevice> {
+    /// Backing storage block device
     device: B,
+    /// Last operation sense data
+    sense: Option<SenseData>,
+    vendor_id: [u8; 8],
+    product_id: [u8; 16],
 }
 
 impl<B: BlockDevice> Scsi<B> {
-    pub fn new(device: B) -> Self {
+    pub fn new(device: B, vendor: &str, product: &str) -> Self {
-        Self { device }
+        let mut vendor_id = [b' '; 8];
+        fill_from_slice(&mut vendor_id, vendor.as_bytes());
+
+        let mut product_id = [b' '; 16];
+        fill_from_slice(&mut product_id, product.as_bytes());
+
+        Self {
+            device,
+            sense: None,
+            vendor_id,
+            product_id,
+        }
+    }
+
+    async fn handle_command_out(
+        &mut self,
+        lun: u8,
+        cmd: &[u8],
+        pipe: &mut impl transport::DataPipeOut,
+    ) -> Result<(), InternalError> {
+        if lun != 0 {
+            return Err(InternalError::LunsNotSupported);
+        }
+
+        let op_code = cmd.get(0).ok_or(InternalError::CommandParseError)?;
+
+        match *op_code {
+            TestUnitReadyCommand::OPCODE => {
+                let req = TestUnitReadyCommand::from_bytes(cmd).ok_or(InternalError::CommandParseError)?;
+                debug!("{:?}", req);
+                self.device.status().map_err(|e| e.into())
+            }
+            PreventAllowMediumRemoval::OPCODE => {
+                let req = PreventAllowMediumRemoval::from_bytes(cmd).ok_or(InternalError::CommandParseError)?;
+                debug!("{:?}", req);
+
+                // From spec:
+                // If the device server does not support the medium changer prevent state, it shall terminate the
+                // PREVENT ALLOW MEDIUM REMOVAL command with CHECK CONDITION status with the sense
+                // key set to ILLEGAL REQUEST and the additional sense code set to INVALID FIELD IN CDB.
+                Err(InternalError::CustomSenseData(SenseData {
+                    key: SenseKey::IllegalRequest,
+                    asc: AdditionalSenseCode::InvalidFieldInCdb,
+                }))
+            }
+            Write10Command::OPCODE => {
+                let req = Write10Command::from_bytes(cmd).ok_or(InternalError::CommandParseError)?;
+                debug!("{:?}", req);
+
+                let mut data = MaybeUninit::<[u8; 512]>::uninit();
+
+                let start_lba = req.lba();
+                let transfer_length = req.transfer_length() as u32;
+
+                if start_lba + transfer_length > self.device.max_lba()? + 1 {
+                    return Err(InternalError::LbaOutOfRange);
+                }
+
+                for lba in start_lba..start_lba + transfer_length {
+                    pipe.read(unsafe { data.assume_init_mut() }).await?;
+                    self.device.write_block(lba, unsafe { data.assume_init_ref() }).await?;
+                }
+
+                Ok(())
+            }
+            _ => Err(InternalError::UnknownOpcode),
+        }
+    }
+
+    async fn handle_command_in(
+        &mut self,
+        lun: u8,
+        cmd: &[u8],
+        pipe: &mut impl transport::DataPipeIn,
+    ) -> Result<(), InternalError> {
+        if lun != 0 {
+            return Err(InternalError::LunsNotSupported);
+        }
+
+        let op_code = cmd.get(0).ok_or(InternalError::CommandParseError)?;
+
+        match *op_code {
+            InquiryCommand::OPCODE => {
+                let req = InquiryCommand::from_bytes(cmd).ok_or(InternalError::CommandParseError)?;
+                debug!("{:#?}", req);
+
+                let mut resp = InquiryResponse::new();
+                resp.set_peripheral_device_type(PeripheralDeviceType::DirectAccessBlock);
+                resp.set_peripheral_qualifier(PeripheralQualifier::Connected);
+                resp.set_removable_medium(true);
+                resp.set_version(SpcVersion::Spc3);
+                resp.set_response_data_format(ResponseDataFormat::Standard);
+                resp.set_hierarchical_support(false);
+                resp.set_normal_aca(false);
+                resp.set_additional_length((InquiryResponse::SIZE - 4) as u8);
+                resp.set_protect(false);
+                resp.set_third_party_copy(false);
+                resp.set_target_port_group_support(TargetPortGroupSupport::Unsupported);
+                resp.set_access_controls_coordinator(false);
+                resp.set_scc_supported(false);
+                resp.set_multi_port(false);
+                resp.set_enclosure_services(false);
+                resp.set_vendor_identification(&self.vendor_id);
+                resp.set_product_identification(&self.product_id);
+                resp.set_product_revision_level(&[b' '; 4]);
+
+                pipe.write(&resp.data).await?;
+                Ok(())
+            }
+            ModeSense6Command::OPCODE => {
+                let req = ModeSense6Command::from_bytes(cmd).ok_or(InternalError::CommandParseError)?;
+                debug!("{:?}", req);
+
+                let mut header = ModeParameterHeader6::new();
+                header.set_mode_data_length(ModeParameterHeader6::SIZE as u8 - 1);
+                pipe.write(&header.data).await?;
+
+                Ok(())
+            }
+            RequestSenseCommand::OPCODE => {
+                let req = RequestSenseCommand::from_bytes(cmd).ok_or(InternalError::CommandParseError)?;
+                debug!("{:?}", req);
+
+                let mut resp = RequestSenseResponse::new();
+                resp.set_response_code(ResponseCode::CurrentFixedSenseData);
+
+                let len = RequestSenseResponse::SIZE.min(req.allocation_length() as usize);
+                resp.set_additional_sense_length((len - 7) as u8);
+
+                match &self.sense {
+                    Some(sense) => {
+                        resp.set_sense_key(sense.key);
+                        resp.set_additional_sense_code(sense.asc.asc());
+                        resp.set_additional_sense_code_qualifier(sense.asc.ascq());
+                    }
+                    None => {
+                        resp.set_sense_key(SenseKey::NoSense);
+                    }
+                }
+
+                pipe.write(&resp.data[..len]).await?;
+                Ok(())
+            }
+            ReadCapacity10Command::OPCODE => {
+                let req = ReadCapacity10Command::from_bytes(cmd).ok_or(InternalError::CommandParseError)?;
+                debug!("{:?}", req);
+
+                let mut resp = ReadCapacity10Response::new();
+                resp.set_max_lba(self.device.max_lba()?);
+                resp.set_block_size(self.device.block_size()? as u32);
+
+                pipe.write(&resp.data).await?;
+                Ok(())
+            }
+            ReadFormatCapacitiesCommand::OPCODE => {
+                let req = ReadFormatCapacitiesCommand::from_bytes(cmd).ok_or(InternalError::CommandParseError)?;
+                debug!("{:?}", req);
+
+                let mut resp = ReadFormatCapacitiesResponse::new();
+                resp.set_capacity_list_length(8);
+                resp.set_max_lba(self.device.max_lba()?);
+                resp.set_block_size(self.device.block_size()? as u32);
+
+                pipe.write(&resp.data).await?;
+                return Ok(());
+            }
+            Read10Command::OPCODE => {
+                let req = Read10Command::from_bytes(cmd).ok_or(InternalError::CommandParseError)?;
+                debug!("{:?}", req);
+
+                let mut data = MaybeUninit::<[u8; 512]>::uninit();
+
+                let start_lba = req.lba();
+                let transfer_length = req.transfer_length() as u32;
+
+                if start_lba + transfer_length > self.device.max_lba()? + 1 {
+                    return Err(InternalError::LbaOutOfRange);
+                }
+
+                for lba in start_lba..start_lba + transfer_length {
+                    self.device.read_block(lba, unsafe { data.assume_init_mut() }).await?;
+
+                    pipe.write(unsafe { data.assume_init_ref() }).await?;
+                }
+
+                Ok(())
+            }
+            _ => Err(InternalError::UnknownOpcode),
+        }
     }
 }
 
 impl<B: BlockDevice> CommandSetHandler for Scsi<B> {
+    const MSC_SUBCLASS: MscSubclass = MscSubclass::ScsiTransparentCommandSet;
+    const MAX_LUN: u8 = 0;
+
     async fn command_out(
         &mut self,
         lun: u8,
         cmd: &[u8],
         pipe: &mut impl transport::DataPipeOut,
     ) -> Result<(), transport::CommandError> {
-        assert!(lun == 0, "LUNs are not supported");
+        match self.handle_command_out(lun, cmd, pipe).await {
-
+            Ok(_) => {
-        let op_code = cmd[0];
+                self.sense = None;
-        match op_code {
+                Ok(())
-            TestUnitReadyCommand::OPCODE => {
+            }
-                info!("TestUnitReadyCommand: {:#?}", TestUnitReadyCommand::from_bytes(cmd));
+            Err(e) => {
-                return Ok(());
+                error!("command_out error op={}, err={:?}", cmd.get(0), e);
+                self.sense = Some(e.into_sense_data());
+                Err(match e {
+                    InternalError::DataPipeError(e) => e.into(),
+                    _ => transport::CommandError::CommandError,
+                })
             }
-            PreventAllowMediumRemoval::OPCODE => match PreventAllowMediumRemoval::from_bytes(cmd) {
-                Some(req) => {
-                    info!("PreventAllowMediumRemoval: {:?}", req);
-                    return Ok(());
-                }
-                None => error!("Error parsing PreventAllowMediumRemoval"),
-            },
-            Write10Command::OPCODE => match Write10Command::from_bytes(cmd) {
-                Some(req) => {
-                    info!("Write10Command: {:?}", req);
-
-                    let mut data = MaybeUninit::<[u8; 512]>::uninit();
-
-                    let start_lba = req.lba();
-                    let transfer_length = req.transfer_length() as u32;
-
-                    if start_lba + transfer_length - 1 > self.device.num_blocks() {
-                        return Err(transport::CommandError::CommandError);
-                    }
-
-                    for lba in start_lba..start_lba + transfer_length {
-                        pipe.read(unsafe { data.assume_init_mut() }).await?;
-                        self.device.write_block(lba, unsafe { data.assume_init_ref() }).unwrap();
-                    }
-
-                    return Ok(());
-                }
-                None => error!("Error parsing Write10Command"),
-            },
-            _ => warn!("Unknown OUT opcode: {}", op_code),
         }
-
-        Err(transport::CommandError::CommandError)
     }
 
     async fn command_in(
@@ -84,135 +266,112 @@ impl<B: BlockDevice> CommandSetHandler for Scsi<B> {
         cmd: &[u8],
         pipe: &mut impl transport::DataPipeIn,
     ) -> Result<(), transport::CommandError> {
-        assert!(lun == 0, "LUNs are not supported");
+        match self.handle_command_in(lun, cmd, pipe).await {
-
+            Ok(_) => {
-        let op_code = cmd[0];
+                self.sense = None;
-        info!("op_code: {}", op_code);
+                Ok(())
-        match op_code {
+            }
-            InquiryCommand::OPCODE => match InquiryCommand::from_bytes(cmd) {
+            Err(e) => {
-                Some(req) => {
+                error!("command_in error op={}, err={:?}", cmd.get(0), e);
-                    info!("inquiry: {:#?}", req);
+                self.sense = Some(e.into_sense_data());
-
+                Err(match e {
-                    let vendor_ident = b"FAKE    ";
+                    InternalError::DataPipeError(e) => e.into(),
-                    let product_ident = b"PRODUCT         ";
+                    _ => transport::CommandError::CommandError,
-
+                })
-                    let mut resp = InquiryResponse::new();
+            }
-                    resp.set_peripheral_device_type(PeripheralDeviceType::DirectAccessBlock);
-                    resp.set_peripheral_qualifier(PeripheralQualifier::Connected);
-                    resp.set_removable_medium(true);
-                    resp.set_version(SpcVersion::Spc3);
-                    resp.set_response_data_format(ResponseDataFormat::Standard);
-                    resp.set_hierarchical_support(false);
-                    resp.set_normal_aca(false);
-                    resp.set_additional_length((InquiryResponse::SIZE - 4) as u8);
-                    resp.set_protect(false);
-                    resp.set_third_party_copy(false);
-                    resp.set_target_port_group_support(TargetPortGroupSupport::Unsupported);
-                    resp.set_access_controls_coordinator(false);
-                    resp.set_scc_supported(false);
-                    resp.set_multi_port(false);
-                    resp.set_enclosure_services(false);
-                    resp.set_vendor_identification(vendor_ident);
-                    resp.set_product_identification(product_ident);
-                    resp.set_product_revision_level(&[b' '; 4]);
-
-                    pipe.write(&resp.data).await?;
-                    return Ok(());
-                }
-                None => error!("Error parsing InquiryCommand"),
-            },
-            ModeSense6Command::OPCODE => match ModeSense6Command::from_bytes(cmd) {
-                Some(req) => {
-                    info!("ModeSense6Command: {:?}", req);
-
-                    // let mut buf = [0u8; ModeParameterHeader6::SIZE + CachingModePage::SIZE];
-
-                    // let mut header = ModeParameterHeader6::from_bytes(&mut buf[0..ModeSense6Command::SIZE]).unwrap();
-                    // header.set_mode_data_length((ModeParameterHeader6::SIZE + CachingModePage::SIZE - 1) as u8);
-
-                    // let mut caching_mode_page =
-                    //     CachingModePage::from_bytes(&mut buf[ModeParameterHeader6::SIZE..]).unwrap();
-                    // caching_mode_page.set_page_code(PageCode::CachingModePage);
-                    // caching_mode_page.set_page_length(CachingModePage::SIZE as u8);
-                    // caching_mode_page.set_read_cache_disable(true);
-                    // caching_mode_page.set_write_cache_enable(false);
-
-                    // pipe.write(&buf).await?;
-
-                    let mut header = ModeParameterHeader6::new();
-                    header.set_mode_data_length(ModeParameterHeader6::SIZE as u8 - 1);
-                    pipe.write(&header.data).await?;
-
-                    return Ok(());
-                }
-                None => error!("Error parsing ModeSense6Command"),
-            },
-            RequestSenseCommand::OPCODE => match RequestSenseCommand::from_bytes(cmd) {
-                Some(req) => {
-                    info!("RequestSenseCommand: {:?}", req);
-
-                    let mut resp = RequestSenseResponse::new();
-                    resp.set_response_code(ResponseCode::CurrentFixedSenseData);
-                    resp.set_sense_key(SenseKey::NoSense);
-
-                    pipe.write(&resp.data).await?;
-                    return Ok(());
-                }
-                None => error!("Error parsing RequestSenseCommand"),
-            },
-            ReadCapacity10Command::OPCODE => match ReadCapacity10Command::from_bytes(cmd) {
-                Some(req) => {
-                    info!("ReadCapacity10Command: {:?}", req);
-
-                    let mut resp = ReadCapacity10Response::new();
-                    resp.set_max_lba(self.device.num_blocks());
-                    resp.set_block_size(self.device.block_size() as u32);
-
-                    pipe.write(&resp.data).await?;
-                    return Ok(());
-                }
-                None => error!("Error parsing ReadCapacity10Command"),
-            },
-            ReadFormatCapacitiesCommand::OPCODE => match ReadFormatCapacitiesCommand::from_bytes(cmd) {
-                Some(req) => {
-                    info!("ReadFormatCapacitiesCommand: {:?}", req);
-
-                    let mut resp = ReadFormatCapacitiesResponse::new();
-                    resp.set_capacity_list_length(8);
-                    resp.set_max_lba(self.device.num_blocks());
-                    resp.set_block_size(self.device.block_size() as u32);
-
-                    pipe.write(&resp.data).await?;
-                    return Ok(());
-                }
-                None => error!("Error parsing ReadFormatCapacitiesCommand"),
-            },
-            Read10Command::OPCODE => match Read10Command::from_bytes(cmd) {
-                Some(req) => {
-                    info!("Read10: {:?} {:?}", req, cmd);
-
-                    let mut data = MaybeUninit::<[u8; 512]>::uninit();
-
-                    let start_lba = req.lba();
-                    let transfer_length = req.transfer_length() as u32;
-
-                    if start_lba + transfer_length - 1 > self.device.num_blocks() {
-                        return Err(transport::CommandError::CommandError);
-                    }
-
-                    for lba in start_lba..start_lba + transfer_length {
-                        self.device.read_block(lba, unsafe { data.assume_init_mut() }).unwrap();
-
-                        pipe.write(unsafe { data.assume_init_ref() }).await?;
-                    }
-
-                    return Ok(());
-                }
-                None => error!("Error parsing Read10Command"),
-            },
-            _ => warn!("Unknown IN opcode: {}", op_code),
         }
-
-        Err(transport::CommandError::CommandError)
     }
 }
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+enum InternalError {
+    /// Unknown opcode
+    UnknownOpcode,
+    /// Command could not be parsed
+    CommandParseError,
+    /// Logical block address exceeded max_lba
+    LbaOutOfRange,
+    /// LUNs not supported
+    LunsNotSupported,
+    /// Block device error
+    BlockDeviceError(BlockDeviceError),
+    /// Data pipe error
+    DataPipeError(transport::DataPipeError),
+    /// Custom sense data
+    CustomSenseData(SenseData),
+}
+
+impl InternalError {
+    fn into_sense_data(self) -> SenseData {
+        match self {
+            InternalError::UnknownOpcode => SenseData {
+                key: SenseKey::IllegalRequest,
+                asc: AdditionalSenseCode::InvalidCommandOperationCode,
+            },
+            InternalError::CommandParseError => SenseData {
+                key: SenseKey::IllegalRequest,
+                asc: AdditionalSenseCode::InvalidFieldInCdb,
+            },
+            InternalError::LbaOutOfRange => SenseData {
+                key: SenseKey::IllegalRequest,
+                asc: AdditionalSenseCode::LogicalBlockAddressOutOfRange,
+            },
+            InternalError::LunsNotSupported => SenseData {
+                key: SenseKey::IllegalRequest,
+                asc: AdditionalSenseCode::LogicalUnitNotSupported,
+            },
+            InternalError::BlockDeviceError(e) => match e {
+                BlockDeviceError::MediumNotPresent => SenseData {
+                    key: SenseKey::NotReady,
+                    asc: AdditionalSenseCode::MediumNotPresent,
+                },
+                BlockDeviceError::LbaOutOfRange => SenseData {
+                    key: SenseKey::IllegalRequest,
+                    asc: AdditionalSenseCode::LogicalBlockAddressOutOfRange,
+                },
+                BlockDeviceError::HardwareError => SenseData {
+                    key: SenseKey::HardwareError,
+                    asc: AdditionalSenseCode::NoAdditionalSenseInformation,
+                },
+                BlockDeviceError::ReadError => SenseData {
+                    key: SenseKey::MediumError,
+                    asc: AdditionalSenseCode::UnrecoveredReadError,
+                },
+                BlockDeviceError::WriteError => SenseData {
+                    key: SenseKey::MediumError,
+                    asc: AdditionalSenseCode::WriteError,
+                },
+                BlockDeviceError::EraseError => SenseData {
+                    key: SenseKey::MediumError,
+                    asc: AdditionalSenseCode::EraseFailure,
+                },
+            },
+            InternalError::DataPipeError(_) => SenseData {
+                // Not sure if this is correct.
+                // It's hard to find information on what happens when USB transport fails.
+                key: SenseKey::AbortedCommand,
+                asc: AdditionalSenseCode::NoAdditionalSenseInformation,
+            },
+            InternalError::CustomSenseData(sense) => sense,
+        }
+    }
+}
+
+impl From<BlockDeviceError> for InternalError {
+    fn from(value: BlockDeviceError) -> Self {
+        Self::BlockDeviceError(value)
+    }
+}
+
+impl From<transport::DataPipeError> for InternalError {
+    fn from(value: transport::DataPipeError) -> Self {
+        Self::DataPipeError(value)
+    }
+}
+
+// Why is this not in the standard library?
+fn fill_from_slice(dst: &mut [u8], src: &[u8]) {
+    let limit = dst.len().min(src.len());
+    dst[..limit].copy_from_slice(&src[..limit]);
+}

embassy-usb/src/class/msc/transport/bulk_only/mod.rs

@@ -8,7 +8,7 @@ use embassy_usb_driver::{Direction, Endpoint, EndpointError, EndpointIn, Endpoin
 use self::cbw::CommandBlockWrapper;
 use self::csw::{CommandStatus, CommandStatusWrapper};
 use super::{CommandError, CommandSetHandler, DataPipeError, DataPipeIn, DataPipeOut};
-use crate::class::msc::{MscProtocol, MscSubclass, USB_CLASS_MSC};
+use crate::class::msc::{MscProtocol, USB_CLASS_MSC};
 use crate::control::{ControlHandler, InResponse, Request, RequestType};
 use crate::driver::Driver;
 use crate::Builder;
@@ -57,25 +57,19 @@ pub struct BulkOnlyTransport<'d, D: Driver<'d>, C: CommandSetHandler> {
 }
 
 impl<'d, D: Driver<'d>, C: CommandSetHandler> BulkOnlyTransport<'d, D, C> {
-    pub fn new(
+    pub fn new(builder: &mut Builder<'d, D>, state: &'d mut State, max_packet_size: u16, handler: C) -> Self {
-        builder: &mut Builder<'d, D>,
+        assert!(C::MAX_LUN < 16, "BulkOnlyTransport supports maximum 16 LUNs");
-        state: &'d mut State,
-        subclass: MscSubclass,
-        max_packet_size: u16,
-        max_lun: u8,
-        handler: C,
-    ) -> Self {
-        assert!(max_lun < 16, "BulkOnlyTransport supports maximum 16 LUNs");
 
-        let control = state.control.write(Control { max_lun });
+        let control = state.control.write(Control { max_lun: C::MAX_LUN });
 
-        let mut func = builder.function(USB_CLASS_MSC, subclass as _, MscProtocol::BulkOnlyTransport as _);
+        let subclass = C::MSC_SUBCLASS as u8;
+        let mut func = builder.function(USB_CLASS_MSC, subclass, MscProtocol::BulkOnlyTransport as _);
 
         // Control interface
         let mut iface = func.interface();
         iface.handler(control);
 
-        let mut alt = iface.alt_setting(USB_CLASS_MSC, subclass as _, MscProtocol::BulkOnlyTransport as _);
+        let mut alt = iface.alt_setting(USB_CLASS_MSC, subclass, MscProtocol::BulkOnlyTransport as _);
 
         let read_ep = alt.endpoint_bulk_out(max_packet_size);
         let write_ep = alt.endpoint_bulk_in(max_packet_size);

embassy-usb/src/class/msc/transport/mod.rs

@@ -1,5 +1,7 @@
 use embassy_usb_driver::EndpointError;
 
+use super::MscSubclass;
+
 pub mod bulk_only;
 
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
@@ -54,6 +56,12 @@ pub trait DataPipeOut {
 ///
 /// This trait is tailored to bulk-only transport and may require changes for other transports.
 pub trait CommandSetHandler {
+    /// MSC command set
+    const MSC_SUBCLASS: MscSubclass;
+
+    /// Maximum number of logical units. Set to zero if LUNs are not supported.
+    const MAX_LUN: u8;
+
     /// Handles command where data is sent to device.
     async fn command_out(&mut self, lun: u8, cmd: &[u8], pipe: &mut impl DataPipeOut) -> Result<(), CommandError>;
 

examples/stm32f4/src/bin/usb_msc.rs

@@ -1,173 +0,0 @@
-#![no_std]
-#![no_main]
-#![feature(type_alias_impl_trait)]
-#![feature(async_fn_in_trait)]
-
-use defmt::{panic, todo, *};
-use embassy_executor::Spawner;
-use embassy_stm32::time::mhz;
-use embassy_stm32::usb_otg::{Driver, Instance};
-use embassy_stm32::{interrupt, Config};
-use embassy_usb::class::cdc_acm::{CdcAcmClass, State};
-use embassy_usb::class::msc::subclass::scsi::block_device::BlockDevice;
-use embassy_usb::class::msc::subclass::scsi::Scsi;
-use embassy_usb::class::msc::transport::bulk_only::BulkOnlyTransport;
-use embassy_usb::class::msc::transport::CommandSetHandler;
-use embassy_usb::class::msc::MscSubclass;
-use embassy_usb::driver::EndpointError;
-use embassy_usb::Builder;
-use futures::future::join;
-use {defmt_rtt as _, panic_probe as _};
-
-// struct CommandSet {}
-
-// impl CommandSetHandler for CommandSet {
-//     async fn command_out(
-//         &mut self,
-//         lun: u8,
-//         cmd: &[u8],
-//         pipe: &mut impl embassy_usb::class::msc::transport::DataPipeOut,
-//     ) -> Result<(), embassy_usb::class::msc::transport::CommandError> {
-//         info!("CMD_OUT: {:?}", cmd);
-//         Ok(())
-//     }
-
-//     async fn command_in(
-//         &mut self,
-//         lun: u8,
-//         cmd: &[u8],
-//         pipe: &mut impl embassy_usb::class::msc::transport::DataPipeIn,
-//     ) -> Result<(), embassy_usb::class::msc::transport::CommandError> {
-//         info!("CMD_IN: {:?}", cmd);
-//         Ok(())
-//     }
-// }
-
-struct Device {
-    data: [u8; 512 * 128],
-}
-
-impl BlockDevice for Device {
-    fn block_size(&self) -> usize {
-        512
-    }
-
-    fn num_blocks(&self) -> u32 {
-        (self.data.len() / self.block_size()) as u32 - 1
-    }
-
-    fn read_block(
-        &self,
-        lba: u32,
-        block: &mut [u8],
-    ) -> Result<(), embassy_usb::class::msc::subclass::scsi::block_device::BlockDeviceError> {
-        block.copy_from_slice(&self.data[lba as usize * 512..(lba as usize + 1) * 512]);
-        Ok(())
-    }
-
-    fn write_block(
-        &mut self,
-        lba: u32,
-        block: &[u8],
-    ) -> Result<(), embassy_usb::class::msc::subclass::scsi::block_device::BlockDeviceError> {
-        self.data[lba as usize * 512..(lba as usize + 1) * 512].copy_from_slice(block);
-        Ok(())
-    }
-}
-
-#[embassy_executor::main]
-async fn main(_spawner: Spawner) {
-    info!("Hello World!");
-
-    let mut config = Config::default();
-    config.rcc.pll48 = true;
-    config.rcc.sys_ck = Some(mhz(48));
-
-    let p = embassy_stm32::init(config);
-
-    // Create the driver, from the HAL.
-    let irq = interrupt::take!(OTG_FS);
-    let mut ep_out_buffer = [0u8; 256];
-    let driver = Driver::new_fs(p.USB_OTG_FS, irq, p.PA12, p.PA11, &mut ep_out_buffer);
-
-    // Create embassy-usb Config
-    let mut config = embassy_usb::Config::new(0xc0de, 0xcafe);
-    config.manufacturer = Some("Embassy");
-    config.product = Some("USB-serial example");
-    config.serial_number = Some("12345678");
-
-    // Required for windows compatiblity.
-    // https://developer.nordicsemi.com/nRF_Connect_SDK/doc/1.9.1/kconfig/CONFIG_CDC_ACM_IAD.html#help
-    config.device_class = 0xEF;
-    config.device_sub_class = 0x02;
-    config.device_protocol = 0x01;
-    config.composite_with_iads = true;
-
-    // Create embassy-usb DeviceBuilder using the driver and config.
-    // It needs some buffers for building the descriptors.
-    let mut device_descriptor = [0; 256];
-    let mut config_descriptor = [0; 256];
-    let mut bos_descriptor = [0; 256];
-    let mut control_buf = [0; 64];
-
-    let mut state = Default::default();
-
-    let mut builder = Builder::new(
-        driver,
-        config,
-        &mut device_descriptor,
-        &mut config_descriptor,
-        &mut bos_descriptor,
-        &mut control_buf,
-        None,
-    );
-
-    let scsi = Scsi::new(Device { data: [0u8; 512 * 128] });
-
-    let mut msc = BulkOnlyTransport::new(
-        &mut builder,
-        &mut state,
-        MscSubclass::ScsiTransparentCommandSet,
-        64,
-        0,
-        scsi,
-    );
-
-    // Build the builder.
-    let mut usb = builder.build();
-
-    // Run the USB device.
-    let usb_fut = usb.run();
-
-    // Do stuff with the class!
-    let echo_fut = async {
-        loop {
-            msc.run().await;
-        }
-    };
-
-    // Run everything concurrently.
-    // If we had made everything `'static` above instead, we could do this using separate tasks instead.
-    join(usb_fut, echo_fut).await;
-}
-
-struct Disconnected {}
-
-impl From<EndpointError> for Disconnected {
-    fn from(val: EndpointError) -> Self {
-        match val {
-            EndpointError::BufferOverflow => panic!("Buffer overflow"),
-            EndpointError::Disabled => Disconnected {},
-        }
-    }
-}
-
-async fn echo<'d, T: Instance + 'd>(class: &mut CdcAcmClass<'d, Driver<'d, T>>) -> Result<(), Disconnected> {
-    let mut buf = [0; 64];
-    loop {
-        let n = class.read_packet(&mut buf).await?;
-        let data = &buf[..n];
-        info!("data: {:x}", data);
-        class.write_packet(data).await?;
-    }
-}

examples/stm32f4/src/bin/usb_msc_ram.rs

@@ -0,0 +1,111 @@
+#![no_std]
+#![no_main]
+#![feature(type_alias_impl_trait)]
+#![feature(async_fn_in_trait)]
+
+use defmt::*;
+use embassy_executor::Spawner;
+use embassy_stm32::time::mhz;
+use embassy_stm32::usb_otg::Driver;
+use embassy_stm32::{interrupt, Config};
+use embassy_usb::class::msc::subclass::scsi::block_device::{BlockDevice, BlockDeviceError};
+use embassy_usb::class::msc::subclass::scsi::Scsi;
+use embassy_usb::class::msc::transport::bulk_only::BulkOnlyTransport;
+use embassy_usb::Builder;
+use futures::future::join;
+use {defmt_rtt as _, panic_probe as _};
+
+struct RamBlockDevice {
+    data: [u8; 512 * 128],
+}
+
+impl BlockDevice for RamBlockDevice {
+    fn status(&self) -> Result<(), BlockDeviceError> {
+        Ok(())
+    }
+
+    fn block_size(&self) -> Result<usize, BlockDeviceError> {
+        Ok(512)
+    }
+
+    fn max_lba(&self) -> Result<u32, BlockDeviceError> {
+        Ok((self.data.len() / self.block_size().unwrap()) as u32 - 1)
+    }
+
+    async fn read_block(&self, lba: u32, block: &mut [u8]) -> Result<(), BlockDeviceError> {
+        block.copy_from_slice(&self.data[lba as usize * 512..(lba as usize + 1) * 512]);
+        Ok(())
+    }
+
+    async fn write_block(&mut self, lba: u32, block: &[u8]) -> Result<(), BlockDeviceError> {
+        self.data[lba as usize * 512..(lba as usize + 1) * 512].copy_from_slice(block);
+        Ok(())
+    }
+}
+
+#[embassy_executor::main]
+async fn main(_spawner: Spawner) {
+    info!("Hello World!");
+
+    let mut config = Config::default();
+    config.rcc.pll48 = true;
+    config.rcc.sys_ck = Some(mhz(48));
+
+    let p = embassy_stm32::init(config);
+
+    // Create the driver, from the HAL.
+    let irq = interrupt::take!(OTG_FS);
+    let mut ep_out_buffer = [0u8; 256];
+    let driver = Driver::new_fs(p.USB_OTG_FS, irq, p.PA12, p.PA11, &mut ep_out_buffer);
+
+    // Create embassy-usb Config
+    let mut config = embassy_usb::Config::new(0xc0de, 0xcafe);
+    config.manufacturer = Some("Embassy");
+    config.product = Some("MSC example");
+    config.serial_number = Some("12345678");
+
+    // Required for windows compatiblity.
+    // https://developer.nordicsemi.com/nRF_Connect_SDK/doc/1.9.1/kconfig/CONFIG_CDC_ACM_IAD.html#help
+    config.device_class = 0xEF;
+    config.device_sub_class = 0x02;
+    config.device_protocol = 0x01;
+    config.composite_with_iads = true;
+
+    // Create embassy-usb DeviceBuilder using the driver and config.
+    // It needs some buffers for building the descriptors.
+    let mut device_descriptor = [0; 256];
+    let mut config_descriptor = [0; 256];
+    let mut bos_descriptor = [0; 256];
+    let mut control_buf = [0; 64];
+
+    let mut state = Default::default();
+
+    let mut builder = Builder::new(
+        driver,
+        config,
+        &mut device_descriptor,
+        &mut config_descriptor,
+        &mut bos_descriptor,
+        &mut control_buf,
+        None,
+    );
+
+    // Create SCSI target for our block device
+    let scsi = Scsi::new(RamBlockDevice { data: [0u8; 512 * 128] }, "Embassy", "MSC");
+
+    // Use bulk-only transport for our SCSI target
+    let mut msc_transport = BulkOnlyTransport::new(&mut builder, &mut state, 64, scsi);
+
+    // Build the builder.
+    let mut usb = builder.build();
+
+    // Run the USB device.
+    let usb_fut = usb.run();
+
+    // Run mass storage transport
+    let msc_fut = msc_transport.run();
+
+    // Run everything concurrently.
+    // If we had made everything `'static` above instead, we could do this using separate tasks instead.
+    join(usb_fut, msc_fut).await;
+}