Skip to content

cpu/cc2538: fix RSSI computation#22464

Open
maribu wants to merge 7 commits into
RIOT-OS:masterfrom
maribu:cpu/cc2538/rssi-fix
Open

cpu/cc2538: fix RSSI computation#22464
maribu wants to merge 7 commits into
RIOT-OS:masterfrom
maribu:cpu/cc2538/rssi-fix

Conversation

@maribu

@maribu maribu commented Jul 11, 2026

Copy link
Copy Markdown
Member

Contribution description

  • use int instead of int8_t to avoid loosing precision
  • the offset is already negative, so add it instead of subtracting it.

Testing procedure

One could send to an CC2538 board with another board next to it using different TX power settings and check if the RSSI is reasonable.

Issues/PRs references

Split out of #22463

Declaration of AI-Tools / LLMs usage:

AI-Tools / LLMs that were used are:

  • none

- use `int` instead of `int8_t` to avoid loosing precision
- the offset is already negative, so add it instead of subtracting it.
@github-actions github-actions Bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels Jul 11, 2026
@maribu maribu requested a review from jia200x July 11, 2026 08:59
@crasbe crasbe added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 11, 2026
@riot-ci

riot-ci commented Jul 11, 2026

Copy link
Copy Markdown

Murdock results

✔️ PASSED

390554d Update cpu/cc2538/radio/cc2538_rf_radio_ops.c

Success Failures Total Runtime
11128 0 11128 11m:31s

Artifacts

@AnnsAnns AnnsAnns added the AI: None Stated AI was not (stated to be) used in this PR/Issue label Jul 13, 2026

@AnnsAnns AnnsAnns left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One could send to an CC2538 board with another board next to it using different TX power settings and check if the RSSI is reasonable.

This sounds like a "I will simply trust maribu on this one" situation 😛

@AnnsAnns AnnsAnns added this pull request to the merge queue Jul 13, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 13, 2026
@maribu

maribu commented Jul 13, 2026

Copy link
Copy Markdown
Member Author

This sounds like a "I will simply trust maribu on this one" situation 😛

Hmm, we might want to actually test this. I just guessed the change from the code context. (So basically did Vibe-code this change, just with the guessing done by me rather than a GPU.)

The code in master does however indeed not only look fishy to the LLVM's integer overflow checker, but also to me.

@AnnsAnns

Copy link
Copy Markdown
Member

This sounds like a "I will simply trust maribu on this one" situation 😛

Hmm, we might want to actually test this. I just guessed the change from the code context. (So basically did Vibe-code this change, just with the guessing done by me rather than a GPU.)

The code in master does however indeed not only look fishy to the LLVM's integer overflow checker, but also to me.

Agentic vibe based merging, Microsoft better pay me one trillion for that

@kfessel kfessel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unobfuscate please

Comment thread cpu/cc2538/radio/cc2538_rf_radio_ops.c Outdated
Comment thread cpu/cc2538/radio/cc2538_rf_radio_ops.c Outdated
Co-authored-by: Karl Fessel <karl.fessel@gmail.com>
@kfessel

kfessel commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

int8_t rssi_val is read from the device
‎IEEE802154_RADIO_RSSI_OFFSET is (-174)
CC2538_RSSI_OFFSET is (-73)
->
const int hw_rssi_min = −247

so if the signed 8 bit value ( rage [-128 127] is bigger than −247)
its a rssi is a valid value
else
rssi is 0

is that correct?

might that register be unsigned?

@kfessel kfessel dismissed their stale review July 15, 2026 16:53

this is addressed

Comment thread cpu/cc2538/radio/cc2538_rf_radio_ops.c Outdated

@kfessel kfessel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unobfuscated code made flawes visible

see the suggestion in other comment - thread

Co-authored-by: Karl Fessel <karl.fessel@gmail.com>
Comment thread cpu/cc2538/radio/cc2538_rf_radio_ops.c Outdated
@kfessel kfessel dismissed their stale review July 16, 2026 21:02

addressed

@kfessel

kfessel commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

squash is missing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: None Stated AI was not (stated to be) used in this PR/Issue 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.

5 participants