Skip to main content
N
nicgreenway
Visitor II
August 27, 2024
Solved

FDCAN HAL bug in assert-check for number of filters

  • August 27, 2024
  • 2 replies
  • 1155 views

Hi All - minor issue I noticed earlier on a Nucleo H563ZI board when configuring the FDCAN filters.

With asserts enabled, if I set the allowed maximum filters to a non-zero number and try to set a filter on a higher filter index, it raises an assert as expected.  But if the allowed number of filters is 0, it does not.

I believe this is because the assert is implemented at line 1332 in stm32h5xx_hal_fdcan.c as:

 

 

assert_param(IS_FDCAN_MAX_VALUE(sFilterConfig->FilterIndex, (hfdcan->Init.ExtFiltersNbr - 1U)));

 

 

Init.ExtFiltersNbr is an unsigned integer, so (Init.ExtFiltersNbr - 1) as passed to the IS_FDCAN_MAX_VALUE test will be wrapped-around to a large number?

This may be better implemented as:

 

 

assert_param(IS_FDCAN_MAX_VALUE((sFilterConfig->FilterIndex + 1U), hfdcan->Init.ExtFiltersNbr ));

 

 

Nic

      This topic has been closed for replies.
      Best answer by mƎALLEm

      Hello,

      Received an internal feedback regarding your original request:

      1- Adding +1 to the left member of the comparison is indeed the simplest solution instead of removing one to the right member and 

      assert_param(IS_FDCAN_MAX_VALUE(sFilterConfig->FilterIndex, (hfdcan->Init.ExtFiltersNbr - 1)));

      will be replaced by:

      assert_param(IS_FDCAN_MAX_VALUE((sFilterConfig->FilterIndex + 1U), hfdcan->Init.ExtFiltersNbr ));

      2- Internal team do not agree with your last comment i.e. that we need to directly look at the register instead of the structure, because of possible desynchro.
      This is not possible: Calling FDCAN_init will call directly FDCAN_CalcultateRamBlockAddresses(), which will update RXGFC register with both StdFiltersNbr and ExtFiltersNbr, so theses variables will be in sync with the register.
      If the user wants to change theses parameters (that are in the init struct), he will obviously need to redo the init.

      2 replies

      M
      mƎALLEm
      Technical Moderator
      August 27, 2024

      Hello, 

      To me it's a bug but need to check that internally. I'll get back to you as soon as I have an answer.

      Internal ticket number 189560 (not accessible by community users).

      N
      nicgreenwayAuthor
      Visitor II
      August 28, 2024

      It would actually be better if the assert could check the appropriate bits from the FDCAN register (RXGFC[LSE[3:0]]), since if hfdcan->Init.ExtFiltersNbr is accidentally set after running HAL_FDCAN_Init then the value is not copied across: it may appear correct in ExtFiltersNbr but be incorrect in the register, in which case the filter appears to be applied correctly, but will not actually be run.

      M
      mƎALLEmAnswer
      Technical Moderator
      September 20, 2024

      Hello,

      Received an internal feedback regarding your original request:

      1- Adding +1 to the left member of the comparison is indeed the simplest solution instead of removing one to the right member and 

      assert_param(IS_FDCAN_MAX_VALUE(sFilterConfig->FilterIndex, (hfdcan->Init.ExtFiltersNbr - 1)));

      will be replaced by:

      assert_param(IS_FDCAN_MAX_VALUE((sFilterConfig->FilterIndex + 1U), hfdcan->Init.ExtFiltersNbr ));

      2- Internal team do not agree with your last comment i.e. that we need to directly look at the register instead of the structure, because of possible desynchro.
      This is not possible: Calling FDCAN_init will call directly FDCAN_CalcultateRamBlockAddresses(), which will update RXGFC register with both StdFiltersNbr and ExtFiltersNbr, so theses variables will be in sync with the register.
      If the user wants to change theses parameters (that are in the init struct), he will obviously need to redo the init.

      Terms & ConditionsAccessibility statement