From 2618813c70dec3c8c8bd97d90a44d675a7986e72 Mon Sep 17 00:00:00 2001 From: Hasnain Virk Date: Fri, 22 Mar 2019 15:12:37 +0200 Subject: [PATCH 1/3] Ack timeout must be at least 2 seconds While calculating ack timeout, we were ending up getting a random value which may become less than 2 seconds. This is not allowed as per v1.0.2 specification. To fix the issue we now take the random number from 0 to 2000 ms and then add that to the fixed 2000 ms ack timeout value, guaranteeing a value at least equal to 2000 ms. --- features/lorawan/lorastack/phy/LoRaPHY.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/features/lorawan/lorastack/phy/LoRaPHY.cpp b/features/lorawan/lorastack/phy/LoRaPHY.cpp index dd3cba9217..8bf57ee691 100644 --- a/features/lorawan/lorastack/phy/LoRaPHY.cpp +++ b/features/lorawan/lorastack/phy/LoRaPHY.cpp @@ -628,8 +628,7 @@ uint16_t LoRaPHY::get_maximum_frame_counter_gap() uint32_t LoRaPHY::get_ack_timeout() { uint16_t ack_timeout_rnd = phy_params.ack_timeout_rnd; - return (phy_params.ack_timeout - + get_random(-ack_timeout_rnd, ack_timeout_rnd)); + return (phy_params.ack_timeout + get_random(0, ack_timeout_rnd)); } uint32_t LoRaPHY::get_default_rx2_frequency() From 2691b83c4e8f9e28fee71b35b54a067c9a52c88e Mon Sep 17 00:00:00 2001 From: Hasnain Virk Date: Fri, 22 Mar 2019 15:05:03 +0200 Subject: [PATCH 2/3] Fixing premature RX2 abort A bug while setting up RX start timers would result in premature closusre of RX2 window. The 'ack_Timeout_timer' would be invoked prematurely and at that time RX2 window may be being demodulating. This resulted in massive instability with any test that relied on Confirmed traffic or lower data rates. To fix the issue, we must know the length of the RX window in milliseconds and for this purpose we have extended the 'get_rx_window_params(...)' API. The length of the time the window may remain open must be accounted for while setting up 'ack_timeout_timer'. --- features/lorawan/lorastack/mac/LoRaMac.cpp | 12 +++++++++--- features/lorawan/lorastack/phy/LoRaPHY.cpp | 7 +++++-- features/lorawan/lorastack/phy/LoRaPHY.h | 3 ++- features/lorawan/system/lorawan_data_structures.h | 6 +++++- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/features/lorawan/lorastack/mac/LoRaMac.cpp b/features/lorawan/lorastack/mac/LoRaMac.cpp index a971fb9cd4..132346f8db 100644 --- a/features/lorawan/lorastack/mac/LoRaMac.cpp +++ b/features/lorawan/lorastack/mac/LoRaMac.cpp @@ -667,13 +667,14 @@ void LoRaMac::on_radio_tx_done(lorawan_time_t timestamp) if (get_device_class() == CLASS_C) { _lora_time.start(_rx2_closure_timer_for_class_c, (_params.rx_window2_delay - time_diff) + - _params.rx_window2_config.window_timeout); + _params.rx_window2_config.window_timeout_ms); } // start timer after which ack wait will timeout (for Confirmed messages) if (_params.is_node_ack_requested) { _lora_time.start(_params.timers.ack_timeout_timer, (_params.rx_window2_delay - time_diff) + + _params.rx_window2_config.window_timeout_ms + _lora_phy->get_ack_timeout()); } } else { @@ -1097,6 +1098,8 @@ lorawan_status_t LoRaMac::schedule_tx() { channel_selection_params_t next_channel; lorawan_time_t backoff_time = 0; + lorawan_time_t aggregated_timeoff = 0; + uint8_t channel = 0; uint8_t fopts_len = 0; if (_params.sys_params.max_duty_cycle == 255) { @@ -1122,9 +1125,12 @@ lorawan_status_t LoRaMac::schedule_tx() next_channel.last_aggregate_tx_time = _params.timers.aggregated_last_tx_time; lorawan_status_t status = _lora_phy->set_next_channel(&next_channel, - &_params.channel, + &channel, &backoff_time, - &_params.timers.aggregated_timeoff); + &aggregated_timeoff); + + _params.channel = channel; + _params.timers.aggregated_timeoff = aggregated_timeoff; switch (status) { case LORAWAN_STATUS_NO_CHANNEL_FOUND: diff --git a/features/lorawan/lorastack/phy/LoRaPHY.cpp b/features/lorawan/lorastack/phy/LoRaPHY.cpp index 8bf57ee691..c13f07d18a 100644 --- a/features/lorawan/lorastack/phy/LoRaPHY.cpp +++ b/features/lorawan/lorastack/phy/LoRaPHY.cpp @@ -406,7 +406,8 @@ float LoRaPHY::compute_symb_timeout_fsk(uint8_t phy_dr) void LoRaPHY::get_rx_window_params(float t_symb, uint8_t min_rx_symb, float error_fudge, float wakeup_time, - uint32_t *window_length, int32_t *window_offset, + uint32_t *window_length, uint32_t *window_length_ms, + int32_t *window_offset, uint8_t phy_dr) { float target_rx_window_offset; @@ -442,6 +443,7 @@ void LoRaPHY::get_rx_window_params(float t_symb, uint8_t min_rx_symb, // Setting the window_length in terms of 'symbols' for LoRa modulation or // in terms of 'bytes' for FSK *window_length = (uint32_t) ceil(window_len_in_ms / t_symb); + *window_length_ms = window_len_in_ms; } int8_t LoRaPHY::compute_tx_power(int8_t tx_power_idx, float max_eirp, @@ -848,7 +850,8 @@ void LoRaPHY::compute_rx_win_params(int8_t datarate, uint8_t min_rx_symbols, } get_rx_window_params(t_symbol, min_rx_symbols, (float) rx_error, MBED_CONF_LORA_WAKEUP_TIME, - &rx_conf_params->window_timeout, &rx_conf_params->window_offset, + &rx_conf_params->window_timeout, &rx_conf_params->window_timeout_ms, + &rx_conf_params->window_offset, rx_conf_params->datarate); } diff --git a/features/lorawan/lorastack/phy/LoRaPHY.h b/features/lorawan/lorastack/phy/LoRaPHY.h index a522afc4f9..71b8ba094c 100644 --- a/features/lorawan/lorastack/phy/LoRaPHY.h +++ b/features/lorawan/lorastack/phy/LoRaPHY.h @@ -626,7 +626,8 @@ protected: */ void get_rx_window_params(float t_symbol, uint8_t min_rx_symbols, float rx_error, float wakeup_time, - uint32_t *window_length, int32_t *window_offset, + uint32_t *window_length, uint32_t *window_length_ms, + int32_t *window_offset, uint8_t phy_dr); /** diff --git a/features/lorawan/system/lorawan_data_structures.h b/features/lorawan/system/lorawan_data_structures.h index 141f7db0c4..e0aeb14476 100644 --- a/features/lorawan/system/lorawan_data_structures.h +++ b/features/lorawan/system/lorawan_data_structures.h @@ -1007,9 +1007,13 @@ typedef struct { */ uint32_t frequency; /*! - * The RX window timeout + * The RX window timeout - Symbols */ uint32_t window_timeout; + /*! + * The RX window timeout - Milliseconds + */ + uint32_t window_timeout_ms; /*! * The RX window offset */ From 4c1b8a881659497272993b3aefdb4e47eff7c5c6 Mon Sep 17 00:00:00 2001 From: Hasnain Virk Date: Fri, 22 Mar 2019 15:45:35 +0200 Subject: [PATCH 3/3] Unit test fix LoRaPHY_stub get_rx_window_params() API is changed. To reflect that changing the stub. --- UNITTESTS/stubs/LoRaPHY_stub.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/UNITTESTS/stubs/LoRaPHY_stub.cpp b/UNITTESTS/stubs/LoRaPHY_stub.cpp index 53dce9fe4f..5c0f8f53ec 100644 --- a/UNITTESTS/stubs/LoRaPHY_stub.cpp +++ b/UNITTESTS/stubs/LoRaPHY_stub.cpp @@ -173,7 +173,8 @@ uint8_t LoRaPHY::verify_link_ADR_req(verify_adr_params_t *verify_params, void LoRaPHY::get_rx_window_params(float t_symbol, uint8_t min_rx_symbols, float rx_error, float wakeup_time, - uint32_t *window_length, int32_t *window_offset, + uint32_t *window_length, uint32_t *window_length_ms, + int32_t *window_offset, uint8_t phy_dr) { }