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 QodoReport u-blox capabilities and add capability-gated NavIC support
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
|
Thanks, two of the three were real and are fixed in the push above. The concurrency limit outliving a receiver. Correct, and it is worse than a stale MSP field: Reading past the payload. Also correct. The loop now takes whole extensions only, The schema version. Bumped, to 2.1.1. The rule in |
|
The configurator side is open as iNavFlight/inav-configurator#2778. |
|
Taking the stricter reading on the registry version, as offered above: it is 2.2.0 now rather than 2.1.1. New payload fields do extend what the registry describes while leaving anything reading the old layout working, and on reflection that is what the minor bump in the rule is for, otherwise nothing would ever qualify for one. |
|
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 #11979 249 targets built. Find your board's
|
The configurator has no way of knowing what the receiver in front of it can actually do. It shows the same list of settings for every u-blox module, so a NEO-F10N is offered Glonass it does not have, and the hardware version alone cannot tell an F10 from an M10 because both report 000A0000. INAV already asks the receiver: UBX-MON-GNSS says which constellations are present, which are running and how many can run at once, and the MON-VER extensions carry the module name and the augmentation systems. None of it left the flight controller. MSP_GPSSTATISTICS now carries it, appended after the hardware version so older configurators keep reading the fields they know.
INAV never configured NavIC, so a receiver that has it, such as the u-blox F10, kept whatever it was shipped with, which is off. There was no setting to change that short of writing the receiver's flash from u-center. gps_ublox_use_navic, off by default, now sets CFG-SIGNAL-NAVIC_ENA and NAVIC_L5_ENA. The keys go only to receivers that list NAVIC in their MON-VER extensions, and in a CFG-VALSET of their own: a receiver refuses a whole message over one key it does not know, so they cannot share one with the constellations every receiver gets. The new field goes at the end of gpsConfig_t, where pgLoad gives it its default on an existing configuration, as was done for autoBaudMax.
The reply now carries what the receiver reported about itself: the major constellations it has and which of them are running, the augmentation systems that only MON-VER lists, how many it can track at once, and its module name. README.md regenerated from the registry with gen_msp_md.py.
…e receiver Two things came out of review. The MON-VER extension loop took any remaining byte as the start of a record and then read a fixed thirty from it, so a frame whose length is not a round number of records had its last line read past the payload; it now takes whole records only, and the module name is copied with the room left in its own record. And capMaxGnss survived a protocol restart while the masks beside it were cleared, which both reported a swapped receiver's limit and ended the capability poll before the new receiver had answered. The message registry gets the patch bump its own rule asks for.
New payload fields extend what the registry describes while leaving anything reading the old layout working, which is the minor bump its own rule asks for.
307b3ea to
d651af6
Compare
2 participants
What happens
The u-blox driver already asks every receiver what it is and what it has. UBX-MON-GNSS gives the major constellations it supports, which of them are running, and how many it can track at once; UBX-MON-VER gives the firmware, the module name and the list of augmentation systems. INAV keeps the first three in
ubx_capabilitiesand throws the rest away, and none of it leaves the flight controller.So the configurator has one byte to work with, the hardware version, and has to offer the same three switches to everyone. A NEO-F10N has no GLONASS at all, yet the GLONASS switch is there; NavIC exists on the F10 and on nothing else INAV supports, so it is offered to no one; and the tab can only say "u-blox M10" when the receiver has been telling us it is a NEO-M10N all along.
The changes
MSP_GPSSTATISTICS says what the receiver reported. Five fields appended after
hwVersion: the supported and enabled MON-GNSS masks, the augmentation systems from the MON-VER extensions, how many constellations the receiver can run at once, and the module name as a length-prefixed string. Appended, so anything reading the old layout keeps working. Zero everywhere when the provider is not u-blox or the receiver has not answered yet, which the configurator reads as "unknown" rather than "not supported".The MON-VER extensions are parsed. Each 30-byte extension line is scanned for
MOD=, which is the module's own name, and for theSBAS;QZSS;NAVICstyle list of augmentation systems, which UBX-MON-GNSS does not carry. Both are cleared when the protocol thread restarts, so a receiver swapped on a bench does not inherit the previous one's answers.gps_ublox_use_navic. NavIC is the Indian regional system, visible over India and the region around it. Its keys,CFG-SIGNAL-NAVIC_ENAandCFG-SIGNAL-NAVIC_L5_ENA, exist on the F10 and not on the M10, and a single unknown key makes a receiver reject the whole CFG-VALSET, so NavIC goes out in a message of its own and only to a receiver that has listed NAVIC in its MON-VER extensions. Default off, as the F10 ships it.Testing
SITL on Windows against emulated M8, M10 and F10 receivers that reject unknown keys the way the real ones do, reading what INAV writes to the wire:
gps_ublox_use_navicThe constellation VALSET goes out unchanged in all four, which is the point of sending NavIC separately.
gps_ublox_unittestandgps_null_port_unittestpass.Not tried on a real F10 yet.
Notes
The configurator side, which uses these fields to offer only the constellations the receiver has and to put the module name in the tab title, is a separate pull request; it degrades to today's behaviour against a flight controller without this change, since the fields come back as zero.