Keep UBX-NAV-SIG frames longer than the receive buffer

[Raffi1202]

Problem

With a u-blox ZED-F9P on INAV 8.0.0 the Configurator GPS tab shows the Errors counter climbing (342 of 654 messages in the reporter's screenshot) while Sats shows 32, and the CLI shows no satellite information (gpssats is empty). Disabling UBX-NAV-SIG on the receiver makes the errors disappear. Reported in #10941. Fixes #10941.

Cause

src/main/io/gps_ublox.c:900-904 on maintenance-10.x: any frame whose declared payload length exceeds MAX_UBLOX_PAYLOAD_SIZE is dropped, gpsStats.errors is incremented and the parser resyncs. MAX_UBLOX_PAYLOAD_SIZE is UBLOX_MAX_SIGNALS * 16 + 8 = 1032 bytes (src/main/io/gps_ublox.h:35-38), so the buffer holds 64 NAV-SIG signals. A multi-band receiver reports two or three signals per satellite, so NAV-SIG regularly carries more than 64 signals and every such frame is discarded. On receivers with protocol version above 23.01 INAV enables NAV-SIG and disables NAV-SAT (gps_ublox.c:1069-1077), so satelites[] is never filled and gpssats stays empty. The code is identical on maintenance-11.x.

Change

• Oversized UBX-NAV-SIG and UBX-NAV-SAT frames up to UBLOX_MAX_ACCEPTED_PAYLOAD_SIZE (255 * 16 + 8 bytes, the most a U1 record count can declare) are received to the end and checksummed. Only the first MAX_UBLOX_PAYLOAD_SIZE bytes are stored, so records beyond the buffer are dropped and the parser stays byte aligned. The existing handlers already stop at UBLOX_MAX_SIGNALS.
• After the eight-byte header of such a frame the declared length must equal 8 + count * record size; otherwise the frame is counted as an error and the parser resyncs at once, so a corrupt length cannot swallow the following frames.
• Every other oversized message keeps the old drop-and-resync path.
• _payload_length is clamped to MAX_UBLOX_PAYLOAD_SIZE before the frame handlers run.

No buffer size changes; RAM use is unchanged.

Test

Host harness that compiles the real gps_ublox.c and feeds synthetic frames byte by byte into gpsNewFrameUBLOX(), built with ASan/UBSan, run against maintenance-10.x and this branch:

Input	maintenance-10.x	this PR
NAV-SIG 40 / 64 signals + NAV-PVT	0 errors, 40 / 64 slots	same
NAV-SIG 65 / 96 / 255 signals + NAV-PVT	1 error, 0 slots, PVT parsed	0 errors, 64 slots, PVT parsed
10 epochs NAV-PVT + NAV-SIG (96 signals)	10 errors, 10 packets	0 errors, 20 packets
NAV-SAT 100 satellites + NAV-PVT	1 error, 0 slots	0 errors, 64 slots
NAV-SIG length 1544 but numSigs 10 + NAV-PVT	1 error, PVT parsed	1 error, PVT parsed
MON-VER 2000 bytes, NAV-SIG length 0xFFFF, bad checksum	1 error each, next PVT parsed	same
NAV-SIG (96 signals) cut off after 200 payload bytes, then 20 NAV-PVT	1 error, 20/20 PVT parsed	1 error, 6/20 PVT parsed
NAV-SIG (64 signals, fits the buffer) cut off after 200 payload bytes, then 20 NAV-PVT	1 error, 11/20 PVT parsed	same
2000 rounds of random bytes and NAV-SIG/NAV-SAT frames	no sanitizer finding	no sanitizer finding

Known limit, inherent to UBX framing (last two table rows): a frame with a consistent header that loses bytes consumes the following data up to its declared length before the checksum fails. maintenance-10.x already has this for every frame up to 1032 bytes (9 of 20 PVT lost above); for NAV-SIG/NAV-SAT the window grows to at most 4088 bytes (14 of 20 PVT lost above).

The likely source of lost bytes is the UART receive ring: it is 256 bytes (src/main/drivers/serial_uart.h:28-42) and the RX interrupt overwrites it without an overflow check (serial_uart_stm32f7xx.c:264-265, serial_uart_stm32f4xx.c:222-223, serial_uart_at32f43x.c:366-367). The GPS task runs at 50 Hz (src/main/fc/fc_tasks.c:589), so at 230400 baud about 460 bytes arrive between two runs, more than the ring holds; at 115200 it is about 230 bytes.

Bound: 4088 bytes take about 355 ms at 115200 baud (710 ms at 57600), below GPS_TIMEOUT of 1000 ms (src/main/io/gps_private.h:27), so a damaged frame cannot cause a GPS timeout at 57600 baud or faster. #12001 (512-byte GPS receive buffer, lower NAV-SIG rate) reduces the exposure further.

Existing unit tests gps_ublox_unittest, gps_null_port_unittest and gps_heartbeat_unittest pass. Not tested on hardware: a comparison of maintenance-10.x and this PR with a ZED-F9P at 115200 and 230400 baud (PVT rate and gpsStats.errors) is still open.

Flash / RAM

arm-none-eabi-size, maintenance-10.x (3931fcd) vs. this PR:

Target	Flash	RAM
MATEKF722	+64 B (FLASH1 97.54 % -> 97.55 %)	±0
MATEKF405	+176 B	±0
IFLIGHT_BLITZ_ATF435	+176 B	±0

Docs

No documentation change: no setting, CLI command or documented limit changes, and UBLOX_MAX_SIGNALS is a build-time define that is not mentioned under docs/.

[qodo-code-review]

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Preserve oversized UBX-NAV-SIG and NAV-SAT frames

🐞 Bug fix 🕐 20-40 Minutes

AI Description

• Preserve oversized NAV-SIG/NAV-SAT frames by checksumming while truncating buffered signal
 records.
• Reject unrelated or protocol-invalid oversized payloads using existing resynchronization behavior.
• Clamp handler-visible lengths and enforce compile-time receive-buffer sizing safety.

Diagram

graph TD
  Stream["UBX Stream"] --> Header["Header Parser"] --> PayloadCheck{"Oversized?"}
  PayloadCheck -->|"Within limit"| Buffer["Bounded Copy"] --> Checksum["Checksum"] --> Handler["NAV Handler"] --> Signals["Signal List"]
  PayloadCheck -->|"Allowed NAV"| Buffer
  PayloadCheck -->|"Rejected"| Resync["Error Resync"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Increase the fixed signal capacity

• ➕ Requires little parser logic change.
• ➕ Displays more signals before truncation occurs.
• ➖ Cannot cover all 255 protocol-level signal records without substantial static RAM.
• ➖ Increases both receive-buffer and satellite-list memory on constrained targets.
• ➖ Only postpones rather than eliminates oversized-frame handling.

2. Stream signal records directly

• ➕ Could copy selected records without retaining a full payload buffer.
• ➕ Could support filtering or prioritizing constellations during reception.
• ➖ Adds record-aware logic to the generic byte parser.
• ➖ Couples checksum state, partial-record handling, and NAV message semantics.
• ➖ Creates a larger and riskier change than bounded truncation.

Recommendation: The PR's bounded-copy approach is preferable because it preserves the existing parser and static-memory model while keeping frame alignment and checksum validation intact. Raising the fixed limit does not solve the protocol maximum, while record-level streaming adds disproportionate state-machine complexity for diagnostic-only data.

Files changed (2) +37 / -8

Bug fix (2) +37 / -8

gps_ublox.cConsume and truncate oversized NAV signal frames safely +25/-7

Consume and truncate oversized NAV signal frames safely

• Allows oversized NAV-SIG and NAV-SAT payloads up to the protocol-derived maximum to be fully consumed and checksummed while storing only bytes that fit. Clamps the handler-visible payload length and bounds satellite-list processing to the retained signal count, while unsupported oversized messages retain the existing error path.

src/main/io/gps_ublox.c

gps_ublox.hDefine oversized-frame limits and buffer invariants +12/-1

Define oversized-frame limits and buffer invariants

• Documents UBLOX_MAX_SIGNALS as stored signal capacity, adds the maximum accepted NAV payload size, and introduces compile-time assertions for accepted-size and receive-buffer safety. It also corrects the NAV-SIG array comment to reflect configurable capacity.

src/main/io/gps_ublox.h

[qodo-free-for-open-source-projects]

Code Review by Qodo

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

1. Bad navigation frames consume updates ✓ Resolved

Description

gpsNewFrameUBLOX() accepts any oversized navigation signal or satellite length up to 4,088 bytes
without checking that it matches the payload's count field. When a malformed frame ends before its
declared length, the parser treats subsequent UBX frames as payload until that length is reached,
discarding their position and speed updates before checksum failure restores synchronization.

Code

src/main/io/gps_ublox.c[R909-911]

+                const bool truncatable = (_class == CLASS_NAV) &&
+                                         (_msg_id == MSG_NAV_SIG || _msg_id == MSG_NAV_SAT) &&
+                                         (_payload_length <= UBLOX_MAX_ACCEPTED_PAYLOAD_SIZE);

Evidence

The new condition accepts oversized NAV-SIG and NAV-SAT declarations solely by message identity and
the 4,088-byte cap. The payload state then advances exclusively by _payload_counter until the
declared length is consumed and does not inspect embedded UBX preambles; checksum failure can reset
the parser only afterward, while the receiver continuously feeds all available serial bytes through
this state machine.

src/main/io/gps_ublox.c[903-933]
src/main/io/gps_ublox.c[935-960]
src/main/io/gps_ublox.c[1210-1221]
src/main/io/gps_ublox.h[41-49]

Agent prompt

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

## Issue description
Oversized NAV-SIG and NAV-SAT frames are accepted based only on class, message ID, and an upper length bound. A malformed declared length can therefore keep the state machine in payload mode while valid subsequent UBX frames are consumed as payload.
## Fix Focus Areas
- src/main/io/gps_ublox.c[903-933]
- src/main/io/gps_ublox.h[41-49]
## Recommended Fix
After receiving the fixed eight-byte NAV-SIG or NAV-SAT header, derive the expected payload length from `numSigs` or `numSvs` and the corresponding record size. If it differs from the declared payload length, increment the error count and reset the parser so subsequent bytes can be scanned for the next UBX preamble; retain full-frame checksumming and prefix buffering for structurally valid oversized frames.

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

---

Tip of the day

💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[github-actions]

RAM / Flash usage vs. base commit 76ee415 — commit 92b479f

Target	Flash Δ	RAM Δ
MATEKF405 ⚠️	+23284 B (+3.33%)	-12636 B (-8.46%)
MATEKF722 ⚠️	+10404 B (+2.21%)	-13256 B (-10.58%)
MATEKF765 ⚠️	+15868 B (+2.15%)	-11512 B (-6.96%)
MATEKH743 ⚠️	+22692 B (+2.93%)	-10688 B (-6.32%)

See RAM/flash optimization guide for techniques to reduce usage.

[github-actions]

Test firmware build ready — commit 92b479f

Download firmware for PR #11921

251 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]

Moved to milestone 10.1 because it changes how the u-blox parser consumes bytes. Please let us know when it has been tested with different types of real GPS receivers (for example M8, M9/M10 and a multi-band F9P). Thank you!