thesolarnomad / lora-serialization

LoraWAN serialization/deserialization library for The Things Network
MIT License
165 stars 27 forks source link

Sometimes, you've just gotta use a 4-byte float. #32

Closed davidbrodrick closed 3 years ago

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-1.0%) to 97.945% when pulling 83f4ea95bb2b58c8cd045aa14ec9aadd1fdbc98a on davidbrodrick:master into 4ced49331d12105b23be018733ad97a8d924c7c3 on thesolarnomad:master.

joscha commented 3 years ago

Nice! Can we add tests for this new data type, please?

davidbrodrick commented 3 years ago

Yeah sorry, I would have added those initially but somehow missed the unit test suite in the library completely until after I'd made the pull request. I'm just taking a look what I need to do to add some coverage.

davidbrodrick commented 3 years ago

Yep, I'll apply an epsilon. Ran out of time last night. Will get back to it shortly.

davidbrodrick commented 3 years ago

Hopefully all good now. 99.99 is IEEE 754 encoded as 99.98999786376953125 so I added an appropriate epsilon in the check. Coverage decreased because I moved the rawfloat from the deep copy comparison in the LoraMessage decode.

joscha commented 3 years ago

Nice work. I can merge once the tests are green.

davidbrodrick commented 3 years ago

Yeah I'm a bit confused. I think all test passed but it indicates a fail because of the coverage.

joscha commented 3 years ago

Ah yes, because of the decrease. That's fine, I'll give in to the 1% :)