search

embassy-rs / embassy

Modern embedded framework, using Rust and async.
https://embassy.dev
Apache License 2.0
5.48k stars 765 forks source link

FDCAN (CAN-FD) peripheral support on STM32 (G0, G4, H5, L5, U5 and H7) #2447

Closed blaa closed 9 months ago

blaa commented 9 months ago

Many STM32 devices with "generic" CAN2.0 peripheral are currently supported via the bxcan library. The ones listed in the title have a different peripheral. Additionaly the H7 is a bit different flavor because why not. Those FDCAN peripherals are currently not supported.

There's plenty of semi-coordinated progress on making the FDCAN embassy driver and many partial not-merged solutions. I've created this issue to summarize all information I could gather.

  1. There's a DRAFT pull-request focusing on H7: https://github.com/embassy-rs/embassy/pull/1692/files last changes 07.2023.
  2. There's some work by @tcbennun for other non-H7 devices from 21.11.2023: https://github.com/MaxiluxSystems/embassy/blob/feature/fdcan/embassy-stm32/src/can/fdcan.rs https://github.com/tcbennun/embassy-fdcan-mvp/blob/master/src/main.rs
    Which was a basis for some other forks.
  3. TheEppcJR from 08.12.2023 made some corrections to tcbennun. Commit descriptions aren't really certain about the change though. ;) https://github.com/TheEppicJR/embassy-fdcan-mvp/blob/h750-can-test/Cargo.toml https://github.com/TheEppicJR/embassy/commits/fdcan-exports/
  4. There's work by Adam Morgan based on tcbennun change from 21.12.2023, which mentions G4: https://github.com/lucimobility/embassy/tree/feature/rebased-fdcan

So my current understanding is to go with the (1) if using H7 or probably (4) if using the rest of devices. But I can't be sure. In near future I'll try (4) with stm32g431 and see how it goes.

blaa commented 9 months ago

I gathered changes from (2) and (4) on new branch to create something that might be mergeable - no debug prints, cleanly split commits, no commented code. Moved the example from FDCAN2/3 to FDCAN1. It compiles example for stm32g4, I haven't yet tried it on hardware at least in loopback mode it seems to work on stm32g431.

Lacking:

https://github.com/blaa/embassy/commits/feature/fdcan/

Two not included parts (were commented) are:

+    /*  
+    pub async fn flush(&self, mb: bxcan::Mailbox) { 
+        poll_fn(|cx| { 
+            T::state().tx_waker.register(cx.waker());
+            if T::regs().tsr().read().tme(mb.index()) {
+                return Poll::Ready(());
+            }
+   
+            Poll::Pending
+        })
+        .await;
+    }*/ 

+// TODO: impl Drop prevents us from changing modes without dropping the old one. 
+//  there is a solution here but I need to think about it more 
+/* 
+impl<'d, T: Instance, M: FdcanOperatingMode> Drop for Fdcan<'d, T, M> { 
+    fn drop(&mut self) {  
+        // Cannot call `free()` because it moves the instance. 
+        // Manually reset the peripheral. 
+        T::regs().cccr().write(|w| w.set_init(true));
+        T::disable();  
+    }  
+}  
+ */ 
+
kevswims commented 9 months ago

@blaa let myself and @aaamorgan know if you need a hand reviewing or implementing anything for this. We are quite interested in getting what we built in (4) above merged in and if you have combined those forks that seems like a good place to start working from. Happy to help with docs, test code, and any hardware testing that's needed. We have hardware that has a G4 with two transceivers that can be setup to talk to each other if that helps any for testing.

I'm on the Embassy matrix as kevlan if you want to ping me there to chat through anything as well.

cschuhen commented 9 months ago

Hi have been kicking this can a little further down the road at: https://github.com/embassy-rs/embassy/compare/main...cschuhen:embassy:feature/fdcan I added a HIL (internal loopback) test and added basic documentation for all of the public's that needed it. Tonight (Brisbane time) I intend to clean the changesets up a bit, squishing and reverting some of my own testing changes. Hopefully then it will be in shape to PR and get opinions from the core embassy developers.

blaa commented 9 months ago

Heh, just an hour minutes ago I started documenting this code. I'll stop and leave it to you then - maybe add some comments somewhere (maybe here). If you wish I can create PR later or leave it to you - as you wish.

I've checked two TODOs in comments and added additional docs to Bit0Error and Bit1Error (but I see you did too! Great work)

My partial local diff follows

diff --git a/embassy-stm32/src/can/bxcan.rs b/embassy-stm32/src/can/bxcan.rs
index cc87b256..96f12584 100644
--- a/embassy-stm32/src/can/bxcan.rs
+++ b/embassy-stm32/src/can/bxcan.rs
@@ -16,6 +16,8 @@ use crate::rcc::RccPeripheral;
 use crate::time::Hertz;
 use crate::{interrupt, peripherals, Peripheral};

+use super::calc_bit_timings;
+
 /// Contains CAN frame and additional metadata.
 ///
 /// Timestamp is available if `time` feature is enabled.
diff --git a/embassy-stm32/src/can/fdcan.rs b/embassy-stm32/src/can/fdcan.rs
index 08f3398a..9c54ca15 100644
--- a/embassy-stm32/src/can/fdcan.rs
+++ b/embassy-stm32/src/can/fdcan.rs
@@ -18,17 +18,25 @@ use crate::interrupt::typelevel::Interrupt;
 use crate::rcc::RccPeripheral;
 use crate::{interrupt, peripherals, Peripheral};

+// as far as I can tell, embedded-hal/can doesn't have any fdcan frame support
+/// CAN frame (header + data) upon reception.
 pub struct RxFrame {
+    /// CAN Header
     pub header: RxFrameInfo,
+    /// CAN or FDCAN (8-64 bytes) of data
     pub data: Data,
 }

+/// CAN frame that will be transmitted.
 pub struct TxFrame {
+    /// CAN Header
     pub header: TxFrameHeader,
+    /// CAN or FDCAN (8-64 bytes) of data
     pub data: Data,
 }

 impl TxFrame {
+    /// Validate input data and return new TxFrame.
     pub fn new(header: TxFrameHeader, data: &[u8]) -> Option<Self> {
         if data.len() < header.len as usize {
             return None;
@@ -51,10 +59,12 @@ impl TxFrame {
         Some(TxFrame { header, data })
     }

+    /// Access frame data (without a CAN header)
     pub fn data(&self) -> &[u8] {
         &self.data.bytes[..(self.header.len as usize)]
     }
 }
+
 impl RxFrame {
     pub(crate) fn new(header: RxFrameInfo, data: &[u8]) -> Self {
         let data = Data::new(&data).unwrap_or_else(|| Data::empty());
@@ -62,6 +72,7 @@ impl RxFrame {
         RxFrame { header, data }
     }

+    /// Access frame payload
     pub fn data(&self) -> &[u8] {
         &self.data.bytes[..(self.header.len as usize)]
     }
@@ -91,6 +102,7 @@ impl Data {
         Some(Self { bytes })
     }

+    /// Access internal raw bytes.
     pub fn raw(&self) -> &[u8] {
         &self.bytes
     }

(...)

#[derive(Debug)]
+/// Possible CAN Bus errors.
 pub enum BusError {
+    /// Error: More than 5 equal bits in sequence are not allowed on the bus.
     Stuff,
+    /// Error: Fixed part of received frame was invalid.
     Form,
+    /// Error: Transmitted messages was not acked.
     Acknowledge,
+    /// Error: Error during transmit: sent dominant but read recessive.
     BitRecessive,
+    /// Error: Arbitration error during transmit: sent recessive but read dominant.
     BitDominant,
+    /// Error: Invalid CRC of received message.
     Crc,
+    /// Error: Other software error
     Software,
+    /// Error: Bus is in off state.
     BusOff,
+    /// Error: Bus is in passive state.
     BusPassive,
+    /// Error: Bus is in warning state.
     BusWarning,
 }

@@ -185,8 +216,10 @@ impl BusError {
     fn try_from(lec: LastErrorCode) -> Option<BusError> {
         match lec {
             LastErrorCode::AckError => Some(BusError::Acknowledge),
-            LastErrorCode::Bit0Error => Some(BusError::BitRecessive), // TODO: verify
-            LastErrorCode::Bit1Error => Some(BusError::BitDominant),  // TODO: verify
+            // `0` data bit encodes a dominant state. `1` data bit is recessive.
+            // Bit0Error: During transmit, the node wanted to send a 0 but monitored a 1
+            LastErrorCode::Bit0Error => Some(BusError::BitRecessive),
+            LastErrorCode::Bit1Error => Some(BusError::BitDominant),
             LastErrorCode::CRCError => Some(BusError::Crc),
             LastErrorCode::FormError => Some(BusError::Form),
             LastErrorCode::StuffError => Some(BusError::Stuff),
cschuhen commented 9 months ago

I have pushed more changesets and opened a PR but CI is failing.

cschuhen commented 9 months ago

I have cleaned up the PR.

cschuhen commented 9 months ago

I have pushed some more fixes after review comments.

Also, for those interested, I have a few more experimental changesets here on my fdcan_experimental branch: https://github.com/cschuhen/embassy/tree/feature/fdcan_experimental I was planning to leave these until after the main merge.

cschuhen commented 9 months ago

I have pushed some more fixes after review comments.

Also, for those interested, I have a few more experimental changesets here on my fdcan_experimental branch: https://github.com/cschuhen/embassy/tree/feature/fdcan_experimental I was planning to leave these until after the main merge.

This branch is redundant now. The important bits were merged into fdcan_r2

cschuhen commented 9 months ago

@blaa Can this be closed now?

cschuhen commented 8 months ago

For those interested, follow up PR: https://github.com/embassy-rs/embassy/pull/2571