-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add GPS hardware version to MSP and optimize defaults for M8/M9/M10 #11262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: maintenance-9.x
Are you sure you want to change the base?
Add GPS hardware version to MSP and optimize defaults for M8/M9/M10 #11262
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
PR Compliance Guide 🔍All compliance sections have been disabled in the configurations. |
| sbufWriteU16(dst, gpsSol.hdop); | ||
| sbufWriteU16(dst, gpsSol.eph); | ||
| sbufWriteU16(dst, gpsSol.epv); | ||
| sbufWriteU32(dst, gpsState.hwVersion); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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]
| 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.
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