Skip to content

spotToStart needs to be at least uint16_t in extract* functions #109

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

Closed
egonbeermat opened this issue Feb 11, 2022 · 5 comments · Fixed by #111
Closed

spotToStart needs to be at least uint16_t in extract* functions #109

egonbeermat opened this issue Feb 11, 2022 · 5 comments · Fixed by #111

Comments

@egonbeermat
Copy link

In these function definitions, spotToStart needs to be at least uint16_t:

	uint32_t extractLong(ubxPacket *msg, uint8_t spotToStart); 
	int32_t extractSignedLong(ubxPacket *msg, uint8_t spotToStart); 
	uint16_t extractInt(ubxPacket *msg, uint8_t spotToStart); 
	int16_t extractSignedInt(ubxPacket *msg, int8_t spotToStart);
	uint8_t extractByte(ubxPacket *msg, uint8_t spotToStart); 
	int8_t extractSignedChar(ubxPacket *msg, uint8_t spotToStart); 

ubxPacket can be > 255 bytes. The UBX-NAV-SAT message contains an array of satellites; once this array size passes 20, corrupted data is returned from processUBXpacket when processing the incoming data using these extract functions, as "spotToStart" has a value > 255.

Can be tested with the CallbackExample7_NAV_SAT.ino - If the satellite list is > 20 satellites, corrupted data is displayed. Changing these function definitions to uint16_t resolves the issue.

@PaulZC
Copy link
Collaborator

PaulZC commented Feb 11, 2022

Hi Ian (@egonbeermat ),

Yes. My bad. I spotted this a few days ago while working on other messages which can cross the 256 boundary. It is fixed in the release_candidate branch and will be included in the next release, sometime next week.

Best wishes,
Paul

@egonbeermat
Copy link
Author

Paul - thanks, that's great! I've been playing with the library for a couple of days, with (mostly) good results so far. I have had an issue when trying to use two callbacks for different items (one is fine), and still not quite figured out how to get getDOP() and getPVT() working together with their respective autoDOP / autoPVT functions, but I'm putting that down to unfamiliarity on my part so far. It certainly is a lot easier using this library than going native, so thanks and keep up the good work!

20220211_144535

@PaulZC
Copy link
Collaborator

PaulZC commented Feb 11, 2022

Hi Ian,

Thank you for the nice feedback! It is always good to have happy users.

That looks like a great project. Please do share it open-source if you can.

I've been making improvements to the callbacks too. The old way was for the callback to receive a complete copy of the UBX data. That worked very well on some platforms (ESP32) but less well on others (Apollo3 / Artemis included). The new way is for the callback to receive a pointer to the UBX data. It is much kinder on the stack.

Have a look through the callback examples. There are some that use multiple callbacks.

Please do give the release_candidate branch a try. Let me know if you see any gremlins or other weirdness. The official release is coming in a few days but I've got a couple of things to resolve first.

Have a great weekend,
Paul

@egonbeermat
Copy link
Author

Paul - I downloaded the release candidate 3 days ago, and modified my code to use the callback pointer method. Everything seems to be working like a charm, so thanks!

My issue when using multiple callbacks seemed to boil down to how frequently I polled checkCallbacks(). I was initially having issues using more than one callback; turning on debugging showed a checksum issue was preventing the message being parsed and processed. My polling rate was 250ms. Once I lowered this to 50ms, the checksum issue went away. I think this occurs if the polling frequency is lower than the frequency that NavigationRate & MeasurementRate cause messages to be produced. Not sure if this is an actual issue or not, but just making you aware.

Also, do you accept pull requests? I have implemented the following and tested/verified via u-center. They are config values relevant to the accuracy of the fix produced:

  bool setMinimumElevation(uint8_t newMinElevation, uint16_t maxWait = defaultMaxWait);
  uint8_t getMinimumElevation(uint16_t maxWait = defaultMaxWait);
  bool setPositionDOPMask(uint16_t newPositionDOPMask, uint16_t maxWait = defaultMaxWait);
  uint16_t getPositionDOPMask(uint16_t maxWait = defaultMaxWait);

Thanks again for the library!

@PaulZC
Copy link
Collaborator

PaulZC commented Feb 14, 2022

Hi Ian,

You will run into problems if you don't call checkCallbacks frequently enough. You will either overflow your processor's internal serial RX buffer when using serial, or the module's internal I2C or SPI buffers. It will depend on how many messages you have enabled and how long they all are... My advice is to call checkCallbacks as often as possible. There are internal timers which prevent the I2C and SPI busses from being hammered too hard. The interval is set automatically when you call setMeasurementRate or setNavigationFrequency. But you can set it manually too by calling setI2CpollingWait or setSPIpollingWait.

We do indeed accept pull requests - thank you. Please see CONTRIBUTING.md for guidance.

Best wishes,
Paul

@PaulZC PaulZC linked a pull request Feb 20, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants