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.

Sign up for GitHub

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

Jump to bottom

Add NMEA GST Message for GPSInformation.Accuracies #9874

Closed
wants to merge 17 commits into from
Closed

Add NMEA GST Message for GPSInformation.Accuracies #9874

wants to merge 17 commits into from

Conversation

jiargei
Copy link
Contributor

@jiargei jiargei commented Apr 26, 2019 •
edited by jef-n
Loading

Description

The core feature "GPSInformation" allways shows -1.0m vor Vertical and Horizontal Accuracy... This is handled by the NMEA message GST which was not implemented yet... I added that.

As "accuracy" definition I used the approach described at Calculating Vrms and Hrms from nmea data

Fixes #21933

@luipir
Copy link
Contributor

luipir commented Apr 26, 2019

hi @jiargei thanks for you contribution
I'm not used with this part of code... but did you check if an updated version of the external lib http://nmea.sourceforge.net can solve already the problem?

@luipir
Copy link
Contributor

luipir commented Apr 26, 2019

seems not, nmea upstream trunk does not manage GPGST... make sense to contribute upstream to nemea library and then update our repo?
Just my2c

@jiargei
Copy link
Contributor Author

jiargei commented Apr 26, 2019

Hej @luipir
No, i posted that issue there too.... sorry for that proken build.. I thought I don't have to stay to the checklist... proven wrong as so often...
I'm building it right now... if it works, i send my changes to nmealib too....

@luipir
Copy link
Contributor

luipir commented Apr 26, 2019

if it works, i send my changes to nmealib too

great

@jiargei
Copy link
Contributor Author

jiargei commented Apr 26, 2019

Hej @luipir !

It's working...

@luipir
Copy link
Contributor

luipir commented Apr 26, 2019

BTW I would wait and align to upstream nmea lib if you are not in a rush

jef-n
jef-n reviewed Apr 27, 2019
View reviewed changes
external/nmea/parse.c Outdated
@@ -133,25 +133,28 @@ int nmea_pack_type( const char *buff, int buff_sz )
"GPGSV",
"GPRMC",
"GPVTG",
"GNRMC",
"GNRMC",
"GPGST",
};

NMEA_ASSERT( buff );

if ( buff_sz < 5 )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

< 6

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed that...

@jiargei
Update parse.c
43814a8 
- Corrected Buffer Sizecomparison to 6
- defined Variable for buffer_size
@stale
Copy link

stale bot commented May 19, 2019

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@stale stale bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label May 19, 2019
@luipir
Copy link
Contributor

luipir commented May 19, 2019

@jiargei any news from upstream ? can you share the PR link to the upstream library?

@stale stale bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label May 19, 2019
@nyalldawson nyalldawson added Feature Frozen Feature freeze - Do not merge! labels May 19, 2019
@nyalldawson nyalldawson added Bugfix and removed Feature Frozen Feature freeze - Do not merge! labels May 19, 2019
@nyalldawson
Copy link
Collaborator

I've re-tagged this as a bug fix -- it fixes an existing feature which doesn't work

@nyalldawson nyalldawson added this to the 3.8.0 milestone May 19, 2019
@qgib qgib mentioned this pull request May 25, 2019
Closed
@nyalldawson nyalldawson added Bug Either a bug report, or a bug fix. Let's hope for the latter! and removed Bugfix labels May 27, 2019
@jiargei
Copy link
Contributor Author

jiargei commented Jun 3, 2019

Hej! I appreciate that! Nice work btw!

@stale
Copy link

stale bot commented Jun 17, 2019

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@stale stale bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jun 17, 2019
@lbartoletti
Copy link
Member

@jiargei any news from upstream ? can you share the PR link to the upstream library?

For reference https://sourceforge.net/p/nmea/bugs/18/

@stale stale bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jun 19, 2019
@elpaso elpaso mentioned this pull request Jun 19, 2019
Merged
@nyalldawson
Copy link
Collaborator

Superseded by #30274

@jiargei
Copy link
Contributor Author

jiargei commented Jun 20, 2019

Hej @nyalldawson... If this issue is closed does it mean it wont get implemented?

@nyalldawson
Copy link
Collaborator

@jiargei - no, see the note in #30274

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jef-n jef-n jef-n left review comments

Assignees
No one assigned
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter!
Projects
None yet
Milestone
3.8.0
Development

Successfully merging this pull request may close these issues.

Zoom to layer fails using views
5 participants
@jiargei @luipir @nyalldawson @lbartoletti @jef-n