Skip to content

Conversation

@henrikbrixandersen
Copy link
Member

Fix the test for sending frames with the CAN_FRAME_ESI flag set. Sending frames with this flag set in software is never allowed.

Fixes: 8023a58

Fix the test for sending frames with the CAN_FRAME_ESI flag set. Sending
frames with this flag set in software is never allowed.

Fixes: 8023a58

Signed-off-by: Henrik Brix Andersen <[email protected]>
@henrikbrixandersen henrikbrixandersen added the Hotfix Fix for issues blocking development, i.e. upstream CI issues, tests failing in upstream CI , etc. label Oct 24, 2024
@henrikbrixandersen henrikbrixandersen requested a review from a team October 24, 2024 14:15
@henrikbrixandersen
Copy link
Member Author

The test introduced in 8023a58 is wrong and holding back both local pre-v4.0.0-rc1 testing and PRs targeting v4.0.0-rc1.

@carlescufi carlescufi merged commit 1e83368 into zephyrproject-rtos:main Oct 24, 2024
22 checks passed
@nordic-segl
Copy link
Contributor

The test introduced in 8023a58 is wrong and holding back both local pre-v4.0.0-rc1 testing and PRs targeting v4.0.0-rc1.

In my opinion, test introduced in 8023a58 was correct.
It was checking that "Sending frames with this flag set in software is never allowed."
PR #76134 was failing this test.
Wrong was that I and @duynguyenxa thought that sending CAN_FRAME_ESI | CAN_FRAME_FDF was correct.
Thus, I think both test variants should be there.

@henrikbrixandersen : Could You comment on whether there are any errors in my reasoning? [low priority]

* @brief Test error when CAN FD Error State Indicator (ESI) is set on transmit frame.
*
* CAN FD Error State Indicator (ESI) indicates that the transmitting node is
* in error-passive state. Only valid in combination with CAN_FRAME_FDF.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was copied from the CAN API documentation at
https://docs.zephyrproject.org/latest/doxygen/html/group__can__interface.html#ga83f8b7af6eb45e43aaac355de3e69e52
Is it worth to make changes there too?

@henrikbrixandersen henrikbrixandersen deleted the can_tests_api_fix_totally_misunderstood_esi_flag_test branch October 28, 2024 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: CAN Hotfix Fix for issues blocking development, i.e. upstream CI issues, tests failing in upstream CI , etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants