ntrip: Added support for NTRIP Rev1 servers #1440
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
|
This is potentially worth merging, but please squash your commits rather than including 3 that have the exact same message and only modify one file. |
|
Apologies. I just assumed you'd squash merge into main (that's what I normally do). I will see if I can squash on my side first. |
|
Okay squashed on my side now. Thanks again for considering a merge! |
|
Tested with ntrip.data.gnss.ga.gov.au - no issues there. Happy to merge once the CI's green EDIT: Could you change the commit message to "ntrip: Added support for NTRIP Rev1 servers"? We usually prefix with the subsystem that's been changed. |
jheising
changed the title
|
Okay @stephendade, the commit has been amended. Thanks! |
|
Thanks! merging |
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.
I ran into an issue getting the popular RTK base station system, rtkbase working with MAVProxy. Doing some deeper analysis, it looks like rtkbase uses the
str2strutility provided in the popular RTKLIB project.What I found is that the str2str NTRIP caster returns a response that looks like this:
Notice how the server response does not include any headers and just goes straight to the response body without a blank line? Because of this, the ntrip.py library will never find a blank line, and
self.found_headerwill never equalTruewhich means the ntrip library will always be "stuck" waiting for headers.At first I debated whether or not this library should be updated in order to fix the "bad behavior" of an NTRIP server. But after a bit of research I found that according to this, it is actually valid and expected behavior under the NTRIP Rev1 protocol. So in a sense we're just adding support for NTRIP Rev1 with this small update.
Potential issues:
I think it is also possible for NTRIP Rev1 to optionally contain headers, so there might be a case where if the headers are over 4096 characters in length that this might falsely think the headers are finished and parsed, when they aren't. But it's not clear to me that this would cause anything bad to happen since the headers are not used in any capacity.