Skip to content
This repository was archived by the owner on Jan 28, 2021. It is now read-only.

"Survey-in" time truncated #118

Closed
ubussema opened this issue Aug 5, 2020 · 4 comments
Closed

"Survey-in" time truncated #118

ubussema opened this issue Aug 5, 2020 · 4 comments

Comments

@ubussema
Copy link

ubussema commented Aug 5, 2020

Hej,

I am currently working on a ZED-F9P project and would like to use the survey-in mode.

Hoping that this is the most appropriate place for my contribution, I will quickly explain the issue:

The library limits the time to uint16 with following comment:

//svinMinDur is U4 (uint32_t) but we'll only use a uint16_t (waiting more than 65535 seconds seems excessive!)
payloadCfg[24] = observationTime & 0xFF; //svinMinDur in seconds
payloadCfg[25] = observationTime >> 8;   //svinMinDur in seconds
payloadCfg[26] = 0;                      //Truncate to 16 bits
payloadCfg[27] = 0;                      //Truncate to 16 bits

However, when reading https://portal.u-blox.com/s/question/0D52p00008nvqoTCAQ/f9p-base-surveyin-position-accuracy it seems to actually make sense to have longer observation times if you want to get more precision. I have copied their graph here below to reflect this. 65000s is around 18h.

image

Is there an important reason to truncate the survey-in time to uint16? Else it might be a good idea to include the additional 16 bits.

What do you think?

@PaulZC
Copy link
Collaborator

PaulZC commented Aug 5, 2020

Hi @ubussema ,

Thank you for raising this issue - it is a valid point.

We decided to keep the observation time as 16-bits (uint16_t) for historical reasons. The first version of setSurveyMode defined observationTime as 16-bits and so we decided to keep it that way (even though really it should be 32-bits).

If we change observationTime to 32-bits now, it probably won't be compatible with code anyone has written which expects it to be 16-bits.

I can think of a couple of ways forward: we can either change the library and probably use an overloaded function which accepts 16-bits or 32-bits; or it can be done with a custom packet. The custom packet is easier for us as it means we don't need to change the library! ;-)

Please have a look at Example20_SendCustomCommand. Everything you need is in there. You just need to copy and paste the packetCfg definition from setSurveyMode into the customCfg & customPayload (remembering to 'enable' bytes 26 and 27).

Please let me know what you think.

Best wishes,
Paul

@ubussema
Copy link
Author

ubussema commented Aug 5, 2020

Hej @PaulZC , thanks for your quick answer.

My thought is that the custom command is certainly a valid workaround. Thanks for the hint!
I will however probably not be the last one who encounters this issue. Your idea about the overload function sounds good and it might be worth amending the library with such.

What's your view on this?

@PaulZC
Copy link
Collaborator

PaulZC commented Aug 10, 2020

Hi @ubussema ,
Unfortunately I don't have time to look at this at the moment. If you would like to add the changes and submit a Pull Request, that would be great.
Best wishes,
Paul

@nseidle
Copy link
Member

nseidle commented Oct 28, 2020

Hi @ubussema - I recently merged some good changes to getVal/setVal methods. You should be able to set the survey in value directly using them. Checkout Example 8 for a starting point.

I'm going to close this issue. If you decide to add the 32 bit setting to the library, we welcome the PR.

@nseidle nseidle closed this as completed Oct 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants