FDCAN HAL bug in assert-check for number of filters

[nicgreenway]

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

[SofLit]

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).

[nicgreenway]

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.

[SofLit · Accepted solution]

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.