FDCAN issues

[richardeoin]

I happened to spot some (potential) issues in the FDCAN support

• TXBCR TXBCF and TXBTO registers are bitmaps, not ordinals fdcan.rs#L1396
• TXBRP register is a bitmap fdcan.rs#L1415
• Tx buffer element ID_W is missing its top bit txbuffer_element.rs#L130 See RM0440 rev5 44.3.6
• accept_all_into_fifo1() has the action for FIFO0 filter.rs#L40 filter.rs#L83

[David-OConnor]

Related: Thoughts on a standalone fdCAN crate, like the bxCAN one?

[teamplayer3]

Implementation suggestions: Would be nice to get the raw ID from ExtendedId and StandardId from outside the crate. Maybe make as_raw() public or some other way.
A nice way would be to have a crate to abstract this for every can implementation, so the ID types can be used in crates which should support any can implementation.

[luctius]

I happened to spot some (potential) issues in the FDCAN support

* `TXBCR` `TXBCF` and `TXBTO` registers are bitmaps, not ordinals [fdcan.rs#L1396](https://github.com/stm32-rs/stm32g4xx-hal/blob/master/src/fdcan.rs#L1396)

* `TXBRP` register is a bitmap [fdcan.rs#L1415](https://github.com/stm32-rs/stm32g4xx-hal/blob/master/src/fdcan.rs#L1415)

* Tx buffer element `ID_W` is missing its top bit [txbuffer_element.rs#L130](https://github.com/stm32-rs/stm32g4xx-hal/blob/master/src/fdcan/message_ram/txbuffer_element.rs#L130) See RM0440 rev5 44.3.6

* `accept_all_into_fifo1()` has the action for FIFO0 [filter.rs#L40](https://github.com/stm32-rs/stm32g4xx-hal/blob/master/src/fdcan/filter.rs#L40) [filter.rs#L83](https://github.com/stm32-rs/stm32g4xx-hal/blob/master/src/fdcan/filter.rs#L83)

Ooh, good points, nicely spotted!
I'll work on that tomorrow.

Related: Thoughts on a standalone fdCAN crate, like the bxCAN one?

I'm all for it; I needed the fdcan support, so I made it for the g4 as a stopgap measure, but please go ahead and copy this into a stand-alone crate. I had that idea but this seemed like a good first stepping stone to me.

Implementation suggestions: Would be nice to get the raw ID from ExtendedId and StandardId from outside the crate. Maybe make as_raw() public or some other way.

I'll add this

A nice way would be to have a crate to abstract this for every can implementation, so the ID types can be used in crates which should support any can implementation.

That also sounds like a good idea to me!

[adamgreig]

For shared traits, there's already the new CAN traits that were recently merged into embedded-hal here, which came from the embedded-can crate with some slight modifications. Right now the latest embedded-hal version doesn't yet include the new CAN traits, but it should soon (perhaps even in a new 0.2 release); in any event it seems like using that trait for the ID types would be nice for cross compatibility.

Having a separate fdcan crate which could be shared between the various HALs sounds good too, please shout if you need any help creating a repo in stm32-rs.

[richardeoin]

please go ahead and copy this into a stand-alone crate

I've made a standalone crate here. It includes a svd2rust-style PAC that applies to the FDCAN register mappings for G0 G4 L5 and H7, although unfortunately the H7 (and MP1) registers are different enough from everything else that I decided to use feature flags to switch between them.

I would also support using either the embedded-can (probably the most practical at the moment) or embedded-hal traits.

[David-OConnor]

I think it's better to leave the traits out of it, in the style of bxCAN and stm32-usbd. Or make them optional.

Great work.