-
Notifications
You must be signed in to change notification settings - Fork 85
Add support for ublox 7 series chips #113
Comments
I'm all for a PR that demos compatibility with series 7 (and/or 6 as well) but I can't dedicate time to it right now. |
Based on the code in meshtastic@512833e I've created a patch (attached) which works against current SparkFun_Ublox_Arduino_Library and which could be used to create a PR. The only thing I've added was a fix to parse horizontal and vertical accuracies from POSLLH:
@geeksville do you want to create a PR with this updated patch so that you can get credit for it? The only thing remaining to be done is to remove hardcoding and to use the hardware capability determination I suggested above to invoke the proper parsing routine based on chip series. |
thank you for your nice note. I don't need credit though a mention in the commit is nice. |
Done, @nseidle pls take it from here. |
@letranloc that may work fine for you locally but with the above suggestion in library code you could get an out-of-bounds read with certain messages (and modules). The current code doesn't even reach offset 83 in that message payload (skips headVeh, magDec and magAcc among others) so the issue in the above approach is masked and the code silently works. That's why parsing based on chip capabilities is the correct approach - the maintainers should double-check the protocol versions that trigger the particular invocation. An elegant way to map parsing code to the evolving message payload contents could be a further improvement; so not to use too many switches and/or ifs. Also, the above approach doesn't address the lack of support for UBX-NAV-HPPOSLLH in 7 (and 6) which is handled by @geeksville's code. |
First of all, thanks to everyone who has contributed to this library thus far. Thanks @nseidle for doing a good job responding to requests and supporting the current code base.
I've been a SparkFun customer since at least 2014 and I have made more than a dozen orders with your company to date. I've also visited your campus in 2015. I say this not to throw my weight around, but simply to show that I'm a supporter.
Onto the issue: I have a 7 series chip and the current Ublox library doesn't support it - I'm not getting any valid data using the get methods on SFE_UBLOX_GPS. I noticed in #80 that @geeksville has done the preliminary investigation into the issue (his chip is a bit older though: neo6) and has made a good first stab at a proof of concept.
Can you guys leverage the findings and code in #80 to add support for older chips into your library? I know you said that if it's a reasonable merge, you can do it.
I agree that the candidate code needs be cleaned up a bit first before inclusion in the library code, to remove hardcoded stuff etc. Ideally, the target solution should check the capabilities of the chip and keep that state to be used in the execution of getPVT() or a similar strategy. Looks like the equivalent of the call SFE_UBLOX_GPS.minfo.hwVersion (anything below "00080000" for non-PVT solution) or something leveraging the value in SFE_UBLOX_GPS.getProtocolVersionHigh() would be good candidates for such capabilities determination - I don't have the u-blox specs in front of me. Adding this sort of strategy now may end up being needed in the future anyway.
Lastly, you're probably already aware of this, but your company and specifically your u-blox library are already trending on the u-blox.com website, giving you the well deserved credit and exposure. It may be a matter of time before more requests like this start coming in..
https://portal.u-blox.com/s/question/0D52p00008HKD5jCAH/i2c-configuration-using-arduino
https://portal.u-blox.com/s/question/0D52p000090aP1bCAE/maxm8-linux-cc-i2cddc-example-source-code
https://www.u-blox.com/en/blogs/stories/sparkfun%E2%80%99s-list-u-blox-shields-just-got-longer
The text was updated successfully, but these errors were encountered: