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

Explicitly add the dwell time duration #49

Merged
merged 1 commit into from
Apr 25, 2022
Merged

Conversation

adriansmares
Copy link
Contributor

@adriansmares adriansmares commented Apr 22, 2022

Summary

References #48

Changes

  • Explicitly add the dwell time duration to the dwell time configuration while enabling the dwell time
    • From what regulations I could find about the AS923 countries, the 400ms duration was used everywhere
    • Should anyone stray away from this common value, we will fix it in a frequency plan dedicated to that country

Notes for Reviewers

The lack of duration fails the validation:
https://github.com/TheThingsNetwork/lorawan-stack/blob/9d58a370c158a60072ed841370a5e7a97b67f419/pkg/frequencyplans/frequencyplans.go#L429-L431
During my tests I only verified that disabling works, which was not enough.

I will add CI checks as part of #38 which will catch these issues in the future.

Checklist

  • Scope: The referenced issue is addressed, there are no unrelated changes.
  • Compatibility: The changes are backwards compatible, they don't break existing deployments.
  • Testing: The changes are tested.
  • Documentation: Relevant documentation is added or updated.

@adriansmares adriansmares added this to the 2022 Q2 milestone Apr 22, 2022
@adriansmares adriansmares self-assigned this Apr 22, 2022
@adriansmares adriansmares changed the title Explicitely add the dwell time duration Explicitly add the dwell time duration Apr 22, 2022
@adriansmares adriansmares merged commit 6d17f94 into master Apr 25, 2022
@adriansmares adriansmares deleted the fix/add-dt-duration branch August 12, 2022 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants