can: Documentation and example cleanup

Author: timokroeger

Cargo.toml

@@ -98,7 +98,6 @@ codegen-units = 1
 [profile.release]
 codegen-units = 1
 debug = true
-opt-level = "s"
 lto = true
 
 [[example]]

examples/can-echo.rs

@@ -1,3 +1,5 @@
+//! Simple CAN example. Requires a transceiver connected to PB5 and PB6.
+
 #![no_main]
 #![no_std]
 
@@ -22,14 +24,13 @@ fn main() -> ! {
     // resonator must be used.
     rcc.cfgr.use_hse(8.mhz()).freeze(&mut flash.acr);
 
-    let mut gpiob = dp.GPIOB.split(&mut rcc.apb2);
+    let mut can2 = Can::new(dp.CAN2, &mut rcc.apb1);
 
-    // Select some pins for CAN2.
+    // Select pins for CAN2.
+    let mut gpiob = dp.GPIOB.split(&mut rcc.apb2);
     let can_rx_pin = gpiob.pb5.into_floating_input(&mut gpiob.crl);
     let can_tx_pin = gpiob.pb6.into_alternate_push_pull(&mut gpiob.crl);
     let mut afio = dp.AFIO.constrain(&mut rcc.apb2);
-
-    let mut can2 = Can::new(dp.CAN2, &mut rcc.apb1);
     can2.assign_pins((can_tx_pin, can_rx_pin), &mut afio.mapr);
 
     can2.configure(|config| {
@@ -38,9 +39,6 @@ fn main() -> ! {
         config.set_bit_timing(0x001c_0003);
     });
 
-    // Get the transmitter part.
-    let mut tx = can2.take_tx().unwrap();
-
     // Filters are required to use the receiver part of CAN2.
     // Because the filter banks are part of CAN1 we first need to enable CAN1
     // and split the filters between the peripherals to use them for CAN2.
@@ -51,7 +49,10 @@ fn main() -> ! {
     let (_filters1, mut filters2) = can1.split_filters(0).unwrap();
     assert_eq!(filters2.num_available(), NUM_FILTER_BANKS);
     filters2.add(&Filter::accept_all()).unwrap(); // Receive all messages.
+
+    // Split the peripheral into transmitter and receiver parts.
     let mut rx = can2.take_rx(filters2).unwrap();
+    let mut tx = can2.take_tx().unwrap();
 
     // Sync to the bus and start normal operation.
     block!(can2.enable()).ok();

examples/can-loopback.rs

@@ -1,9 +1,13 @@
+//! Showcases advanced CAN filter capabilities.
+//! Does not require additional transceiver hardware.
+
 #![no_main]
 #![no_std]
 
 use panic_halt as _;
 
 use cortex_m_rt::entry;
+use embedded_hal::digital::v2::OutputPin;
 use nb::block;
 use stm32f1xx_hal::{
     can::{Can, Filter, Frame},
@@ -37,10 +41,8 @@ fn main() -> ! {
         config.set_silent(true);
     });
 
-    // Get the transmitter part.
-    let mut tx = can.take_tx().unwrap();
-
     // Use advanced configurations for the first three filter banks.
+    // More details can be found in the reference manual of the device.
     #[cfg(not(feature = "connectivity"))]
     let mut filters = can
         .split_filters_advanced(0x0000_0006, 0xFFFF_FFFA, 0x0000_0007)
@@ -52,6 +54,9 @@ fn main() -> ! {
 
     assert_eq!(filters.num_available(), 8);
 
+    // The order of the added filters is important: it must match configuration
+    // of the `split_filters_advanced()` method.
+
     // Matches 0, 1, 2
     filters
         .add_standard([
@@ -75,7 +80,9 @@ fn main() -> ! {
         ])
         .unwrap();
 
+    // Split the peripheral into transmitter and receiver parts.
     let mut rx = can.take_rx(filters).unwrap();
+    let mut tx = can.take_tx().unwrap();
 
     // Sync to the bus and start normal operation.
     block!(can.enable()).ok();
@@ -95,5 +102,9 @@ fn main() -> ! {
         assert!(rx.receive().is_err());
     }
 
+    let mut gpiob = dp.GPIOB.split(&mut rcc.apb2);
+    let mut led = gpiob.pb9.into_push_pull_output(&mut gpiob.crh);
+    led.set_high().unwrap();
+
     loop {}
 }

examples/can-rtfm.rs

@@ -6,8 +6,8 @@
 //! transmission the interrupt is reentered and more data is fetched from the
 //! queue.
 //! Received frames are simply echoed back. In contrast to the naive `can-echo`
-//! example the echo messages are also correctly prioritized by the transmit
-//! queue.
+//! example all messages are also correctly prioritized by the transmit queue.
+
 #![no_main]
 #![no_std]
 
@@ -64,26 +64,24 @@ const APP: () = {
             .pclk2(72.mhz())
             .freeze(&mut flash.acr);
 
-        let mut gpiob = cx.device.GPIOB.split(&mut rcc.apb2);
-        let mut afio = cx.device.AFIO.constrain(&mut rcc.apb2);
+        let mut can2 = Can::new(cx.device.CAN2, &mut rcc.apb1);
 
+        // Select pins for CAN2.
+        let mut gpiob = cx.device.GPIOB.split(&mut rcc.apb2);
         let can_rx_pin = gpiob.pb5.into_floating_input(&mut gpiob.crl);
         let can_tx_pin = gpiob.pb6.into_alternate_push_pull(&mut gpiob.crl);
-
-        let mut can2 = Can::new(cx.device.CAN2, &mut rcc.apb1);
+        let mut afio = cx.device.AFIO.constrain(&mut rcc.apb2);
         can2.assign_pins((can_tx_pin, can_rx_pin), &mut afio.mapr);
 
         can2.configure(|config| {
             // APB1 (PCLK1): 36MHz, Bit rate: 125kBit/s, Sample Point 87.5%
             // Value was calculated with http://www.bittiming.can-wiki.info/
-            config.set_bit_timing(0x001c0011);
+            config.set_bit_timing(0x001c_0011);
         });
 
-        let mut can_tx = can2.take_tx().unwrap();
-        can_tx.enable_interrupt();
-        let can_tx_queue = BinaryHeap::new();
-        CanFramePool::grow(CAN_POOL_MEMORY);
-
+        // Filters are required to use the receiver part of CAN2.
+        // Because the filter banks are part of CAN1 we first need to enable CAN1
+        // and split the filters between the peripherals to use them for CAN2.
         let mut can1 = Can::new(cx.device.CAN1, &mut rcc.apb1);
         let (_, mut filters) = can1.split_filters(0).unwrap();
 
@@ -100,6 +98,13 @@ const APP: () = {
         let mut can_rx = can2.take_rx(filters).unwrap();
         can_rx.enable_interrupts();
 
+        let mut can_tx = can2.take_tx().unwrap();
+        can_tx.enable_interrupt();
+
+        let can_tx_queue = BinaryHeap::new();
+        CanFramePool::grow(CAN_POOL_MEMORY);
+
+        // Sync to the bus and start normal operation.
         can2.enable().ok();
 
         init::LateResources {

src/can.rs

@@ -45,10 +45,9 @@ use core::{
 /// Can be either a standard identifier (11bit, Range: 0..0x3FF) or a
 /// extendended identifier (29bit , Range: 0..0x1FFFFFFF).
 ///
-/// The `Ord` trait is can be used to determine the frame’s priority this ID
-/// belongs to. This works because the EID and RTR flags are included in the
-/// underlying integer representaiton.
-/// Lower identifier values mean higher priority. Additionally standard frames
+/// The `Ord` trait can be used to determine the frame’s priority this ID
+/// belongs to.
+/// Lower identifier values have a higher priority. Additionally standard frames
 /// have a higher priority than extended frames and data frames have a higher
 /// priority than remote frames.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
@@ -67,15 +66,15 @@ impl Id {
 
     /// Creates a new standard identifier (11bit, Range: 0..0x7FF)
     ///
-    /// Ids outside the allowed range are silently truncated.
+    /// IDs outside the allowed range are silently truncated.
     pub fn new_standard(id: u32) -> Self {
         assert!(id < 0x7FF);
         Self(id << Self::STANDARD_SHIFT)
     }
 
     /// Creates a new extendended identifier (29bit , Range: 0..0x1FFFFFFF).
     ///
-    /// Ids outside the allowed range are silently truncated.
+    /// IDs outside the allowed range are silently truncated.
     pub fn new_extended(id: u32) -> Id {
         assert!(id < 0x1FFF_FFFF);
         Self(id << Self::EXTENDED_SHIFT | Self::IDE_MASK)
@@ -161,12 +160,12 @@ impl Frame {
         frame
     }
 
-    /// Creates ane new frame with a standard identifier.
+    /// Creates a new frame with a standard identifier.
     pub fn new_standard(id: u32, data: &[u8]) -> Self {
         Self::new(Id::new_standard(id), data)
     }
 
-    /// Creates ane new frame with an extended identifier.
+    /// Creates a new frame with an extended identifier.
     pub fn new_extended(id: u32, data: &[u8]) -> Self {
         Self::new(Id::new_extended(id), data)
     }
@@ -181,22 +180,22 @@ impl Frame {
         self
     }
 
-    /// Returns true if this `Frame` is a extended frame
+    /// Returns true if this frame is an extended frame
     pub fn is_extended(&self) -> bool {
         self.id.is_extended()
     }
 
-    /// Returns true if this `Frame` is a standard frame
+    /// Returns true if this frame is a standard frame
     pub fn is_standard(&self) -> bool {
         self.id.is_standard()
     }
 
-    /// Returns true if this `Frame` is a remote frame
+    /// Returns true if this frame is a remote frame
     pub fn is_remote_frame(&self) -> bool {
         self.id.rtr()
     }
 
-    /// Returns true if this `Frame` is a data frame
+    /// Returns true if this frame is a data frame
     pub fn is_data_frame(&self) -> bool {
         !self.is_remote_frame()
     }
@@ -297,8 +296,11 @@ impl traits::Pins for (PB6<Alternate<PushPull>>, PB5<Input<Floating>>) {
     }
 }
 
+/// Number of supported filter banks.
 #[cfg(not(feature = "connectivity"))]
 pub const NUM_FILTER_BANKS: usize = 14;
+
+/// Number of supported filter banks.
 #[cfg(feature = "connectivity")]
 pub const NUM_FILTER_BANKS: usize = 28;
 
@@ -309,7 +311,7 @@ pub struct CanConfig<Instance> {
 
 impl<Instance> CanConfig<Instance>
 where
-    Instance: traits::Instance<Bus = APB1>,
+    Instance: traits::Instance,
 {
     /// Configures the bit timings.
     ///
@@ -322,13 +324,14 @@ where
         });
     }
 
-    /// Enables or disables loopback mode: Internally connects the TX to RX.
+    /// Enables or disables loopback mode: Internally connects the TX and RX
+    /// signals together.
     pub fn set_loopback(&mut self, enabled: bool) {
         let can = unsafe { &*Instance::REGISTERS };
         can.btr.modify(|_, w| w.lbkm().bit(enabled));
     }
 
-    /// Enables or disables silent mode: Disconnects TX from the pin.
+    /// Enables or disables silent mode: Disconnects the TX signal from the pin.
     pub fn set_silent(&mut self, enabled: bool) {
         let can = unsafe { &*Instance::REGISTERS };
         can.btr.modify(|_, w| w.silm().bit(enabled));
@@ -417,7 +420,6 @@ where
         let can = unsafe { &*Instance::REGISTERS };
         let msr = can.msr.read();
         if msr.slak().bit_is_set() {
-            // TODO: Make automatic bus-off management configurable.
             can.mcr
                 .modify(|_, w| w.abom().set_bit().sleep().clear_bit());
             Err(nb::Error::WouldBlock)
@@ -486,7 +488,7 @@ impl Can<CAN1> {
     /// `split_idx` can be in the range `0..NUM_FILTER_BANKS` and decides the number
     /// of filters assigned to each peripheral. A value of `0` means all filter
     /// banks are used for CAN2 while `NUM_FILTER_BANKS` reserves all filter banks
-    /// for CAN2.
+    /// for CAN1.
     #[cfg(feature = "connectivity")]
     pub fn split_filters(&mut self, split_idx: usize) -> Option<(Filters<CAN1>, Filters<CAN2>)> {
         // Set all filter banks to 32bit scale and mask mode.
@@ -570,7 +572,7 @@ impl Can<CAN1> {
     }
 }
 
-/// A masked filter configuration.
+/// Filter with an optional mask.
 pub struct Filter {
     id: u32,
     mask: u32,
@@ -636,9 +638,17 @@ impl Filter {
             }
     }
 
-    fn as_16bit_filter(reg: u32) -> u32 {
+    fn reg_to_16bit(reg: u32) -> u32 {
         (reg & Id::STANDARD_MASK) >> 16 | (reg & Id::IDE_MASK) << 1 | (reg & Id::RTR_MASK) << 3
     }
+
+    fn id_to_16bit(&self) -> u32 {
+        Self::reg_to_16bit(self.id)
+    }
+
+    fn mask_to_16bit(&self) -> u32 {
+        Self::reg_to_16bit(self.mask)
+    }
 }
 
 /// Interface to the filter banks of a CAN peripheral.
@@ -663,6 +673,9 @@ where
     }
 
     /// Returns the number of available filters.
+    ///
+    /// This can number can be larger than the number of filter banks if
+    /// `Can::split_filters_advanced()` was used.
     pub fn num_available(&self) -> usize {
         let can = unsafe { &*CAN1::ptr() };
 
@@ -714,8 +727,8 @@ where
         self.check_config(idx, false, false)?;
         self.write_filter_bank(
             idx,
-            Filter::as_16bit_filter(filters[0].mask) << 16 | Filter::as_16bit_filter(filters[0].id),
-            Filter::as_16bit_filter(filters[1].mask) << 16 | Filter::as_16bit_filter(filters[1].id),
+            filters[0].mask_to_16bit() << 16 | filters[0].id_to_16bit(),
+            filters[1].mask_to_16bit() << 16 | filters[1].id_to_16bit(),
         );
         Ok(())
     }
@@ -735,8 +748,8 @@ where
         self.check_config(idx, true, false)?;
         self.write_filter_bank(
             idx,
-            Filter::as_16bit_filter(filters[1].id) << 16 | Filter::as_16bit_filter(filters[0].id),
-            Filter::as_16bit_filter(filters[3].id) << 16 | Filter::as_16bit_filter(filters[2].id),
+            filters[1].id_to_16bit() << 16 | filters[0].id_to_16bit(),
+            filters[3].id_to_16bit() << 16 | filters[2].id_to_16bit(),
         );
         Ok(())
     }
@@ -772,7 +785,7 @@ where
         self.count += 1;
     }
 
-    /// Disables all enabled filters.
+    /// Disables all enabled filter banks.
     pub fn clear(&mut self) {
         let can = unsafe { &*CAN1::ptr() };
 
@@ -805,9 +818,9 @@ where
     /// Puts a CAN frame in a free transmit mailbox for transmission on the bus.
     ///
     /// Frames are transmitted to the bus based on their priority (identifier).
-    /// Transmit order is preserved for frames with identical identifiers.
-    /// If all transmit mailboxes are full a higher priority frame replaces the
-    /// lowest priority frame which is returned as `Ok(Some(frame))`.
+    /// Transmit order is preserved for frames with of identifiers.
+    /// If all transmit mailboxes are full, a higher priority frame replaces the
+    /// lowest priority frame, which is returned as `Ok(Some(frame))`.
     pub fn transmit(&mut self, frame: &Frame) -> nb::Result<Option<Frame>, Infallible> {
         let can = unsafe { &*Instance::REGISTERS };