helium / mappers

Mappers Frontend and API
Apache License 2.0
69 stars 26 forks source link

Onicolaos handle float correctly #129

Closed onicolaos closed 6 months ago

onicolaos commented 6 months ago

In the case when the altitude is float it is still a number that can be coerced or casted to an integer. here we validate the value anyway if it is a number and round it to store it as an integer into the database

jthiller commented 6 months ago

This looks good to me. I just took a look at docs and I don't think we need to change anything. It lists Int but this change is more about handling the case wherein they haven't set that field correctly. https://docs.helium.com/iot/coverage-mapping/api/

@kent-williams would love a second set of eyes if you get some time

onicolaos commented 6 months ago

it seems that chirpstack does somewhere an implicit conversion int -> float if that makes sense

stefanbraun-private commented 6 months ago

it seems that chirpstack does somewhere an implicit conversion int -> float if that makes sense

It seems that all integers and floats are converted into JSON number type (float)...

I found a closed issue in ChirpStack-repository about implicit conversion of CayenneLPP-encoded integers to float: https://github.com/chirpstack/chirpstack/issues/77 (there I added my question why metadata like "fPort", "dr", "fCnt" are NOT converted...?!?)

kent-williams commented 6 months ago

LGTM @jthiller