Message flags correction

Uplink multicast is not allowed. Proprietary messages cannot be
of type unconfirmed and unconfirmed.

Author: Hasnain Virk

features/lorawan/LoRaWANStack.cpp

@@ -64,6 +64,12 @@ using namespace events;
     #endif
 #endif
 
+/**
+ * Bit mask for message flags
+ */
+
+#define MSG_FLAG_MASK                         0x0F
+
 /*****************************************************************************
  * Constructor                                                               *
  ****************************************************************************/
@@ -292,10 +298,17 @@ int16_t LoRaWANStack::handle_tx(const uint8_t port, const uint8_t* data,
         return status;
     }
 
-    if (flags == 0 ||
-        (flags & MSG_FLAG_MASK) == (MSG_CONFIRMED_FLAG|MSG_UNCONFIRMED_FLAG)) {
-        tr_error("CONFIRMED and UNCONFIRMED are mutually exclusive for send()");
-        return LORAWAN_STATUS_PARAMETER_INVALID;
+    // All the flags mutually exclusive. In addition to that MSG_MULTICAST_FLAG cannot be
+    // used for uplink.
+    switch (flags & MSG_FLAG_MASK) {
+        case MSG_UNCONFIRMED_FLAG:
+        case MSG_CONFIRMED_FLAG:
+        case MSG_PROPRIETARY_FLAG:
+            break;
+
+        default:
+            tr_error("Invalid send flags");
+            return LORAWAN_STATUS_PARAMETER_INVALID;
     }
 
     int16_t len = _loramac.prepare_ongoing_tx(port, data, length, flags, _num_retry);
@@ -533,8 +546,8 @@ void LoRaWANStack::process_reception(const uint8_t* const payload, uint16_t size
                 state_controller(DEVICE_STATE_STATUS_CHECK);
             }
         } else {
-            // handle UNCONFIRMED case here, RX slots were turned off due to
-            // valid packet reception
+            // handle UNCONFIRMED, PROPRIETARY case here, RX slots were turned off due to
+            // valid packet reception, so we generate a TX_DONE event
             _loramac.post_process_mcps_req();
             _ctrl_flags |= TX_DONE_FLAG;
             state_controller(DEVICE_STATE_STATUS_CHECK);

features/lorawan/lorastack/mac/LoRaMac.cpp

@@ -179,7 +179,7 @@ void LoRaMac::post_process_mcps_req()
         }
 
     } else {
-        //UNCONFIRMED
+        //UNCONFIRMED or PROPRIETARY
         if (_params.is_ul_frame_counter_fixed == false) {
             _params.ul_frame_counter++;
         }
@@ -1217,26 +1217,27 @@ int16_t LoRaMac::prepare_ongoing_tx(const uint8_t port,
         }
     }
 
-    // Handles all unconfirmed messages, including proprietary and multicast
-    if ((flags & MSG_FLAG_MASK) == MSG_UNCONFIRMED_FLAG
-            || (flags & MSG_FLAG_MASK) == MSG_UNCONFIRMED_MULTICAST
-            || (flags & MSG_FLAG_MASK) == MSG_UNCONFIRMED_PROPRIETARY) {
-
+    // Handles unconfirmed messages
+    if (flags & MSG_UNCONFIRMED_FLAG) {
          _ongoing_tx_msg.type = MCPS_UNCONFIRMED;
          _ongoing_tx_msg.fport = port;
          _ongoing_tx_msg.nb_trials = 1;
     }
 
-    // Handles all confirmed messages, including proprietary and multicast
-    if ((flags & MSG_FLAG_MASK) == MSG_CONFIRMED_FLAG
-            || (flags & MSG_FLAG_MASK) == MSG_CONFIRMED_MULTICAST
-            || (flags & MSG_FLAG_MASK) == MSG_CONFIRMED_PROPRIETARY) {
-
+    // Handles confirmed messages
+    if (flags & MSG_CONFIRMED_FLAG) {
         _ongoing_tx_msg.type = MCPS_CONFIRMED;
         _ongoing_tx_msg.fport = port;
         _ongoing_tx_msg.nb_trials = num_retries;
     }
 
+    // Handles proprietary messages
+    if (flags & MSG_PROPRIETARY_FLAG) {
+        _ongoing_tx_msg.type = MCPS_PROPRIETARY;
+        _ongoing_tx_msg.fport = port;
+        _ongoing_tx_msg.nb_trials = 1;
+    }
+
     tr_info("RTS = %u bytes, PEND = %u, Port: %u",
             _ongoing_tx_msg.f_buffer_size, _ongoing_tx_msg.pending_size,
             _ongoing_tx_msg.fport);
@@ -1269,7 +1270,6 @@ lorawan_status_t LoRaMac::send_ongoing_tx()
         machdr.bits.mtype = FRAME_TYPE_DATA_CONFIRMED_UP;
         _params.max_ack_timeout_retries = _ongoing_tx_msg.nb_trials;
     } else if (_ongoing_tx_msg.type == MCPS_PROPRIETARY) {
-        //TODO: Is this dead code currently??? Nobody sets this type
         machdr.bits.mtype = FRAME_TYPE_PROPRIETARY;
     } else {
         return LORAWAN_STATUS_SERVICE_UNKNOWN;

features/lorawan/lorawan_types.h

@@ -36,38 +36,16 @@
 
 /**
  * Option Flags for send(), receive() APIs
+ *
+ * Special Notes for UPLINK:
+ *  i)  All of the flags are mutually exclusive.
+ *  ii) MSG_MULTICAST_FLAG cannot be used.
  */
 #define MSG_UNCONFIRMED_FLAG                  0x01
 #define MSG_CONFIRMED_FLAG                    0x02
 #define MSG_MULTICAST_FLAG                    0x04
 #define MSG_PROPRIETARY_FLAG                  0x08
 
-/**
- * Bit mask for message flags
- */
-
-#define MSG_FLAG_MASK                         0x0F
-
-/**
- * Mask for unconfirmed multicast message
- */
-#define MSG_UNCONFIRMED_MULTICAST              0x05
-
-/**
- * Mask for confirmed multicast message
- */
-#define MSG_CONFIRMED_MULTICAST                0x06
-
-/**
- * Mask for unconfirmed message proprietary message
- */
-#define MSG_UNCONFIRMED_PROPRIETARY            0x09
-
-/**
- * Mask for confirmed proprietary message
- */
-#define MSG_CONFIRMED_PROPRIETARY              0x0A
-
 /**
  * LoRaWAN device classes definition.
  *