Use Result instead of Option for Frame creation.

9 files changed, 78 insertions, 100 deletions

diff --git a/embassy-stm32/src/can/bx/mod.rs b/embassy-stm32/src/can/bx/mod.rs
index 650d4414b..ae8bafaa8 100644
--- a/embassy-stm32/src/can/bx/mod.rs
+++ b/embassy-stm32/src/can/bx/mod.rs
@@ -44,7 +44,6 @@ pub type Data = crate::can::frame::ClassicData;
 pub type Frame = crate::can::frame::ClassicFrame;
 
 use crate::can::bx::filter::MasterFilters;
-use crate::can::frame::ClassicData;
 
 /// A bxCAN peripheral instance.
 ///
@@ -799,10 +798,7 @@ where
             data[4..8].copy_from_slice(&mb.tdhr().read().0.to_ne_bytes());
             let len = mb.tdtr().read().dlc();
 
-            Some(Frame::new(
-                Header::new(id, len, false),
-                ClassicData::new(&data).unwrap(),
-            ))
+            Some(Frame::new(Header::new(id, len, false), &data).unwrap())
         } else {
             // Abort request failed because the frame was already sent (or being sent) on
             // the bus. All mailboxes are now free. This can happen for small prescaler
@@ -944,10 +940,7 @@ fn receive_fifo(canregs: crate::pac::can::Can, fifo_nr: usize) -> nb::Result<Fra
     // Release the mailbox.
     rfr.write(|w| w.set_rfom(true));
 
-    Ok(Frame::new(
-        Header::new(id, len, false),
-        ClassicData::new(&data).unwrap(),
-    ))
+    Ok(Frame::new(Header::new(id, len, false), &data).unwrap())
 }
 
 /// Identifies one of the two receive FIFOs.
diff --git a/embassy-stm32/src/can/bxcan.rs b/embassy-stm32/src/can/bxcan.rs
index 4d96df8aa..bb7cc3d7f 100644
--- a/embassy-stm32/src/can/bxcan.rs
+++ b/embassy-stm32/src/can/bxcan.rs
@@ -274,7 +274,7 @@ impl<'d, T: Instance> Can<'d, T> {
             data[0..4].copy_from_slice(&fifo.rdlr().read().0.to_ne_bytes());
             data[4..8].copy_from_slice(&fifo.rdhr().read().0.to_ne_bytes());
 
-            let frame = Frame::new(Header::new(id, data_len, false), Data::new(&data).unwrap());
+            let frame = Frame::new(Header::new(id, data_len, false), &data).unwrap();
             let envelope = Envelope {
                 #[cfg(feature = "time")]
                 ts,
diff --git a/embassy-stm32/src/can/enums.rs b/embassy-stm32/src/can/enums.rs
index 36139a45c..651de9194 100644
--- a/embassy-stm32/src/can/enums.rs
+++ b/embassy-stm32/src/can/enums.rs
@@ -28,3 +28,15 @@ pub enum BusError {
     ///  At least one of error counter has reached the Error_Warning limit of 96.
     BusWarning,
 }
+
+/// Frame Create Errors
+#[derive(Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub enum FrameCreateError {
+    /// Data in header does not match supplied.
+    NotEnoughData,
+    /// Invalid data length not 0-8 for Classic packet or valid for FD.
+    InvalidDataLength,
+    /// Invalid ID.
+    InvalidCanId,
+}
diff --git a/embassy-stm32/src/can/fd/peripheral.rs b/embassy-stm32/src/can/fd/peripheral.rs
index cce4e5e8d..e14977d91 100644
--- a/embassy-stm32/src/can/fd/peripheral.rs
+++ b/embassy-stm32/src/can/fd/peripheral.rs
@@ -56,7 +56,10 @@ impl Registers {
         match maybe_header {
             Some((header, ts)) => {
                 let data = &buffer[0..header.len() as usize];
-                Some((F::from_header(header, data)?, ts))
+                match F::from_header(header, data) {
+                    Ok(frame) => Some((frame, ts)),
+                    Err(_) => None,
+                }
             }
             None => None,
         }
diff --git a/embassy-stm32/src/can/frame.rs b/embassy-stm32/src/can/frame.rs
index 0dc74d299..14fc32c51 100644
--- a/embassy-stm32/src/can/frame.rs
+++ b/embassy-stm32/src/can/frame.rs
@@ -1,6 +1,8 @@
 //! Definition for CAN Frames
 use bit_field::BitField;
 
+use crate::can::enums::FrameCreateError;
+
 /// CAN Header, without meta data
 #[derive(Debug, Copy, Clone)]
 pub struct Header {
@@ -82,7 +84,7 @@ impl Header {
 /// and to retrieve the Header from a frame
 pub trait CanHeader: Sized {
     /// Construct frame from header and payload
-    fn from_header(header: Header, data: &[u8]) -> Option<Self>;
+    fn from_header(header: Header, data: &[u8]) -> Result<Self, FrameCreateError>;
 
     /// Get this frame's header struct
     fn header(&self) -> &Header;
@@ -103,15 +105,15 @@ impl ClassicData {
     ///
     /// Returns `None` if `data` is more than 64 bytes (which is the maximum) or
     /// cannot be represented with an FDCAN DLC.
-    pub fn new(data: &[u8]) -> Option<Self> {
-        if !FdData::is_valid_len(data.len()) {
-            return None;
+    pub fn new(data: &[u8]) -> Result<Self, FrameCreateError> {
+        if data.len() > 8 {
+            return Err(FrameCreateError::InvalidDataLength);
         }
 
         let mut bytes = [0; 8];
         bytes[..data.len()].copy_from_slice(data);
 
-        Some(Self { bytes })
+        Ok(Self { bytes })
     }
 
     /// Raw read access to data.
@@ -134,12 +136,6 @@ impl ClassicData {
     }
 }
 
-impl From<&[u8]> for ClassicData {
-    fn from(d: &[u8]) -> Self {
-        ClassicData::new(d).unwrap()
-    }
-}
-
 /// Frame with up to 8 bytes of data payload as per Classic CAN
 #[derive(Debug, Copy, Clone)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
@@ -150,59 +146,42 @@ pub struct ClassicFrame {
 
 impl ClassicFrame {
     /// Create a new CAN classic Frame
-    pub fn new(can_header: Header, data: impl Into<ClassicData>) -> ClassicFrame {
-        ClassicFrame {
-            can_header,
-            data: data.into(),
-        }
+    pub fn new(can_header: Header, raw_data: &[u8]) -> Result<Self, FrameCreateError> {
+        let data = ClassicData::new(raw_data)?;
+        Ok(ClassicFrame { can_header, data: data })
     }
 
     /// Creates a new data frame.
-    pub fn new_data(id: impl Into<embedded_can::Id>, data: &[u8]) -> Self {
+    pub fn new_data(id: impl Into<embedded_can::Id>, data: &[u8]) -> Result<Self, FrameCreateError> {
         let eid: embedded_can::Id = id.into();
         let header = Header::new(eid, data.len() as u8, false);
         Self::new(header, data)
     }
 
     /// Create new extended frame
-    pub fn new_extended(raw_id: u32, raw_data: &[u8]) -> Option<Self> {
+    pub fn new_extended(raw_id: u32, raw_data: &[u8]) -> Result<Self, FrameCreateError> {
         if let Some(id) = embedded_can::ExtendedId::new(raw_id) {
-            match ClassicData::new(raw_data) {
-                Some(data) => Some(ClassicFrame::new(
-                    Header::new(id.into(), raw_data.len() as u8, false),
-                    data,
-                )),
-                None => None,
-            }
+            Self::new(Header::new(id.into(), raw_data.len() as u8, false), raw_data)
         } else {
-            None
+            Err(FrameCreateError::InvalidCanId)
         }
     }
 
     /// Create new standard frame
-    pub fn new_standard(raw_id: u16, raw_data: &[u8]) -> Option<Self> {
+    pub fn new_standard(raw_id: u16, raw_data: &[u8]) -> Result<Self, FrameCreateError> {
         if let Some(id) = embedded_can::StandardId::new(raw_id) {
-            match ClassicData::new(raw_data) {
-                Some(data) => Some(ClassicFrame::new(
-                    Header::new(id.into(), raw_data.len() as u8, false),
-                    data,
-                )),
-                None => None,
-            }
+            Self::new(Header::new(id.into(), raw_data.len() as u8, false), raw_data)
         } else {
-            None
+            Err(FrameCreateError::InvalidCanId)
         }
     }
 
     /// Create new remote frame
-    pub fn new_remote(id: impl Into<embedded_can::Id>, len: usize) -> Option<Self> {
+    pub fn new_remote(id: impl Into<embedded_can::Id>, len: usize) -> Result<Self, FrameCreateError> {
         if len <= 8usize {
-            Some(ClassicFrame::new(
-                Header::new(id.into(), len as u8, true),
-                ClassicData::empty(),
-            ))
+            Self::new(Header::new(id.into(), len as u8, true), &[0; 8])
         } else {
-            None
+            Err(FrameCreateError::InvalidDataLength)
         }
     }
 
@@ -229,20 +208,19 @@ impl ClassicFrame {
 
 impl embedded_can::Frame for ClassicFrame {
     fn new(id: impl Into<embedded_can::Id>, raw_data: &[u8]) -> Option<Self> {
-        match ClassicData::new(raw_data) {
-            Some(data) => Some(ClassicFrame::new(
-                Header::new(id.into(), raw_data.len() as u8, false),
-                data,
-            )),
-            None => None,
+        let frameopt = ClassicFrame::new(Header::new(id.into(), raw_data.len() as u8, false), raw_data);
+        match frameopt {
+            Ok(frame) => Some(frame),
+            Err(_) => None,
         }
     }
     fn new_remote(id: impl Into<embedded_can::Id>, len: usize) -> Option<Self> {
         if len <= 8 {
-            Some(ClassicFrame::new(
-                Header::new(id.into(), len as u8, true),
-                ClassicData::empty(),
-            ))
+            let frameopt = ClassicFrame::new(Header::new(id.into(), len as u8, true), &[0; 8]);
+            match frameopt {
+                Ok(frame) => Some(frame),
+                Err(_) => None,
+            }
         } else {
             None
         }
@@ -268,8 +246,8 @@ impl embedded_can::Frame for ClassicFrame {
 }
 
 impl CanHeader for ClassicFrame {
-    fn from_header(header: Header, data: &[u8]) -> Option<Self> {
-        Some(Self::new(header, ClassicData::new(data)?))
+    fn from_header(header: Header, data: &[u8]) -> Result<Self, FrameCreateError> {
+        Self::new(header, data)
     }
 
     fn header(&self) -> &Header {
@@ -290,15 +268,15 @@ impl FdData {
     ///
     /// Returns `None` if `data` is more than 64 bytes (which is the maximum) or
     /// cannot be represented with an FDCAN DLC.
-    pub fn new(data: &[u8]) -> Option<Self> {
+    pub fn new(data: &[u8]) -> Result<Self, FrameCreateError> {
         if !FdData::is_valid_len(data.len()) {
-            return None;
+            return Err(FrameCreateError::InvalidDataLength);
         }
 
         let mut bytes = [0; 64];
         bytes[..data.len()].copy_from_slice(data);
 
-        Some(Self { bytes })
+        Ok(Self { bytes })
     }
 
     /// Raw read access to data.
@@ -337,40 +315,35 @@ pub struct FdFrame {
 
 impl FdFrame {
     /// Create a new CAN classic Frame
-    pub fn new(can_header: Header, data: FdData) -> FdFrame {
-        FdFrame { can_header, data }
+    pub fn new(can_header: Header, raw_data: &[u8]) -> Result<Self, FrameCreateError> {
+        let data = FdData::new(raw_data)?;
+        Ok(FdFrame { can_header, data })
     }
 
     /// Create new extended frame
-    pub fn new_extended(raw_id: u32, raw_data: &[u8]) -> Option<Self> {
+    pub fn new_extended(raw_id: u32, raw_data: &[u8]) -> Result<Self, FrameCreateError> {
         if let Some(id) = embedded_can::ExtendedId::new(raw_id) {
-            match FdData::new(raw_data) {
-                Some(data) => Some(FdFrame::new(Header::new(id.into(), raw_data.len() as u8, false), data)),
-                None => None,
-            }
+            Self::new(Header::new(id.into(), raw_data.len() as u8, false), raw_data)
         } else {
-            None
+            Err(FrameCreateError::InvalidCanId)
         }
     }
 
     /// Create new standard frame
-    pub fn new_standard(raw_id: u16, raw_data: &[u8]) -> Option<Self> {
+    pub fn new_standard(raw_id: u16, raw_data: &[u8]) -> Result<Self, FrameCreateError> {
         if let Some(id) = embedded_can::StandardId::new(raw_id) {
-            match FdData::new(raw_data) {
-                Some(data) => Some(FdFrame::new(Header::new(id.into(), raw_data.len() as u8, false), data)),
-                None => None,
-            }
+            Self::new(Header::new(id.into(), raw_data.len() as u8, false), raw_data)
         } else {
-            None
+            Err(FrameCreateError::InvalidCanId)
         }
     }
 
     /// Create new remote frame
-    pub fn new_remote(id: impl Into<embedded_can::Id>, len: usize) -> Option<Self> {
+    pub fn new_remote(id: impl Into<embedded_can::Id>, len: usize) -> Result<Self, FrameCreateError> {
         if len <= 8 {
-            Some(FdFrame::new(Header::new(id.into(), len as u8, true), FdData::empty()))
+            Self::new(Header::new(id.into(), len as u8, true), &[0; 8])
         } else {
-            None
+            Err(FrameCreateError::InvalidDataLength)
         }
     }
 
@@ -392,20 +365,17 @@ impl FdFrame {
 
 impl embedded_can::Frame for FdFrame {
     fn new(id: impl Into<embedded_can::Id>, raw_data: &[u8]) -> Option<Self> {
-        match FdData::new(raw_data) {
-            Some(data) => Some(FdFrame::new(
-                Header::new_fd(id.into(), raw_data.len() as u8, false, true),
-                data,
-            )),
-            None => None,
+        match FdFrame::new(Header::new_fd(id.into(), raw_data.len() as u8, false, true), raw_data) {
+            Ok(frame) => Some(frame),
+            Err(_) => None,
         }
     }
     fn new_remote(id: impl Into<embedded_can::Id>, len: usize) -> Option<Self> {
         if len <= 8 {
-            Some(FdFrame::new(
-                Header::new_fd(id.into(), len as u8, true, true),
-                FdData::empty(),
-            ))
+            match FdFrame::new(Header::new_fd(id.into(), len as u8, true, true), &[0; 64]) {
+                Ok(frame) => Some(frame),
+                Err(_) => None,
+            }
         } else {
             None
         }
@@ -432,8 +402,8 @@ impl embedded_can::Frame for FdFrame {
 }
 
 impl CanHeader for FdFrame {
-    fn from_header(header: Header, data: &[u8]) -> Option<Self> {
-        Some(Self::new(header, FdData::new(data)?))
+    fn from_header(header: Header, data: &[u8]) -> Result<Self, FrameCreateError> {
+        Self::new(header, data)
     }
 
     fn header(&self) -> &Header {
diff --git a/examples/stm32f1/src/bin/can.rs b/examples/stm32f1/src/bin/can.rs
index a43fb4427..ac337e8a0 100644
--- a/examples/stm32f1/src/bin/can.rs
+++ b/examples/stm32f1/src/bin/can.rs
@@ -46,7 +46,7 @@ async fn main(_spawner: Spawner) {
 
     let mut i: u8 = 0;
     loop {
-        let tx_frame = Frame::new_data(unwrap!(StandardId::new(i as _)), &[i, 0, 1, 2, 3, 4, 5, 6]);
+        let tx_frame = Frame::new_data(unwrap!(StandardId::new(i as _)), &[i, 0, 1, 2, 3, 4, 5, 6]).unwrap();
         can.write(&tx_frame).await;
 
         match can.read().await {
diff --git a/examples/stm32f4/src/bin/can.rs b/examples/stm32f4/src/bin/can.rs
index 2ed631a46..71b9453eb 100644
--- a/examples/stm32f4/src/bin/can.rs
+++ b/examples/stm32f4/src/bin/can.rs
@@ -51,7 +51,7 @@ async fn main(_spawner: Spawner) {
 
     let mut i: u8 = 0;
     loop {
-        let tx_frame = Frame::new_data(unwrap!(StandardId::new(i as _)), &[i]);
+        let tx_frame = Frame::new_data(unwrap!(StandardId::new(i as _)), &[i]).unwrap();
         let tx_ts = Instant::now();
         can.write(&tx_frame).await;
 
diff --git a/examples/stm32f7/src/bin/can.rs b/examples/stm32f7/src/bin/can.rs
index 2701196ed..221ac2a05 100644
--- a/examples/stm32f7/src/bin/can.rs
+++ b/examples/stm32f7/src/bin/can.rs
@@ -26,7 +26,7 @@ bind_interrupts!(struct Irqs {
 #[embassy_executor::task]
 pub async fn send_can_message(tx: &'static mut CanTx<'static, CAN3>) {
     loop {
-        let frame = Frame::new_data(unwrap!(StandardId::new(0 as _)), &[0]);
+        let frame = Frame::new_data(unwrap!(StandardId::new(0 as _)), &[0]).unwrap();
         tx.write(&frame).await;
         embassy_time::Timer::after_secs(1).await;
     }
diff --git a/tests/stm32/src/bin/can.rs b/tests/stm32/src/bin/can.rs
index e36137b38..c08c69a3b 100644
--- a/tests/stm32/src/bin/can.rs
+++ b/tests/stm32/src/bin/can.rs
@@ -60,7 +60,7 @@ async fn main(_spawner: Spawner) {
 
     let mut i: u8 = 0;
     loop {
-        let tx_frame = Frame::new_data(unwrap!(StandardId::new(i as _)), &[i]);
+        let tx_frame = Frame::new_data(unwrap!(StandardId::new(i as _)), &[i]).unwrap();
 
         info!("Transmitting frame...");
         let tx_ts = Instant::now();