Skip to content

show RSSI dbm as negative number on OSD consistently - #11769

Open
RomanLut wants to merge 2 commits into
iNavFlight:masterfrom
RomanLut:submit_mavlink_rssi
Open

show RSSI dbm as negative number on OSD consistently#11769
RomanLut wants to merge 2 commits into
iNavFlight:masterfrom
RomanLut:submit_mavlink_rssi

Conversation

@RomanLut

@RomanLut RomanLut commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

INAV parses MAVLink RADIO_STATUS messages differently depending on the configured radio type.

In GENERIC mode: rssi field is positive number and is assigned as is to the uplink RSSI indicator.
In ELRS mode: remrssi is poitive number and is negated for the uplink RSSI indicator.

This change negates rssi in generic mode so RSSI is displayed as a negative dBm value, consistently with ELRS and other receivers in INAV.

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

Branch Targeting Suggestion

You've targeted the master branch with this PR. Please consider if a version branch might be more appropriate:

  • maintenance-9.x - If your change is backward-compatible and won't create compatibility issues between INAV firmware and Configurator 9.x versions. This will allow your PR to be included in the next 9.x release.

  • maintenance-10.x - If your change introduces compatibility requirements between firmware and configurator that would break 9.x compatibility. This is for PRs which will be included in INAV 10.x

If master is the correct target for this change, no action is needed.


This is an automated suggestion to help route contributions to the appropriate branch.

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 8, 2026

Copy link
Copy Markdown

PR Summary by Qodo

Show MAVLink RSSI dBm as negative on OSD for GENERIC radio type

🐞 Bug fix 🕐 Less than 10 minutes

Grey Divider

AI Description

• Negate MAVLink RADIO_STATUS.rssi in GENERIC mode to represent dBm correctly.
• Align RSSI sign convention with ELRS and other INAV receiver paths.
• Keep SNR and LQ calculations unchanged while fixing OSD RSSI consistency.
Diagram

graph TD
  A["MAVLink RADIO_STATUS"] --> B["mavlinkParseRxStats()"] --> E{radio_type}
  E -->|"SIK"| F["Scale RSSI"] --> C["rxLinkStatistics.uplinkRSSI"]
  E -->|"ELRS"| G["Negate remrssi"] --> C
  E -->|"GENERIC"| H["Negate rssi"] --> C --> D["OSD RSSI dBm"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Normalize sign at OSD render time
  • ➕ Keeps raw telemetry parsing closer to message fields
  • ➕ Allows per-display formatting choices without affecting other consumers
  • ➖ OSD is not the only consumer of uplinkRSSI (e.g., MSP, logic conditions)
  • ➖ Spreads unit/sign normalization across layers, increasing inconsistency risk
2. Add explicit config/flag for RSSI sign convention
  • ➕ Supports radios/implementations that may already report negative dBm
  • ➕ Avoids assuming all GENERIC senders use positive magnitudes
  • ➖ Adds configuration surface area and user confusion
  • ➖ Harder to validate than a consistent internal convention

Recommendation: Keep the PR’s approach: normalize to negative dBm at the MAVLink parsing boundary so all downstream consumers (OSD, MSP, logic) see a consistent uplinkRSSI convention. Consider adding a sign-convention option only if real-world GENERIC senders are found to already provide signed dBm.

Files changed (1) +1 / -1

Bug fix (1) +1 / -1
mavlink.cNegate GENERIC MAVLink RSSI to match negative dBm convention +1/-1

Negate GENERIC MAVLink RSSI to match negative dBm convention

• Changes MAVLINK_RADIO_GENERIC/default handling to store uplinkRSSI as -msg->rssi instead of msg->rssi. This aligns the displayed and consumed RSSI sign convention with the ELRS MAVLink path while leaving SNR and LQ logic intact.

src/main/telemetry/mavlink.c

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 8, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Invalid RSSI becomes -255 ✓ Resolved 🐞 Bug ≡ Correctness
Description
In mavlinkParseRxStats() GENERIC mode now negates msg->rssi without handling MAVLink’s
invalid/unknown sentinel (UINT8_MAX/255), producing uplinkRSSI = -255. This value is consumed as a
real dBm reading (e.g., by OSD alarm checks), causing false low-RSSI alarms/logic triggers when RSSI
is actually unknown.
Code

src/main/telemetry/mavlink.c[1325]

+            rxLinkStatistics.uplinkRSSI = -msg->rssi;
Evidence
The MAVLink message definition explicitly documents rssi as a uint8_t with UINT8_MAX meaning
invalid/unknown; the PR change negates that field unconditionally, so invalid/unknown becomes -255.
OSD logic reads rxLinkStatistics.uplinkRSSI as an int16_t dBm and triggers blinking when it is
below the configured alarm threshold, so -255 is treated as an extremely low RSSI rather than
‘unknown’.

src/main/telemetry/mavlink.c[1310-1328]
lib/main/MAVLink/common/mavlink_msg_radio_status.h[7-15]
src/main/io/osd.c[2700-2710]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`mavlinkParseRxStats()` in GENERIC mode unconditionally assigns `rxLinkStatistics.uplinkRSSI = -msg->rssi;`. MAVLink defines `rssi == UINT8_MAX (255)` as invalid/unknown, so the new code turns this into `-255`, which downstream code interprets as an extremely poor (but valid) dBm.
### Issue Context
- MAVLink `RADIO_STATUS.rssi` is `uint8_t` with values `[0..254]`, `255` = invalid/unknown.
- The same function already treats `msg->rssi != 255` specially for LQ, indicating the sentinel is expected.
- OSD displays `rxLinkStatistics.uplinkRSSI` directly and compares it to `osdConfig()->rssi_dbm_alarm`, so `-255` can trigger alarms even though RSSI is unknown.
### Fix Focus Areas
- src/main/telemetry/mavlink.c[1318-1328]
### Suggested change
- Only negate when `msg->rssi != UINT8_MAX`; otherwise set `uplinkRSSI` to a defined “unknown” representation that won’t be misinterpreted as a real low dBm (e.g. `0`, or a dedicated sentinel plus consumer handling).
- Make the cast explicit to avoid any ambiguity: `-(int16_t)msg->rssi`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Invalid RSSI becomes -255 ✓ Resolved 🐞 Bug ≡ Correctness
Description
In mavlinkParseRxStats() GENERIC mode now negates msg->rssi without handling MAVLink’s
invalid/unknown sentinel (UINT8_MAX/255), producing uplinkRSSI = -255. This value is consumed as a
real dBm reading (e.g., by OSD alarm checks), causing false low-RSSI alarms/logic triggers when RSSI
is actually unknown.
Code

src/main/telemetry/mavlink.c[1325]

+            rxLinkStatistics.uplinkRSSI = -msg->rssi;
Evidence
The MAVLink message definition explicitly documents rssi as a uint8_t with UINT8_MAX meaning
invalid/unknown; the PR change negates that field unconditionally, so invalid/unknown becomes -255.
OSD logic reads rxLinkStatistics.uplinkRSSI as an int16_t dBm and triggers blinking when it is
below the configured alarm threshold, so -255 is treated as an extremely low RSSI rather than
‘unknown’.

src/main/telemetry/mavlink.c[1310-1328]
lib/main/MAVLink/common/mavlink_msg_radio_status.h[7-15]
src/main/io/osd.c[2700-2710]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`mavlinkParseRxStats()` in GENERIC mode unconditionally assigns `rxLinkStatistics.uplinkRSSI = -msg->rssi;`. MAVLink defines `rssi == UINT8_MAX (255)` as invalid/unknown, so the new code turns this into `-255`, which downstream code interprets as an extremely poor (but valid) dBm.

### Issue Context
- MAVLink `RADIO_STATUS.rssi` is `uint8_t` with values `[0..254]`, `255` = invalid/unknown.
- The same function already treats `msg->rssi != 255` specially for LQ, indicating the sentinel is expected.
- OSD displays `rxLinkStatistics.uplinkRSSI` directly and compares it to `osdConfig()->rssi_dbm_alarm`, so `-255` can trigger alarms even though RSSI is unknown.

### Fix Focus Areas
- src/main/telemetry/mavlink.c[1318-1328]

### Suggested change
- Only negate when `msg->rssi != UINT8_MAX`; otherwise set `uplinkRSSI` to a defined “unknown” representation that won’t be misinterpreted as a real low dBm (e.g. `0`, or a dedicated sentinel plus consumer handling).
- Make the cast explicit to avoid any ambiguity: `-(int16_t)msg->rssi`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread src/main/telemetry/mavlink.c Outdated
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

Test firmware build ready — commit 045c4d3

Download firmware for PR #11769

243 targets built. Find your board's .hex file by name on that page (e.g. MATEKF405SE.hex). Files are individually downloadable — no GitHub login required.

Development build for testing only. Use Full Chip Erase when flashing.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant