Clean up TWAI docs and example.

esp-hal-common/src/twai/filter.rs

@@ -1,3 +1,7 @@
+//! # Two-wire Automotive Interface (TWAI) Filters
+//!
+//! These are acceptance filters that limit which packets are received by the TWAI peripheral.
+
 #[cfg(feature = "eh1")]
 use embedded_can::{ExtendedId, StandardId};
 #[cfg(not(feature = "eh1"))]
@@ -10,17 +14,19 @@ pub enum FilterType {
 }
 
 pub trait Filter {
-    // The type of the filter.
+    /// The type of the filter.
     const FILTER_TYPE: FilterType;
     fn filter_type(&self) -> FilterType {
         Self::FILTER_TYPE
     }
 
+    /// Get the register level representation of the filter.
     fn to_registers(&self) -> [u8; 8];
 }
 
 pub type BitFilter<const N: usize> = [u8; N];
 
+/// Internal macro used to convert a byte from a bytestring into a bit inside a given code and mask.
 macro_rules! set_bit_from_byte {
     ($code:expr, $mask:expr, $byte:expr, $shift:expr) => {
         match $byte {
@@ -254,8 +260,10 @@ impl Filter for SingleExtendedFilter {
     }
 }
 
+/// A filter that matches against two standard 11-bit standard ids.
 ///
-/// TODO: is this how we actually want to store the two filters?
+/// The first filter part can match a packet's id, rtr bit, and the first byte of the payload.
+/// The second filter part can match a packet's id and rtr bit.
 ///
 /// Warning: This is not a perfect filter. Extended ids that match the bit layout of this filter
 /// will also be accepted.
@@ -426,6 +434,7 @@ impl DualExtendedFilter {
             raw: code_mask_to_register_array(acceptance_code, acceptance_mask),
         }
     }
+    /// Create a new filter matching the first 16 bits of two 29-bit ids.
     ///
     /// The masks indicate which bits of the code the filter should match against. Set bits
     /// in the mask indicate that the corresponding bit in the code should match.

esp-hal-common/src/twai/mod.rs

@@ -1,3 +1,8 @@
+//! # Two-wire Automotive Interface (TWAI)
+//!
+//! This driver manages the ISO 11898-1 (CAN Specification 2.0) compatible TWAI controllers. It
+//! supports Standard Frame Format (11-bit) and Extended Frame Format (29-bit) frame identifiers.
+
 use core::slice::{from_raw_parts, from_raw_parts_mut};
 
 use crate::{
@@ -19,7 +24,7 @@ use self::filter::{Filter, FilterType};
 
 pub mod filter;
 
-/// Structure backing the embedded_hal::can::Frame trait.
+/// Structure backing the embedded_hal::can::Frame/embedded_can::Frame trait.
 #[derive(Debug)]
 pub struct EspTwaiFrame {
     id: Id,
@@ -118,6 +123,7 @@ pub struct TimingConfig {
 }
 
 /// A selection of pre-determined baudrates for the TWAI driver.
+/// Currently these timings are sourced from the ESP IDF C driver which assumes an APB clock of 80MHz.
 pub enum BaudRate {
     B125K,
     B250K,
@@ -136,7 +142,7 @@ impl BaudRate {
     // #define TWAI_TIMING_CONFIG_500KBITS()   {.brp = 8, .tseg_1 = 15, .tseg_2 = 4, .sjw = 3, .triple_sampling = false}
     // #define TWAI_TIMING_CONFIG_800KBITS()   {.brp = 4, .tseg_1 = 16, .tseg_2 = 8, .sjw = 3, .triple_sampling = false}
     // #define TWAI_TIMING_CONFIG_1MBITS()     {.brp = 4, .tseg_1 = 15, .tseg_2 = 4, .sjw = 3, .triple_sampling = false}
-    fn timing(self) -> TimingConfig {
+    const fn timing(self) -> TimingConfig {
         match self {
             Self::B125K => TimingConfig {
                 baud_rate_prescaler: 32,
@@ -247,7 +253,11 @@ where
             });
     }
 
-    /// Set up the acceptance filter on the device to accept the specified filters.
+    /// Set up the acceptance filter on the device.
+    ///
+    /// NOTE: On a bus with mixed 11-bit and 29-bit packet id's, you may experience an
+    /// 11-bit filter match against a 29-bit frame and vice versa. Your application should check
+    /// the id again once a frame has been received to make sure it is the expected value.
     ///
     /// [ESP32C3 Reference Manual](https://www.espressif.com/sites/default/files/documentation/esp32-c3_technical_reference_manual_en.pdf#subsubsection.29.4.6)
     pub fn set_filter(&mut self, filter: impl Filter) {
@@ -267,11 +277,11 @@ where
         }
     }
 
-    /// Set the Error warning threshold.
+    /// Set the error warning threshold.
     ///
-    /// In the case when any of an error counter value exceeds
-    /// the threshold, or all the error counter values are below the threshold, an error warning
-    /// interrupt will be triggered (given the enable signal is valid).
+    /// In the case when any of an error counter value exceeds the threshold, or all the
+    /// error counter values are below the threshold, an error warning interrupt will be
+    /// triggered (given the enable signal is valid).
     pub fn set_error_warning_limit(&mut self, limit: u8) {
         self.peripheral
             .register_block()
@@ -296,8 +306,8 @@ where
 
 /// An active TWAI peripheral in Normal Mode.
 ///
-/// According to the [ESP32C3 Reference Manual](https://www.espressif.com/sites/default/files/documentation/esp32-c3_technical_reference_manual_en.pdf) 29.4.1.2 Operation Mode, in "Normal Mode":
-/// The TWAI controller can transmit and receive messages including error signals (such as error and overload Frames)
+/// In this mode, the TWAI controller can transmit and receive messages including error
+/// signals (such as error and overload frames).
 pub struct Twai<T> {
     peripheral: T,
 }
@@ -347,7 +357,7 @@ where
 
     /// Get the number of messages that the peripheral has available in the receive FIFO.
     ///
-    /// Note that this may not be the number of messages in the receive FIFO due to
+    /// Note that this may not be the number of valid messages in the receive FIFO due to
     /// fifo overflow/overrun.
     pub fn num_available_messages(&self) -> u8 {
         self.peripheral
@@ -435,7 +445,7 @@ where
     /// [ESP32C3 Reference Manual](https://www.espressif.com/sites/default/files/documentation/esp32-c3_technical_reference_manual_en.pdf#subsubsection.29.4.4.2)
     ///
     /// NOTE: TODO: This may not work if using the self reception/self test functionality. See
-    /// notes 1 and 2 in the Frame Identifier section of the reference manual.
+    /// notes 1 and 2 in the "Frame Identifier" section of the reference manual.
     ///
     fn transmit(&mut self, frame: &Self::Frame) -> nb::Result<Option<Self::Frame>, Self::Error> {
         let status = self.peripheral.register_block().status.read();
@@ -525,8 +535,11 @@ where
             .cmd
             .write(|w| w.tx_req().set_bit());
 
+        // Success in readying packet for transmit. No packets can be replaced in the transmit
+        // buffer so return None in accordance with the embedded-can/embedded-hal trait.
         nb::Result::Ok(None)
     }
+    /// Return a received frame if there are any available.
     fn receive(&mut self) -> nb::Result<Self::Frame, Self::Error> {
         let status = self.peripheral.register_block().status.read();
 
@@ -655,6 +668,7 @@ pub trait Instance {
     fn register_block(&self) -> &RegisterBlock;
 }
 
+#[cfg(any(esp32c3))]
 impl Instance for crate::pac::TWAI {
     #[inline(always)]
     fn register_block(&self) -> &RegisterBlock {

esp32c3-hal/examples/twai.rs

@@ -47,7 +47,7 @@ fn main() -> ! {
     let can_rx_pin = io.pins.gpio3;
 
     // The speed of the CAN bus.
-    let can_baudrate = twai::BaudRate::B1000K;
+    const CAN_BAUDRATE: twai::BaudRate = twai::BaudRate::B1000K;
 
     // Begin configuring the TWAI peripheral. The peripheral is in a reset like state that
     // prevents transmission but allows configuration.
@@ -57,18 +57,17 @@ fn main() -> ! {
         can_rx_pin,
         &mut system.peripheral_clock_control,
         &clocks,
-        can_baudrate,
+        CAN_BAUDRATE,
     );
 
     // Partially filter the incoming messages to reduce overhead of receiving undesired messages.
     // Note that due to how the hardware filters messages, standard ids and extended ids may both
     // match a filter. Frame ids should be explicitly checked in the application instead of fully
     // relying on these partial acceptance filters to exactly match.
-    // TODO: even though this is a single, standard id filter, extended ids will also match this filter.
     // A filter that matches standard ids of an even value.
-    let filter =
+    const FILTER: twai::filter::SingleStandardFilter =
         twai::filter::SingleStandardFilter::new(b"xxxxxxxxxx0", b"x", [b"xxxxxxxx", b"xxxxxxxx"]);
-    can_config.set_filter(filter);
+    can_config.set_filter(FILTER);
 
     // Start the peripheral. This locks the configuration settings of the peripheral and puts it
     // into operation mode, allowing packets to be sent and received.