Skip to content

RemoteID/PositionManager: fix GCS Live GNSS position#14587

Open
julianoes wants to merge 2 commits into
mavlink:masterfrom
julianoes:pr-remoteid-live-gnss-position
Open

RemoteID/PositionManager: fix GCS Live GNSS position#14587
julianoes wants to merge 2 commits into
mavlink:masterfrom
julianoes:pr-remoteid-live-gnss-position

Conversation

@julianoes

@julianoes julianoes commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Two fixes so the Live GNSS Remote ID GCS position works on Android / in FAA regions.

  • Freshness from local arrival time: _updateLastGCSPositionInfo() stored update.timestamp() (the source's clock), which on Android is offset from the system clock the freshness check compares against — making gcsGPSGood flap at 1 Hz near the 5 s threshold. Now stamps local arrival time.
  • Keep altitude on poor vertical accuracy: altitude was dropped above a strict 10 m, leaving a non-3D coordinate that the FAA path rejects. Now accept altitude on any 3D fix within threshold (or with no reported accuracy), and raise the threshold to 100 m to match the horizontal one.

julianoes added 2 commits July 6, 2026 14:16
The freshness check in _sendSystem() compared update.timestamp() (the
position source's own clock) against the local wall clock. On Android
that timestamp is offset from the system clock, and when the offset sat
near the 5s ALLOWED_GPS_DELAY threshold the computed age swept across it
every send cycle, flapping gcsGPSGood green/red at 1 Hz despite a healthy
1 Hz fix.

Record the local arrival time instead, so the check measures the true
time since the last valid fix and is immune to GPS-vs-system clock skew.
Altitude was dropped whenever the fix's vertical accuracy exceeded a strict
10m gate (or wasn't reported), which the Android position source almost never
meets, leaving gcsPosition.altitude() as NaN. Remote ID in FAA regions
mandates an operator altitude, so this made Live GNSS unusable there.

Accept the altitude on any 3D fix when the vertical accuracy is within
threshold or not reported at all, and raise the vertical threshold to 100m to
match the horizontal one.

static constexpr qreal kMinHorizonalAccuracyMeters = 100.;
static constexpr qreal kMinVerticalAccuracyMeters = 10.;
static constexpr qreal kMinVerticalAccuracyMeters = 100.;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This might need discussion. It solved the problem for me with Herelink but might not be generic.

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 30.62%. Comparing base (f29efd3) to head (fced979).
⚠️ Report is 125 commits behind head on master.

Files with missing lines Patch % Lines
src/PositionManager/PositionManager.cpp 25.00% 1 Missing and 2 partials ⚠️
src/Vehicle/RemoteIDManager.cc 0.00% 0 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (20.00%) is below the target coverage (30.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14587      +/-   ##
==========================================
+ Coverage   25.47%   30.62%   +5.15%     
==========================================
  Files         769      785      +16     
  Lines       65912    66780     +868     
  Branches    30495    30940     +445     
==========================================
+ Hits        16788    20452    +3664     
+ Misses      37285    32389    -4896     
- Partials    11839    13939    +2100     
Flag Coverage Δ
unittests 30.62% <20.00%> (+5.15%) ⬆️

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

Files with missing lines Coverage Δ
src/PositionManager/PositionManager.h 80.00% <ø> (+40.00%) ⬆️
src/Vehicle/RemoteIDManager.cc 18.27% <0.00%> (-2.59%) ⬇️
src/PositionManager/PositionManager.cpp 15.83% <25.00%> (+0.57%) ⬆️

... and 434 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 c85eb6e...fced979. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Jul 6, 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: 25 passed, 0 skipped
linux-coverage-unit: 103 passed, 0 skipped
Total: 128 passed, 0 skipped

Code Coverage

Coverage: 66.3%

No baseline available for comparison

Artifact Sizes

Artifact Size
QGroundControl 221.84 MB
QGroundControl-aarch64 179.41 MB
QGroundControl-installer-AMD64 137.91 MB
QGroundControl-installer-AMD64-ARM64 80.44 MB
QGroundControl-installer-ARM64 109.20 MB
QGroundControl-linux 87.09 MB
QGroundControl-mac 87.08 MB
QGroundControl-windows 86.14 MB
QGroundControl-x86_64 191.64 MB
No baseline available for comparison

Updated: 2026-07-06 03:18:21 UTC • Commit: fced979 • 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

This PR improves the reliability of the Remote ID “Live GNSS” GCS position pipeline, primarily for Android and FAA-region requirements, by addressing timestamp freshness handling and altitude retention for 3D coordinates.

Changes:

  • Stamp GCS GNSS “last update” using local arrival time (UTC) instead of the source-provided timestamp to prevent freshness flapping on platforms with offset provider clocks (e.g. Android).
  • Increase the vertical accuracy gate from 10 m → 100 m and accept altitude when vertical accuracy is acceptable or not reported, to avoid dropping altitude and failing FAA 3D coordinate requirements.
  • Adjust altitude-setting logic in QGCPositionManager to use the new vertical-accuracy behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/Vehicle/RemoteIDManager.cc Changes freshness timestamping for Live GNSS updates to use local arrival time.
src/PositionManager/PositionManager.h Raises the vertical accuracy threshold constant to 100 m.
src/PositionManager/PositionManager.cpp Updates altitude acceptance logic to retain altitude under broader (and Android-compatible) conditions.

Comment on lines +132 to 134
if ((update.coordinate().type() == QGeoCoordinate::Coordinate3D) && verticalAccuracyOk) {
newGCSPosition.setAltitude(update.coordinate().altitude());
}
Comment on lines 490 to +495
if (update.isValid()) {
_lastGeoPositionTimeStamp = update.timestamp().toUTC();
// Stamp the local arrival time rather than update.timestamp(). The freshness
// check in _sendSystem() compares this against the local wall clock, and on some
// platforms (e.g. Android) the position source's own timestamp is offset from the
// system clock, which made gcsGPSGood flap green/red even with a healthy fix.
_lastGeoPositionTimeStamp = QDateTime::currentDateTimeUtc();
@DonLakeFlyer

Copy link
Copy Markdown
Contributor

@julianoes Can you look at the review?

@DonLakeFlyer

Copy link
Copy Markdown
Contributor

PR #14587 — Implications of the kMinVerticalAccuracyMeters Change (10 m → 100 m)

PR: RemoteID/PositionManager: fix GCS Live GNSS position

The 10 m → 100 m change in PositionManager.h (together with the accept-when-unreported logic in PositionManager.cpp _positionUpdated()) controls when the GCS device's altitude is copied into gcsPosition. Implications, by consumer:

1. Remote ID (the intended beneficiary)

RemoteIDManager::_sendSystem() broadcasts gcsPosition.altitude() as the operator geodetic altitude in OPEN_DRONE_ID_SYSTEM, and the FAA-region check hard-fails if the coordinate isn't 3D. Phones commonly report vertical accuracy in the 15–50 m range, so the old 10 m gate silently stripped altitude and made gcsGPSGood red in FAA regions. Raising to 100 m fixes that. The message carries no operator-altitude accuracy field, so broadcasting a ±100 m altitude is regulatorily better than broadcasting "unknown" — this is the right trade for Remote ID.

2. Follow Me — the real safety-relevant implication

FollowMe.cc sends gcsCoordinate.altitude() as altMetersAMSL in FOLLOW_TARGET. If the firmware is configured to track target altitude, the vehicle could now be commanded to follow an altitude that's up to ~100 m wrong (and accuracy figures are 1-σ, so occasionally worse), where previously it would have gotten the stale/last-good altitude instead. Mitigating factor: FollowMe forwards the vertical std-dev in pos_std_dev[2], so firmware can de-weight it — but PX4's follow-me altitude modes don't necessarily do so.

3. Set-home-to-GCS

Vehicle::_updateHomepoint() sends MAV_CMD_DO_SET_HOME with gcsPosition.altitude() when "update home position" is enabled. Home altitude is the reference for relative-altitude display and RTL behavior; a 100 m error there is operationally significant. Previously the loose altitude simply wasn't there (the coordinate kept the old/NaN altitude).

4. NTRIP GGA

NTRIPGgaProvider.cc sends the position to the caster for VRS selection; a 100 m altitude error is harmless here.

5. The "no reported accuracy → accept" part is the bigger loosening

Independent of the 10→100 constant, the new verticalAccuracyOk = true default means any 3D fix from a source that doesn't report VerticalAccuracy (e.g. plain NMEA without GST sentences) now passes its altitude straight through, with unknown quality. That's arguably correct for Remote ID, but it applies to Follow Me and set-home too.

6. Minor pre-existing quirk it doesn't change

When the gate does reject, newGCSPosition keeps the previous altitude while lat/lon update — so consumers can get a fresh 2D position paired with a stale altitude. Not made worse by this PR, just noting the gate never clears altitude.

Summary

The constant change is consistent (the horizontal gate is already 100 m via kMinHorizonalAccuracyMeters) and is what makes FAA Remote ID usable on phones. The cost is that Follow Me and update-home-position now consume altitude of much lower guaranteed quality. If that's a concern, the gate could be kept per-consumer (e.g. Remote ID reads altitude regardless, Follow Me keeps a stricter check) rather than loosened globally.

@DonLakeFlyer

Copy link
Copy Markdown
Contributor

2 and 3 are the main problems. 3 was already a bit whacky, but 2 will lead to real problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants