Add GPS hardware version to MSP and optimize defaults for M8/M9/M10#11262
Open
sensei-hacker wants to merge 3 commits intoiNavFlight:maintenance-9.xfrom
Open
Add GPS hardware version to MSP and optimize defaults for M8/M9/M10#11262sensei-hacker wants to merge 3 commits intoiNavFlight:maintenance-9.xfrom
sensei-hacker wants to merge 3 commits intoiNavFlight:maintenance-9.xfrom
Conversation
Add hwVersion field to MSP_GPSSTATISTICS (166) for GPS module detection: - Field added at end of message (backward compatible) - Exposes gpsState.hwVersion (800=M8, 900=M9, 1000=M10, 0=Unknown) - Enables configurator auto-detection of GPS module type Changes: - src/main/fc/fc_msp.c: Add sbufWriteU32(dst, gpsState.hwVersion) - docs/development/msp/msp_messages.json: Document hwVersion field Used by configurator GPS preset UI to automatically configure optimal constellation/rate settings based on hardware capability (M8/M9/M10). Backward compatible: Old configurators ignore extra bytes, new configurators check byteLength before reading hwVersion field. Related configurator PR: feature-gps-preset-ui branch Research: claude/developer/docs/gps/m9-16-satellite-limitation-official.md
Changed defaults to provide better out-of-box accuracy: - gps_ublox_use_galileo: OFF → ON - gps_ublox_use_beidou: OFF → ON - gps_ublox_use_glonass: OFF (unchanged) - gps_ublox_nav_hz: 10 → 8 Rationale: - 3 constellations (GPS+Galileo+Beidou) provide excellent coverage - 8Hz allows M9 modules to use 32 satellites (vs 16 at ≥10Hz) - Safe for M8 (handles 8Hz easily) - Optimal for M10 with 3 constellations at default clock - Glonass remains OFF to avoid M10 processing overhead Updated nav_hz description to document M9's 16-satellite limitation at ≥10Hz, discovered through u-blox forum research and Clive Turvey's code analysis. Regenerated Settings.md from settings.yaml. Related: GPS preset UI feature
Contributor
PR Compliance Guide 🔍All compliance sections have been disabled in the configurations. |
Comment on lines
1009
to
+1012
| sbufWriteU16(dst, gpsSol.hdop); | ||
| sbufWriteU16(dst, gpsSol.eph); | ||
| sbufWriteU16(dst, gpsSol.epv); | ||
| sbufWriteU32(dst, gpsState.hwVersion); |
Contributor
There was a problem hiding this comment.
Suggestion: Gate the new hwVersion field behind an explicit buffer/feature check (or protocol/version check) so legacy/size-assuming consumers don't break and serialization can't overrun a constrained buffer. [Learned best practice, importance: 6]
Suggested change
| sbufWriteU16(dst, gpsSol.hdop); | |
| sbufWriteU16(dst, gpsSol.eph); | |
| sbufWriteU16(dst, gpsSol.epv); | |
| sbufWriteU32(dst, gpsState.hwVersion); | |
| sbufWriteU16(dst, gpsSol.hdop); | |
| sbufWriteU16(dst, gpsSol.eph); | |
| sbufWriteU16(dst, gpsSol.epv); | |
| if (sbufBytesRemaining(dst) >= 4) { | |
| sbufWriteU32(dst, gpsState.hwVersion); | |
| } |
Addresses Qodo code review suggestion to prevent sending uninitialized memory over MSP_GPSSTATISTICS. While global variables are zero-initialized in C, explicit initialization is better practice: - Makes intent clear in code - Works for all GPS providers (UBLOX, MSP, FAKE) - Future-proof if gpsState becomes non-global - Documents that 0 means UNKNOWN u-blox driver also initializes to UBX_HW_VERSION_UNKNOWN (0) during configuration, but this ensures all code paths are safe.
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.
User description
Summary
Extends MSP_GPSSTATISTICS to include GPS hardware version (hwVersion) for auto-detection, and updates GPS defaults for better out-of-box accuracy across all u-blox modules.
This enables the configurator to automatically detect GPS module type (M8/M9/M10) and apply optimal presets.
Changes
MSP Extension
gpsState.hwVersionGPS Defaults
Rationale for Defaults based on research on the u-blox forum, PX4 discussion, Clive Turvey (u-blox expert), and Jetrell's testing
Updated
gps_ublox_nav_hzdescription in settings.yaml to document M9's 16-satellite limitation at ≥10Hz.Research & Citations
This work is based on research and community contributions:
u-blox Forum Research
Clive Turvey's Code Analysis
PX4 GPS Driver Decision
Field Testing
Testing
Tested ✅
Testing Needed⚠️
Related PR
Documentation
Testing help appreciated! If you have M9 or M10 modules connected, please test and report auto-detection
PR Type
Enhancement
Description
Extend MSP_GPSSTATISTICS with GPS hardware version field
hwVersionfield for auto-detection of M8/M9/M10 modulesOptimize GPS defaults for improved accuracy across all u-blox modules
gps_ublox_nav_hzfrom 10Hz to 8Hz (enables 32 satellites on M9)Document M9 satellite limitation and rationale in settings
Diagram Walkthrough
File Walkthrough
fc_msp.c
Add hwVersion field to MSP_GPSSTATISTICSsrc/main/fc/fc_msp.c
sbufWriteU32(dst, gpsState.hwVersion)to MSP_GPSSTATISTICS messagemsp_messages.json
Document GPS hardware version field in MSPdocs/development/msp/msp_messages.json
hwVersionfield in MSP_GPSSTATISTICS (message 166)800=UBLOX8, 900=UBLOX9, 1000=UBLOX10, 0=UNKNOWN)
uint32_twith units as "Version code"Settings.md
Update GPS settings documentation with new defaultsdocs/Settings.md
gps_ublox_nav_hzdefault value from 10 to 8 in tablegps_ublox_use_beidoudefault from OFF to ONgps_ublox_use_galileodefault from OFF to ONsettings.yaml
Update GPS defaults and navigation rate documentationsrc/main/fc/settings.yaml
gps_ublox_nav_hzdefault from 10 to 8 Hzgps_ublox_use_galileoby default (OFF → ON)gps_ublox_use_beidouby default (OFF → ON)gps_ublox_nav_hzdescription with M9 satellite limitationdetails