Conversation
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
PR Summary by QodoAvoid unsupported GLONASS keys on u-blox F10 receivers
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
cc20741 to
42e8c23
Compare
|
Good catch, and it reproduces. Measured in SITL with an emulated M10 that answers everything, then goes silent long enough for the protocol to restart, then comes back but loses the one MON-GNSS reply. The constellation VALSET that reaches the receiver after the restart:
So the report is right. I did not take the suggested fix, though. Clearing
That trades one regression for a worse one. The decision now reads the support mask directly, which is cleared in the same place the other masks are: const bool noGlonass = ubx_capabilities.supported && !gpsUbloxHasGlonass();An empty mask means MON-GNSS never answered, so nothing is known and every key goes out, exactly as before this PR. No new state, no change to the wait, no change in timing. After the change:
Branch force pushed. |
42e8c23 to
e8f77e1
Compare
MrScothh
changed the title
MrScothh
changed the title
An F10 reports the M10 hardware version, so INAV takes the CFG-VALSET path for it, and then sends two things it cannot accept. Its configuration database has no Glonass group at all, and its Beidou L1 signal is B1C, not the B1I that INAV picks by default. Either one makes the receiver reject the whole message, so SBAS, Galileo, BeiDou and QZSS never get applied. UBX-MON-GNSS is already polled before the configuration starts and reports which constellations the receiver has, so leave the Glonass keys out when it says there are none. An empty mask means MON-GNSS never answered, and then nothing is known, so every key goes out as before. The dual band receivers name themselves in the MON-VER extensions, which are already being read for the constellation list, so pick B1C there. It is what they have, and their two bands cannot be configured apart.
UBX-MON-GNSS says how many major constellations the receiver can track at once, and INAV read the number without using it. Asking for more gets the configuration refused: the receiver answers NAK and keeps what it had, as the M8 protocol description puts it for UBX-CFG-GNSS. INAV then reset Galileo, BeiDou and GLONASS to their defaults in its own settings without sending them, so the receiver ran one configuration, the configurator showed another, and the next save of any kind replaced the user's choice with the defaults. The constellations to enable are now worked out once, from the selection, the constellations the receiver has and how many it can take. When the selection does not fit, the ones INAV leaves off by default go first: GLONASS, then BeiDou, then Galileo. The settings are not touched, so the same configuration gets everything it asked for on a receiver that can run it. Both the CFG-VALSET path for M10 and later and the CFG-GNSS path for M8 and M9 use it. The count is only used next to a fresh MON-GNSS mask, since it is not cleared between detection attempts.
df62a3e to
1e25a27
Compare
|
RAM / Flash usage vs. base commit
See RAM/flash optimization guide for techniques to reduce usage. |
|
Test firmware build ready — commit Download firmware for PR #11965 249 targets built. Find your board's
|
2 participants
Two things keep a u-blox receiver from being configured with what it can actually
run, both in
configureGNSS10()and both ending the same way: the receiver answersNAK, applies none of the message, and INAV quietly rewrites the user's constellation
settings to their defaults without sending them.
1. The F10 is offered keys and signals it does not have
A u-blox F10 reports
hwVersion000A0000, the same string an M10 reports, so INAV correctly identifies it as protocol 27+ and configures it through CFG-VALSET. It then sends two things that receiver cannot accept.Its configuration database has no Glonass group at all. The F10 interface description (UBX-23002975) shows the CFG-SIGNAL group jumping straight from
CFG-SIGNAL-QZSS_ENA(0x10310024) toCFG-SIGNAL-NAVIC_ENA(0x10310026): key 0x10310025, which isCFG-SIGNAL-GLO_ENAon an M10, is not there, and neither is 0x10310018.And its Beidou L1 signal is B1C, not B1I.
configureGNSS10()picks B1I unless Glonass is on, which on an F10 both enables a signal it cannot receive and turns off the one it uses. The NEO-F10N integration manual is explicit about what happens then:A CFG-VALSET is all or nothing, so either of those makes the receiver answer NAK and apply none of the message. SBAS, Galileo, BeiDou and QZSS go down with them, and INAV then resets
gps_ublox_use_galileo,gps_ublox_use_beidouandgps_ublox_use_glonassto their defaults.This is the whole F10 family: NEO-F10N, DAN-F10N and the boards built on them. What a real NEO-F10N answers to MON-VER:
GPS;GAL;BDS, with noGLO, andSPGL1L5for the dual band firmware.What changes for the F10
Both pieces of information are already on hand.
INAV polls UBX-MON-GNSS before the configuration starts and keeps the supported-constellation mask, and
gpsUbloxHasGlonass()is already there. The two Glonass keys are last in the array, so leaving them out makes the message shorter rather than different. They are only dropped when the mask says so: an empty mask means MON-GNSS never answered, nothing is known, and every key goes out as before.The MON-VER extensions are already being walked for the constellation list and the protocol version, so the same walk now notices
L1L5and the Beidou choice follows it.Nothing changes for a receiver that reports Glonass.
2. Any receiver can be asked for more constellations than it can run
UBX-MON-GNSS also says how many major constellations the receiver can track at once,
and INAV reads that number into
ubx_capabilities.capMaxGnsswithout ever using it.Ask for more and the answer is the same NAK as above. The u-blox 8 protocol
description says so for UBX-CFG-GNSS:
and the M10 and F10 integration manuals say it for the configuration interface:
This is not hypothetical on the receivers INAV supports. A NEO-M8 tracks up to three
concurrent GNSS, so INAV's own defaults, GPS with Galileo and BeiDou, already fill it:
turning GLONASS on as well is refused, and today that loses the whole constellation
configuration rather than one constellation. The M10 platform tracks four, so nothing
is trimmed there.
What happens after the NAK is the part worth fixing. INAV resets the three settings to
their defaults in its own configuration but never sends them, so the receiver keeps
running whatever it had, the configurator shows the defaults instead of the choice
that was made, and the next save of any kind persists them.
What changes there
The constellations to ask for are now worked out once, from the selection, the
constellations the receiver reports and how many it says it can run:
anyway;
go first: GLONASS, then BeiDou, then Galileo;
on a receiver that can run it;
detection attempts.
Both paths use it, CFG-VALSET for M10 and later and CFG-GNSS for M8 and M9, so the
M8 case above stops being a refusal and becomes three constellations that work.
Testing
SITL on Windows against an emulated u-blox that answers MON-VER and MON-GNSS and streams NAV-PVT. The F10 emulator applies its manual's rules and NAKs the message if it sees a Glonass key, B1I enabled, or B1C turned off while B2a is on. What reaches the receiver in the constellation VALSET:
gps_ublox_use_glonass = ONgps_ublox_use_glonass = ONBefore the change the two F10 cases were rejected, the first for both reasons and the second for the Glonass keys alone, and no part of the constellation configuration reached the receiver. The two M10 cases are byte for byte what they were.
For the second fix, with the emulated receiver refusing anything past its reported
limit, as the manuals describe:
gps_ublox_use_glonassrewritten to OFFAlso checked, because the capability state outlives a protocol restart: an M10 that goes quiet long enough for the state machine to restart, then comes back with its MON-GNSS reply lost, still gets all 12 keys, exactly as it does on
maintenance-10.x.Not tested on hardware yet. The emulators follow the published rules, but a real
NEO-F10N, and an M8 with all four constellations selected, would both be worth a
check.