Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NTRIP: use pynmeagps for correct gga generation #1323

Merged
merged 1 commit into from
Feb 28, 2024

Conversation

richahert
Copy link

The MAVproxy NTRIP client does not work with several NTRIP casters, because the GGA messages are sent in an incompatible format. this is described in #1131

This PR solves that problem, by using the same library for gga generation that pygpsclient uses (pynmeagps), which I experienced to be working with all casters.

The ntrip client of pygpsclient works quite well, so I might open an Issue in the future to request that the MAVproxy ntrip could be refactored as a wraper of that NTRIP client. It has more active developers than the built-in NTRIP Client of MAVproxy.

For now, this PR should provide a solution that is tested with skylark and I am confident that it also solves the problem with topcon, but I can't test it atm, because I don't have an active subscription.

@rmackay9
Copy link
Contributor

Hi @richahert,

Could you rebase this on master to remove the merge commits?

@richahert
Copy link
Author

Hi @rmackay9 ,
I rebased the branch so the PR has one clean commit.

@rmackay9
Copy link
Contributor

Hi @richahert,

Thanks for that. You might want to also prefix the commit message with "ntrip:" or "NTRIP:" to be consistent with other commits in this repo but again, I'm not a maintainer so this is just a suggestion and I'm not sure how strict Tridge and others are.

If this gets stuck and not merged then it might need to be brought up on one of the dev calls to get merged. The best choice is probably the small EU Dev Call which is later today.

@tridge tridge merged commit 7efff6d into ArduPilot:master Feb 28, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants