sensebox / node-sketch-templater

Arduino sketch templates used by the openSenseMap-api
MIT License
2 stars 2 forks source link

Encoding of lux and uv in LoRaWAN template seems wrong #26

Open avbentem opened 4 years ago

avbentem commented 4 years ago

I assume the following code is expected to nicely yield 3 bytes that can be decoded using some simple shifting (like decoded = bytes[0] | bytes[1]<<8 | bytes[2]<<16 rather than some weird bytes[0] + 255 * (bytes[1] | bytes[2]<<8)):

message.addUint8(lux % 255);
message.addUint16(lux / 255);

However, when using modulo and division to get the LSB and MSBs, one should use 256 (or 0x100), not 255. Like 0x123456 % 255 yields the unexpected 0x9C, not 0x56. And 0x123456 / 255 yields 0x1246, not 0x1234.

Using 256 in the calculations fixes that. Other options would be:

message.addUint8(lux);
message.addUint16(lux >> 8);

...or:

message.addUint8(lux);
message.addUint8(lux >> 8);
message.addUint8(lux >> 16);

Note that https://github.com/sensebox/board-support-watterott-fork/tree/master/arduino/samd/libraries/LoraMessage also uses shifting for writeUint16, so encodes 0x1234 into the bytes 0x34 and 0x12 as expected.

But of course, please ensure that my assumption of your intentions is valid!

avbentem commented 4 years ago

I still feel that this is a bug... If you disagree, then please let me know so I can delete the pull request/branch.