NTRIP: refactor manager into composable transport + support classes#14557
Open
HTRamsey wants to merge 1 commit into
Open
NTRIP: refactor manager into composable transport + support classes#14557HTRamsey wants to merge 1 commit into
HTRamsey wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (21.75%) is below the target coverage (30.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #14557 +/- ##
==========================================
+ Coverage 25.47% 30.09% +4.62%
==========================================
Files 769 787 +18
Lines 65912 67372 +1460
Branches 30495 31225 +730
==========================================
+ Hits 16788 20273 +3485
+ Misses 37285 33341 -3944
- Partials 11839 13758 +1919
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 376 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
HTRamsey
force-pushed
the
feature/ntrip-improvements
branch
from
61b982c to
a6cbe1a
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR restructures QGroundControl’s NTRIP/RTK correction pipeline into a more modular design: an event-driven NTRIPManager state machine with a pluggable NTRIPTransport, plus focused support classes (GGA provider, connection stats, source-table controller) and new shared utilities (RTCM parsing, NMEA helpers, data-rate tracking). It also adds/updates QML UI pieces and Settings to expose connection/corrections status and configuration options, with expanded unit coverage.
Changes:
- Refactors NTRIP into composable components (
NTRIPManager,NTRIPTransportConfig,NTRIPGgaProvider,NTRIPConnectionStats,NTRIPSourceTableController) and typed errors (NTRIPError). - Adds shared utilities for RTCM framing/CRC (
RTCMParser), NMEA checksum/GGA (NMEAUtils), bandwidth tracking (DataRateTracker), and UDP forwarding (UdpForwarder), and wires them into RTCM/NTRIP flows. - Introduces new/updated QML controls and an NTRIP QML module for connection/corrections status + settings UI updates; adds/extends unit tests accordingly.
Reviewed changes
Copilot reviewed 75 out of 76 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Utilities/DataRateTrackerTest.h | Adds unit-test declarations for DataRateTracker. |
| test/Utilities/DataRateTrackerTest.cc | Implements DataRateTracker unit tests (accumulation/reset/conversion). |
| test/Utilities/CMakeLists.txt | Registers DataRateTrackerTest in Utilities tests. |
| test/GPS/UdpForwarderTest.h | Adds unit-test declarations for UdpForwarder. |
| test/GPS/UdpForwarderTest.cc | Implements UDP forwarding tests using a local QUdpSocket receiver. |
| test/GPS/RTCMTestHelper.h | Adds RTCM test helper header (referenced in test CMake). |
| test/GPS/RTCMParserTest.h | Extends RTCM parser tests with edge cases and extractValidFrames tests. |
| test/GPS/RTCMParserTest.cc | Expands parser test suite and updates to new RTCMParser API/constants. |
| test/GPS/NTRIPSourceTableTest.cc | Updates tests to new value-type mountpoint parsing + model roles. |
| test/GPS/NTRIPSourceTableControllerTest.h | Adds unit-test declarations for source-table controller. |
| test/GPS/NTRIPSourceTableControllerTest.cc | Adds controller tests for fetch state, cache TTL, and mountpoint selection. |
| test/GPS/NTRIPManagerTest.h | Re-scopes manager tests to connection/reconnect state-machine behavior. |
| test/GPS/NTRIPManagerTest.cc | Updates manager tests to new reconnect timer/backoff internals and singleton cleanup. |
| test/GPS/NTRIPHttpTransportTest.h | Adds transport config validation + HTTP request building test cases. |
| test/GPS/NTRIPHttpTransportTest.cc | Updates transport tests to new whitelist/parser, NMEAUtils, and request building. |
| test/GPS/NTRIPGgaProviderTest.h | Adds new tests focused on GGA formatting and edge cases. |
| test/GPS/NTRIPGgaProviderTest.cc | Implements GGA tests using NMEAUtils::makeGGA as canonical generator. |
| test/GPS/NTRIPConnectionStatsTest.h | Adds unit-test declarations for connection stats. |
| test/GPS/NTRIPConnectionStatsTest.cc | Implements stats tests (bytes/messages/rate/correction age). |
| test/GPS/MockNTRIPTransport.h | Adds a mock NTRIPTransport for manager/transport tests. |
| test/GPS/GpsTestHelpers.h | Updates RTCM frame builder to use new RTCMParser::kPreamble. |
| test/GPS/CMakeLists.txt | Registers new GPS/NTRIP/RTCM-related unit tests and helpers. |
| src/Utilities/RTCMParser.h | Adds shared RTCM3 frame parser with CRC and frame extraction helper. |
| src/Utilities/RTCMParser.cc | Implements parser state machine + CRC-24Q and framing extraction. |
| src/Utilities/NMEAUtils.h | Adds shared NMEA checksum verification/repair and GGA sentence builder. |
| src/Utilities/NMEAUtils.cc | Implements checksum logic and GGA generation. |
| src/Utilities/Network/UdpForwarder.h | Adds UDP forwarder QObject for outbound RTCM forwarding. |
| src/Utilities/Network/UdpForwarder.cc | Implements UDP forwarder lifecycle/config/forwarding logic. |
| src/Utilities/Network/CMakeLists.txt | Adds UdpForwarder to QGCNetwork library sources. |
| src/Utilities/DataRateTracker.h | Adds lightweight rolling data-rate tracker (non-QObject). |
| src/Utilities/DataRateTracker.cc | Implements windowed rate calculation and reset behavior. |
| src/Utilities/CMakeLists.txt | Adds new utility sources (DataRateTracker/NMEAUtils/RTCMParser) to build. |
| src/Settings/NTRIPSettings.h | Adds new NTRIP setting facts (self-signed certs, GGA source/interval). |
| src/Settings/NTRIPSettings.cc | Declares new NTRIP settings facts. |
| src/Settings/NTRIP.SettingsGroup.json | Adds UI metadata for self-signed TLS, GGA source, and GGA interval settings. |
| src/QmlControls/FixStatusDot.qml | Adds reusable status-dot control. |
| src/QmlControls/ConnectionStatusRow.qml | Adds reusable “status + button” row control. |
| src/QmlControls/CMakeLists.txt | Registers new QML controls in the Controls module. |
| src/GPS/RTCM/RTCMUdpInput.h | Refactors UDP RTCM input to own a QUdpSocket* and support validation via RTCMParser. |
| src/GPS/RTCM/RTCMUdpInput.cc | Implements updated UDP RTCM input with optional CRC-valid frame extraction. |
| src/GPS/RTCM/RTCMMavlink.h | Integrates DataRateTracker and exposes bandwidth stats via Q_PROPERTY. |
| src/GPS/RTCM/RTCMMavlink.cc | Replaces manual bandwidth timer/counter with DataRateTracker and emits change signals. |
| src/GPS/RTCM/CMakeLists.txt | Adds RTCM submodule sources to build. |
| src/GPS/NTRIP/RTCMUdpInput.cc | Removes old NTRIP-scoped RTCM UDP input implementation. |
| src/GPS/NTRIP/RTCMParser.h | Removes old NTRIP-scoped RTCM parser header. |
| src/GPS/NTRIP/RTCMParser.cc | Removes old NTRIP-scoped RTCM parser implementation. |
| src/GPS/NTRIP/NTRIPTransportConfig.h | Introduces typed NTRIP transport configuration and diff helpers. |
| src/GPS/NTRIP/NTRIPTransportConfig.cc | Implements config validation, caster identity, and whitelist parsing. |
| src/GPS/NTRIP/NTRIPTransport.h | Introduces transport interface with signals for manager state machine. |
| src/GPS/NTRIP/NTRIPSourceTableController.h | Adds controller owning source-table fetch lifecycle, caching, and QML exposure. |
| src/GPS/NTRIP/NTRIPSourceTableController.cc | Implements source-table fetch with caching, size guard, and error handling. |
| src/GPS/NTRIP/NTRIPSourceTable.h | Refactors source-table model to a single QAbstractListModel over value-type rows. |
| src/GPS/NTRIP/NTRIPSourceTable.cc | Implements STR parsing with coordinate validation, role-based model, and distance sorting. |
| src/GPS/NTRIP/NTRIPMountpointList.qml | Adds reusable mountpoint list view for browsing/selecting mountpoints. |
| src/GPS/NTRIP/NTRIPManager.h | Refactors manager into explicit event-driven state machine, wiring stats/GGA/source-table/UDP forwarding. |
| src/GPS/NTRIP/NTRIPHttpTransport.h | Refactors HTTP transport as NTRIPTransport, adds request builder and chrono timers. |
| src/GPS/NTRIP/NTRIPHttpTransport.cc | Updates transport logic (TLS handling, ICY support, header cap, RTCM parsing + whitelist). |
| src/GPS/NTRIP/NTRIPGgaProvider.h | Adds configurable GGA provider with position-source selection and interval control. |
| src/GPS/NTRIP/NTRIPGgaProvider.cc | Implements provider with default position sources and fast-retry behavior pre-fix. |
| src/GPS/NTRIP/NTRIPError.h | Adds typed error enum exposed via Qt metaobject system. |
| src/GPS/NTRIP/NTRIPConnectionStatusRow.qml | Adds NTRIP-specific status-row mapping connection state to UI. |
| src/GPS/NTRIP/NTRIPConnectionStatus.qml | Adds NTRIP corrections status panel (bytes, rate, message types, GGA source, etc.). |
| src/GPS/NTRIP/NTRIPConnectionStats.h | Adds QObject stats tracker for QML (bytes/messages/rate/staleness/message-id counts). |
| src/GPS/NTRIP/NTRIPConnectionStats.cc | Implements periodic signal emission and per-message accounting. |
| src/GPS/NTRIP/CMakeLists.txt | Adds new NTRIP sources and registers a dedicated NTRIP QML module. |
| src/GPS/GPSRtk.h | Removes per-instance RTCMMavlink* member and modernizes formatting. |
| src/GPS/GPSRtk.cc | Routes serial RTK corrections through shared NTRIPManager::rtcmMavlink(). |
| src/GPS/CMakeLists.txt | Moves RTCM sources into src/GPS/RTCM subdirectory and keeps RTCM+NTRIP built without serial. |
| src/AppSettings/pages/NTRIP.SettingsUI.json | Updates NTRIP settings UI layout and adds GGA reporting controls + validation toggle. |
| src/AppSettings/NtripServerSettings.qml | Adds UI toggle for allowing self-signed TLS certs when TLS enabled. |
| src/AppSettings/NtripMountpointBrowser.qml | Switches mountpoint browsing UI to new controller/model and new NTRIPMountpointList delegate. |
| src/AppSettings/NtripConnectionStatus.qml | Removes old connection-status UI component. |
| src/AppSettings/NtripConnectionSettings.qml | Adds new connection settings component combining connection row + status panel. |
| src/AppSettings/CMakeLists.txt | Registers new NTRIP connection settings QML file. |
Comment on lines
+34
to
+36
| if (_fetchStatus == FetchStatus::InProgress) { | ||
| return; | ||
| } |
Comment on lines
+8
to
+16
| _rateTimer.callOnTimeout(this, [this]() { | ||
| if (_rateTracker.bytesPerSec() > 0) { | ||
| emit bytesReceivedChanged(); | ||
| emit dataRateChanged(); | ||
| } | ||
| if (_prevMessagesReceived != _messagesReceived) { | ||
| _prevMessagesReceived = _messagesReceived; | ||
| emit messagesReceivedChanged(); | ||
| } |
Comment on lines
+37
to
+44
| // rateUpdated is only true once the 1-second window elapses. | ||
| // Within a fast test that window almost certainly has not elapsed, | ||
| // so we assert false — and totalBytes is the durable invariant. | ||
| // (If somehow the window expires in CI we skip the rateUpdated check.) | ||
| if (!tracker.rateUpdated()) { | ||
| QVERIFY(!tracker.rateUpdated()); | ||
| } | ||
| } |
Comment on lines
+358
to
+362
| const QByteArray remainingData = _httpResponseBuf.mid(hdrEnd + 4); | ||
| _httpResponseBuf.clear(); | ||
|
|
||
| if (status.code == 401) { | ||
| qCWarning(NTRIPHttpTransportLog) << "Authentication failed:" << status.reason; | ||
| emit error(tr("Authentication failed (401): check username and password")); | ||
| return; | ||
| if (!remainingData.isEmpty()) { | ||
| qCDebug(NTRIPHttpTransportLog) << "Processing trailing data:" << remainingData.size() << "bytes"; |
Comment on lines
+321
to
+326
| const QByteArray remainingData = _httpResponseBuf.mid(firstLineEnd + 2); | ||
| _httpResponseBuf.clear(); | ||
| if (!remainingData.isEmpty()) { | ||
| _parseRtcm(remainingData); | ||
| } | ||
| return; |
HTRamsey
force-pushed
the
feature/ntrip-improvements
branch
from
a6cbe1a to
402566f
Compare
Restructure the NTRIP client around a state-machine NTRIPManager with a pluggable NTRIPTransport (HTTP/TCP/TLS), and split out focused collaborators: GgaProvider, ReconnectPolicy, ConnectionStats, UdpForwarder, and a SourceTable controller, plus a typed NTRIPError. Add DataRateTracker and NMEAUtils utilities, an NTRIP corrections status QML panel/module, and settings for self-signed certs, GGA source/interval, and UDP forwarding. The SourceTable controller owns its QNetworkReply directly (no separate fetcher object) and RTCMUdpInput/UdpForwarder own their QUdpSocket directly (no UdpEndpoint wrapper) — the socket/reply lifecycle lives with its owner. Hardening in this change: - SourceTable controller caps response body at 8 MB (OOM guard) - TLS error path aborts via the captured socket (no nullable-_socket deref) - Caster lat/lon validated finite + in range before distance math - Reject ':' in Basic-auth username (RFC 7617); host control-char guard moved ahead of connect() - RTCM parser re-feeds the byte after overflow reset (no lost preamble) - SourceTable controller aborts superseded replies and invalidates its fetch cache on error (a failed fetch never serves a stale Success) Tests cover RTCM parsing/recovery, HTTP transport, GGA, reconnect policy, connection stats, source-table controller, NMEA utils, and data-rate tracking.
HTRamsey
force-pushed
the
feature/ntrip-improvements
branch
from
402566f to
7fa39d7
Compare
github-actions
Bot
added
the
github_actions
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if: runner.os == 'Linux' | ||
| shell: bash | ||
| # Retry: a slow apt mirror can stall the download past the job timeout; fast-fail and retry. | ||
| uses: nick-fields/retry@v4 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Restructures the NTRIP client around a state-machine
NTRIPManagerwith a pluggableNTRIPTransport(HTTP/TCP/TLS), splitting the monolith into focused collaborators:GgaProvider,ReconnectPolicy,ConnectionStats,UdpForwarder, and aSourceTablecontroller, plus a typedNTRIPError. AddsDataRateTrackerandNMEAUtilsutilities, an NTRIP corrections status QML panel/module, and settings for self-signed certs, GGA source/interval, and UDP forwarding.Socket/reply lifecycle lives with its owner: the SourceTable controller owns its
QNetworkReplydirectly andRTCMUdpInput/UdpForwarderown theirQUdpSocketdirectly (no wrapper objects).Hardening
_socketderef):in Basic-auth username (RFC 7617); host control-char guard moved ahead ofconnect()Success)Tests
Unit coverage for RTCM parsing/recovery, HTTP transport, GGA, reconnect policy, connection stats, source-table controller, NMEA utils, and data-rate tracking.
Notes