adafruit / Adafruit_CircuitPython_GPS

GPS parsing module for CircuitPython. Meant to parse NMEA data from serial GPS modules.
MIT License
75 stars 58 forks source link

fix minute parsing #102

Closed jkittner closed 5 months ago

jkittner commented 1 year ago

This fixes the issue that came up in #99. resolves #95

I tried to simplify this a little to get the best possible precision. We cannot be more accurate than what's in the actual NMEA message. The previous approach was to parse the sentence (which contains degrees and minutes) to decimal degrees and then convert them back. During this conversion they forgot to divide by 60, which caused the incorrect minutes (as seen in #99). Since we already get the degrees and minutes, we can directly parse them and provide them without any loss of precision.

I added tests, that verify, that for all sentence types, the degrees and minutes match exactly the values provided in the original raw NMEA message.

They do not almost match, when it comes to the number of digits, but that's maybe due to different versions?

dhalbert commented 11 months ago

Hi @jkittner Do you have time to finish this off? Thanks.

jkittner commented 11 months ago

@dhalbert sorry - I was quite busy with my thesis, conferences and other stuff. I had a look a while back and figured it's not an easy fix with the way parsing is done currently.

I think we should consider using and existing or implementing a proper NMEA parser first and then just accessing the results and checking if they are available depending on the current message type. The way it is implemented now, the parsing is done incrementally in bits and pieces here and there because the different msg types were probably added incrementally

I'll have another look and check if there maybe is an easier, temporary fix for that...

jkittner commented 9 months ago

@tekktrik @dhalbert - I came up with a temporary fix for this. Things should now work as expected.

jkittner commented 7 months ago

@tekktrik @dhalbert do you maybe have time to have a final look at this?

jkittner commented 7 months ago

The pylint version is so old it doesn't work with my python version (3.12) anymore so I had to use -- no-verify and of course I violated the black style - should have used SKIP=pylint instaed...

tekktrik commented 5 months ago

The pylint version is so old it doesn't work with my python version (3.12) anymore so I had to use -- no-verify and of course I violated the black style - should have used SKIP=pylint instaed...

We're working on a fix for this! Sorry for the delay, this looks great! Thanks for your work and patience!