setVal is 16 bit and needs 8 and 32 bit versions

[PaulZC]

Hi,
Looking at the code for setVal, although the comment in the code says that it sets an 8-bit value, value is actually taken to be 16 bits and is copied into payloadCfg[8] and payloadCfg[9].
8 and 32 bit versions are needed. Strictly, a 64-bit version is needed too for double precision values.
My approach would be to create setVal8, setVal16 and setVal32.
Let me know if you need a pull request.
Many thanks!
Paul

[nseidle]

Did you know you can link to specific lines of code? Please do so I can see what you're referring to without guessing (don't want to be wrong!).

Are you saying this comment is a typo? Ya. Looks like it.

I would love a PR! Please feel free.

Is there a reason to have setVal8/16/32 rather than just three setVals overloaded with uint8_t value, uint16_t value, etc? I think we would need a getVal8, getVal16, getVal32.

[PaulZC]

Yes, an overloaded setVal would be way more elegant. I'll give it a try. Watch this space...

It is this comment that is misleading. The way the code is currently written, the comment should be:

//Given a key, set a 16-bit value

Thanks!
Paul

[PaulZC]

So... I tried defining overloaded versions of setVal, which would accept uint8_t, uint16_t or uint32_t values but ran into ambiguity problems. Basically, if we define a value of say 100, or even 0x01, the compiler does not know if it should be a uint8_t, uint16_t or uint32_t and barfs with an ambiguity.
You can work around this by casting the data type in the function call:

setVal(0x12345678, (uint8_t)0x01);

but that's not exactly pretty and breaks the setVal example.

Another solution would be to define a dictionary of every keyID and its associated value width, but that's a lot of work and it would need to be updated every time ublox add new keyIDs.

So, I decided to go with my original idea of defining 8, 16 and 32 but versions of setVal (setVal8, setVal16 and setVal32). setVal still exists - deleting it would break the setVal example - but now it simply calls setVal16 and returns the result.

Please see PR #30 for the code changes.

Thanks!
Paul

[nseidle]

Nice work @PaulZC! Thanks for the PR and the work on this.