search

TheThingsNetwork / lorawan-stack

The Things Stack, an Open Source LoRaWAN Network Server
https://www.thethingsindustries.com/stack/
Apache License 2.0
978 stars 308 forks source link

'Temperature' variable type error #3633

Closed wong-hao closed 3 years ago

wong-hao commented 3 years ago

Summary

Temperature of the Gateway is not signed integer, it is float. ...

Steps to Reproduce

  1. You just need to read the protocol document carefully to find this obvious error.
  2. ...
  3. ...

What do you see now?

...

What do you want to see instead?

Change the temp variable type from int to float. ...

Environment

The reference design SX1302CxxxGW1 using the SX1302 digital baseband chip. ...

How do you propose to implement this?

Just change the variable type here. ...

How do you propose to test this?

...

Can you do this yourself and submit a Pull Request?

I hope you do So because It's such a small change that it's boring to submit a pull request. ...

KrishnaIyer commented 3 years ago

https://github.com/Lora-net/sx1302_hal/blob/master/packet_forwarder/PROTOCOL.md This is the SX1302 reference.

temp field isn't defined in the SX1301 reference.

This field must've been added somewhere along the way? @johanstokking maybe you have some context here.

Just change the variable type here.

We can't simply edit gateway facing fields. Check our compatibility commitment for more details.

We'd probably need to able to check if a gateway uses SX1301 or the SX1302 ref (added to the Gateway proto) and refactor the translation. This isn't a simple change. We can discuss this now and move to Next Up.

wong-hao commented 3 years ago

https://github.com/Lora-net/sx1302_hal/blob/master/packet_forwarder/PROTOCOL.md This is the SX1302 reference.

temp field isn't defined in the SX1301 reference.

This field must've been added somewhere along the way? @johanstokking maybe you have some context here.

Just change the variable type here.

We can't simply edit gateway facing fields. Check our compatibility commitment for more details.

We'd probably need to able to check if a gateway uses SX1301 or the SX1302 ref (added to the Gateway proto) and refactor the translation. This isn't a simple change. We can discuss this now and move to Next Up.

I can provide an experimental phenomenon for your analysis When I use sx1302, if I don't handle this error, TTN console can't receive my message. When I comment out the temperature field in the semtech UDP packet forwarder code (let the uploaded data not contain the temperature attribute, which is exactly the same as sx1301), TTN can receive the gateway message.

johanstokking commented 3 years ago

Just change the variable type here.

We can't simply edit gateway facing fields. Check our compatibility commitment for more details.

We aren't sending these values back to the gateway.

It's safe to change this to *float32.