Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpu/cc2538: mask length byte before checking CRC [backport 2024.10] #21038

Merged

Conversation

maribu
Copy link
Member

@maribu maribu commented Nov 25, 2024

Backport of #20998

Contribution description

This is a follow-up for #20956.

I found that the solution applied there wasn’t robust, because when receiving short packets quickly after each other the RX FIFO might be filling up with a new packet while handling the IRQ. Then, the number of bytes in the FIFO will be bigger than the packet length of the first packet. Even though the second packet will be dropped anyway, the length of the first packet can still be correctly obtained from the length byte in the packet.

Namely, from section 23.9.5.1 of the CC2538 User’s Guide it follows that indeed the radio will mask out bit 7 of the length byte:
image

Then, it continues to fill the RX FIFO with this number of bytes as follows from section 23.9.3:
image

This means that with this approach, always the correct byte is used for checking the CRC result.

Testing procedure

Flashed this PR on two devices, and tested with long packets that the error from #20955 didn't appear. When testing short packets quickly after each other, in case the old crc_loc was not equal to the corrected pkt_len, the pkt_len had the expected number of bytes, while crc_loc was usually a couple bytes more than that.

Issues/PRs references

This is a more proper fix for #20955.

@github-actions github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels Nov 25, 2024
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 25, 2024
@maribu maribu enabled auto-merge November 25, 2024 12:21
@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 25, 2024
@riot-ci
Copy link

riot-ci commented Nov 25, 2024

Murdock results

✔️ PASSED

1278388 cpu/cc2538: mask length byte before checking CRC

Success Failures Total Runtime
10214 0 10215 18m:46s

Artifacts

@maribu maribu added this pull request to the merge queue Nov 25, 2024
Merged via the queue into RIOT-OS:2024.10-branch with commit 1b61216 Nov 26, 2024
29 checks passed
@benpicco benpicco added this to the Release 2024.10 milestone Nov 27, 2024
@benpicco benpicco added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Nov 28, 2024
@maribu maribu deleted the backport/2024.10/cc2538_crc_bit branch December 15, 2024 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants