Skip to content

Add extra GPS satellite statistics OSD widget#11371

Open
tipoman9 wants to merge 3 commits intoiNavFlight:maintenance-10.xfrom
tipoman9:OSD_GPS_Extra_Stats_v2
Open

Add extra GPS satellite statistics OSD widget#11371
tipoman9 wants to merge 3 commits intoiNavFlight:maintenance-10.xfrom
tipoman9:OSD_GPS_Extra_Stats_v2

Conversation

@tipoman9
Copy link
Copy Markdown

Provide a lightweight periodic non-ACK UBX poll for MON RF (UBX class 0x0A id 0x38), only when the OSD widget is used, that does not interfere with configuration ACK handling.
Extract useful RF diagnostics (noisePerMS) and condensed per-constellation satellite statistics directly on OSD for quick in field diagnostics.
The satellite grouping counts and "good" definition follow UBX NAV signal semantics (good when quality == UBLOX_SIG_QUALITY_CODE_LOCK_TIME_SYNC).

Some consumer-grade GPS units support only 3 out of 4 constellations, so the satellite statistics help identify which constellation provides better coverage at a given location.
Additionally, the noise level metric can help detect potential interference between the VTx or telemetry Rx antenna and the GPS antenna, allowing the pilot to take corrective actions such as adjusting transmission power or increasing antenna separation.

The is how the widget represents the extra statistics:

image

10 GPS satellites visible, >4 locked.
5 Glonass sats visible, 3 locked

image

8 GPS sats visible, 2 locked.
5 Glonass sats visible, 3 locked

A small change in the configurator is needed, the corresponding PR is created iNavFlight/inav-configurator#2576

@github-actions
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.

@tipoman9
Copy link
Copy Markdown
Author

tipoman9 commented Mar 4, 2026

The code has been updated to fix the failing check.
Could a maintainer please approve the workflow run?
Thanks!

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 7, 2026

Test firmware build ready — commit 8a83048

Download firmware for PR #11371

223 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.

@sensei-hacker sensei-hacker changed the base branch from master to maintenance-9.x March 7, 2026 19:08
@sensei-hacker sensei-hacker changed the base branch from maintenance-9.x to maintenance-10.x March 7, 2026 19:16
@sensei-hacker
Copy link
Copy Markdown
Member

sensei-hacker commented Apr 12, 2026

Thanks for this PR — the RF diagnostics feature looks genuinely useful.
A few things worth checking before merge that were flagged by a tool I used to reference this code against the Ublox documentation:

noisePerMS — is only the low byte being read?

According to the u-blox M9 Interface Description (UBX-21022436, §3.14.10 UBX-MON-RF, p. 131), the per-block layout (stride 24 bytes) has noisePerMS as a U2 field at byte offset 16 + n·24 — for block 0 that's absolute payload offset 0x10. The PR reads _buffer.bytes[0x10] (one byte), which would silently truncate any noise value whose raw count exceeds 255. Is this intentional (e.g., values in practice fit in a byte), or should the full U2 be read?

// If reading both bytes:
monRfNoisePerMs = (uint16_t)_buffer.bytes[0x10] | ((uint16_t)_buffer.bytes[0x11] << 8);
// bounds check would then need: _payload_length > 0x11

(Per the same spec, cwSuppression at offset 20 + n·24 = 0x14 for block 0 is a U1, so the single-byte read there is correct.)

There is also a typo in the variable name: monCWSuppresion is missing one s - might should be monCWSSuppression or similar.


Polling and ACK state interleaving

The placement of pollMonRf() after ptSpawn(gpsConfigure) returns looks correct — it only fires in steady state. One edge case to check: could the 1 Hz MON-RF poll fire in the same scheduler tick as the periodic pollGnssCapabilities() call? If _ack_state is UBX_ACK_WAITING at that moment, the extra bytes injected into the serial stream could interleave with a pending MON-GNSS response. A guard like the following might be worth adding:

if (gpsState.hwVersion > UBX_HW_VERSION_UBLOX8 && osdMonRfWidgetEnabled
    && _ack_state != UBX_ACK_WAITING
    && (millis() - lastMonRfMs) > 1000) {

OSD widget enable — is there a disable path?

gpsSetOsdMonRfWidgetEnabled(true) is called each draw cycle when the element is rendered, but is there a path that calls it with false? If a user removes the widget from their layout, would MON-RF polling continue indefinitely? This may be intentional (always poll on M9+ in steady state), but if so, the flag and function name could be simplified.


volatile on osdMonRfWidgetEnabled

Is the volatile qualifier on osdMonRfWidgetEnabled intentional? In INAV's cooperative threading model the OSD draw function (writer) and the GPS protothread (reader) both run on the main loop — there's no ISR or preemptive concurrency. The qualifier doesn't cause harm, but it may imply ISR-safety concerns to future readers where none exist.


#ifdef USE_GPS_PROTO_UBLOX guard in osd.c

The OSD_GPS_EXTRA_STATS case calls gpsGetUbloxSatelite() and gpsSetOsdMonRfWidgetEnabled(), which are defined inside #if defined(USE_GPS) && defined(USE_GPS_PROTO_UBLOX) in gps_ublox.c. The CI build passed all 223 targets, so this may already be handled, but it would be worth confirming the element is guarded for builds where GPS is enabled but uBlox protocol is not.


These are mostly questions rather than blocking issues. The protothread placement of the poll and the M9+ version gate both look correct.

@tipoman9 tipoman9 force-pushed the OSD_GPS_Extra_Stats_v2 branch from 1a62359 to a6e4952 Compare April 14, 2026 23:14
@tipoman9
Copy link
Copy Markdown
Author

tipoman9 commented Apr 14, 2026

I moved the new pollMonRf() after the current pollGnssCapabilities() and made them never execute on the same cycle.
Converted noisePerMS from uint8_t to uint16_t and decode it as a little-endian U2, added AutoGainControl field parsing and a new gpsGetMonAGCPercent() metrics to be extracted.
Some CLI changes for better diagnostics also included.
Don't think OSD widget disable path is needed. This would happen only if the users removes the widged from the config screen and then the FC will restart after the configurator exits.

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.

2 participants