Conversation
At 230400 a GPS can deliver up to 460 bytes between two runs of the task that drains its port, and a 256 byte buffer drops whatever does not fit. On a bench with a NEO-F10N at 230400 the receiver was never identified: its 258 byte MON-VER reply cannot survive in a buffer smaller than itself while the regular navigation messages keep arriving. With 1 KB buffers the same receiver was identified immediately, with no timeouts and no errors. The F7 and H7 have the RAM for it: 4.5 to 6 KB for all the ports a target defines, against 96 KB free on the smallest F7 target in the maintenance build and 363 KB on the smallest H7 one. The F405 targets have 13 to 18 KB left and the AT32F435 targets between 1.5 and 3 KB, so they keep what they have. The note about occupied sizes being returned as uint8_t no longer held: the driver functions and the ring indices are all 32 bit.
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 QodoIncrease F7/H7 UART receive buffers to 1 KB
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 add REVIEW.md to your repo root and Qodo follows it on every PR |
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
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 this fixes
A receive buffer has to hold what arrives between two runs of the task that drains it, which
for the GPS is every 20 ms. At 115200 that is 230 bytes and 256 is enough, which is why the
current size has served; at 230400 it is 460 bytes and the buffer overruns on every burst.
What that costs is not a few bytes here and there. On an H743 bench with a NEO-F10N at
230400, INAV never identified the receiver at all: packets kept arriving, more than 2800 of
them in 80 seconds, while the hardware version stayed unknown and the timeout counter climbed
to 26. UBX-MON-VER, the reply that carries it, takes 11 ms of line at that rate, so an
overrun always lands in the middle of it and the answer is lost, every five seconds, forever.
With 1 KB buffers the same receiver was identified immediately, with no timeouts and no
errors.
What I changed
The receive buffers are 1 KB on F7 and H7 and stay at 256 bytes everywhere else.
The note about occupied sizes being returned as
uint8_tno longer held: the driverfunctions and the ring indices are all 32 bit.
Testing
On a TBS Lucid H7 Wing the change adds 5376 bytes of RAM, 768 for each of the seven ports the
target defines, and the receiver at 230400 is identified in about 2 s rather than not at all.
Built for MATEKF722SE, where RAM goes from 45.7% to 47.7%, and for MATEKF405SE, which is
unchanged.