-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
STM32: FDCAN fix for FIFO1 usage and handling of Error interrupts. #8057
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ports/stm32/pyb_can.c
Outdated
| #if MICROPY_HW_ENABLE_FDCAN | ||
| // TODO implement for FDCAN | ||
| return mp_const_none; | ||
| // Done: check implementation for FDCAN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this comment can be removed now, because it works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, it can. I will put it on my todo list. Next to the G4 PR...
|
Hi, does this PR fix this issue #8169 ? |
No, this is an other problem, the problem you are facing is located in the pyb_can.c file, in function pyb_can_send. There the data size is limited to to original CAN spec of 8 bytes. Not the FDCAN updated values. Fir this PR I focused on getting the peripheral working. |
Yes but FDCAN is initialized in |
|
FYI I sent this #8185, it enables FD frames + BRS support, and fixes the Message RAM usage. I also documented how the message RAM is partitioned and what each section is used for. You should probably rebase on that after it's merged. There's minimal changes to |
|
I was not aware of this PR and also made a PR #8206 for enabling the rxcallbacks on FD CAN interfaces. This PR seems to be more complete in its implementation. |
|
I have tested this PR, it works very well. Also nice to see the |
|
Thanks for the effort on this @H-Grobben and thanks @JohnieBraaf for testing. Squashed and merged in 160e4d9 and 517e82e with some minor clean up (removed duplicate definition of some FIFO macros, and removed duplicated code in |
FDCAN peripheral was not performing as expected, so I debugged and tested it.
Original code where this was based on used a independent state with regards to the interrupt. During heavy bus error conditions the internal state could become out-of-sync with the interrupts.
Testing on NucleoG474 and using PR #6911 (which is not yet merged..) but checked the HAL function to work with H7 family
More explanation:
During the development of an application using the CAN communication, we found a interrupt-run-away in some situations. We found that the error interrupt triggered (al be it Warning, Passive or Bus-Off, all triggered it) the run-away. We could not get out of the non-responsive state without a reset.
I checked the code, and found two things: the error interrupt is enabled but not cleared in the interrupt routine, and an internal variable 'State' that was used to track the message received state (empty, new, full, overflow) that was not directly related to interrupt that indicated the state.
In this pull request I fixed these issue by adding more values for the interrupt reason (warning, passive, bus off) and clearing the error interrupts, and making the internal state directly dependent on the interrupt state for received messages.
While testing the fixes I also found that introducing the FIFO1 in the CAN receive stage, another issue existed. Even if the messages are received into the FIFO1 (by selecting message filtering for FIFO0 and FIFO1), the interrupt firing was indicating FIFO0 Rx. The configuration of the interrupts for this is now also fixed. The CAN peripheral has 2 interrupt lines going into the NVIC controller. The assignment of the interrupt reasons to these 2 interrupt lines was missing. Now the reception of FIFO1 messages, triggers the 2nd interrupt line. Other interrupts (Rx FIFO0 and Bus Error) are assigned to the 1st interrupt line.