Fix: the GPS timeout ignores what the protocol asks for, so some receivers are never configured - #11977
Open
MrScothh wants to merge 1 commit into
Open
Fix: the GPS timeout ignores what the protocol asks for, so some receivers are never configured#11977MrScothh wants to merge 1 commit into
MrScothh wants to merge 1 commit into
Conversation
Contributor
|
ⓘ 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 QodoHonor protocol-requested GPS setup timeouts
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you |
|
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 #11977 249 targets built. Find your board's
|
gpsSetProtocolTimeout() stores the timeout it is given, but the check in gpsUpdate() never read it: it compared against GPS_TIMEOUT until 2022 and against baseTimeoutMs since, one second for every protocol but NMEA. The longer waits the u-blox driver asks for, three seconds while it detects the receiver and the full sweep during autobaud, were cut to that second. Detection fits in a second only when every poll is answered at once. A receiver that does not answer UBX-MON-GNSS uses up the second on two of the three polls, times out, and starts over from autobaud, so it is never configured at all: against an emulated M10 that stays silent on MON-GNSS, INAV sent MON-VER six times and MON-GNSS eleven times in twelve seconds and no configuration. The check now uses the timeout that was asked for, but never less than the base one. The configuration steps ask for GPS_SHORT_TIMEOUT, half a second, and have always had a second in practice; tightening them now could restart receivers that configure fine today, so they keep it. With the change the silent receiver is detected once and configured like any other.
MrScothh
force-pushed
the
fix/gps-protocol-timeout
branch
from
September 20, 2026 09:20
65f7927 to
c351ce3
Compare
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
2 participants
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.
What happens
gpsSetProtocolTimeout()takes a timeout and stores it ingpsState.timeoutMs. Nothing ever reads it. The check ingpsUpdate()compared againstGPS_TIMEOUTuntil #8423 in 2022 and againstgpsState.baseTimeoutMssince, so every phase gets one second, two for NMEA, whatever the driver asked for:The u-blox driver asks for more in two places, and has since the variable timeout was introduced in 2018:
Detection asks for three seconds because that is what it needs: three polls for UBX-MON-VER and three for UBX-MON-GNSS, half a second each. It gets one second, which is enough only when every poll is answered at once.
A receiver that does not answer UBX-MON-GNSS spends that second on two unanswered polls, the link is declared lost, and the state machine starts over from autobaud. It never reaches
gpsConfigure(), so it is never configured at all: no rate, no dynamic model, no constellations, no message configuration. Against an emulated M10 that answers MON-VER but stays silent on MON-GNSS, in twelve seconds:maintenance-10.xThe same receiver answering MON-GNSS is configured identically before and after, in one pass.
The change
One line: the check uses the timeout that was asked for, but never less than the base one.
MAX()rather than the stored value alone on purpose. The configuration steps ask forGPS_SHORT_TIMEOUT, half a second, and have had a full second in practice for eight years. Honouring that literally would tighten them for the first time and could restart receivers that configure fine today, which is not what this fixes. Only the phases that ask for longer change: the autobaud sweep and detection.Nothing else reads
timeoutMs, so this is the whole change.Testing
SITL on Windows, against an emulated u-blox that answers MON-VER, streams NAV-PVT, and is switched between answering MON-GNSS and staying silent. Message counts above are from that run, taken from the receiver side.
gps_null_port_unittestandgps_ublox_unittestpass.Notes
gps.cis common to every GPS provider, so this is worth a careful look. In practice the only driver that asks for anything other than the base timeout is the u-blox one; NMEA, MSP, CRSF and DroneCAN always passbaseTimeoutMs, so for them the comparison is unchanged.Independent of my other open GPS pull requests: they touch
gps_ublox.c, this one touchesgps.c.