Prevent GPS injection from choking the GPS driver #12710
Merged
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
Sending a continuous stream of injection messages can cause the GPS driver to get stuck indefinitely in the handling loop.
bkueng
reviewed
Aug 15, 2019
| @@ -416,7 +416,10 @@ void GPS::handleInjectDataTopic() | |||
| { | |||
| bool updated = false; | |||
|
|
|||
| const size_t max_num_injections = 6; | |||
There was a problem hiding this comment.
Magic numbers :-) Can you add a comment why it's 6?
There was a problem hiding this comment.
Problem is I don't remember exactly the argument for 6. I think the injection data that belongs together is split into multiple messages for multiple sattelite networks. So Galileo, Baidu etc.
Let me ask someone who knows for sure.
bkueng
previously approved these changes
Aug 19, 2019
dagar
approved these changes
Aug 23, 2019
bozkurthan
pushed a commit
to bozkurthan/Firmware
that referenced
this pull request
Sep 4, 2019
Sending a continuous stream of injection messages can cause the GPS driver to get stuck indefinitely in the handling loop.
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.
Solves #11716
FYI @tdnet12434
Describe problem solved by the proposed pull request
Sending a continuous stream of injection data can cause the GPS driver to get stuck indefinitely in the handling loop.
Test data / coverage
We tested with a proprietary module able to spam GPS injection data for a few seconds.
@tdnet12434 came to the same conclusion individually: #11716 (comment)
Describe your preferred solution
This proposed solution is a bit hacky, since it uses a hard-coded integer for the maximum number of consecutive injection message to handle. We are limiting to 6 consecutive messages, since a GPS injection data set can be split and sent as multiple messages. Reading only a single message per loop might yield incomplete injection data. According to our knowledge, no injection data set is larger than 3 messages.
Describe possible alternatives
Any solution not involving a magic number.