Skip to content

fix(GPS): correct GPS_RTCM_DATA fragmentation for MAVLink reassembly - #14772

Merged
DonLakeFlyer merged 3 commits into
mavlink:masterfrom
dakejahl:fix/gps-rtcm-data-fragmentation
Aug 8, 2026
Merged

fix(GPS): correct GPS_RTCM_DATA fragmentation for MAVLink reassembly#14772
DonLakeFlyer merged 3 commits into
mavlink:masterfrom
dakejahl:fix/gps-rtcm-data-fragmentation

Conversation

@dakejahl

@dakejahl dakejahl commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Make QGC's GPS_RTCM_DATA packing match the MAVLink reassembly contract used by ArduPilot and PX4.

Problem

RTCMMavlink treated payloads of size 180 as fragmented without a completion marker, omitted the required zero-length terminator for exact multiples of 180 under four fragments, and could emit fragment IDs beyond 0–3 for RTCM frames larger than 720 bytes. ArduPilot discards incomplete sequences; PX4 carries an explicit compatibility workaround for older QGC senders.

Solution

  • Pack ≤180 bytes as a single unfragmented message
  • Fragment 181–720 bytes and append a zero-length terminator when length is an exact multiple of 180 with fewer than four fragments
  • Stream >720 byte payloads as successive unfragmented chunks (protocol max reassembled size is 4×180)
  • Emit one validated RTCM frame per UDP signal so each frame gets its own sequence
  • Remove the now-unused RTCMParser::extractValidFrames — concatenating frames back into one blob is the pattern that broke sequencing
  • Cover packing rules (including all fragment-count/terminator boundaries) and UDP per-frame emission with unit tests

Match MAVLink/ArduPilot/PX4 rules: unfragmented packets up to 180 bytes,
zero-length terminator for exact 180 multiples under 4 fragments, and
stream oversized (>720) payloads as unfragmented chunks instead of
overflowing the 2-bit fragment ID. Emit one UDP-validated RTCM frame per
sequence.
@dakejahl
dakejahl requested a review from HTRamsey as a code owner August 7, 2026 02:23
@dakejahl
dakejahl marked this pull request as draft August 7, 2026 02:25
RTCMUdpInput now emits one CRC-valid frame per signal instead of
concatenating frames, which left extractValidFrames without callers.
Concatenated multi-frame output is exactly the shape that broke
GPS_RTCM_DATA sequencing, so drop it rather than leave it around.
Add RTCMUdpInputTest: raw passthrough with validation off, one signal
per CRC-valid frame, bad-CRC frames dropped mid-stream, and parser
state carried across split datagrams. RTCMUdpInput now resolves port 0
to the actual bound port so tests can bind ephemerally without racing
for a fixed port.

Also cover the 541..719 pack() range (four fragments with a non-full
tail, no terminator).
@github-actions github-actions Bot added size/M and removed size/S labels Aug 7, 2026
@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 53.75000% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.72%. Comparing base (f29efd3) to head (20a96c1).
⚠️ Report is 245 commits behind head on master.

Files with missing lines Patch % Lines
src/GPS/RTCM/RTCMMavlink.cc 53.84% 18 Missing and 12 partials ⚠️
src/GPS/RTCM/RTCMUdpInput.cc 53.33% 1 Missing and 6 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14772      +/-   ##
==========================================
+ Coverage   25.47%   32.72%   +7.25%     
==========================================
  Files         769      784      +15     
  Lines       65912    67602    +1690     
  Branches    30495    31311     +816     
==========================================
+ Hits        16788    22120    +5332     
+ Misses      37285    30596    -6689     
- Partials    11839    14886    +3047     
Flag Coverage Δ
unittests 32.72% <53.75%> (+7.25%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/GPS/RTCM/RTCMMavlink.h 0.00% <ø> (ø)
src/GPS/RTCM/RTCMParser.cc 85.29% <ø> (ø)
src/GPS/RTCM/RTCMParser.h 66.66% <ø> (ø)
src/GPS/RTCM/RTCMUdpInput.h 100.00% <ø> (ø)
src/GPS/RTCM/RTCMUdpInput.cc 48.68% <53.33%> (ø)
src/GPS/RTCM/RTCMMavlink.cc 36.89% <53.84%> (ø)

... and 471 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9faca48...20a96c1. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Build Results

Platform Status

Platform Status Details
Linux Passed View
Windows Passed View
MacOS Passed View
Android Passed View

All builds passed.

Pre-commit

Check Status Details
pre-commit Failed (non-blocking) View

Pre-commit hooks: 0 passed, 0 failed, 0 skipped.

Test Results

linux-coverage-integration: 37 passed, 0 skipped
linux-coverage-unit: 119 passed, 0 skipped
linux-sanitizers-integration: 20 passed, 0 skipped
linux-sanitizers-unit: 119 passed, 0 skipped
Total: 295 passed, 0 skipped

Code Coverage

Coverage Baseline Change
69.5% 69.4% +0.1%

Artifact Sizes

Artifact Size Δ from master
QGroundControl 221.81 MB -4.38 MB (decrease)
QGroundControl-aarch64 179.35 MB +0.00 MB (increase)
QGroundControl-installer-AMD64 137.86 MB -0.44 MB (decrease)
QGroundControl-installer-AMD64-ARM64 80.36 MB -0.22 MB (decrease)
QGroundControl-installer-ARM64 109.12 MB +0.01 MB (increase)
QGroundControl-linux 87.04 MB -78.73 MB (decrease)
QGroundControl-mac 87.04 MB +0.00 MB (increase)
QGroundControl-x86_64 191.59 MB +0.01 MB (increase)
Total size decreased by 83.75 MB

Updated: 2026-08-07 03:26:44 UTC • Commit: 20a96c1 • Triggered by: Android

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Corrects RTCM-to-MAVLink packetization to satisfy ArduPilot and PX4 reassembly requirements.

Changes:

  • Handles fragmentation boundaries, terminators, and oversized frames correctly.
  • Emits validated UDP RTCM frames individually.
  • Adds comprehensive packetization and UDP tests.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/GPS/RTCM/RTCMMavlink.h Defines packetization API and limits.
src/GPS/RTCM/RTCMMavlink.cc Implements compliant fragmentation and streaming.
src/GPS/RTCM/RTCMUdpInput.h Documents per-frame validated emission.
src/GPS/RTCM/RTCMUdpInput.cc Emits each valid RTCM frame separately.
src/GPS/RTCM/RTCMParser.h Removes obsolete extraction API.
src/GPS/RTCM/RTCMParser.cc Removes concatenating extraction implementation.
test/GPS/RTCMMavlinkTest.h Declares packetization tests.
test/GPS/RTCMMavlinkTest.cc Tests fragmentation boundaries and flags.
test/GPS/RTCMUdpInputTest.h Declares UDP input tests.
test/GPS/RTCMUdpInputTest.cc Tests validation and per-frame emission.
test/GPS/RTCMParserTest.h Removes obsolete test declarations.
test/GPS/RTCMParserTest.cc Removes extraction tests.
test/GPS/CMakeLists.txt Registers the new unit tests.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@dakejahl
dakejahl marked this pull request as ready for review August 8, 2026 03:02
@DonLakeFlyer DonLakeFlyer added this to the Release V5.1 milestone Aug 8, 2026
@DonLakeFlyer
DonLakeFlyer merged commit 74c3b8e into mavlink:master Aug 8, 2026
45 of 48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants