[LoRaWAN] Resolve warnings, fix bugs for fixed bands (#1021)

* [LoRaWAN] Resolve warnings * [LoRaWAN] Fixed bands: improve initial datarate, fix CFList bug * [LoRaWAN] Improve MAC debug output formatting * Fix hexdump debug level * Remove unnecessary error, add new ones to keywords * [LoRaWAN] Discard useless check --------- Co-authored-by: StevenCellist <[email protected]>
jgromes · Mar 18, 2024 · cfc4259 · cfc4259
1 parent 44f6c1d
commit cfc4259
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 76 deletions.
diff --git a/keywords.txt b/keywords.txt
@@ -430,9 +430,10 @@ RADIOLIB_ERR_INVALID_CHANNEL	LITERAL1
 RADIOLIB_ERR_INVALID_CID	LITERAL1
 RADIOLIB_ERR_UPLINK_UNAVAILABLE	LITERAL1
 RADIOLIB_ERR_COMMAND_QUEUE_FULL	LITERAL1
-RADIOLIB_ERR_COMMAND_QUEUE_EMPTY	LITERAL1
 RADIOLIB_ERR_COMMAND_QUEUE_ITEM_NOT_FOUND	LITERAL1
 RADIOLIB_ERR_JOIN_NONCE_INVALID	LITERAL1
 RADIOLIB_ERR_N_FCNT_DOWN_INVALID	LITERAL1
 RADIOLIB_ERR_A_FCNT_DOWN_INVALID	LITERAL1
-RADIOLIB_ERR_DATA_RATE_INVALID	LITERAL1
+RADIOLIB_ERR_DATA_RATE_INVALID	LITERAL1
+RADIOLIB_ERR_DWELL_TIME_EXCEEDED LITERAL1
+RADIOLIB_ERR_CHECKSUM_MISMATCH	LITERAL1
diff --git a/src/BuildOpt.h b/src/BuildOpt.h
@@ -479,7 +479,7 @@
     #define RADIOLIB_DEBUG_PRINT_FLOAT(LEVEL, VAL, DECIMALS) RADIOLIB_DEBUG_PRINT(LEVEL "%.3f", VAL)
   #endif
 
-  #define RADIOLIB_DEBUG_HEXDUMP(LEVEL, ...) RADIOLIB_DEBUG_PRINT(LEVEL); Module::hexdump(__VA_ARGS__)
+  #define RADIOLIB_DEBUG_HEXDUMP(LEVEL, ...) Module::hexdump(LEVEL, __VA_ARGS__)
 #else
   #define RADIOLIB_DEBUG_PRINT(...) {}
   #define RADIOLIB_DEBUG_PRINTLN(...) {}

diff --git a/src/Module.cpp b/src/Module.cpp
@@ -407,7 +407,7 @@ uint32_t Module::reflect(uint32_t in, uint8_t bits) {
 }
 
 #if RADIOLIB_DEBUG
-void Module::hexdump(uint8_t* data, size_t len, uint32_t offset, uint8_t width, bool be) {
+void Module::hexdump(const char* level, uint8_t* data, size_t len, uint32_t offset, uint8_t width, bool be) {
   size_t rem_len = len;
   for(size_t i = 0; i < len; i+=16) {
     char str[80];
@@ -446,20 +446,23 @@ void Module::hexdump(uint8_t* data, size_t len, uint32_t offset, uint8_t width,
     for(size_t j = line_len; j < 16; j++) {
       sprintf(&str[58 + j], "   ");
     }
+    if(level) {
+      RADIOLIB_DEBUG_PRINT(level);
+    }
     RADIOLIB_DEBUG_PRINT(str);
     RADIOLIB_DEBUG_PRINTLN();
     rem_len -= 16;
   }
 }
 
-void Module::regdump(uint16_t start, size_t len) {
+void Module::regdump(const char* level, uint16_t start, size_t len) {
   #if RADIOLIB_STATIC_ONLY
     uint8_t buff[RADIOLIB_STATIC_ARRAY_SIZE];
   #else
     uint8_t* buff = new uint8_t[len];
   #endif
   SPIreadRegisterBurst(start, len, buff);
-  hexdump(buff, len, start);
+  hexdump(level, buff, len, start);
   #if !RADIOLIB_STATIC_ONLY
     delete[] buff;
   #endif

diff --git a/src/Module.h b/src/Module.h
@@ -471,19 +471,21 @@ class Module {
     #if RADIOLIB_DEBUG
     /*!
       \brief Function to dump data as hex into the debug port.
+      \param level RadioLib debug level, set to NULL to not print.
       \param data Data to dump.
       \param len Number of bytes to dump.
       \param width Word width (1 for uint8_t, 2 for uint16_t, 4 for uint32_t).
       \param be Print multi-byte data as big endian. Defaults to false.
     */
-    static void hexdump(uint8_t* data, size_t len, uint32_t offset = 0, uint8_t width = 1, bool be = false);
+    static void hexdump(const char* level, uint8_t* data, size_t len, uint32_t offset = 0, uint8_t width = 1, bool be = false);
 
     /*!
       \brief Function to dump device registers as hex into the debug port.
+      \param level RadioLib debug level, set to NULL to not print.
       \param start First address to dump.
       \param len Number of bytes to dump.
     */
-    void regdump(uint16_t start, size_t len);
+    void regdump(const char* level, uint16_t start, size_t len);
     #endif
 
     #if RADIOLIB_DEBUG and defined(RADIOLIB_BUILD_ARDUINO)

diff --git a/src/TypeDef.h b/src/TypeDef.h
@@ -528,40 +528,35 @@
 */
 #define RADIOLIB_ERR_COMMAND_QUEUE_FULL                         (-1109)
 
-/*!
-  \brief Unable to pop existing MAC command because the queue is empty.
-*/
-#define RADIOLIB_ERR_COMMAND_QUEUE_EMPTY                        (-1110)
-
 /*!
   \brief Unable to delete MAC command because it was not found in the queue.
 */
-#define RADIOLIB_ERR_COMMAND_QUEUE_ITEM_NOT_FOUND               (-1111)
+#define RADIOLIB_ERR_COMMAND_QUEUE_ITEM_NOT_FOUND               (-1110)
 
 /*!
   \brief Unable to join network because JoinNonce is not higher than saved value.
 */
-#define RADIOLIB_ERR_JOIN_NONCE_INVALID                         (-1112)
+#define RADIOLIB_ERR_JOIN_NONCE_INVALID                         (-1111)
 
 /*!
   \brief Received downlink Network frame counter is invalid (lower than last heard value).
 */
-#define RADIOLIB_ERR_N_FCNT_DOWN_INVALID                        (-1113)
+#define RADIOLIB_ERR_N_FCNT_DOWN_INVALID                        (-1112)
 
 /*!
   \brief Received downlink Application frame counter is invalid (lower than last heard value).
 */
-#define RADIOLIB_ERR_A_FCNT_DOWN_INVALID                        (-1114)
+#define RADIOLIB_ERR_A_FCNT_DOWN_INVALID                        (-1113)
 
 /*!
   \brief Uplink payload length at this datarate exceeds the active dwell time limitations.
 */
-#define RADIOLIB_ERR_DWELL_TIME_EXCEEDED                        (-1115)
+#define RADIOLIB_ERR_DWELL_TIME_EXCEEDED                        (-1114)
 
 /*!
   \brief The buffer integrity check did not match the supplied checksum value.
 */
-#define RADIOLIB_ERR_CHECKSUM_MISMATCH                          (-1116)
+#define RADIOLIB_ERR_CHECKSUM_MISMATCH                          (-1115)
 
 /*!
   \}

diff --git a/src/protocols/LoRaWAN/LoRaWAN.cpp b/src/protocols/LoRaWAN/LoRaWAN.cpp
@@ -258,7 +258,6 @@ int16_t LoRaWANNode::restoreChannels() {
     this->setupChannelsFix(this->subBand);
   }
 
-  Module* mod = this->phyLayer->getMod();
   uint8_t bufferZeroes[5] = { 0 };
   if(this->band->bandType == RADIOLIB_LORAWAN_BAND_DYNAMIC) {
     uint8_t *startChannelsUp = &this->bufferSession[RADIOLIB_LORAWAN_SESSION_UL_CHANNELS];
@@ -309,38 +308,72 @@ int16_t LoRaWANNode::restoreChannels() {
   return(RADIOLIB_ERR_NONE);
 }
 
-void LoRaWANNode::beginCommon(uint8_t joinDr) {
+void LoRaWANNode::beginCommon(uint8_t initialDr) {
   // in case a new session is started while there is an ongoing session
   // clear the MAC queues completely
   memset(&(this->commandsUp), 0, sizeof(LoRaWANMacCommandQueue_t));
   memset(&(this->commandsDown), 0, sizeof(LoRaWANMacCommandQueue_t));
 
-  LoRaWANMacCommand_t cmd = {
-    .cid = RADIOLIB_LORAWAN_MAC_LINK_ADR,
-    .payload = { 0 }, 
-    .len = MacTable[RADIOLIB_LORAWAN_MAC_LINK_ADR].lenDn,
-    .repeat = 0,
-  };
+  uint8_t drUp = 0;
   if(this->band->bandType == RADIOLIB_LORAWAN_BAND_DYNAMIC) {
-    uint8_t drUp = 0;
-    // if join datarate is user-specified and valid, select that value; otherwise use
-    if(joinDr != RADIOLIB_LORAWAN_DATA_RATE_UNUSED) {
-      if(joinDr >= this->band->txFreqs[0].drMin && joinDr <= this->band->txFreqs[0].drMax) {
-        drUp = joinDr;
+    // if join datarate is user-specified and valid, select that value
+    if(initialDr != RADIOLIB_LORAWAN_DATA_RATE_UNUSED) {
+      if(initialDr >= this->band->txFreqs[0].drMin && initialDr <= this->band->txFreqs[0].drMax) {
+        drUp = initialDr;
       } else {
-        RADIOLIB_DEBUG_PROTOCOL_PRINTLN("Datarate %d is not valid (min: %d, max %d) - using default", 
-                                joinDr, this->band->txFreqs[0].drMin, this->band->txFreqs[0].drMax);
-        joinDr = RADIOLIB_LORAWAN_DATA_RATE_UNUSED;
+        // if there is no channel that allowed the user-specified datarate, revert to default datarate
+        RADIOLIB_DEBUG_PROTOCOL_PRINTLN("Datarate %d is not valid - using default", initialDr);
+        initialDr = RADIOLIB_LORAWAN_DATA_RATE_UNUSED;
       }
-    } 
-    if(joinDr == RADIOLIB_LORAWAN_DATA_RATE_UNUSED) {
+    }
+
+    // if there is no (channel that allowed the) user-specified datarate, use a default datarate
+    // we use the floor of the average datarate of the first default channel
+    if(initialDr == RADIOLIB_LORAWAN_DATA_RATE_UNUSED) {
       drUp = (this->band->txFreqs[0].drMin + this->band->txFreqs[0].drMax) / 2;
     }
-    cmd.payload[0] = (drUp << 4);
+
   } else {
-    uint8_t drJr = this->band->txSpans[0].joinRequestDataRate;
-    cmd.payload[0] = (drJr << 4);
+    // if the user specified a certain datarate, check if any of the configured channels allows it
+    if(initialDr != RADIOLIB_LORAWAN_DATA_RATE_UNUSED) {
+      uint8_t i = 0; 
+      for(; i < RADIOLIB_LORAWAN_NUM_AVAILABLE_CHANNELS; i++) {
+        if(this->availableChannels[RADIOLIB_LORAWAN_CHANNEL_DIR_UPLINK][i].enabled) {
+          if(initialDr >= this->availableChannels[RADIOLIB_LORAWAN_CHANNEL_DIR_UPLINK][i].drMin
+            && initialDr <= this->availableChannels[RADIOLIB_LORAWAN_CHANNEL_DIR_UPLINK][i].drMax) {
+              break;
+          }
+        }
+      }
+      // if there is no channel that allowed the user-specified datarate, revert to default datarate
+      if(i == RADIOLIB_LORAWAN_NUM_AVAILABLE_CHANNELS) {
+        RADIOLIB_DEBUG_PROTOCOL_PRINTLN("Datarate %d is not valid - using default", initialDr);
+        initialDr = RADIOLIB_LORAWAN_DATA_RATE_UNUSED;
+      }
+    }
+
+    // if there is no (channel that allowed the) user-specified datarate, use a default datarate
+    // we use the join-request datarate for one of the available channels
+    if(initialDr == RADIOLIB_LORAWAN_DATA_RATE_UNUSED) {
+      // randomly select one of 8 or 9 channels and find corresponding datarate
+      uint8_t numChannels = this->band->numTxSpans == 1 ? 8 : 9;
+      uint8_t rand = this->phyLayer->random(numChannels) + 1;     // range 1-8 or 1-9
+      if(rand <= 8) {
+        drUp = this->band->txSpans[0].joinRequestDataRate;        // if one of the first 8 channels, select datarate of span 0
+      } else {
+        drUp = this->band->txSpans[1].joinRequestDataRate;        // if ninth channel, select datarate of span 1
+      }
+    }
+
   }
+
+  LoRaWANMacCommand_t cmd = {
+    .cid = RADIOLIB_LORAWAN_MAC_LINK_ADR,
+    .payload = { 0 }, 
+    .len = MacTable[RADIOLIB_LORAWAN_MAC_LINK_ADR].lenDn,
+    .repeat = 0,
+  };
+  cmd.payload[0]  = (drUp << 4);      // set uplink datarate
   cmd.payload[0] |= 0;                // default to max Tx Power
   cmd.payload[3]  = (1 << 7);         // set the RFU bit, which means that the channel mask gets ignored
   (void)execMacCommand(&cmd);
@@ -457,7 +490,12 @@ int16_t LoRaWANNode::beginOTAA(uint64_t joinEUI, uint64_t devEUI, uint8_t* nwkKe
   }
   RADIOLIB_ASSERT(state);
 
-  // setup all MAC properties to default values  
+  // on fixed bands, the join-datarate is specified per specification
+  // therefore, we ignore the value that was specified by the user
+  if(this->band->bandType == RADIOLIB_LORAWAN_BAND_FIXED) {
+    joinDr = RADIOLIB_LORAWAN_DATA_RATE_UNUSED;
+  }
+  // setup all MAC properties to default values
   this->beginCommon(joinDr);
 
   // set the physical layer configuration
@@ -694,7 +732,7 @@ int16_t LoRaWANNode::beginOTAA(uint64_t joinEUI, uint64_t devEUI, uint8_t* nwkKe
   return(RADIOLIB_ERR_NONE);
 }
 
-int16_t LoRaWANNode::beginABP(uint32_t addr, uint8_t* nwkSKey, uint8_t* appSKey, uint8_t* fNwkSIntKey, uint8_t* sNwkSIntKey, bool force) {
+int16_t LoRaWANNode::beginABP(uint32_t addr, uint8_t* nwkSKey, uint8_t* appSKey, uint8_t* fNwkSIntKey, uint8_t* sNwkSIntKey, bool force, uint8_t initialDr) {
   // if not forced and already joined, don't do anything
   if(!force && this->isJoined()) {
     RADIOLIB_DEBUG_PROTOCOL_PRINTLN("beginABP(): Did not rejoin: session already active");
@@ -744,7 +782,7 @@ int16_t LoRaWANNode::beginABP(uint32_t addr, uint8_t* nwkSKey, uint8_t* appSKey,
   }
 
   // setup all MAC properties to default values
-  this->beginCommon();
+  this->beginCommon(initialDr);
 
   // set the physical layer configuration
   state = this->setPhyProperties();
@@ -780,8 +818,6 @@ bool LoRaWANNode::isJoined() {
 }
 
 int16_t LoRaWANNode::saveSession() {
-  Module* mod = this->phyLayer->getMod();
-
   // store DevAddr and all keys
   LoRaWANNode::hton<uint32_t>(&this->bufferSession[RADIOLIB_LORAWAN_SESSION_DEV_ADDR], this->devAddr);
   memcpy(&this->bufferSession[RADIOLIB_LORAWAN_SESSION_APP_SKEY], this->appSKey, RADIOLIB_AES128_BLOCK_SIZE);
@@ -1734,16 +1770,6 @@ int16_t LoRaWANNode::setupChannelsFix(uint8_t subBand) {
   for(size_t i = 0; i < RADIOLIB_LORAWAN_NUM_AVAILABLE_CHANNELS; i++) {
     this->availableChannels[RADIOLIB_LORAWAN_CHANNEL_DIR_UPLINK][i] = RADIOLIB_LORAWAN_CHANNEL_NONE;
   }
-
-  // randomly select one of 8 or 9 channels and find corresponding datarate
-  uint8_t numChannels = this->band->numTxSpans == 1 ? 8 : 9;
-  uint8_t rand = this->phyLayer->random(numChannels) + 1;     // range 1-8 or 1-9
-  uint8_t drJR = RADIOLIB_LORAWAN_DATA_RATE_UNUSED;
-  if(rand <= 8) {
-    drJR = this->band->txSpans[0].joinRequestDataRate;        // if one of the first 8 channels, select datarate of span 0
-  } else {
-    drJR = this->band->txSpans[1].joinRequestDataRate;        // if ninth channel, select datarate of span 1
-  }
 
   // if no subband is selected by user, cycle through banks of 8 using devNonce value
   if(subBand == 0) {
@@ -1814,18 +1840,17 @@ int16_t LoRaWANNode::processCFList(uint8_t* cfList) {
       .len = 0,
       .repeat = 0,
     };
-    cmd.payload[0] = 0xFF;  // same datarate and payload
 
     // in case of mask-type bands, copy those frequencies that are masked true into the available TX channels
-    size_t numChMasks = 3 + this->band->numTxSpans;   // 4 masks for bands with 2 spans, 5 spans for bands with 1 span
+    size_t numChMasks = 3 + this->band->numTxSpans;       // 4 masks for bands with 2 spans, 5 spans for bands with 1 span
     for(size_t chMaskCntl = 0; chMaskCntl < numChMasks; chMaskCntl++) {
       cmd.len = MacTable[RADIOLIB_LORAWAN_MAC_LINK_ADR].lenDn;
-      cmd.payload[3] = chMaskCntl << 4;               // NbTrans = 0 -> keep the same
+      cmd.payload[0] = 0xFF;                              // same datarate and payload
+      memcpy(&cmd.payload[1], &cfList[chMaskCntl*2], 2);  // copy mask
+      cmd.payload[3] = chMaskCntl << 4;                   // set chMaskCntl, set NbTrans = 0 -> keep the same
       cmd.repeat = (chMaskCntl + 1);
-      memcpy(&cmd.payload[1], &cfList[chMaskCntl*2], 2);
       (void)execMacCommand(&cmd);
     }
-    // delete the ADR response
   }
 
   return(RADIOLIB_ERR_NONE);
@@ -2097,10 +2122,6 @@ int16_t LoRaWANNode::pushMacCommand(LoRaWANMacCommand_t* cmd, LoRaWANMacCommandQ
 }
 
 int16_t LoRaWANNode::deleteMacCommand(uint8_t cid, LoRaWANMacCommandQueue_t* queue, uint8_t* payload) {
-  if(queue->numCommands == 0) {
-    return(RADIOLIB_ERR_COMMAND_QUEUE_EMPTY);
-  }
-
   for(size_t index = 0; index < queue->numCommands; index++) {
     if(queue->commands[index].cid == cid) {
       // if a pointer to a payload is supplied, copy the command's payload over
@@ -2123,11 +2144,8 @@ int16_t LoRaWANNode::deleteMacCommand(uint8_t cid, LoRaWANMacCommandQueue_t* que
 }
 
 bool LoRaWANNode::execMacCommand(LoRaWANMacCommand_t* cmd) {
-  RADIOLIB_DEBUG_PROTOCOL_PRINT("[MAC] 0x%02X %s", cmd->cid, cmd->len ? "= 0x" : "");
-  for(uint8_t i = 0; i < cmd->len; i++) {
-    RADIOLIB_DEBUG_PROTOCOL_PRINT("%02X", cmd->payload[i]);
-  }
-  RADIOLIB_DEBUG_PROTOCOL_PRINTLN();
+  RADIOLIB_DEBUG_PROTOCOL_PRINTLN("[MAC] 0x%02X", cmd->cid);
+  RADIOLIB_DEBUG_PROTOCOL_HEXDUMP(cmd->payload, cmd->len);
 
   if(cmd->cid >= RADIOLIB_LORAWAN_MAC_PROPRIETARY) {
     // TODO call user-provided callback for proprietary MAC commands?
@@ -2228,7 +2246,6 @@ bool LoRaWANNode::execMacCommand(LoRaWANMacCommand_t* cmd) {
           chMaskAck = (uint8_t)this->applyChannelMaskDyn(chMaskCntl, chMask);
 
         } else {                // RADIOLIB_LORAWAN_BAND_FIXED
-          bool clearChannels = false;
           if(cmd->repeat == 1) {
             // if this is the first ADR command in the queue, clear all saved channels
             // so we can apply the new channel mask
@@ -2270,7 +2287,7 @@ bool LoRaWANNode::execMacCommand(LoRaWANMacCommand_t* cmd) {
       } else {                // RADIOLIB_LORAWAN_BAND_FIXED
 
         // save Tx/Dr to the Link ADR position in the session buffer 
-        uint8_t bufTxDr[cmd->len] = { 0 };
+        uint8_t bufTxDr[RADIOLIB_LORAWAN_MAX_MAC_COMMAND_LEN_DOWN] = { 0 };
         bufTxDr[0] = cmd->payload[0];
         bufTxDr[3] = 1 << 7;
         memcpy(&this->bufferSession[RADIOLIB_LORAWAN_SESSION_LINK_ADR], bufTxDr, cmd->len);
@@ -2672,9 +2689,8 @@ bool LoRaWANNode::applyChannelMaskFix(uint8_t chMaskCntl, uint16_t chMask) {
   }
   if(this->band->numTxSpans == 2 && chMaskCntl == 6) {
     // all channels on (but we revert to selected subband)
-    if(this->subBand >= 0) {
-      this->setupChannelsFix(this->subBand);
-    }
+    this->setupChannelsFix(this->subBand);
+
     // a '1' enables a single channel from second span
     LoRaWANChannel_t chnl;
     for(uint8_t i = 0; i < 8; i++) {

diff --git a/src/protocols/LoRaWAN/LoRaWAN.h b/src/protocols/LoRaWAN/LoRaWAN.h
@@ -528,9 +528,10 @@ class LoRaWANNode {
       \param fNwkSIntKey Pointer to the Forwarding network session (LoRaWAN 1.1), unused for LoRaWAN 1.0.
       \param sNwkSIntKey Pointer to the Serving network session (LoRaWAN 1.1), unused for LoRaWAN 1.0.
       \param force Set to true to force a new session, even if one exists.
+      \param initialDr The datarate at which to send the first uplink and any subsequent uplinks (unless ADR is enabled)
       \returns \ref status_codes
     */
-    int16_t beginABP(uint32_t addr, uint8_t* nwkSKey, uint8_t* appSKey, uint8_t* fNwkSIntKey = NULL, uint8_t* sNwkSIntKey = NULL, bool force = false);
+    int16_t beginABP(uint32_t addr, uint8_t* nwkSKey, uint8_t* appSKey, uint8_t* fNwkSIntKey = NULL, uint8_t* sNwkSIntKey = NULL, bool force = false, uint8_t initialDr = RADIOLIB_LORAWAN_DATA_RATE_UNUSED);
 
     /*! \brief Whether there is an ongoing session active */
     bool isJoined();
@@ -806,7 +807,7 @@ class LoRaWANNode {
 
     static int16_t checkBufferCommon(uint8_t *buffer, uint16_t size);
 
-    void beginCommon(uint8_t joinDr = RADIOLIB_LORAWAN_DATA_RATE_UNUSED);
+    void beginCommon(uint8_t initialDr);
 
     // a buffer that holds all LW base parameters that should persist at all times!
     uint8_t bufferNonces[RADIOLIB_LORAWAN_NONCES_BUF_SIZE] = { 0 };