Stability improvements & CRYPTO_ERROR addition

Hasnain Virk · Hasnain Virk · commit 9973eb3e793b · 2018-05-24T15:59:07.000+03:00
General stability improvements are performed.
A flag is added if a Class C RX2 window is open.
We shouldn't open it again if its already opened.

TX_CRYPTO_ERROR is renamed to CRYPTO_ERROR.
Keeping TX_CRYPTO_ERROR for backwards compatibility.
diff --git a/features/lorawan/LoRaWANStack.cpp b/features/lorawan/LoRaWANStack.cpp
@@ -608,55 +608,59 @@ void LoRaWANStack::process_reception(const uint8_t* const payload, uint16_t size
         mlme_confirm_handler();
     }
 
-    if (_loramac.nwk_joined()) {
-        if (_loramac.get_mcps_confirmation()->req_type == MCPS_CONFIRMED) {
-            // if ack was not received, we will try retransmission after
-            // ACK_TIMEOUT. handle_data_frame() already disables ACK_TIMEOUT timer
-            // if ack was received. Otherwise, following method will be called in
-            // LoRaMac.cpp, on_ack_timeout_timer_event().
-            if (_loramac.get_mcps_indication()->is_ack_recvd) {
-                tr_debug("Ack=OK, NbTrials=%d", _loramac.get_mcps_confirmation()->nb_retries);
-                _loramac.post_process_mcps_req();
-                _ctrl_flags |= TX_DONE_FLAG;
-                _ctrl_flags &= ~TX_ONGOING_FLAG;
-                state_controller(DEVICE_STATE_STATUS_CHECK);
-            } else {
-                if (!_loramac.continue_sending_process()) {
-                    tr_error("Retries exhausted for Class A device");
-                    _ctrl_flags &= ~TX_DONE_FLAG;
-                    _ctrl_flags |= TX_ONGOING_FLAG;
-                    state_controller(DEVICE_STATE_STATUS_CHECK);
-                }
-            }
-        } else {
-            // handle UNCONFIRMED, PROPRIETARY case here, RX slots were turned off due to
-            // valid packet reception, so we generate a TX_DONE event
+    if (!_loramac.nwk_joined()) {
+        return;
+    }
+
+    // if the outgoing message was of CONFIRMED type
+    if (_loramac.get_mcps_confirmation()->req_type == MCPS_CONFIRMED) {
+        // if ack was not received, we will try retransmission after
+        // ACK_TIMEOUT. handle_data_frame() already disables ACK_TIMEOUT timer
+        // if ack was received. Otherwise, following method will be called in
+        // LoRaMac.cpp, on_ack_timeout_timer_event().
+        if (_loramac.get_mcps_indication()->is_ack_recvd) {
+            tr_debug("Ack=OK, NbTrials=%d",
+                     _loramac.get_mcps_confirmation()->nb_retries);
             _loramac.post_process_mcps_req();
             _ctrl_flags |= TX_DONE_FLAG;
+            _ctrl_flags &= ~TX_ONGOING_FLAG;
             state_controller(DEVICE_STATE_STATUS_CHECK);
+        } else {
+            if (!_loramac.continue_sending_process()) {
+                tr_error("Retries exhausted for Class A device");
+                _ctrl_flags &= ~TX_DONE_FLAG;
+                _ctrl_flags |= TX_ONGOING_FLAG;
+                state_controller(DEVICE_STATE_STATUS_CHECK);
+            }
         }
+    } else {
+        // handle UNCONFIRMED case here, RX slots were turned off due to
+        // valid packet reception
+        _loramac.post_process_mcps_req();
+        _ctrl_flags |= TX_DONE_FLAG;
+        state_controller(DEVICE_STATE_STATUS_CHECK);
+    }
 
-        // handle any pending MCPS indication
-        if (_loramac.get_mcps_indication()->pending) {
-            _loramac.post_process_mcps_ind();
-            _ctrl_flags |= MSG_RECVD_FLAG;
-            state_controller(DEVICE_STATE_STATUS_CHECK);
-        }
+    // handle any pending MCPS indication
+    if (_loramac.get_mcps_indication()->pending) {
+        _loramac.post_process_mcps_ind();
+        _ctrl_flags |= MSG_RECVD_FLAG;
+        state_controller(DEVICE_STATE_STATUS_CHECK);
+    }
 
-        // change the state only if a TX cycle completes for Class A
-        // For class C it's not needed as it will already be in receiving
-        // state, no matter if the TX cycle completed or not.
-        if (!(_ctrl_flags & TX_ONGOING_FLAG)) {
-            // we are done here, update the state
-              state_machine_run_to_completion();
-        }
+    // change the state only if a TX cycle completes for Class A
+    // For class C it's not needed as it will already be in receiving
+    // state, no matter if the TX cycle completed or not.
+    if (!(_ctrl_flags & TX_ONGOING_FLAG)) {
+        // we are done here, update the state
+        state_machine_run_to_completion();
+    }
 
-        if (_loramac.get_mlme_indication()->pending) {
-            tr_debug("MLME Indication pending");
-            _loramac.post_process_mlme_ind();
-            tr_debug("Automatic uplink requested");
-            mlme_indication_handler();
-        }
+    if (_loramac.get_mlme_indication()->pending) {
+        tr_debug("MLME Indication pending");
+        _loramac.post_process_mlme_ind();
+        tr_debug("Immediate Uplink requested");
+        mlme_indication_handler();
     }
 
     _ready_for_rx = true;
@@ -765,6 +769,7 @@ void LoRaWANStack::send_automatic_uplink_message(const uint8_t port)
 {
     const int16_t ret = handle_tx(port, NULL, 0, MSG_CONFIRMED_FLAG, true, true);
     if (ret < 0) {
+        tr_debug("Failed to generate AUTOMATIC UPLINK, error code = %d", ret);
         send_event_to_application(AUTOMATIC_UPLINK_ERROR);
     }
 }
@@ -871,8 +876,16 @@ void LoRaWANStack::mlme_confirm_handler()
             state_controller(DEVICE_STATE_CONNECTED);
         } else {
             tr_error("Joining error: %d", _loramac.get_mlme_confirmation()->status);
-            _device_current_state = DEVICE_STATE_AWAITING_JOIN_ACCEPT;
-            state_controller(DEVICE_STATE_JOINING);
+            if (_loramac.get_mlme_confirmation()->status == LORAMAC_EVENT_INFO_STATUS_CRYPTO_FAIL) {
+                // fatal error
+                _device_current_state = DEVICE_STATE_IDLE;
+                send_event_to_application(CRYPTO_ERROR);
+            } else {
+                // non-fatal, retry if possible
+                _device_current_state = DEVICE_STATE_AWAITING_JOIN_ACCEPT;
+                state_controller(DEVICE_STATE_JOINING);
+            }
+
         }
     }
 }
diff --git a/features/lorawan/lorastack/mac/LoRaMac.cpp b/features/lorawan/lorastack/mac/LoRaMac.cpp
@@ -83,6 +83,7 @@ LoRaMac::LoRaMac()
       _mlme_indication(),
       _mlme_confirmation(),
       _is_nwk_joined(false),
+      _continuous_rx2_window_open(false),
       _device_class(CLASS_A)
 {
     _params.keys.dev_eui = NULL;
@@ -196,7 +197,9 @@ void LoRaMac::on_radio_tx_done(void)
         _lora_phy.put_radio_to_sleep();
     } else {
         // this will open a continuous RX2 window until time==RECV_DELAY1
-        open_rx2_window();
+        if (!_continuous_rx2_window_open) {
+            open_rx2_window();
+        }
     }
 
     if(_params.is_rx_window_enabled == true) {
@@ -226,8 +229,7 @@ void LoRaMac::on_radio_tx_done(void)
 /**
  * This part handles incoming frames in response to Radio RX Interrupt
  */
-void LoRaMac::handle_join_accept_frame(const uint8_t *payload,
-                                       uint16_t size)
+void LoRaMac::handle_join_accept_frame(const uint8_t *payload, uint16_t size)
 {
     uint32_t mic = 0;
     uint32_t mic_rx = 0;
@@ -258,7 +260,7 @@ void LoRaMac::handle_join_accept_frame(const uint8_t *payload,
     mic_rx |= ((uint32_t) _params.rx_buffer[size - LORAMAC_MFR_LEN + 3] << 24);
 
     if (mic_rx == mic) {
-
+        _lora_time.stop(_params.timers.rx_window2_timer);
         if (_lora_crypto.compute_skeys_for_join_frame(_params.keys.app_key,
                                                       APPKEY_KEY_LENGTH,
                                                       _params.rx_buffer + 1,
@@ -517,6 +519,7 @@ void LoRaMac::handle_data_frame(const uint8_t* const payload,
         if (!is_multicast) {
             // We are not the destination of this frame.
             _mcps_indication.status = LORAMAC_EVENT_INFO_STATUS_ADDRESS_FAIL;
+            _mcps_indication.pending = false;
             return;
         }
     } else {
@@ -533,14 +536,14 @@ void LoRaMac::handle_data_frame(const uint8_t* const payload,
     if (!message_integrity_check(payload, size, &ptr_pos, address,
                                  &downlink_counter, nwk_skey)) {
         tr_error("MIC failed");
+        _mcps_indication.status = LORAMAC_EVENT_INFO_STATUS_MIC_FAIL;
+        _mcps_indication.pending = false;
         return;
     }
 
     // message is intended for us and MIC have passed, stop RX2 Window
     // Spec: 3.3.4 Receiver Activity during the receive windows
-    if (_params.rx_slot == RX_SLOT_WIN_2) {
-        _lora_time.stop(_params.timers.rx_window2_timer);
-    }
+    _lora_time.stop(_params.timers.rx_window2_timer);
 
     _mcps_confirmation.ack_received = false;
     _mcps_indication.is_ack_recvd = false;
@@ -612,7 +615,7 @@ void LoRaMac::handle_data_frame(const uint8_t* const payload,
     // to take retransmissions and repetitions into account. Error cases
     // will be handled in function OnMacStateCheckTimerEvent.
     if (_params.is_node_ack_requested) {
-        if (fctrl.bits.ack == 1) {
+        if (fctrl.bits.ack) {
             _mac_commands.clear_command_buffer();
             _mcps_confirmation.ack_received = fctrl.bits.ack;
             _mcps_indication.is_ack_recvd = fctrl.bits.ack;
@@ -641,7 +644,7 @@ void LoRaMac::handle_data_frame(const uint8_t* const payload,
         _mcps_indication.buffer_size = size - ptr_pos;
     }
 
-    // only stop act timer, if the ack is actuall recieved
+    // only stop act timer, if the ack is actually recieved
     if (_mcps_confirmation.ack_received) {
         _lora_time.stop(_params.timers.ack_timeout_timer);
     }
@@ -665,7 +668,9 @@ void LoRaMac::on_radio_rx_done(const uint8_t* const payload, uint16_t size,
     }
 
     if (_device_class == CLASS_C) {
-         open_rx2_window();
+        if (!_continuous_rx2_window_open) {
+            open_rx2_window();
+        }
      } else {
          _lora_phy.put_radio_to_sleep();
      }
@@ -841,12 +846,14 @@ void LoRaMac::on_backoff_timer_expiry(void)
     Lock lock(*this);
     lorawan_status_t status = schedule_tx();
     MBED_ASSERT(status==LORAWAN_STATUS_OK);
+    (void) status;
 }
 
 void LoRaMac::open_rx1_window(void)
 {
     Lock lock(*this);
     tr_debug("Opening RX1 Window");
+    _continuous_rx2_window_open = false;
     _lora_time.stop(_params.timers.rx_window1_timer);
     _params.rx_slot = RX_SLOT_WIN_1;
 
@@ -880,7 +887,13 @@ void LoRaMac::open_rx2_window()
     _params.rx_window2_config.is_repeater_supported = _params.is_repeater_supported;
     _params.rx_window2_config.rx_slot = RX_SLOT_WIN_2;
 
-    _params.rx_window2_config.is_rx_continuous = get_device_class()==CLASS_C ? true : false;
+    if (get_device_class() == CLASS_C) {
+        _continuous_rx2_window_open = true;
+        _params.rx_window2_config.is_rx_continuous = true;
+    } else {
+        _continuous_rx2_window_open = false;
+        _params.rx_window2_config.is_rx_continuous = false;
+    }
 
     _mcps_indication.rx_datarate = _params.rx_window2_config.datarate;
 
@@ -911,8 +924,11 @@ void LoRaMac::on_ack_timeout_timer_event(void)
     tr_debug("ACK_TIMEOUT Elapses, Retrying ...");
     _lora_time.stop(_params.timers.ack_timeout_timer);
 
-    // reduce data rate
-    if ((_params.ack_timeout_retry_counter % 2)) {
+    // reduce data rate on every 2nd attempt if and only if the
+    // ADR is on
+    if ((_params.ack_timeout_retry_counter % 2)
+            && (_params.sys_params.adr_on)) {
+        tr_debug("Trading datarate for range");
         _params.sys_params.channel_data_rate = _lora_phy.get_next_lower_tx_datarate(
                                                _params.sys_params.channel_data_rate);
     }
@@ -933,6 +949,7 @@ void LoRaMac::on_ack_timeout_timer_event(void)
         // now that is a critical failure
         lorawan_status_t status = handle_retransmission();
         MBED_ASSERT(status==LORAWAN_STATUS_OK);
+        (void) status;
     }
 
     _params.ack_timeout_retry_counter++;
@@ -1501,6 +1518,7 @@ lorawan_status_t LoRaMac::prepare_frame(loramac_mhdr_t *machdr,
             }
 
             if (_params.is_srv_ack_requested == true) {
+                tr_debug("Acking to NS");
                 fctrl->bits.ack = 1;
             }
 
diff --git a/features/lorawan/lorastack/mac/LoRaMac.h b/features/lorawan/lorastack/mac/LoRaMac.h
@@ -675,6 +675,8 @@ class LoRaMac {
 
     bool _is_nwk_joined;
 
+    bool _continuous_rx2_window_open;
+
     device_class_t _device_class;
 
 #if defined(LORAWAN_COMPLIANCE_TEST)
diff --git a/features/lorawan/lorawan_types.h b/features/lorawan/lorawan_types.h
@@ -192,7 +192,7 @@ typedef struct lorawan_connect {
  * TX_DONE              - When a packet is sent
  * TX_TIMEOUT,          - When stack was unable to send packet in TX window
  * TX_ERROR,            - A general TX error
- * TX_CRYPTO_ERROR,     - If MIC fails, or any other crypto relted error
+ * CRYPTO_ERROR,        - A crypto error indicating wrong keys
  * TX_SCHEDULING_ERROR, - When stack is unable to schedule packet
  * RX_DONE,             - When there is something to receive
  * RX_TIMEOUT,          - Not yet mapped
@@ -209,7 +209,8 @@ typedef enum lora_events {
     TX_DONE,
     TX_TIMEOUT,
     TX_ERROR,
-    TX_CRYPTO_ERROR,
+    CRYPTO_ERROR,
+    TX_CRYPTO_ERROR = CRYPTO_ERROR, //keeping this for backward compatibility
     TX_SCHEDULING_ERROR,
     RX_DONE,
     RX_TIMEOUT,
