STMicroelectronics / STM32CubeG4

STM32Cube MCU Full Package for the STM32G4 series - (HAL + LL Drivers, CMSIS Core, CMSIS Device, MW libraries plus a set of Projects running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits))
Other
182 stars 98 forks source link

FDCAN RxHeader member IsFilterMatchingFrame and FilterIndex lacking documentation #18

Closed willtoth closed 1 year ago

willtoth commented 3 years ago

Description

The FDCAN_RxHeaderTypeDef member IsFilterMatchingFrame and FilterIndex naming and documentation are confusing and do not convey the behavior when a non matching frame is received.

  uint32_t FilterIndex;           /*!< Specifies the index of matching Rx acceptance filter element.
                                       This parameter must be a number between:
                                        - 0 and (SRAMCAN_FLS_NBR-1), if IdType is FDCAN_STANDARD_ID
                                        - 0 and (SRAMCAN_FLE_NBR-1), if IdType is FDCAN_EXTENDED_ID */

  uint32_t IsFilterMatchingFrame; /*!< Specifies whether the accepted frame did not match any Rx filter.
                                         Acceptance of non-matching frames may be enabled via
                                         HAL_FDCAN_ConfigGlobalFilter().
                                         This parameter can be 0 or 1                                    */

https://github.com/STMicroelectronics/STM32CubeG4/blob/master/Drivers/STM32G4xx_HAL_Driver/Inc/stm32g4xx_hal_fdcan.h#L231-L239

Expected Behavior

Based on the description of these two elements, I would expect FilterIndex to always be between 0 and (SRAMCAN_FLE_NBR-1) for extended frames (i.e. between 0 and 8).

The naming of IsFilterMatchingFrame would make be think that a value of 0 means that the frame did not match a filter. Reading the documentation for this element is a bit ambiguous (the description is opposite of the variable name).

Actual Behavior

When a filter is matched, FilterIndex is set to the filter number as expected, but IsFilterMatchingFrame is set to 0.

When a filter is not matched, FilterIndex is set to 63, which is not documented here, and IsFilterMatchingFrame is set to 1.

Fix

It is probably well too late to change the name or value of IsFilterMatchingFrame, so updating the documentation to be more clear would be the only fix. Something like:

  uint32_t FilterIndex;           /*!< Specifies the index of matching Rx acceptance filter element.
                                       This parameter must be a number between:
                                        - 0 and (SRAMCAN_FLS_NBR-1), if IdType is FDCAN_STANDARD_ID
                                        - 0 and (SRAMCAN_FLE_NBR-1), if IdType is FDCAN_EXTENDED_ID 

                                       When the frame is a Non-Filter matching frame, the value will be <xx>*/

  uint32_t IsFilterMatchingFrame; /*!< Specifies whether the accepted frame did not match any Rx filter.
                                         Acceptance of non-matching frames may be enabled via
                                         HAL_FDCAN_ConfigGlobalFilter().
                                         This parameter will be 0 if the frame matched an Rx filter or 1 if it did not match any Rx filter                                   */
ASELSTM commented 3 years ago

Hi @willtoth,

Thank you for this other report. It has been forwarded to our development teams. We will get back to you as soon as they provide us with their feedback.

Best regards,

ASELSTM commented 3 years ago

Hi @willtoth,

Sorry for the delay in coming back to you. You request has been approved by our development teams.

Regarding the FilterIndex our development team has cross-checked the possible values indicated in the reference manual and confirmed that when the received frame did not match any Rx filter element the value is actually invalid. Moreover, it is a RAM zone (Message RAM area of the FDCAN) so this field can have any value not only 63.

The description will be then updated as follows :

  uint32_t FilterIndex;           /*!< Specifies the index of matching Rx acceptance filter element.
                                       This parameter must be a number between:
                                        - 0 and (SRAMCAN_FLS_NBR-1), if IdType is FDCAN_STANDARD_ID
                                        - 0 and (SRAMCAN_FLE_NBR-1), if IdType is FDCAN_EXTENDED_ID 
+                                      When the frame is a Non-Filter matching frame, the value is invalid.  */

  uint32_t IsFilterMatchingFrame; /*!< Specifies whether the accepted frame did not match any Rx filter.
                                         Acceptance of non-matching frames may be enabled via
                                         HAL_FDCAN_ConfigGlobalFilter().
                                         This parameter can be 0 or 1                                   
+                                        This parameter takes 0 if the frame matched an Rx filter or 1 if it did not match any Rx filter */

This fix will be implemented and made available in the frame of a future release. Thank you once more for your contribution as well as for the effort and time you put into reporting this concern.

With regards,

ASELSTM commented 3 years ago

ST Internal Reference: 121878

RJMSTM commented 1 year ago

Fixed in commit c4132af65fe74ddd7f54aced85a0f4acce736405