ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.67k stars 2.98k forks source link

Wrong algorithm for CAN timing settings in can_api.c (STM32G474RE) #15309

Open chdelfs opened 2 years ago

chdelfs commented 2 years ago

Description of defect

Algorithm for calculating the CAN bus timing settings in method int can_frequency(can_t obj, int f) appears to be wrong. It differs from the method applied in _can_init_freq_direct(can_t obj, const can_pinmap_t *pinmap, int hz) which yields - to my understanding - the correct settings.

The main difference is in the file targets/TARGET_STM/can_api.c in line 145 vs line 312: can_init_freq_direct() uses macro IS_FDCAN_NOMINAL_TSEG1 whereas can_frequency() uses macro IS_FDCAN_DATA_TSEG1 for calculation of the nominalPrescaler. Macro IS_FDCAN_DATA_TSEG1 is meaningful only for FDCAN which is not supported by Mbed.

The impact is that the CAN bus timing settings lead to a deviation from the desired CAN transmission speed. Other CAN controllers will not be able to receive CAN messages and - therefore- will not acknowledge CAN transmissions.
This leads to transmission errors: either other parties on the CAN bus send error frames or they do not send an acknowledge and finally this leads to a "bus off" state in the STM32G4 CAN controller.

Target(s) affected by this defect ?

 I faced this issue on NucleoG474RE board, but likely all STM32G4 boards are affected.

Toolchain(s) (name and version) displaying this defect ?

 GCC 10.2.1 arm-none-eabi

What version of Mbed-os are you using (tag or sha) ?

6.16.0

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

mbed-cmake

How is this defect reproduced ?

I can reproduce the issue by:
  1. Use board NucleoG474RE and a second CAN enabled board (in my case LPC4088 with older Mbed, but another Nucleo G474RE board should do the job). Setup CAN connection (requires CAN transceivers).
  2. Create main() with CAN instance for both boards.
  3. Set CAN frequency using method frequency() to 100kHz (default frequency) for one NucleoG474RE board.
  4. Send messages to the other board in a loop.
  5. Check for transmit errors. NucleoG474RE will enter "bus off" due to too many transmission errors.

    I faced the issue in https://forums.mbed.com/t/no-successful-can-bus-sending-with-nucleog474re.

mbedmain commented 2 years ago

@chdelfs thank you for raising this issue.Please take a look at the following comments:

Could you add some more detail to the description? A good description should be at least 25 words. What Mbed OS version are you using? How can we reproduce your issue?

NOTE: If there are fields which are not applicable then please just add 'n/a' or 'None'. This indicates to us that at least all the fields have been considered. Please update the issue header with the missing information.

chdelfs commented 2 years ago

My suggestion is that can_api.c is modified in line 312: Change from: while (!IS_FDCAN_DATA_TSEG1(ntq / nominalPrescaler)) { to: while (!IS_FDCAN_NOMINAL_TSEG1(ntq / nominalPrescaler)) {

After changing the Mbed source as above, I could successfully use CAN bus on NucleoG474RE.

0xc0170 commented 2 years ago

cc @ARMmbed/team-st-mcd

0xc0170 commented 2 years ago

My suggestion is that can_api.c is modified in line 312:

Would you send a pull request addressing this?

chdelfs commented 2 years ago

PR is created: https://github.com/ARMmbed/mbed-os/pull/15317

jeromecoutant commented 2 years ago

Hi

15317 approved

Thx @chdelfs