TheThingsNetwork / arduino-device-lib

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

Feature upgrades for ABP, add errorReadout, poll after Retry Class A and more #277

Closed flhofer closed 2 years ago

flhofer commented 2 years ago

Hello! [EDITED]

I made some changes to the library for a project I worked on, so I created a pull request for the most useful ones.

Changes in general function.

New methods

Other changes

Final note

I tested the code with a TTN Uno board on a private LoRaWan server. There may be some typos as I had to manually apply some changes from my version as it holds way more features. And yes: I will push docs and examples to the branch while this request gets rolling. I know it takes some time for review, and it's better to get started soon. 😃

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

jpmeijers commented 2 years ago

The overall quality of this PR looks very good to me. Two small issues:

flhofer commented 2 years ago

@jpmeijers Thanks for the quick reply! Didn't expect that. I have more proposals. I will apply your proposed changes and upload also some documentation soon. Some I noticed too

flhofer commented 2 years ago

@jpmeijers Your comment on TTN_UNSUCESSFUL_RECEIVE made me recheck the code. I actually found that too. Indeed, the submission I made yesterday was onle the changes of last year. I added thus now the rest and made a first documentation. I might still do some refactoring.. 😃

More updates soon

flhofer commented 2 years ago

@jpmeijers @johanstokking Refactoring done. Also fixed a duty cycle preset issue for EU868. The default 8 channels span over 2 bands, G and G1, both set to 1%. Thus each of them may use 1/3 for channel 1-3 or 1/5th for channel 4-8 of the band's duty cycle of 1%, not 1/8 of 1% as it was until now.

Explanations here

jpmeijers commented 2 years ago

Looking at the commands this library sends to the RN2483 when configured for EU868, I agree it does not look exactly as we want. The library uses the same configureChannels() for both OTAA and ABP. I think this needs to change. In the case of OTAA only the LoRaWAN default channels for the region should be configured, as the rest will be pushed by the network in the Join Accept. Only in the case of ABP we should manually configure all the channels and their duty cycles.

This is however a bigger change, and not a simple fix we should do in this PR.

@johanstokking let's merge this PR first, then we can work on fixing the RX2 window, and other issues with the configured channels.

johanstokking commented 2 years ago

@johanstokking let's merge this PR first, then we can work on fixing the RX2 window, and other issues with the configured channels.

I agree. Thanks a lot @flhofer for implementing and @jpmeijers for reviewing.