Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

protocol: LoRaWAN: Allow configuring scanGuard #1158

Merged
merged 1 commit into from
Jul 13, 2024

Conversation

alistair23
Copy link
Contributor

The current code uses scanGuard to increase the Rx window. Sometimes the default 10ms scanGuard isn't enough. So allow uses of LoRaWAN to set a larger scanGuard.

@HeadBoffin
Copy link
Collaborator

Does it come with a honking great warning about the consequential impact on the battery life of the device when someone over enthusiastically sets it to something silly like 1000?

@jgromes
Copy link
Owner

jgromes commented Jul 10, 2024

@alistair23 Thank you for the contribution, I think it's a valid idea. Though as @HeadBoffin suggests, it would be good to have a warning in the Doxygen description, to make it clear that this is for advanced users only.

It is important to realize that by exposing this, we essentially allow the users to break the LoRaWAN Rx delay if they set some unexpectedly large value - for example, larger than the Rx delay itself, I don't know how will it behave at that point.

@StevenCellist
Copy link
Collaborator

The value must be restricted to the interval [0,500). 500 means that the Rx2 window would open at the very moment the Rx1 window closes, higher than that means negative delay (which is handled but means not complying to LW). So either a very very big warning or a setter that returns ERR_UNKNOWN if out of range to urge users to visit an exorcist... ;)
(Although I'm not really for a setter since it means more functions.)

The current code uses scanGuard to increase the Rx window. Sometimes the
default 10ms scanGuard isn't enough. So allow uses of LoRaWAN to set a
larger scanGuard.

Signed-off-by: Alistair Francis <[email protected]>
@alistair23 alistair23 force-pushed the alistair/scan-guard branch from 331aeff to 5b9cad0 Compare July 11, 2024 00:10
@alistair23
Copy link
Contributor Author

Fixed!

@jgromes jgromes merged commit a93dd1a into jgromes:master Jul 13, 2024
30 checks passed
@jgromes
Copy link
Owner

jgromes commented Jul 13, 2024

@alistair23 thank you for the contribution, merged!

@alistair23 alistair23 deleted the alistair/scan-guard branch July 14, 2024 09:49
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.

4 participants