TheThingsIndustries / generic-node-se

Generic Node Sensor Edition
https://www.genericnode.com
Other
108 stars 31 forks source link

FreeRTOS downlink fix #202

Closed mcserved closed 3 years ago

mcserved commented 3 years ago

Summary:

This PR increases the window of the RX retrieval. Additionally it removes the forced ADR sequence and fixes some documentation. Closes #201 #187.

Changes:

See the graph here: basic_lorawan freertos_lorawan (new) freertos
5s14ms 5s129ms 4s980ms
218ms 271ms 531ms
781ms 754ms 492ms
218ms 271ms 531ms

Here a picture of the events (compared to the picture in issue #187) image

Noticeable from both is that the retrieval times are now within the basic_lorawan application's retrieval window, but also extend far beyond this. So the power consumption is increased by this. I choose 200 specifically as it would be slightly before the 5s mark.

Notes for Reviewers:

Downlinks seem to be better, although not always perfect for me, although this happens with basic_lorawan as well so this can be due to the setup/antenna performance

elsalahy commented 3 years ago

@marnixcro I think this solves an issue and creates a new one as increasing the Rx window from 200 to 500 is not a welcomed addition from the POV of the LoRaWAN specifications or power consumption.

Please check if we can do anything about this? Otherwise Please open a backlog/technical debt issue to reduce the lorawanConfigRX_MAX_TIMING_ERROR and to fix the underlying OS time drift.

mcserved commented 3 years ago

I'll close this PR, and wait for issue #203 to be fixed.