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

add NMEA Input library #12289

Closed
wants to merge 6 commits into from
Closed

Conversation

IamPete1
Copy link
Member

This adds a new NMEA input library and uses it for WindVane, GPS and Rangefinder. I have only tested a with windvane. It is hoped this will make it easier to add new NMEA inputs in the future (AIS). Hopefully it also removes a bit of duplicated code.

This is getting to the limit of my C++ so apology's if i have done it in a odd way.

@rmackay9
Copy link
Contributor

From a practical point of view, this would be much easier to get into master if the GPS parts were removed. Touching the NMEA GPS is such a critical part of the code that it will require a much higher level of review and testing vs the changes for the windvane and range finder which are rarely used. If it were me I'd take it in steps, get the range finder and windvane changes in, then put the NMEA GPS changes in a follow-up PR.

@IamPete1
Copy link
Member Author

rebased and dropped the GPS bit

@IamPete1
Copy link
Member Author

IamPete1 commented Nov 3, 2019

rebased

@IamPete1
Copy link
Member Author

IamPete1 commented Nov 4, 2019

fixed bad rebase

@IamPete1
Copy link
Member Author

I'm going to close this, there is no saving in code without the GPS drivers and its a big job to re-test the rangefinders (and I don't have any) let alone the GPS stuff if that were to be included.

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.

None yet

3 participants