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-37), 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 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.

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 count can declare) are now received to the end and checksummed; only the first MAX_UBLOX_PAYLOAD_SIZE bytes are stored, so signals beyond UBLOX_MAX_SIGNALS are dropped and the parser stays byte aligned. After the eight-byte header the declared length is compared with 8 + count * record size; a mismatch is counted as an error and resyncs, and 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 and the NAV-SIG handler uses the clamped count for both of its loops. Two STATIC_ASSERTs pin UBLOX_MAX_ACCEPTED_PAYLOAD_SIZE >= MAX_UBLOX_PAYLOAD_SIZE and sizeof(ubx_nav_sig) <= UBLOX_BUFFER_SIZE.

Test

Not run on hardware or SITL. Cause verified by reading src/main/io/gps_ublox.c:900-904 and the message setup at gps_ublox.c:1069-1077 on maintenance-10.x; The Qodo finding on the first commit (a short frame with an oversized declared length would swallow the following frames) is addressed by the header length check in ad33673. The existing navSigStructureSizes unit test (src/test/unit/gps_ublox_unittest.cc:92) only checks struct sizes and is unchanged.

Flash / RAM

Not measured yet. The upstream firmware CI has not been released for this PR, so no size report exists.

Docs

No documentation change needed: 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 🐞 Bug ☼ Reliability

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 switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗