feat(gps): fix GPS support and add M10 dual-band GNSS configuration#200
feat(gps): fix GPS support and add M10 dual-band GNSS configuration#200rtlopez merged 3 commits intortlopez:masterfrom
Conversation
Fix GPS initialization issues and add M10 support with dual-band L1+L5 and user-configurable multi-constellation GNSS. Replace NMEA with UBX protocol for better reliability. Features: - M10 dual-band L1+L5 for sub-meter accuracy - Multi-constellation support: GPS, GLONASS, Galileo, BeiDou, QZSS, SBAS - CLI configuration with preset modes (0-5) and individual control - Auto-detection of M8/M9/M10 modules - 25Hz update rate on M10 at 230400 baud Changes: - Remove NMEA parser from PR rtlopez#199(closed), switch to UBX-only protocol - Add GPS_M10 enum and dual-band support - Add GNSS configuration parameters to GpsConfig - Implement configureGnss() for version-aware setup - Update docs/setup.md with GPS configuration guide - Update docs/README.md and docs/wiring.md Configuration: - gps_gnss_mode: 0=Auto, 1=GPS-only, 2=GPS+GLO, 3=GPS+GAL, 4=GPS+BDS, 5=All - gps_enable_dual_band: Auto L1+L5 on M10 - Individual flags: enableGPS, enableGLONASS, enableGalileo, etc. Tested with M8/M9/M10 modules, all GNSS modes working.
docs/setup.md
Outdated
| In the `Configuration` tab, under `Other Features`, enable **GPS** or use CLI: | ||
|
|
||
| ``` | ||
| set feature_mask = 128 |
There was a problem hiding this comment.
this syntax is not valid here, also paramter name is incorrect. It has changed recently and now it should be
set feature_gps 1
docs/setup.md
Outdated
|
|
||
| ``` | ||
| # Minimum satellites required for valid fix | ||
| set gps_min_sats = 8 |
There was a problem hiding this comment.
invalid CLI syntax, = is not required.
docs/setup.md
Outdated
|
|
||
| ### GPS Features | ||
|
|
||
| #### Return to Home (RTH) |
There was a problem hiding this comment.
There is no GPS rescue in this PR, why it is documented here?
docs/wiring.md
Outdated
| | `pin_buzzer` | 0 | 5 | Status buzzer | | ||
| | `pin_led` | 26 | - | Status led | | ||
| | `pin_buzzer` | 26 | 5 | Status buzzer | | ||
| | `pin_led` | 2 | - | Status led | |
There was a problem hiding this comment.
pin_buzzer and pin_led document values that are valid for latest stable release (v0.2.1), do not update yet.
lib/Espfc/src/Connect/Cli.cpp
Outdated
| { | ||
| printGpsStatus(s, true); | ||
| } | ||
| else if(strcmp_P(cmd.args[0], PSTR("gps_home")) == 0) |
There was a problem hiding this comment.
once you added home position status to gps command, there is no need for gps_home command. It will be more clear to set home as gps sub-command, for example gps set_home.
lib/Espfc/src/Sensor/GpsSensor.cpp
Outdated
| sin(phi1) * cos(phi2) * cos(deltaLambda); | ||
|
|
||
| double theta = atan2(y, x); | ||
| double bearing = fmod((theta * 180.0 / M_PI + 360.0), 360.0); |
There was a problem hiding this comment.
same issue as haversineDistance()
lib/Espfc/src/Sensor/GpsSensor.cpp
Outdated
| return (int16_t)bearing; | ||
| } | ||
|
|
||
| void GpsSensor::setHomePosition() const |
There was a problem hiding this comment.
it is alredy implemented, see Model::setGpsHome() and Actuator::updateArmed()
lib/Espfc/src/Sensor/GpsSensor.cpp
Outdated
| _model.state.gps.accuracy.vertical = m.vAcc; // mm | ||
| _model.state.gps.accuracy.speed = m.sAcc; // mm/s | ||
| _model.state.gps.accuracy.heading = m.headAcc; // deg * 1e5 | ||
| _model.state.gps.accuracy.horizontal = m.hAcc; |
There was a problem hiding this comment.
please revert comments with units. They are on purpose here and extremely usefull for me.
lib/Espfc/src/Sensor/GpsSensor.hpp
Outdated
| SET_BAUD, | ||
| DISABLE_NMEA, | ||
| GET_VERSION, | ||
| CONFIGURE_GNSS, |
There was a problem hiding this comment.
I'm not sure if CONFIGURE_GNSS is in conflict with ENABLE_SBAS. Anyway, I think, it would be safer to run configure_gnss as last phase.
README.md
Outdated
| * In flight PID Tuning | ||
| * Buzzer, Led and voltage monitor | ||
| * Failsafe mode | ||
| * GPS rescue |
There was a problem hiding this comment.
there is no GPS rescue here, this may confuse others.
|
@rtlopez thanks for your suggestions and corrections, let me work on them to resolve asap |
- restore BAUDS order (9600 first for cold-start detection) - restore DISABLE_NMEA state and processNmea() for safe frame skipping - move CONFIGURE_GNSS to last config phase (after ENABLE_SBAS) - replace raw CFG-GNSS byte array with UbxCfgGnss7 struct + send() - replace double in haversine/bearing with float equirectangular approx - remove setHomePosition/shouldSetHome; use Model::setGpsHome() instead - merge gps_home command into gps set_home/clear_home subcommands - restore unit comments on accuracy fields (mm, mm/s, deg*1e5) - remove GPS rescue config fields and docs (not in this PR) - fix CLI syntax in docs (set feature_gps 1, no = in set commands) - revert pin_buzzer/pin_led defaults in wiring.md - rewrite test_gps as standalone math tests (no Arduino dependency)
|
Hi @rtlopez, thanks for the detailed review — I've addressed all the feedback in the latest commit. Regarding splitting into smaller PRs: I understand the reasoning, but in this case the three areas are tightly coupled at the implementation level: M10 detection (GET_VERSION → handleVersion()) is what determines whether configureGnss() enables dual-band — the GNSS configuration step has no effect without it. I've kept each logical area clearly separated within the code itself, and the review feedback has all been addressed. Happy to discuss further if you'd prefer a different approach. |
|
Hi @amanpd7 now it looks pretty acceptable for me. I'll try to do some tests over the weekend and if all pass, I'll merge it. Thank you! |
|
@rtlopez Hi and thanks for reviewing so quickly, I'll also be testing these new changes by this weekend and will update here accordingly. |
|
Hi @amanpd7 I've tested on HGLRC M100 today and cannot make it working. I got this log I did quick look into datasheet, and noticed, that
it is possible, that 34.10, which I have, no longer suport it. please check |
|
Hi @rtlopez I am using HGLRC M100-PRO gps module and did not find this issue. This is a real hardware-specific issue. Root cause: UBX-CFG-GNSS is officially deprecated in u-blox protocol versions > 23.01. Some M10 variants NAK it and error out; others silently accept it. My own M10 module (also PROTVER 34.10) accepted the command without issue, so I missed this during testing. Immediate fix (already identified): In configureGnss(), change the timeout state from ERROR to SET_RATE so a NAK skips GNSS constellation config rather than halting initialization entirely: This lets the M100 initialize successfully with default GNSS settings. Long-term fix: Parse the PROTVER field from MON-VER extended strings (already read in GET_VERSION state) and use UBX-CFG-VALSET with CFG-SIGNAL-* keys for modules with PROTVER > 23.01. I'll add this in a follow-up commit. Can you confirm — does the HGLRC M100 use M10, M10S, or M10C silicon? The MON-VER hardware field (e.g. 00190000) would also help narrow down which variant is rejecting the command. I'll also be pushing a permanent patch for it in this PR. here is logs of my setup for reference |
UBX-CFG-GNSS is deprecated on M10 modules with PROTVER > 23.01 and some variants (e.g. HGLRC M100) return a NAK, halting initialization. Pass SET_RATE as the timeout/NAK fallback state so GNSS constellation config is skipped gracefully and the module continues to initialize.
|
Hi @amanpd7, I don't really like this way of handling errors, but I decided to merge these changes anyway, I hope we can work on this further in the future. Thank you. |
|
Hi @rtlopez thanks for accepting my contribution, the issue you faced was completely on silicon level, chipset i am using is M10 and did not had any errors with same firmware running, this is a very rare occurance but sure my furthur works will be to handle this issue properly and i'll create a patch for this which is very much acceptable. |
Fix GPS initialization issues and add M10 support with dual-band L1+L5 and user-configurable multi-constellation GNSS. Replace NMEA with UBX protocol for better reliability.
Features:
Changes:
Configuration:
Tested with M8/M9/M10 modules, all GNSS modes working.