Skip to content

Don't use unsigned integer type for negative SNR value#8432

Merged
thebentern merged 5 commits into
meshtastic:masterfrom
korbinianbauer:develop
Oct 23, 2025
Merged

Don't use unsigned integer type for negative SNR value#8432
thebentern merged 5 commits into
meshtastic:masterfrom
korbinianbauer:develop

Conversation

@korbinianbauer

@korbinianbauer korbinianbauer commented Oct 23, 2025

Copy link
Copy Markdown
Contributor

RadioInterface::getCWsize(float snr) uses an unsigned integer type (uint32_t) to store a negative number, causing an integer overflow.

On 32-bit systems this is later undone when stuffing the value back into a signed 32-bit long.

On systems with 64-bit longs however, this will result in map() always returning the maximum CW size of 8 and no preference for nodes with low SNR.

Fixes #8430

🤝 Attestations

  • I have tested that my proposed changes behave as described.
  • I have tested that my proposed changes do not cause any obvious regressions on the following devices:
    • Heltec (Lora32) V3
    • LilyGo T-Deck
    • LilyGo T-Beam
    • RAK WisBlock 4631
    • Seeed Studio T-1000E tracker card
    • Other (please specify below)

Only tested code-snippet in isolation:
Before: https://www.onlinegdb.com/zTwCkEfEs
After: https://onlinegdb.com/qe4RSOjAqO

ford-jones and others added 4 commits October 22, 2025 20:08
* Include RSSI in rangetest csv

* Fix typo

* Preserve csv column order

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
SNR-based contention windows are broken on systems with 64-bit long integers.
Fixes #8430
@CLAassistant

CLAassistant commented Oct 23, 2025

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@thebentern thebentern changed the base branch from develop to master October 23, 2025 16:34
@thebentern

Copy link
Copy Markdown
Contributor

Retargeted master. Looks like it's pulling other commits from develop, but those were some I was going to cherry pick anyway

@thebentern thebentern added the bugfix Pull request that fixes bugs label Oct 23, 2025
@thebentern thebentern merged commit 585d9d3 into meshtastic:master Oct 23, 2025
73 of 74 checks passed
jeek pushed a commit to jeek/Meshtastic-Exploiteers-Hacker-Pager that referenced this pull request Jun 30, 2026
Don't use unsigned integer type for negative SNR value
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Pull request that fixes bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: unsigned integer = -20 - SNR based contention window can't work (?)

5 participants