Skip to content

Conversation

@jpmeijers
Copy link
Collaborator

No description provided.

@johanstokking
Copy link
Member

johanstokking commented Jun 7, 2017

This changes behavior as it will try once more.

What's wrong with the original code, is that if retries == 0, it goes in infinite mode. I'd suggest introducing an attempts variable set to retries + 1 and check retries == -1 || attempts-- > 0

@jpmeijers
Copy link
Collaborator Author

In the original code any positive value will make it go into an infinite retry. The original check was just wrong.

The fix does not change the behaviour. Confirmed on a device:
ttn.join(appEui, appKey, 2, 10000); - Tries 2 times to join.
ttn.join(appEui, appKey, 0, 10000); - Does not try to join.
ttn.join(appEui, appKey, -1, 10000); - Infinitely tries to join.
ttn.join(appEui, appKey, -2, 10000); - Does not try to join.
ttn.join(appEui, appKey); - Infinitely tries to join.

@jpmeijers
Copy link
Collaborator Author

I think I see the issue. "Try" vs "retry".

"Number of times to retry after failed or unconfirmed join. Defaults to -1 which means infinite."

So according to the documentation if the value is 0, you want it to try to join once.

jpmeijers added 2 commits June 7, 2017 11:26
Attempts is incremented every time before a join.

Behaviour is now: 
`ttn.join(appEui, appKey, 2);` - Tries 3 times to join.
`ttn.join(appEui, appKey, 0);` - Tries 1 time to join.
`ttn.join(appEui, appKey, -1);` - Tries infinitely to join.
`ttn.join(appEui, appKey, -2);` - Tries 0 times to join.
We will need this fix for the KISS LoRa gadget update.
@jpmeijers
Copy link
Collaborator Author

Behaviour is now:
ttn.join(appEui, appKey, 2); - Tries 3 times to join.
ttn.join(appEui, appKey, 0); - Tries 1 time to join.
ttn.join(appEui, appKey, -1); - Tries infinitely to join.
ttn.join(appEui, appKey, -2); - Tries 0 times to join.

Also did a minor version number bump.

@johanstokking johanstokking merged commit dedd359 into master Jun 7, 2017
@johanstokking johanstokking deleted the fix/infinite-join branch January 15, 2022 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants