search

TheThingsNetwork / arduino-device-lib

Arduino Library for TTN Devices
MIT License
207 stars 96 forks source link

Fixing retries handling on join #205

Closed VyvojRSnet closed 7 years ago

VyvojRSnet commented 7 years ago

There was an bug, that caused inifinity loop till join was successful.

CLAassistant commented 7 years ago

CLA assistant check
All committers have signed the CLA.

johanstokking commented 7 years ago

What is the bug exactly?

From what I see this only introduces an unnecessary infinite retry.

VyvojRSnet commented 7 years ago

At commit 402619e8593ee8200fef1bb448ba7e0bd10eeadf was introduced -1 as infinite retry.

-  while (retries-- >= 0)
+  while (retries == -1 || retries-- >= 0)

But that inserts an an bug. If u set retries for example to 2 this will be result it there is no signal from LoRaWAN GW.

Model: invalid_param
Version: Sending: mac set deveui 0004A30B001EE8CB
Sending: mac set adr off
Sending: mac set deveui 0004A30B001EE8CB
Sending: mac set appeui 8a79139321d2b0f2
Sending: mac set appkey e48f53fecb61b178bd0594c3bb1d6bcd
Sending: mac save 
Sending: mac set rx2 3 869525000
Sending: mac set ch drrange 1 0 6
Sending: mac set ch dcycle 0 799
Sending: mac set ch dcycle 1 799
Sending: mac set ch dcycle 2 799
Sending: mac set ch dcycle 3 799
Sending: mac set ch freq 3 867100000
Sending: mac set ch drrange 3 0 5
Sending: mac set ch status 3 on
Sending: mac set ch dcycle 4 799
Sending: mac set ch freq 4 867300000
Sending: mac set ch drrange 4 0 5
Sending: mac set ch status 4 on
Sending: mac set ch dcycle 5 799
Sending: mac set ch freq 5 867500000
Sending: mac set ch drrange 5 0 5
Sending: mac set ch status 5 on
Sending: mac set ch dcycle 6 799
Sending: mac set ch freq 6 867700000
Sending: mac set ch drrange 6 0 5
Sending: mac set ch status 6 on
Sending: mac set ch dcycle 7 799
Sending: mac set ch freq 7 867900000
Sending: mac set ch drrange 7 0 5
Sending: mac set ch status 7 on
Sending: mac set pwridx 1
Sending: mac set retx 7
Sending: mac set dr 5
JOIN Retries LEFT: 1
Sending: mac join otaa 
Join not accepted: denied
Check your coverage, keys and backend status.
JOIN Retries LEFT: 0
Sending: mac join otaa 
Join not accepted: denied
Check your coverage, keys and backend status.
JOIN Retries LEFT: -1
Sending: mac join otaa 
Join not accepted: denied
Check your coverage, keys and backend status.
JOIN Retries LEFT: -1
Sending: mac join otaa 
Response is not OK: no_free_ch
Send join command failed
JOIN Retries LEFT: -1
Sending: mac join otaa 
Response is not OK: no_free_ch
Send join command failed
........

From 2 you fall down to infinity. BTW: I am using Arduino 1.8.3 IDE with Arduino Mini and RN2483

jpmeijers commented 7 years ago

I agree that we have a bug here. I will look at merging this in on Wednesday. Can you please sign the CLA in the meantime @VyvojRSnet?

VyvojRSnet commented 7 years ago

I've already did, but somehow looks like I didnot. cla-0007

jpmeijers commented 7 years ago

Thanks for this bug report. I can indeed reproduce this. Your fix is also correct, however I prefer a more simple solution. See https://github.com/TheThingsNetwork/arduino-device-lib/pull/207

IntelliDust commented 7 years ago

207 is nicer, shorter, but there is no good practice to modify variable in one part of compound condition.