stm32: Improved CAN FD support. #8185

iabdalkader · 2022-01-16T22:18:31Z

This PR does the following:

Add support for CAN FD frames.
Add support for Bitrate Switching (BRS) for data phase.
Add support for extended (29-bit) frame identifiers.
Add support for a bus with mixed Classic/FD nodes.
Return the matching filter ID and type of received messages.
Fix the maximum range for the nominal and data bit timing parameters.
Optimize the Message RAM usage per FDCAN instance.
Document the usage and different sections of the Message RAM.
Update CAN docs.
Fixes CAN FD not working with STM32 H7 #8169
Fixes Is CAN FD fully supported on STM Nucleo H743 ? #5621
Fixes ports/stm32/can.c: allow mixed id-type bus #3916
Fixes Receive Extended CANBus Frame on H7/H7+ openmv/openmv#1071

Example usage:

To send a classical CAN frame use the default send:

can.send('a' * 8, 1)

To send an FD frame (long frame, up to 64 bytes) set fdf true:

can.send('a' * 64, 1, fdf=True)

To use BRS (bitrate switching) set brs_baudrate (or brs timings), and set brs true, example:

# FD frame + BRS mode. 500 Kbit/s for arbitration phase, 1Mbit/s for data phase.
can = CAN(2, CAN.NORMAL, baudrate=500_000, brs_baudrate=1_000_000, sample_point=80)
can.send('a'*64, 1, fdf=True, brs=True)

To use extended IDs set extframe true, example:

can = CAN(2, CAN.NORMAL, baudrate=500_000, sample_point=75)
# Note you should setfilter() on the receiver with extframe=True
can.send('HelloWorld', 0xFFFF, extframe=True)

All options:

# FD frame + BRS mode + Extended frame ID. 500 Kbit/s for arbitration phase, 1Mbit/s for data phase.
can = CAN(2, CAN.NORMAL, baudrate=500_000, brs_baudrate=1_000_000, sample_point=80)
# Note you should setfilter() on the receiver with extframe=True
can.send('a'*64, 0xFFFF, fdf=True, brs=True, extframe=True)

Testing:

I tested the following scenarios:

Sending/Receiving classic CAN frames from FDCAN <-> FDCAN (STM32H7).
Sending/Receiving FD (long) CAN frames (plus BRS) from FDCAN <-> FDCAN (STM32H7).
Sending/Receiving classic CAN frame FDCAN <-> CAN (H7 <-> F7)
Sending/Filtering&Receiving extended frame IDs (H7 <-> H7).

Notes:

This change doesn't break backwards compatibility with F7 CAN.
~~I renamed extframe to extid because it never really enabled extended frames before, and classic CAN does Not support extended frames anyway.~~
After looking at the CAN specs and CAN FD specs I realized the specs refer to the extended (29-bit) ID as extended frame and extended format respectively. I still think extid is much better, and it is used in the HAL's CAN message structs, that said I reverted it.
The pyb_can_initfilterbanks() functions is problematic, to say the least. This function should Not be a class method for FDCAN because it can't be called before calling can.init() first to initialize the number of standard and extended filters and their Message RAM addresses/offsets based on the Message RAM configuration, and set which type of filter is used (although I init both standard and extended filters but we can optimize this). As for classic CAN, this function calls HAL_CAN_ConfigFilter(NULL, &filter); from can.c. If CAN3 is defined, HAL_CAN_ConfigFilter will dereference a NULL CAN handle pointer.
Following comments on ports/stm32/can.c: allow mixed id-type bus #3916 in particular this comment ports/stm32/can.c: allow mixed id-type bus #3916 (comment), I added extframe keyword argument to send(), setfilter() and clearfilter(), and removed it from init() because it's no longer needed. Also following comments on the same issue, recv() now returns a tuple for the matching filter ID, indicating the frame type (true for extended).

kwagyeman · 2022-01-16T23:10:33Z

Can you fix extended frame mode too?

openmv/openmv#1071

JohnieBraaf · 2022-01-24T15:50:40Z

Changing the variable name that defines the type of CAN Address from extframe to extid does make it more clear.
Especially with the extended frame functionality that CAN FD adds.

I have tested the non-can fd part of this code and it still functions as it did before. I can send and receive normal can frames both with simple and extended id's. I have not looked at the fdf=True type frames.

Not striclty related to this change, but a note for testing this, receiving extid=False type frames on non can-fd interfaces still does not seem to work see #5508
I have nullified the filter in order to receive them on the STM32 F4 and F7 MCU's.

iabdalkader · 2022-01-24T17:24:47Z

Not striclty related to this change, but a note for testing this, receiving extid=False type frames on non can-fd interfaces still does not seem to work see #5508
I have nullified the filter in order to receive them on the STM32 F4 and F7 MCU's.

I have not done any testing with the F4, although I have one if needed. Is this fixed in your PR #8206 ? Also have you seen this PR #8057 it fixes some IRQ issues and whatnot. If the issue is not fixed I can take a look, but this PR is mainly concerned with adding full CAN FD support.

JohnieBraaf · 2022-01-24T17:59:40Z

No, #5508 remains open, but I guess it would be better to fix that in a seperate PR.
Thnx for pointing me to #8057, I was not aware of that PR, it addresses the same issue and more.

iabdalkader · 2022-01-24T18:28:39Z

No, #5508 remains open

Can you tell me how to reproduce this issue ? I can test with of H7, F7 and F4.

JohnieBraaf · 2022-01-24T20:52:11Z

While writing down the steps to reproduce and looking at the code I realized, I should have used MASK16 instead of MASK32.
Sorry for the confusion. I managed to receive non-extended id messages on non fd interfaces with the following filter:
can1.setfilter(bank=0, mode=pyb.CAN.MASK16, fifo=0, params=(0x0, 0x0, 0x0, 0x0))
Thanks for reaching out, the good news is, I can close the old issue.

iabdalkader · 2022-01-24T20:56:45Z

That's great! Thanks for the feedback on the PR.

dpgeorge · 2022-01-27T12:10:24Z

Can you please rebase this on latest master to pick up the other merged FDCAN PR?

iabdalkader · 2022-01-27T13:16:57Z

Can you please rebase this on latest master to pick up the other merged FDCAN PR?

Yes, I'll see what I can do, but I can't fix anything related to STM32G4. That said I documented the message ram usage very well and someone else would be able to fix it.

iabdalkader · 2022-01-27T14:26:10Z

@dpgeorge rebased. I don't like setting the second set of bit timings (the BRS timings) in the Init struct in pyb_can_init_helper() before calling can_init(), but can_init() prototype will have to be changed for both CAN and FDCAN to accept more args (not used by can.c). Do you have any suggestions for a better way to do this ?

iabdalkader · 2022-01-29T00:26:00Z

After looking at the CAN specs and CAN FD specs I realized the specs refer to the extended (29-bit) ID as extended frame and extended format respectively. I still think extid is much better, and it is used in the HAL's CAN message structs, that said I reverted it.

iabdalkader · 2022-01-29T02:03:49Z

Please see the updated notes in the first comment.

iabdalkader · 2022-02-16T21:29:53Z

@dpgeorge I've collected almost all? of the open CAN issues in this PR. All the linked issues should be fixed by this PR. I'm now working on the docs, the state of the CAN docs is really bad. Can you review & approve this so I can account for any API changes while I'm at it ?

iabdalkader · 2022-02-22T13:17:36Z

@dpgeorge docs updated.

dpgeorge · 2022-02-22T13:24:47Z

Thanks for updating. I'll take a look at this next.

dpgeorge · 2022-03-07T06:42:54Z

After an initial review of this PR, here are my comments:

It looks like it'll be too hard and confusing to retain backwards compatibility, so let's forget about that for the moment.
It makes sense to support extframe as an argument to send() and the filter config functions/methods, that's a good change. And hence also remove it from the constructor/init() method.
While I agree that extid is more logical than extframe, the CAN specs and other internet resources on CAN are pretty clear about it being an "extended frame". Even CAN FD calls it an FEFF (FD extended frame format). So I think it's best to match the spec and continue to use extframe. Just need to make the docs clear what this option does.
I see the problem with initfilterbanks() being a static method. But logically it should remain static because its configuring things before the CAN instances are created. Maybe the way it can be implemented is to simply store the nr argument passed in and subsequently use that value to configure the CAN when the objects are actually instantiated.
For recv(): to avoid allocating the 2-tuple, and making the return value nested tuples, I think the second element of the existing returned 4-tuple can become a bitmask with flags. Currently it's a boolean for RTR, but it can be a bit-wise or of RTR|EXTID. Alternatively make the 4-tuple a 5-tuple.

iabdalkader · 2022-03-07T19:20:52Z

Just need to make the docs clear what this option does.

The docs are clear about it and were updated to reflect the changes (removed the extframe arg from init(), added it to send() and the other filter functions).

For recv(): to avoid allocating the 2-tuple, and making the return value nested tuples, I think the second element of the existing returned 4-tuple can become a bitmask with flags. Currently it's a boolean for RTR, but it can be a bit-wise or of RTR|EXTID. Alternatively make the 4-tuple a 5-tuple.

I went with the 5-tuple option, docs/ code comments were both updated to reflect the changes.

Maybe the way it can be implemented is to simply store the nr argument passed in and subsequently use that value to configure the CAN when the objects are actually instantiated.

This is possible, however it's not that simple. The main issue here is that the CAN FD RAM (which is shared between all sections including filtering) is currently configured/split statically (i.e. the number of filters, TX/RX FIFO sizes, etc.. per instance is set statically in fdcan.c). Changing the number of std/ext filters dynamically, would require adjusting all the other sections according to the remaining memory, however, how do we determine which section gets increased/reduced based on the remaining available memory ? I think this will lead us to requiring that users provide the entire memory config/split in initfilterbanks before calling init(). Alternatively, an easier option is to just add an arg to enable using the entire message RAM for 1 CAN instance (use the same memory offset for CAN1/2), which would give the user double the number of filters, double TX/RX FIFO sizes and so on.

EDIT: Maybe providing the CAN memory config would be a good feature for more advanced use cases. The needed args are the number of ext filters, number of standard filters, and the number of TX and RX FIFO elements. It would probably make more sense to rename initfilterbanks to something more generic, and using can_obj->can.Init to set the args.

iabdalkader · 2022-03-29T19:32:35Z

ping @dpgeorge

dpgeorge · 2022-03-29T23:46:01Z

It looks like the only outstanding item here is what to do with initfilterbanks. Based in the discussion above, I have 2 suggestions for what to do.

Option 1

rename initfilterbanks to a more generic name like alloc_resources
alloc_resources(...) will be a class method taking keyword arguments to configure things, and the possible arguments depend on the MCU series (eg F4/F7 just selects CAN2 start bank like, on H7 there is more to configure)
calling alloc_resources() is optional; if it's not called then there are sensible defaults, eg CAN(1) uses all/half available resources

Option 2

remove initfilterbanks completely
add new keyword args to the CAN constructor to specify how many resources it should use, eg num_filter_banks
specifying these args is optional and they default to something sensible, eg all/half resources

Option 2 seems more in line with the rest of the machine API, eg specifying UART buffer size (which comes from the heap, but conceptually is the same as allocating general resources).

* Enable CAN FD frame support and BRS. * Optimize the message RAM usage per FDCAN instance. * Document the usage and different sections of the Message RAM.

* A CAN bus can have mixed classic/FD nodes. Currently, CAN is configured for either standard or extended ID. * This patch allows extended IDs to be filtered and enabled on a per message basis, in send() and set/clearfilter.

* Define the maximum parameters for CAN/FDCAN nominal bit timing, and for FDCAN data bit timing, for bit timing calculations.

iabdalkader · 2022-03-30T15:50:47Z

Option 2 seems more in line with the rest of the machine API

I went with option 2 and removed initfilterbanks completely, added a num_filter_banks keyword arg for classic F4/F7 CAN, which provides the same functionality, and updated the docs accordingly. For FDCAN the configuration remains static. I think the default config is good for now, but I could add the keyword args to configure the common options (like number of standard/extended filters, TX/RX FIFO sizes etc..) and sanity checks to make sure the provided config fits the RAM. Do you want this in the same PR or is it good for now ?

* Add keyword arg to configure the filter bank split.

dpgeorge · 2022-03-30T23:38:22Z

I went with option 2 and removed initfilterbanks completely, added a num_filter_banks keyword arg for classic F4/F7 CAN, which provides the same functionality, and updated the docs accordingly

Fantastic!

For FDCAN the configuration remains static. I think the default config is good for now

Yes I agree, let's leave it static until someone requests for it to be configurable.

iabdalkader · 2022-03-31T08:16:56Z

Not sure why CI is failing, let me know if I need to fix something.

dpgeorge · 2022-03-31T09:44:08Z

There's so much CI, something is always going wrong... but in this case it is #8301 so nothing to worry about for this PR.

dpgeorge · 2022-04-02T11:49:54Z

Rebased and merged in ff287d0 through 5cdf964. And I updated the tests in 71344c1

Thank you @iabdalkader for your efforts on this, I think it is a good improvement to the API and good to have better FD CAN support!

…es-functions Document optional types & functions

iabdalkader mentioned this pull request Jan 16, 2022

CAN FD not working with STM32 H7 #8169

Closed

iabdalkader force-pushed the fdcan_updates branch 2 times, most recently from e3a4441 to 09eb443 Compare January 16, 2022 22:36

iabdalkader mentioned this pull request Jan 16, 2022

STM32: FDCAN fix for FIFO1 usage and handling of Error interrupts. #8057

Closed

iabdalkader force-pushed the fdcan_updates branch 2 times, most recently from 2aed1cb to 9ee8afe Compare January 16, 2022 23:21

dpgeorge added the port-stm32 label Jan 17, 2022

iabdalkader force-pushed the fdcan_updates branch 3 times, most recently from 99af5fb to 5d1ea18 Compare January 17, 2022 20:22

iabdalkader changed the title ~~stm32: Enable CAN FD frame support and BRS.~~ stm32: Enable CAN FD frame support, BRS and extended frame ID support. Jan 17, 2022

iabdalkader changed the title ~~stm32: Enable CAN FD frame support, BRS and extended frame ID support.~~ stm32: Improved CAN FD support. Jan 20, 2022

iabdalkader force-pushed the fdcan_updates branch from 5d1ea18 to f4efec2 Compare January 27, 2022 14:15

iabdalkader force-pushed the fdcan_updates branch 2 times, most recently from faf55b7 to ccf0adf Compare January 29, 2022 01:56

iabdalkader force-pushed the fdcan_updates branch 3 times, most recently from 0ac5a80 to a2fb7af Compare January 30, 2022 18:04

iabdalkader force-pushed the fdcan_updates branch from a2fb7af to 2061103 Compare February 3, 2022 00:10

iabdalkader force-pushed the fdcan_updates branch from 16771b5 to c7cfe83 Compare March 7, 2022 18:11

iabdalkader added 5 commits March 30, 2022 15:07

stm32: Enable CAN FD frame support and BRS.

d577e04

* Enable CAN FD frame support and BRS. * Optimize the message RAM usage per FDCAN instance. * Document the usage and different sections of the Message RAM.

stm32: Add support for CAN FD extended frame ID.

265b4a1

stm32/pyb_can.c: Return the filter ID and type of received messages.

f293ea5

stm32/pyb_can.c: Define the maximum bit timing parameters.

6f847e6

* Define the maximum parameters for CAN/FDCAN nominal bit timing, and for FDCAN data bit timing, for bit timing calculations.

iabdalkader force-pushed the fdcan_updates branch from c7cfe83 to 73329dd Compare March 30, 2022 15:46

iabdalkader added 2 commits March 30, 2022 17:57

stm32/pyb_can.c: Remove initfilterbanks.

ebd7c0b

* Add keyword arg to configure the filter bank split.

docs/library/pyb.CAN.rst: Update CAN docs.

0c52654

iabdalkader force-pushed the fdcan_updates branch from 73329dd to 0c52654 Compare March 30, 2022 15:57

dpgeorge closed this Apr 2, 2022

iabdalkader deleted the fdcan_updates branch April 2, 2022 12:01

tannewt added a commit to tannewt/circuitpython that referenced this pull request Aug 24, 2023

Merge pull request micropython#8185 from jepler/document-optional-typ…

98bd9e5

…es-functions Document optional types & functions

iabdalkader mentioned this pull request Aug 30, 2023

ports/mimxrt: Add machine.CAN driver. #12324

Open

Uh oh!

stm32: Improved CAN FD support. #8185

stm32: Improved CAN FD support. #8185

Uh oh!

Conversation

iabdalkader commented Jan 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This PR does the following:

Example usage:

Testing:

Notes:

Uh oh!

kwagyeman commented Jan 16, 2022

Uh oh!

JohnieBraaf commented Jan 24, 2022

Uh oh!

iabdalkader commented Jan 24, 2022

Uh oh!

JohnieBraaf commented Jan 24, 2022

Uh oh!

iabdalkader commented Jan 24, 2022

Uh oh!

JohnieBraaf commented Jan 24, 2022

Uh oh!

iabdalkader commented Jan 24, 2022

Uh oh!

dpgeorge commented Jan 27, 2022

Uh oh!

iabdalkader commented Jan 27, 2022

Uh oh!

iabdalkader commented Jan 27, 2022

Uh oh!

iabdalkader commented Jan 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iabdalkader commented Jan 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iabdalkader commented Feb 16, 2022

Uh oh!

iabdalkader commented Feb 22, 2022

Uh oh!

dpgeorge commented Feb 22, 2022

Uh oh!

dpgeorge commented Mar 7, 2022

Uh oh!

iabdalkader commented Mar 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iabdalkader commented Mar 29, 2022

Uh oh!

dpgeorge commented Mar 29, 2022

Option 1

Option 2

Uh oh!

iabdalkader commented Mar 30, 2022

Uh oh!

dpgeorge commented Mar 30, 2022

Uh oh!

iabdalkader commented Mar 31, 2022

Uh oh!

dpgeorge commented Mar 31, 2022

Uh oh!

dpgeorge commented Apr 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

iabdalkader commented Jan 16, 2022 •

edited

Loading

iabdalkader commented Jan 29, 2022 •

edited

Loading

iabdalkader commented Jan 29, 2022 •

edited

Loading

iabdalkader commented Mar 7, 2022 •

edited

Loading