Skip to content

Add GnssStatusTel telemetry for the Blueye GNSS receiver - #292

Open
jp-pino wants to merge 3 commits into
masterfrom
claude/gnss-status-telemetry
Open

Add GnssStatusTel telemetry for the Blueye GNSS receiver#292
jp-pino wants to merge 3 commits into
masterfrom
claude/gnss-status-telemetry

Conversation

@jp-pino

@jp-pino jp-pino commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a GnssStatusTel telemetry message backed by new GnssStatus, GnssSatellite, and GnssConstellation types so GNSS behaviour can be diagnosed from dive logs for the first time:

  • Fix quality, satellites used / in view, HDOP
  • Per-axis standard deviations (from GST), COG / SOG, global position
  • Per-satellite SNR by constellation (GPS / GLONASS / Galileo / BeiDou)
  • Driver health: connection state, baud rate, reconnect and receive-error counters, time since last sentence

Merge order

Part of the GNSS driver rework. This PR goes first; the p2_msgs, libguestport, and p2_drone PRs build on it.

🤖 Generated with Claude Code

Adds a GnssStatus message carrying fix quality, satellite counts, HDOP,
per-axis standard deviations, COG/SOG, per-satellite SNR, and driver
health (connection state, baud rate, reconnect and error counters) so
GNSS behaviour can be diagnosed from dive logs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jp-pino jp-pino self-assigned this Aug 28, 2026
@jp-pino
jp-pino requested review from alexagv and follesoe August 28, 2026 14:46
@jp-pino jp-pino added the enhancement New feature or request label Aug 28, 2026
@jp-pino
jp-pino marked this pull request as ready for review August 28, 2026 14:47
Add the fields the driver already knows but the message dropped:

- GnssSatellite gains elevation and azimuth, so a client can plot the
  sky the receiver sees rather than a bare list of signal strengths.
  Both use -1 for "not reported", which is unambiguous because neither
  value is ever negative.
- satellites_in_view now carries the receiver's own in-view count.
  Field 5 kept its number but is renamed satellites_tracked, which is
  what it actually holds: the length of the satellites list, pruned to
  those that reported a signal within the last 30 s.
- is_accuracy_valid distinguishes "no GST sentence yet" from a real
  zero-metre deviation, which the two stddev fields cannot express on
  their own.
- is_course_valid mirrors the bit the driver derives but the status
  message dropped: NMEA RMC leaves the course empty when the receiver
  is stationary, so course_over_ground goes stale without it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SVWisosuY9Cep5iN5huuTc
@follesoe

follesoe commented Aug 29, 2026

Copy link
Copy Markdown
Member

First pass, superseded by the comment below once the scope widened to the whole GSA/geometry set. Kept for the reasoning that has not changed:

  • elevation / azimuth use -1 for "not reported", which is safe here in a way a 0.0 standard deviation is not — neither is ever negative — and avoids proto3 optional where the rest of this repo uses none.
  • is_accuracy_valid rather than switching libguestport's stddev sentinel to NaN: those deviations also feed gnss_rov_avg_stddev and avg_stddev_buffer in the position observer, where a NaN in a rolling variance is worse than a large finite number.

🤖 Generated with Claude Code

Everything here is parsed by the driver and was being discarded before
the wire, and all of it is what support needs to read a bad fix out of
a dive log rather than reproducing it on a bench.

- GnssSatellite.used_in_fix, gated by is_used_in_fix_valid. This is the
  distinction between a satellite the receiver is tracking and one
  actually contributing, which is what a sky plot draws as filled
  against hollow. The gate is needed because a GSA sentence without a
  system id cannot be attributed to a constellation, and a client must
  not read that as "nothing is in the fix".
- fix_mode, pdop and vdop, from the same GSA sentence. HDOP alone says
  nothing about vertical geometry, and 2D against 3D explains an
  altitude that looks wrong.
- The horizontal error ellipse (stddev_semi_major, stddev_semi_minor,
  error_ellipse_orientation), stddev_altitude and range_rms. Two
  axis-aligned deviations cannot describe a tilted uncertainty, which
  is the normal shape when half the sky is behind a quay wall.
- altitude, which is the cheapest sanity check on a surface receiver: a
  multipath fix often keeps a respectable HDOP but puts the antenna
  hundreds of metres off vertically.
- differential_age and differential_station, without which a fix
  quality of 2, 4 or 5 cannot be verified after the fact. Both use -1
  for absent, which is unambiguous as neither is ever negative.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SVWisosuY9Cep5iN5huuTc
@follesoe

Copy link
Copy Markdown
Member

Reviewed against what the driver actually produces and extended in 1c25d36 + c562a3d. GnssStatus goes 16 → 31 fields; everything added is already parsed by libguestport, and used_in_fix needed one new sentence (GSA, BluEye-Robotics/libguestport#393).

Satellite geometry. GnssSatellite gains elevation, azimuth and used_in_fix. Without the first two a satellite is an id and a signal strength — enough for a bar chart, not for plotting the sky the receiver sees, and an obstructed horizon is invisible in the data. used_in_fix is the tracked vs contributing distinction that every GNSS tool draws as filled against hollow. Angles use -1 for "not reported": safe here in a way a 0.0 deviation is not, since neither is ever negative, and it avoids proto3 optional where the rest of this repo uses none.

Four validity bits, each load-bearing. These exist because the alternative is a UI that lies:

field without it
is_used_in_fix_valid a GSA with no system id can't be attributed to a constellation, and a client reads that as "nothing is in the fix"
is_accuracy_valid stddev_for_status() maps the numeric_limits sentinel to 0.0F, so "no GST sentence yet" renders as ± 0.00 m
is_course_valid RMC leaves the course empty when stationary, so a still drone reports the last heading it saw while moving. GnssPosition keeps this bit; GnssStatus dropped it
fix_mode ≠ UNSPECIFIED no way to tell a 2D fix from a 3D one, which is what explains a wrong altitude

satellites_in_view did not mean satellites in view. It held last_report.satellites.size() — entries with a non-empty SNR seen in the last 30 s, always below the receiver's own count and lagging it. Field 5 keeps its number and type but is renamed satellites_tracked; satellites_in_view (17) now carries the count GSV reports on every sentence and the driver was discarding.

Also added, all previously parsed and dropped: pdop/vdop; the horizontal error ellipse (stddev_semi_major, stddev_semi_minor, error_ellipse_orientation), stddev_altitude and range_rms — two axis-aligned deviations cannot describe a tilted uncertainty, which is the normal shape with half the sky behind a quay wall; altitude, the cheapest sanity check on a surface fix, since multipath often keeps a respectable HDOP while putting the antenna hundreds of metres off vertically; and differential_age/differential_station, without which a fix quality of 2, 4 or 5 cannot be verified after the fact.

Skipped: GGA geoid_separation — near constant for a location, legitimately negative so it has no free sentinel, and altitude carries the diagnostic value.

protolint and protoc pass; CI green. Siblings updated to match: BluEye-Robotics/libguestport#393, BluEye-Robotics/p2_msgs#51, BluEye-Robotics/p2_drone#1039.

🤖 Generated with Claude Code

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants