Lora: Split add_mac_command() into separate methods

In order to have smaller and better testable methods, instead of having a big
switch-case -style add_mac_command(), each MAC command is now separated to an
own method.

features/lorawan/lorastack/mac/LoRaMac.cpp

@@ -1432,7 +1432,7 @@ void LoRaMac::setup_link_check_request()
 
     _mlme_confirmation.req_type = MLME_LINK_CHECK;
     _params.flags.bits.mlme_req = 1;
-    mac_commands.add_mac_command(MOTE_MAC_LINK_CHECK_REQ, 0, 0);
+    mac_commands.add_link_check_req();
 }
 
 lorawan_status_t LoRaMac::prepare_join(const lorawan_connect_t *params, bool is_otaa)

features/lorawan/lorastack/mac/LoRaMacCommand.cpp

@@ -51,107 +51,6 @@ LoRaMacCommand::LoRaMacCommand()
     memset(mac_cmd_buffer_to_repeat, 0, sizeof(mac_cmd_buffer_to_repeat));
 }
 
-LoRaMacCommand::~LoRaMacCommand()
-{
-}
-
-lorawan_status_t LoRaMacCommand::add_mac_command(uint8_t cmd, uint8_t p1,
-                                                 uint8_t p2)
-{
-    lorawan_status_t status = LORAWAN_STATUS_BUSY;
-    // The maximum buffer length must take MAC commands to re-send into account.
-    const uint8_t bufLen = LORA_MAC_COMMAND_MAX_LENGTH
-            - mac_cmd_buf_idx_to_repeat;
-
-    switch (cmd) {
-        case MOTE_MAC_LINK_CHECK_REQ:
-            if (mac_cmd_buf_idx < bufLen) {
-                mac_cmd_buffer[mac_cmd_buf_idx++] = cmd;
-                // No payload for this command
-                status = LORAWAN_STATUS_OK;
-            }
-            break;
-        case MOTE_MAC_LINK_ADR_ANS:
-            if (mac_cmd_buf_idx < (bufLen - 1)) {
-                mac_cmd_buffer[mac_cmd_buf_idx++] = cmd;
-                // Margin
-                mac_cmd_buffer[mac_cmd_buf_idx++] = p1;
-                status = LORAWAN_STATUS_OK;
-            }
-            break;
-        case MOTE_MAC_DUTY_CYCLE_ANS:
-            if (mac_cmd_buf_idx < bufLen) {
-                mac_cmd_buffer[mac_cmd_buf_idx++] = cmd;
-                // No payload for this answer
-                status = LORAWAN_STATUS_OK;
-            }
-            break;
-        case MOTE_MAC_RX_PARAM_SETUP_ANS:
-            if (mac_cmd_buf_idx < (bufLen - 1)) {
-                mac_cmd_buffer[mac_cmd_buf_idx++] = cmd;
-                // Status: Datarate ACK, Channel ACK
-                mac_cmd_buffer[mac_cmd_buf_idx++] = p1;
-                // This is a sticky MAC command answer. Setup indication
-//                _lora_mac.set_mlme_schedule_ul_indication();
-                sticky_mac_cmd = true;
-                status = LORAWAN_STATUS_OK;
-            }
-            break;
-        case MOTE_MAC_DEV_STATUS_ANS:
-            if (mac_cmd_buf_idx < (bufLen - 2)) {
-                mac_cmd_buffer[mac_cmd_buf_idx++] = cmd;
-                // 1st byte Battery
-                // 2nd byte Margin
-                mac_cmd_buffer[mac_cmd_buf_idx++] = p1;
-                mac_cmd_buffer[mac_cmd_buf_idx++] = p2;
-                status = LORAWAN_STATUS_OK;
-            }
-            break;
-        case MOTE_MAC_NEW_CHANNEL_ANS:
-            if (mac_cmd_buf_idx < (bufLen - 1)) {
-                mac_cmd_buffer[mac_cmd_buf_idx++] = cmd;
-                // Status: Datarate range OK, Channel frequency OK
-                mac_cmd_buffer[mac_cmd_buf_idx++] = p1;
-                status = LORAWAN_STATUS_OK;
-            }
-            break;
-        case MOTE_MAC_RX_TIMING_SETUP_ANS:
-            if (mac_cmd_buf_idx < bufLen) {
-                mac_cmd_buffer[mac_cmd_buf_idx++] = cmd;
-                // No payload for this answer
-                // This is a sticky MAC command answer. Setup indication
-//                _lora_mac.set_mlme_schedule_ul_indication();
-                sticky_mac_cmd = true;
-                status = LORAWAN_STATUS_OK;
-            }
-            break;
-        case MOTE_MAC_TX_PARAM_SETUP_ANS:
-            if (mac_cmd_buf_idx < bufLen) {
-                mac_cmd_buffer[mac_cmd_buf_idx++] = cmd;
-                // No payload for this answer
-                status = LORAWAN_STATUS_OK;
-            }
-            break;
-        case MOTE_MAC_DL_CHANNEL_ANS:
-            if (mac_cmd_buf_idx < bufLen) {
-                mac_cmd_buffer[mac_cmd_buf_idx++] = cmd;
-                // Status: Uplink frequency exists, Channel frequency OK
-                mac_cmd_buffer[mac_cmd_buf_idx++] = p1;
-                // This is a sticky MAC command answer. Setup indication
-//                _lora_mac.set_mlme_schedule_ul_indication();
-                sticky_mac_cmd = true;
-                status = LORAWAN_STATUS_OK;
-            }
-            break;
-        default:
-            return LORAWAN_STATUS_SERVICE_UNKNOWN;
-    }
-    if (status == LORAWAN_STATUS_OK) {
-        mac_cmd_in_next_tx = true;
-    }
-    return status;
-}
-
 void LoRaMacCommand::clear_command_buffer()
 {
     mac_cmd_buf_idx = 0;
@@ -185,6 +84,7 @@ void LoRaMacCommand::parse_mac_commands_to_repeat()
                 mac_cmd_buffer_to_repeat[cmd_cnt++] = mac_cmd_buffer[i];
                 break;
             }
+
             // NON-STICKY
             case MOTE_MAC_DEV_STATUS_ANS: { // 2 bytes payload
                 i += 2;
@@ -298,7 +198,7 @@ lorawan_status_t LoRaMacCommand::process_mac_commands(uint8_t *payload, uint8_t
 
                 // Add the answers to the buffer
                 for (uint8_t i = 0; i < (linkAdrNbBytesParsed / 5); i++) {
-                    ret_value = add_mac_command(MOTE_MAC_LINK_ADR_ANS, status, 0);
+                    ret_value = add_link_adr_ans(status);
                 }
                 // Update MAC index
                 mac_index += linkAdrNbBytesParsed - 1;
@@ -307,7 +207,7 @@ lorawan_status_t LoRaMacCommand::process_mac_commands(uint8_t *payload, uint8_t
             case SRV_MAC_DUTY_CYCLE_REQ:
                 mac_sys_params.max_duty_cycle = payload[mac_index++];
                 mac_sys_params.aggregated_duty_cycle = 1 << mac_sys_params.max_duty_cycle;
-                ret_value = add_mac_command(MOTE_MAC_DUTY_CYCLE_ANS, 0, 0);
+                ret_value = add_duty_cycle_ans();
                 break;
             case SRV_MAC_RX_PARAM_SETUP_REQ: {
                 rx_param_setup_req_t rxParamSetupReq;
@@ -317,32 +217,26 @@ lorawan_status_t LoRaMacCommand::process_mac_commands(uint8_t *payload, uint8_t
                 mac_index++;
 
                 rxParamSetupReq.frequency = (uint32_t) payload[mac_index++];
-                rxParamSetupReq.frequency |= (uint32_t) payload[mac_index++]
+                rxParamSetupReq.frequency |= (uint32_t) payload[mac_index++] << 8;
-                        << 8;
+                rxParamSetupReq.frequency |= (uint32_t) payload[mac_index++] << 16;
-                rxParamSetupReq.frequency |= (uint32_t) payload[mac_index++]
-                        << 16;
                 rxParamSetupReq.frequency *= 100;
 
                 // Perform request on region
                 status = lora_phy.accept_rx_param_setup_req(&rxParamSetupReq);
 
                 if ((status & 0x07) == 0x07) {
-                    mac_sys_params.rx2_channel.datarate =
+                    mac_sys_params.rx2_channel.datarate = rxParamSetupReq.datarate;
-                            rxParamSetupReq.datarate;
+                    mac_sys_params.rx2_channel.frequency = rxParamSetupReq.frequency;
-                    mac_sys_params.rx2_channel.frequency =
-                            rxParamSetupReq.frequency;
                     mac_sys_params.rx1_dr_offset = rxParamSetupReq.dr_offset;
                 }
-                ret_value = add_mac_command(MOTE_MAC_RX_PARAM_SETUP_ANS, status,
+                ret_value = add_rx_param_setup_ans(status);
-                                            0);
             }
                 break;
             case SRV_MAC_DEV_STATUS_REQ: {
                 uint8_t batteryLevel = BAT_LEVEL_NO_MEASURE;
                 // we don't have a mechanism at the moment to measure
                 // battery levels
-                ret_value = add_mac_command(MOTE_MAC_DEV_STATUS_ANS,
+                ret_value = add_dev_status_ans(batteryLevel, snr & 0x3F);
-                                            batteryLevel, snr & 0x3F);
                 break;
             }
             case SRV_MAC_NEW_CHANNEL_REQ: {
@@ -358,7 +252,7 @@ lorawan_status_t LoRaMacCommand::process_mac_commands(uint8_t *payload, uint8_t
 
                 status = lora_phy.request_new_channel(channel_id, &chParam);
 
-                ret_value = add_mac_command(MOTE_MAC_NEW_CHANNEL_ANS, status, 0);
+                ret_value = add_new_channel_ans(status);
             }
                 break;
             case SRV_MAC_RX_TIMING_SETUP_REQ: {
@@ -369,7 +263,7 @@ lorawan_status_t LoRaMacCommand::process_mac_commands(uint8_t *payload, uint8_t
                 }
                 mac_sys_params.recv_delay1 = delay * 1000;
                 mac_sys_params.recv_delay2 = mac_sys_params.recv_delay1 + 1000;
-                ret_value = add_mac_command(MOTE_MAC_RX_TIMING_SETUP_ANS, 0, 0);
+                ret_value = add_rx_timing_setup_ans();
             }
                 break;
             case SRV_MAC_TX_PARAM_SETUP_REQ: {
@@ -392,19 +286,15 @@ lorawan_status_t LoRaMacCommand::process_mac_commands(uint8_t *payload, uint8_t
                 // Check the status for correctness
                 if (lora_phy.accept_tx_param_setup_req(ul_dwell_time, dl_dwell_time)) {
                     // Accept command
-                    mac_sys_params.uplink_dwell_time =
+                    mac_sys_params.uplink_dwell_time = ul_dwell_time;
-                            ul_dwell_time;
+                    mac_sys_params.downlink_dwell_time = dl_dwell_time;
-                    mac_sys_params.downlink_dwell_time =
+                    mac_sys_params.max_eirp = max_eirp_table[max_eirp];
-                            dl_dwell_time;
-                    mac_sys_params.max_eirp =
-                            max_eirp_table[max_eirp];
                     // Add command response
-                    ret_value = add_mac_command(MOTE_MAC_TX_PARAM_SETUP_ANS, 0, 0);
+                    ret_value = add_tx_param_setup_ans();
                 }
             }
                 break;
             case SRV_MAC_DL_CHANNEL_REQ: {
-
                 uint8_t channel_id = payload[mac_index++];
                 uint32_t rx1_frequency;
 
@@ -412,10 +302,9 @@ lorawan_status_t LoRaMacCommand::process_mac_commands(uint8_t *payload, uint8_t
                 rx1_frequency |= (uint32_t) payload[mac_index++] << 8;
                 rx1_frequency |= (uint32_t) payload[mac_index++] << 16;
                 rx1_frequency *= 100;
-
                 status = lora_phy.dl_channel_request(channel_id, rx1_frequency);
 
-                ret_value = add_mac_command(MOTE_MAC_DL_CHANNEL_ANS, status, 0);
+                ret_value = add_dl_channel_ans(status);
             }
                 break;
             default:
@@ -433,3 +322,129 @@ bool LoRaMacCommand::is_sticky_mac_command_pending()
     }
     return false;
 }
+
+int32_t LoRaMacCommand::cmd_buffer_remaining() const
+{
+    // The maximum buffer length must take MAC commands to re-send into account.
+    return sizeof(mac_cmd_buffer) - mac_cmd_buf_idx_to_repeat - mac_cmd_buf_idx;
+}
+
+lorawan_status_t LoRaMacCommand::add_link_check_req()
+{
+    lorawan_status_t ret = LORAWAN_STATUS_LENGTH_ERROR;
+    if (cmd_buffer_remaining() > 0) {
+        mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_LINK_CHECK_REQ;
+        // No payload for this command
+        ret = LORAWAN_STATUS_OK;
+        mac_cmd_in_next_tx = true;
+    }
+    return ret;
+}
+
+lorawan_status_t LoRaMacCommand::add_link_adr_ans(uint8_t status)
+{
+    lorawan_status_t ret = LORAWAN_STATUS_LENGTH_ERROR;
+    if (cmd_buffer_remaining() > 1) {
+        mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_LINK_ADR_ANS;
+        mac_cmd_buffer[mac_cmd_buf_idx++] = status;
+        ret = LORAWAN_STATUS_OK;
+        mac_cmd_in_next_tx = true;
+    }
+    return ret;
+}
+
+lorawan_status_t LoRaMacCommand::add_duty_cycle_ans()
+{
+    lorawan_status_t ret = LORAWAN_STATUS_LENGTH_ERROR;
+    if (cmd_buffer_remaining() > 0) {
+        mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_DUTY_CYCLE_ANS;
+        // No payload for this answer
+        ret = LORAWAN_STATUS_OK;
+        mac_cmd_in_next_tx = true;
+    }
+    return ret;
+}
+
+lorawan_status_t LoRaMacCommand::add_rx_param_setup_ans(uint8_t status)
+{
+    lorawan_status_t ret = LORAWAN_STATUS_LENGTH_ERROR;
+    if (cmd_buffer_remaining() > 1) {
+        mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_RX_PARAM_SETUP_ANS;
+        // Status: Datarate ACK, Channel ACK
+        mac_cmd_buffer[mac_cmd_buf_idx++] = status;
+        // This is a sticky MAC command answer. Setup indication
+        sticky_mac_cmd = true;
+        ret = LORAWAN_STATUS_OK;
+        mac_cmd_in_next_tx = true;
+    }
+    return ret;
+}
+
+lorawan_status_t LoRaMacCommand::add_dev_status_ans(uint8_t battery, uint8_t margin)
+{
+    lorawan_status_t ret = LORAWAN_STATUS_LENGTH_ERROR;
+    if (cmd_buffer_remaining() > 2) {
+        mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_DEV_STATUS_ANS;
+        // 1st byte Battery
+        // 2nd byte Margin
+        mac_cmd_buffer[mac_cmd_buf_idx++] = battery;
+        mac_cmd_buffer[mac_cmd_buf_idx++] = margin;
+        ret = LORAWAN_STATUS_OK;
+        mac_cmd_in_next_tx = true;
+    }
+    return ret;
+}
+
+lorawan_status_t LoRaMacCommand::add_new_channel_ans(uint8_t status)
+{
+    lorawan_status_t ret = LORAWAN_STATUS_LENGTH_ERROR;
+    if (cmd_buffer_remaining() > 1) {
+        mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_NEW_CHANNEL_ANS;
+        // Status: Datarate range OK, Channel frequency OK
+        mac_cmd_buffer[mac_cmd_buf_idx++] = status;
+        ret = LORAWAN_STATUS_OK;
+        mac_cmd_in_next_tx = true;
+    }
+    return ret;
+}
+
+lorawan_status_t LoRaMacCommand::add_rx_timing_setup_ans()
+{
+    lorawan_status_t ret = LORAWAN_STATUS_LENGTH_ERROR;
+    if (cmd_buffer_remaining() > 0) {
+        mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_RX_TIMING_SETUP_ANS;
+        // No payload for this answer
+        // This is a sticky MAC command answer. Setup indication
+        sticky_mac_cmd = true;
+        ret = LORAWAN_STATUS_OK;
+        mac_cmd_in_next_tx = true;
+    }
+    return ret;
+}
+
+lorawan_status_t LoRaMacCommand::add_tx_param_setup_ans()
+{
+    lorawan_status_t ret = LORAWAN_STATUS_LENGTH_ERROR;
+    if (cmd_buffer_remaining() > 0) {
+        mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_TX_PARAM_SETUP_ANS;
+        // No payload for this answer
+        ret = LORAWAN_STATUS_OK;
+        mac_cmd_in_next_tx = true;
+    }
+    return ret;
+}
+
+lorawan_status_t LoRaMacCommand::add_dl_channel_ans(uint8_t status)
+{
+    lorawan_status_t ret = LORAWAN_STATUS_LENGTH_ERROR;
+    if (cmd_buffer_remaining() > 0) {
+        mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_DL_CHANNEL_ANS;
+        // Status: Uplink frequency exists, Channel frequency OK
+        mac_cmd_buffer[mac_cmd_buf_idx++] = status;
+        // This is a sticky MAC command answer. Setup indication
+        sticky_mac_cmd = true;
+        ret = LORAWAN_STATUS_OK;
+        mac_cmd_in_next_tx = true;
+    }
+    return ret;
+}

features/lorawan/lorastack/mac/LoRaMacCommand.h

@@ -55,26 +55,6 @@ class LoRaMacCommand {
 
 public:
     LoRaMacCommand();
-    ~LoRaMacCommand();
-
-    /**
-     * @brief Adds a new MAC command to be sent.
-     *
-     * @remark MAC layer internal function
-     *
-     * @param [in] cmd    MAC command to be added
-     *                    [MOTE_MAC_LINK_CHECK_REQ,
-     *                     MOTE_MAC_LINK_ADR_ANS,
-     *                     MOTE_MAC_DUTY_CYCLE_ANS,
-     *                     MOTE_MAC_RX2_PARAM_SET_ANS,
-     *                     MOTE_MAC_DEV_STATUS_ANS
-     *                     MOTE_MAC_NEW_CHANNEL_ANS]
-     * @param [in] p1  1st parameter (optional depends on the command)
-     * @param [in] p2  2nd parameter (optional depends on the command)
-     *
-     * @return status  Function status [0: OK, 1: Unknown command, 2: Buffer full]
-     */
-    lorawan_status_t add_mac_command(uint8_t cmd, uint8_t p1, uint8_t p2);
 
     /**
      * @brief Clear MAC command buffer.
@@ -159,6 +139,97 @@ public:
      */
     bool is_sticky_mac_command_pending();
 
+    /**
+     * @brief Adds a new LinkCheckReq MAC command to be sent.
+     *
+     * @return status  Function status: LORAWAN_STATUS_OK: OK,
+     *                                  LORAWAN_STATUS_LENGTH_ERROR: Buffer full
+     */
+    lorawan_status_t add_link_check_req();
+
+private:
+    /**
+     * @brief Get the remaining size of the MAC command buffer
+     *
+     * @return      Remaining free space in buffer (bytes).
+     */
+    int32_t cmd_buffer_remaining() const;
+
+    /**
+     * @brief Adds a new LinkAdrAns MAC command to be sent.
+     *
+     * @param [in] status Status bits
+     *
+     * @return status  Function status: LORAWAN_STATUS_OK: OK,
+     *                                  LORAWAN_STATUS_LENGTH_ERROR: Buffer full
+     */
+    lorawan_status_t add_link_adr_ans(uint8_t status);
+
+    /**
+     * @brief Adds a new DutyCycleAns MAC command to be sent.
+     *
+     * @return status  Function status: LORAWAN_STATUS_OK: OK,
+     *                                  LORAWAN_STATUS_LENGTH_ERROR: Buffer full
+     */
+    lorawan_status_t add_duty_cycle_ans();
+
+    /**
+     * @brief Adds a new RXParamSetupAns MAC command to be sent.
+     *
+     * @param [in] status Status bits
+     *
+     * @return status  Function status: LORAWAN_STATUS_OK: OK,
+     *                                  LORAWAN_STATUS_LENGTH_ERROR: Buffer full
+     */
+    lorawan_status_t add_rx_param_setup_ans(uint8_t status);
+
+    /**
+     * @brief Adds a new DevStatusAns MAC command to be sent.
+     *
+     * @param [in] battery  Battery level
+     * @param [in] margin   Demodulation signal-to-noise ratio (dB)
+     *
+     * @return status  Function status: LORAWAN_STATUS_OK: OK,
+     *                                  LORAWAN_STATUS_LENGTH_ERROR: Buffer full
+     */
+    lorawan_status_t add_dev_status_ans(uint8_t battery, uint8_t margin);
+
+    /**
+     * @brief Adds a new NewChannelAns MAC command to be sent.
+     *
+     * @param [in] status Status bits
+     *
+     * @return status  Function status: LORAWAN_STATUS_OK: OK,
+     *                                  LORAWAN_STATUS_LENGTH_ERROR: Buffer full
+     */
+    lorawan_status_t add_new_channel_ans(uint8_t status);
+
+    /**
+     * @brief Adds a new RXTimingSetupAns MAC command to be sent.
+     *
+     * @return status  Function status: LORAWAN_STATUS_OK: OK,
+     *                                  LORAWAN_STATUS_LENGTH_ERROR: Buffer full
+     */
+    lorawan_status_t add_rx_timing_setup_ans();
+
+    /**
+     * @brief Adds a new TXParamSetupAns MAC command to be sent.
+     *
+     * @return status  Function status: LORAWAN_STATUS_OK: OK,
+     *                                  LORAWAN_STATUS_LENGTH_ERROR: Buffer full
+     */
+    lorawan_status_t add_tx_param_setup_ans();
+
+    /**
+     * @brief Adds a new DlChannelAns MAC command to be sent.
+     *
+     * @param [in] status Status bits
+     *
+     * @return status  Function status: LORAWAN_STATUS_OK: OK,
+     *                                  LORAWAN_STATUS_LENGTH_ERROR: Buffer full
+     */
+    lorawan_status_t add_dl_channel_ans(uint8_t status);
+
 private:
     /**
      * Indicates if the MAC layer wants to send MAC commands