-
Notifications
You must be signed in to change notification settings - Fork 3k
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
CAN: read can do only up to 8 bytes, error if more is reported #15374
CAN: read can do only up to 8 bytes, error if more is reported #15374
Conversation
If HAL implementation writes more than 8 bytes of data, error immediately. CANMessage defines only 8 bytes of data, lenght cannot be > 8. This fixes ARMmbed#15361 Signed-off-by: Martin Kojtal <[email protected]>
I'll check if we need the same fix for 5.15 branch, if yes, I'll create new PR as well. |
CI startd |
The only issue that I can see is that if the FD CAN is used: mbed-os/targets/TARGET_STM/can_api.c Line 21 in 1dd0bce
data field in the CAN_Message structure mbed-os/hal/include/hal/can_helper.h Line 63 in c02e101
|
Jenkins CI Test : ✔️ SUCCESSBuild Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
@Martyx00 correct, it will require rework in the driver itself (contstructor limites also lenght to maximum 8) and assumping the message is predefined to be just 8 bytes in the driver, all HAL implementations use 8 bytes mode only (if they do not, we will catch it via this error). I wonder if this FDCAN1 in HAL makes sense as if a user would something define this, it could lead to similar issues that this PR is fixing - however we will catch it with this fix so overflow should not happen |
I don't know CAN very much... but I thought that ST FD CAN was OK in mbed-os.
In all STM32 newest families, FDCAN IP has replaced CAN IP. |
Not sure if MbedOS officially supports FD CAN. But none of the boards mentioned here: https://os.mbed.com/platforms/?q=&Communication=CAN have FD CAN. I have the Nucleo with the L5 chip somewhere around so I can test if it is possible to use FD CAN with MbedOS in coming days. If it is possible, than there is likely to be some trouble due to the size restriction of the |
Did a quick check in the source code, FD CAN does not seem to be supported. Any limitations of this PR should be taken into account when implementing it in the future. |
@Martyx00 happy with this pull request? |
Yes |
Summary of changes
If HAL implementation writes more than 8 bytes of data, error immediately. CANMessage defines only 8 bytes of data, lenght cannot be > 8.
This fixes #15361
@Martyx00 Please review
Impact of changes
Migration actions required
Documentation
Pull request type
Test results
Reviewers