Refactor CanFrame

- Add missings documentation
- Remove heapless dependency
- Change CanFrame construction interface

Author: Sh3Rm4n

Cargo.toml

@@ -38,10 +38,6 @@ doc-comment = "0.3.3"
 version = "0.1.0"
 optional = true
 
-[dependencies.heapless]
-version = "0.5.5"
-optional = true
-
 [dependencies.bare-metal]
 version = "0.2"
 features = ["const-fn"]
@@ -66,7 +62,7 @@ unproven = ["embedded-hal/unproven"]
 device-selected = []
 direct-call-deprecated = []
 rt = ["stm32f3/rt"]
-can = ["embedded-hal-can", "heapless"]
+can = ["embedded-hal-can"]
 
 gpio-f302 = []
 gpio-f303 = []

examples/can.rs

@@ -42,7 +42,7 @@ fn main() -> ! {
     let can_tx = gpioa.pa12.into_af9(&mut gpioa.moder, &mut gpioa.afrh);
 
     // Initialize the CAN peripheral
-    let can = Can::can(dp.CAN, can_rx, can_tx, &mut rcc.apb1);
+    let can = Can::new(dp.CAN, can_rx, can_tx, &mut rcc.apb1);
 
     // Uncomment the following line to enable CAN interrupts
     // can.listen(Event::Fifo0Fmp);
@@ -66,7 +66,7 @@ fn main() -> ! {
     asm::delay(100_000);
     let data: [u8; 1] = [0];
 
-    let frame = CanFrame::data_frame(CanId::BaseId(ID), &data);
+    let frame = CanFrame::new_data(CanId::BaseId(ID), &data);
 
     block!(can_tx.transmit(&frame)).expect("Cannot send first CAN frame");
 
@@ -81,7 +81,7 @@ fn main() -> ! {
             }
 
             let data: [u8; 1] = [counter];
-            let frame = CanFrame::data_frame(CanId::BaseId(ID), &data);
+            let frame = CanFrame::new_data(CanId::BaseId(ID), &data);
 
             block!(can_tx.transmit(&frame)).expect("Cannot send CAN frame");
         }

src/can.rs

@@ -17,7 +17,6 @@ use crate::gpio::gpioa;
 use crate::gpio::AF9;
 use crate::rcc::APB1;
 use crate::stm32;
-use heapless::{consts::U8, Vec};
 use nb::{self, Error};
 
 use core::sync::atomic::{AtomicU8, Ordering};
@@ -29,7 +28,9 @@ const MAX_EXTENDED_ID: u32 = 0x1FFF_FFFF;
 /// u16 and u32 respectively are used here despite the fact that the upper bits are unused.
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
 pub enum CanId {
+    /// Standard 11bit Identifier (0..=0x7FF)
     BaseId(u16),
+    /// Extended 29bit Identifier (0..=0x1FFF_FFFF)
     ExtendedId(u32),
 }
 
@@ -39,8 +40,12 @@ pub enum CanId {
 /// of actual size, but this could be improved in the future using const-generics.
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct CanFrame {
-    pub id: CanId,
-    pub data: Vec<u8, U8>,
+    /// CAN Frame ID
+    id: CanId,
+    /// Data Length Code (range 0..=8)
+    dlc: usize,
+    /// Data Frame
+    data: [u8; 8],
 }
 
 /// Can Frame Filter Mode
@@ -78,6 +83,7 @@ pub struct CanFilter {
 
 static FILTER_INDEX: AtomicU8 = AtomicU8::new(0);
 
+/// Controll Area Network (CAN) Peripheral
 pub struct Can {
     can: stm32::CAN,
     _rx: gpioa::PA11<AF9>,
@@ -97,14 +103,22 @@ pub struct CanTransmitter {
     _tx: gpioa::PA12<AF9>,
 }
 
+// TODO Use wrapper type around interal pac definition
 /// CAN Interrupt events
 pub enum Event {
+    /// FIFO 0 message pending interrupt
     Fifo0Fmp,
+    /// FIFO 1 message pending interrupt
     Fifo1Fmp,
+    /// FIFO 0 full interupt enable
     Fifo0Full,
+    /// FIFO 1 full interupt enable
     Fifo1Full,
+    /// FIFO 0 overrun interrupt enable
     Fifo0Ovr,
+    /// FIFO 1 overrun interrupt enable
     Fifo1Ovr,
+    /// Transmit mailbox empty interrupt
     Txe,
 }
 
@@ -146,7 +160,7 @@ impl embedded_hal_can::Frame for CanFrame {
 
     #[inline(always)]
     fn data(&self) -> Option<&[u8]> {
-        if self.data.len() > 0 {
+        if self.dlc > 0 {
             Some(&self.data)
         } else {
             None
@@ -238,12 +252,13 @@ impl CanFilterData {
 }
 
 impl Can {
-    pub fn can(
+    /// Initialize the CAN Peripheral
+    pub fn new(
         can: stm32::CAN,
         rx: gpioa::PA11<AF9>,
         tx: gpioa::PA12<AF9>,
         apb1: &mut APB1,
-    ) -> Can {
+    ) -> Self {
         apb1.enr().modify(|_, w| w.canen().enabled());
         can.mcr.modify(|_, w| w.sleep().clear_bit());
         can.mcr.modify(|_, w| w.inrq().set_bit());
@@ -372,29 +387,27 @@ impl embedded_hal_can::Transmitter for CanTransmitter {
                 }),
             }
 
-            if frame.data().is_some() {
-                for j in 0..frame.data.len() {
-                    let val = &frame.data[j];
-
+            if let Some(data) = frame.data() {
+                for (i, d) in data.iter().enumerate() {
                     // NOTE(unsafe): full 8bit write is unsafe via the svd2rust api
                     unsafe {
-                        match j {
-                            0 => tx.tdlr.modify(|_, w| w.data0().bits(*val)),
-                            1 => tx.tdlr.modify(|_, w| w.data1().bits(*val)),
-                            2 => tx.tdlr.modify(|_, w| w.data2().bits(*val)),
-                            3 => tx.tdlr.modify(|_, w| w.data3().bits(*val)),
-                            4 => tx.tdhr.modify(|_, w| w.data4().bits(*val)),
-                            5 => tx.tdhr.modify(|_, w| w.data5().bits(*val)),
-                            6 => tx.tdhr.modify(|_, w| w.data6().bits(*val)),
-                            7 => tx.tdhr.modify(|_, w| w.data7().bits(*val)),
+                        match i {
+                            0 => tx.tdlr.modify(|_, w| w.data0().bits(*d)),
+                            1 => tx.tdlr.modify(|_, w| w.data1().bits(*d)),
+                            2 => tx.tdlr.modify(|_, w| w.data2().bits(*d)),
+                            3 => tx.tdlr.modify(|_, w| w.data3().bits(*d)),
+                            4 => tx.tdhr.modify(|_, w| w.data4().bits(*d)),
+                            5 => tx.tdhr.modify(|_, w| w.data5().bits(*d)),
+                            6 => tx.tdhr.modify(|_, w| w.data6().bits(*d)),
+                            7 => tx.tdhr.modify(|_, w| w.data7().bits(*d)),
                             _ => unreachable!(),
                         }
                     }
                 }
 
                 // NOTE(unsafe): full 8bit write is unsafe via the svd2rust api
                 tx.tdtr
-                    .modify(|_, w| unsafe { w.dlc().bits(frame.data.len() as u8) });
+                    .modify(|_, w| unsafe { w.dlc().bits(data.len() as u8) });
 
                 tx.tir.modify(|_, w| w.rtr().clear_bit());
             } else {
@@ -417,23 +430,23 @@ impl Receiver for CanFifo {
 
         let rx = &can.rx[self.idx];
         if can.rfr[self.idx].read().fmp().bits() > 0 {
-            let mut data = Vec::<_, U8>::new();
+            let mut data: [u8; 8] = [0; 8];
 
-            let len = rx.rdtr.read().dlc().bits() as usize;
+            let len: usize = rx.rdtr.read().dlc().bits().into();
 
             let data_low = rx.rdlr.read();
             let data_high = rx.rdhr.read();
 
             for i in 0..len {
                 match i {
-                    0 => data.push(data_low.data0().bits()).unwrap(),
-                    1 => data.push(data_low.data1().bits()).unwrap(),
-                    2 => data.push(data_low.data2().bits()).unwrap(),
-                    3 => data.push(data_low.data3().bits()).unwrap(),
-                    4 => data.push(data_high.data4().bits()).unwrap(),
-                    5 => data.push(data_high.data5().bits()).unwrap(),
-                    6 => data.push(data_high.data6().bits()).unwrap(),
-                    7 => data.push(data_high.data7().bits()).unwrap(),
+                    0 => data[0] = data_low.data0().bits(),
+                    1 => data[1] = data_low.data1().bits(),
+                    2 => data[2] = data_low.data2().bits(),
+                    3 => data[3] = data_low.data3().bits(),
+                    4 => data[4] = data_high.data4().bits(),
+                    5 => data[5] = data_high.data5().bits(),
+                    6 => data[6] = data_high.data6().bits(),
+                    7 => data[7] = data_high.data7().bits(),
                     _ => unreachable!(),
                 }
             }
@@ -450,7 +463,11 @@ impl Receiver for CanFifo {
             // Release the mailbox
             can.rfr[self.idx].modify(|_, w| w.rfom().set_bit());
 
-            let frame = CanFrame { id: rcv_id, data };
+            let frame = CanFrame {
+                id: rcv_id,
+                dlc: len,
+                data,
+            };
 
             return Ok(frame);
         }
@@ -528,25 +545,38 @@ impl Receiver for CanFifo {
 }
 
 impl CanFrame {
-    pub fn new_with_len(id: CanId, src: &[u8], length: usize) -> CanFrame {
-        assert!(length <= 8, "CAN Frames can have at most 8 data bytes");
-
-        let mut data = Vec::<u8, U8>::new();
-
-        // The vector is always empty and the data size has alreay been checked, this will always succeed
-        data.extend_from_slice(src).unwrap();
+    /// Create a new Can Frame
+    ///
+    /// `src` content is copied into internal buffer
+    /// and handled internally.
+    ///
+    /// # Panics
+    ///
+    /// This function will panic if length of `data` is greater than `8`
+    pub fn new_data(id: CanId, data: &[u8]) -> CanFrame {
+        assert!((0..8).contains(&data.len()));
 
-        CanFrame { id, data }
+        let mut frame = Self {
+            id,
+            dlc: data.len(),
+            data: [0; 8],
+        };
+        frame.data[0..data.len()].copy_from_slice(data);
+        frame
     }
 
-    pub fn data_frame(id: CanId, data: &[u8]) -> CanFrame {
-        CanFrame::new_with_len(id, data, data.len())
-    }
+    /// Crate a new remote Can Frame
+    ///
+    /// # Panics
+    ///
+    /// This function will panic if `dlc` is not inside the vliad range `0..=8`.
+    pub fn new_remote(id: CanId, dlc: usize) -> CanFrame {
+        assert!((0..=8).contains(&dlc));
 
-    pub fn remote_frame(id: CanId) -> CanFrame {
         CanFrame {
             id,
-            data: Vec::<_, _>::new(),
+            dlc,
+            data: [0; 8],
         }
     }
 }