Merge pull request #7459 from hasnainvirk/issue_7232_7432

LoRaWAN: FRMPayload size validity

Author: Cruz Monrreal

features/lorawan/LoRaRadio.h

@@ -281,7 +281,7 @@ class LoRaRadio
      *  @param iq_inverted   Inverts IQ signals (LoRa only)
      *                          FSK : N/A (set to 0).
      *                          LoRa: [0: not inverted, 1: inverted]
-     *  @param timeout       The transmission timeout [us].
+     *  @param timeout       The transmission timeout [ms].
      */
     virtual void set_tx_config(radio_modems_t modem, int8_t power, uint32_t fdev,
                               uint32_t bandwidth, uint32_t datarate,

features/lorawan/lorastack/mac/LoRaMac.cpp

@@ -1033,6 +1033,7 @@ lorawan_status_t LoRaMac::schedule_tx()
 {
     channel_selection_params_t next_channel;
     lorawan_time_t backoff_time = 0;
+    uint8_t fopts_len = 0;
 
     if (_params.sys_params.max_duty_cycle == 255) {
         return LORAWAN_STATUS_DEVICE_OFF;
@@ -1090,9 +1091,25 @@ lorawan_status_t LoRaMac::schedule_tx()
         _params.rx_window2_delay = _params.sys_params.join_accept_delay2
                                    + _params.rx_window2_config.window_offset;
     } else {
-        if (validate_payload_length(_params.tx_buffer_len,
+
+        // if the outgoing message is a proprietary message, it doesn't include any
+        // standard message formatting except port and MHDR.
+        if (_ongoing_tx_msg.type == MCPS_PROPRIETARY) {
+            fopts_len = 0;
+        } else {
+            fopts_len = _mac_commands.get_mac_cmd_length() + _mac_commands.get_repeat_commands_length();
+        }
+
+        // A check was performed for validity of FRMPayload in ::prepare_ongoing_tx() API.
+        // However, owing to the asynch nature of the send() API, we should check the
+        // validity again, as datarate may have changed since we last attempted to transmit.
+        if (validate_payload_length(_ongoing_tx_msg.f_buffer_size,
                                     _params.sys_params.channel_data_rate,
-                                    _mac_commands.get_mac_cmd_length()) == false) {
+                                    fopts_len) == false) {
+            tr_error("Allowed FRMPayload = %d, FRMPayload = %d, MAC commands pending = %d",
+                     _lora_phy->get_max_payload(_params.sys_params.channel_data_rate,
+                                               _params.is_repeater_supported),
+                                               _ongoing_tx_msg.f_buffer_size, fopts_len);
             return LORAWAN_STATUS_LENGTH_ERROR;
         }
         _params.rx_window1_delay = _params.sys_params.recv_delay1
@@ -1215,27 +1232,9 @@ int16_t LoRaMac::prepare_ongoing_tx(const uint8_t port,
                                     uint8_t num_retries)
 {
     _ongoing_tx_msg.port = port;
-
-    uint8_t max_possible_size = get_max_possible_tx_size(length);
-
-    if (max_possible_size > MBED_CONF_LORA_TX_MAX_SIZE) {
-        max_possible_size = MBED_CONF_LORA_TX_MAX_SIZE;
-    }
-
-    if (max_possible_size < length) {
-        tr_info("Cannot transmit %d bytes. Possible TX Size is %d bytes",
-                length, max_possible_size);
-
-        _ongoing_tx_msg.pending_size = length - max_possible_size;
-        _ongoing_tx_msg.f_buffer_size = max_possible_size;
-        memcpy(_ongoing_tx_msg.f_buffer, data, _ongoing_tx_msg.f_buffer_size);
-    } else {
-        _ongoing_tx_msg.f_buffer_size = length;
-        _ongoing_tx_msg.pending_size = 0;
-        if (length > 0) {
-            memcpy(_ongoing_tx_msg.f_buffer, data, length);
-        }
-    }
+    uint8_t max_possible_size = 0;
+    uint8_t fopts_len = _mac_commands.get_mac_cmd_length()
+            + _mac_commands.get_repeat_commands_length();
 
     // Handles unconfirmed messages
     if (flags & MSG_UNCONFIRMED_FLAG) {
@@ -1256,6 +1255,30 @@ int16_t LoRaMac::prepare_ongoing_tx(const uint8_t port,
         _ongoing_tx_msg.type = MCPS_PROPRIETARY;
         _ongoing_tx_msg.fport = port;
         _ongoing_tx_msg.nb_trials = 1;
+        // a proprietary frame only includes an MHDR field which contains MTYPE field.
+        // Everything else is at the discretion of the implementer
+        fopts_len = 0;
+    }
+
+    max_possible_size = get_max_possible_tx_size(fopts_len);
+
+    if (max_possible_size > MBED_CONF_LORA_TX_MAX_SIZE) {
+        max_possible_size = MBED_CONF_LORA_TX_MAX_SIZE;
+    }
+
+    if (max_possible_size < length) {
+        tr_info("Cannot transmit %d bytes. Possible TX Size is %d bytes",
+                length, max_possible_size);
+
+        _ongoing_tx_msg.pending_size = length - max_possible_size;
+        _ongoing_tx_msg.f_buffer_size = max_possible_size;
+        memcpy(_ongoing_tx_msg.f_buffer, data, _ongoing_tx_msg.f_buffer_size);
+    } else {
+        _ongoing_tx_msg.f_buffer_size = length;
+        _ongoing_tx_msg.pending_size = 0;
+        if (length > 0) {
+            memcpy(_ongoing_tx_msg.f_buffer, data, length);
+        }
     }
 
     tr_info("RTS = %u bytes, PEND = %u, Port: %u",
@@ -1566,8 +1589,10 @@ lorawan_status_t LoRaMac::prepare_frame(loramac_mhdr_t *machdr,
 
             _mac_commands.parse_mac_commands_to_repeat();
 
+            // We always add Port Field. Spec leaves it optional.
+            _params.tx_buffer[pkt_header_len++] = frame_port;
+
             if ((payload != NULL) && (_params.tx_buffer_len > 0)) {
-                _params.tx_buffer[pkt_header_len++] = frame_port;
 
                 uint8_t *key = _params.keys.app_skey;
                 uint32_t key_length = sizeof(_params.keys.app_skey) * 8;
@@ -1759,35 +1784,30 @@ void LoRaMac::disconnect()
     reset_mcps_indication();
 }
 
-uint8_t LoRaMac::get_max_possible_tx_size(uint8_t size)
+uint8_t LoRaMac::get_max_possible_tx_size(uint8_t fopts_len)
 {
     uint8_t max_possible_payload_size = 0;
-    uint8_t current_payload_size = 0;
-    uint8_t fopt_len = _mac_commands.get_mac_cmd_length()
-                       + _mac_commands.get_repeat_commands_length();
+    uint8_t allowed_frm_payload_size = 0;
 
     if (_params.sys_params.adr_on) {
         _lora_phy->get_next_ADR(false, _params.sys_params.channel_data_rate,
                                _params.sys_params.channel_tx_power,
                                _params.adr_ack_counter);
     }
 
-    current_payload_size = _lora_phy->get_max_payload(_params.sys_params.channel_data_rate, _params.is_repeater_supported);
+    allowed_frm_payload_size = _lora_phy->get_max_payload(_params.sys_params.channel_data_rate,
+                                                          _params.is_repeater_supported);
 
-    if (current_payload_size >= fopt_len) {
-        max_possible_payload_size = current_payload_size - fopt_len;
+    if (allowed_frm_payload_size >= fopts_len) {
+        max_possible_payload_size = allowed_frm_payload_size - fopts_len;
     } else {
-        max_possible_payload_size = current_payload_size;
-        fopt_len = 0;
+        max_possible_payload_size = allowed_frm_payload_size;
+        fopts_len = 0;
         _mac_commands.clear_command_buffer();
         _mac_commands.clear_repeat_buffer();
     }
 
-    if (validate_payload_length(size, _params.sys_params.channel_data_rate,
-                                fopt_len) == false) {
-        return max_possible_payload_size;
-    }
-    return current_payload_size;
+    return max_possible_payload_size;
 }
 
 bool LoRaMac::nwk_joined()

features/lorawan/lorastack/mac/LoRaMac.h

@@ -93,18 +93,18 @@ class LoRaMac {
     void disconnect(void);
 
     /**
-     * @brief   Queries the LoRaMAC whether it is possible to send the next frame with
-     *          a given payload size. The LoRaMAC takes the scheduled MAC commands into
-     *          account and returns corresponding value.
+     * @brief   Queries the LoRaMAC the maximum possible FRMPayload size to send.
+     *          The LoRaMAC takes the scheduled MAC commands into account and returns
+     *          corresponding value.
      *
-     * @param   size     [in]    The size of the applicable payload to be sent next.
+     * @param   fopts_len     [in]    Number of mac commands in the queue pending.
      *
      * @return  Size of the biggest packet that can be sent.
      *          Please note that if the size of the MAC commands in the queue do
      *          not fit into the payload size on the related datarate, the LoRaMAC will
      *          omit the MAC commands.
      */
-    uint8_t get_max_possible_tx_size(uint8_t size);
+    uint8_t get_max_possible_tx_size(uint8_t fopts_len);
 
     /**
      * @brief nwk_joined Checks if device has joined to network
@@ -316,13 +316,18 @@ class LoRaMac {
 
     /**
      * @brief prepare_ongoing_tx This will prepare (and override) ongoing_tx_msg.
-     * @param port The application port number.
-     * @param data A pointer to the data being sent. The ownership of the
-     *             buffer is not transferred.
-     * @param length The size of data in bytes.
-     * @param flags A flag used to determine what type of
-     *              message is being sent.
-     * @param num_retries Number of retries for a confirmed type message
+     * @param port                          The application port number.
+     *
+     * @param data                          A pointer to the data being sent. The ownership of the
+     *                                      buffer is not transferred.
+     *
+     * @param length                        The size of data in bytes.
+     *
+     * @param flags                         A flag used to determine what type of
+     *                                      message is being sent.
+     *
+     * @param num_retries                   Number of retries for a confirmed type message
+     *
      * @return The number of bytes prepared for sending.
      */
     int16_t prepare_ongoing_tx(const uint8_t port, const uint8_t *data,