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 QodoDetect u-blox baud rates by listening before autobaud commands
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
3a463a1 to
968c682
Compare
|
Both findings were real, thank you. The first was a botched transplant of the NMEA check onto this branch: the nested switch was left open, so any build with the u-blox protocol enabled failed to compile. Fixed, and built for SITL, F405, F722 and H743. The second one I would not have caught on the bench, because the board I measure on also carries #11977, which is what makes the running state honour the timeout the protocol asks for. Rather than depend on that PR, the listening is now done in windows of 250 ms, each announced through gpsSetProtocolTimeout(), and the slow pass listens to six of them in a row instead of one long wait. Nothing the thread waits for is longer than the timeout that declares the receiver lost. Measured again on the bench after the change: ten baud rate changes out of ten, 2.6 to 3.9 s each, no timeouts. |
Autobaud sent the baud rate command at every rate in turn. At each rate that is not the receiver's, the sentence arrives as noise, and a u-blox then ignores its input for about a second. In a sweep that silence falls on the one rate where the command would have been understood, so the receiver is reached only when its rate happens to be the first one tried. Measured on a NEO-F10N left at 230400 by another flight stack, with INAV configured for 115200: the sweep moved it 3 times out of 10, and a board that came up next to it went 20 to 70 seconds without a fix, sometimes never, reporting one timeout after another while the receiver was talking the whole time. So listen first. Each rate is given a window, and the receiver is recognised by what it sends: a UBX frame that passed its checksum, or two sentences shaped like NMEA, which is all a module still in its factory configuration sends. Only then does the command go out, once, at the rate the receiver is actually using. The configured rate is tried first, since a receiver INAV has already set up is the common case, and a second pass listens to several windows in a row, which covers a module sending NMEA once per second. A receiver that says nothing at any rate still gets the old sweep, run as the last pass of the same loop, since nothing else would reach it. The windows are shorter than the timeout that declares the receiver lost, and each one is announced through gpsSetProtocolTimeout(), so that listening long enough never restarts the search before it has heard anything. With this, on the same bench, ten baud changes out of ten succeeded, each in 2 to 4 seconds, with no timeouts. The change costs 416 bytes of flash.
968c682 to
fd31c5d
Compare
1 participant
What this fixes
A u-blox receiver that is not at the configured baud rate is often never found, and when it
is found it takes tens of seconds.
Autobaud walks the baud rate list and sends
$PUBX,41at every rate. At each rate that isnot the receiver's, that sentence reaches it as noise, and a u-blox then ignores its input
for about a second. In a blind sweep that deaf second falls on the one rate where the
command would have been understood, so the receiver is reached only when its rate happens
to be the first one tried.
Measured on a NEO-F10N whose flash had been left at 230400 by another flight stack, with
INAV configured for 115200, on an H743 board:
In normal operation the same board went 20 to 70 seconds before the receiver was
identified, and sometimes never, reporting one timeout after another while the receiver was
transmitting the whole time.
What I changed
Listen before speaking. Each candidate rate gets a window, and the receiver is recognised by
what it sends: a UBX frame that passed its checksum, or two sentences shaped like NMEA,
which is all a module still in its factory configuration sends. Only then does the baud
rate command go out, once, at the rate the receiver is actually using.
common case, and that case now costs 250 ms.
pass of the same loop, since nothing else would reach it.
NMEA sentences are followed in the UBX parser's idle state, where the bytes are passing
anyway, so nothing is added to the receive path. The checksum of a sentence is not worked
out: two whole sentences are asked for, which noise at the wrong rate does not produce.
Testing
On the bench, with the same NEO-F10N and an H743:
none out of five in one direction and three slow ones out of five in the other before;
extra cost is the 250 ms listening window;
gps_ublox_unittestpasses.Built for F405, F722 and H743.
Size
The change costs 416 bytes of flash. The
buildjob fails on ZEEZF7V3, which onmaintenance-10.xitself is already 179 bytes over its flash region (the same job fails onother open PRs by that amount), and this change takes it to 595 over. Measured locally with
the same toolchain as CI.