AP_GPS: Don't reset the entire buffer when handling RTCM data #15532
Conversation
This file contains 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
tridge
requested changes
Oct 13, 2020
libraries/AP_GPS/AP_GPS.cpp
Outdated
| @@ -1311,7 +1312,7 @@ void AP_GPS::handle_gps_rtcm_fragment(uint8_t flags, const uint8_t *data, uint8_ | |||
| if (len < MAVLINK_MSG_GPS_RTCM_DATA_FIELD_DATA_LEN) { | |||
| rtcm_buffer->fragment_count = fragment+1; | |||
| rtcm_buffer->total_length = (MAVLINK_MSG_GPS_RTCM_DATA_FIELD_DATA_LEN*fragment) + len; | |||
| } else if (rtcm_buffer->fragments_received == 0x0F) { | |||
There was a problem hiding this comment.
is this right? can't we be missing an earlier one? could be out of order on UDP
There was a problem hiding this comment.
It's fine either way, we won't flush the buffer until all the fragments have been received, so we must have received the last one before we can test. I did explicitly test this by manually sending packets up out of order from a GCS and I had instrumented the state to see where we were and it behaved correctly.
This is a micro optimization intended to just make handling GPS corrections a bit faster.
WickedShell
force-pushed
the
wickedshell/gps-corrections
branch
from
f7aeed8
to
a9c2cf6
Compare
tridge
approved these changes
Oct 25, 2020
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.
This is a micro optimization intended to just make handling GPS corrections a bit faster.
This tested by injecting data from a GCS with extra tracing prints added to test when we were not injecting data. This also saves 8 bytes of flash on a CubeBlack, but that's definitely not the target here.
It's slightly out of scope at the moment, and I don't have time to do it now, but testing the various paths for the RTCM injection is a good candidate for a CI test in the future (both testing that assembly works, as well as rejecting out of order packets).