Added support for multi-constellation satellite stats. Fixed SNR reporting bug.#44
Conversation
Adds multi-constallation satellite stats parsing.
Config option "LWGPS_CFG_SATS_IN_VIEW_SIZE" for number of stored satellites in view stats slots.
| * ``GPGSA`` or ``GNGSA``: GPS active satellites and dillusion of position | ||
| * ``GPGSV`` or ``GNGSV``: List of satellites in view zone | ||
| * ``GPRMC`` or ``GNRMC``: Recommended minimum specific GPS/Transit data | ||
| * Built-in support for all-constellation GPS statements |
There was a problem hiding this comment.
Here I prefer we keep the original names. It can be as simpleas GPGGA, G*GGA or GNGGA.
|
Do you have a test to confirm it work properly? |
Nope, i just use it currently. It seems to work just fine with simultaneous GPS, Galileo, and GLONASS, but that doesn't prove anything of course. I have now captured a bit of multi-constellation NMEA output from my M10 receiver, plus stats printout: I currently don't use lwgps with the original build system, and thus can't just run the tests without first setting it up just for this purpose, which i don't have the time and nerve for right now. I offer you these fixes as a reference, you may use them without crediting me if it's worth integrating it. I don't mind. Have a nice sunday! |
|
you don't need to build anything on your side in fact. If you enlarge the test suite with the data that show what you wanted to achieve, the Github will run it for us, and we'll see the outcome quickly. I'll sure merge, please make the change as I requested in the review. |
Changed NMEA message list style
Forgot to upload the changed header. Ooops.
|
Hey, i tried out the github actions as you suggested. My forked repo passes the unmodified test suite now. I'll try and add a test case with the multi-constellation NMEA when i come around to it :-) |
MaJerle
left a comment
MaJerle
left a comment
There was a problem hiding this comment.
Sorry for last minute feedback, I just noticed.
| typedef struct { | ||
| uint8_t num; /*!< Satellite number */ | ||
| uint8_t elevation; /*!< Elevation value */ | ||
| uint16_t num; /*!< Satellite number */ |
There was a problem hiding this comment.
Can the satellite number go beyond 255?
There was a problem hiding this comment.
Ah, you're right. This also gives better struct packing.
| #include <stddef.h> | ||
| #include <stdint.h> | ||
| #include "lwgps/lwgps_opt.h" | ||
| #include "lwgps_opt.h" |
There was a problem hiding this comment.
The normal library rule, the way instructions and cmake build is configured is that the include path is set to /src/include folder, forcing the compiler and developer to use the /lwgps/lwgps_xxxxx.h during use.
The same shall remain for the library. The previous way was the correct one. Please return back and then I think we are ready to merge.
There is no test cases for multi constellation NMEA available publicly that can be used to test against? |
Reverted to correct include path.
Reverted back to uint8_t satellite prn.
Reverted back to uint8_t satellite prn.
2 participants
Hello,
i'm currently using your library to parse NMEA from a ublox M10 receiver, which is running at default settings.
I noticed that the number of satellites-in-view vs used-for-positioning satellites is inplausible, when going from an GNSS-denied indoor environment to outdoors: The number of used satellites is intermittently shown to be higher than the number of satellites in view!
I tracked this down to the library not parsing all constellation variants of the G*GSV messages. This clashes with the satellites-in-view value reported by GNGGA, which is for all constellations.
Another issue was the SNR always being reported as 0, even when outdoors with good position and HDoP.
This had to do with GPS multi-band reporting. The multi-band data overwrites the values reported for standard L1 band. The sats_in_view_desc is now cleared only once per NMEA cycle and multi-band/multi-constellation data now handled correctly.
This proposal supports GPS, Galileo, BeiDou and GLONASS and has configurable satellite stats size, with the config option "LWGPS_CFG_SATS_IN_VIEW_SIZE", so the user can track as many satellites as needed.
Regards, Florian.