Merge pull request #452 from mcci-catena/issue447

Fix errors in US and AS923 tests (and AU915 implementation)

Author: terrillmoore

README.md

@@ -120,13 +120,12 @@ The LMIC library provides a fairly complete LoRaWAN Class A and Class B
 implementation, supporting the EU-868, US-915, AU-921, AS-923, and IN-866 bands. Only a limited
 number of features was tested using this port on Arduino hardware, so be careful when using any of the untested features.
 
-The library has only been tested with LoRaWAN 1.0.2 networks and does not have the separated key structure defined by LoRaWAN 1.1.
+The library has only been tested with LoRaWAN 1.0.2/1.03 networks and does not have the separated key structure defined by LoRaWAN 1.1.
 
 What certainly works:
 
 - Sending packets uplink, taking into account duty cycling.
 - Encryption and message integrity checking.
-- Receiving downlink packets in the RX2 window.
 - Custom frequencies and data rate settings.
 - Over-the-air activation (OTAA / joining).
 - Receiving downlink packets in the RX1 and RX2 windows.
@@ -207,9 +206,11 @@ The library supports the following regions:
 `-D CFG_kr920` | `LMIC_REGION_kr920` | 8 | 2.9 | Korea 920-923 MHz ISM
 `-D CFG_in866` | `LMIC_REGION_in866` | 9 | 2.10 | India 865-867 MHz ISM
 
-You should define exactly one of `CFG_...` variables. If you don't,
-the library assumes `CFG_eu868`. The library changes configuration pretty substantially
-according to the region. Some of the differences are listed below.
+The library requires that the compile environment or the project config file define exactly one of `CFG_...` variables. As released, `project_config/lmic_project_config.h` defines `CFG_us915`.  If you build with PlatformIO or other environments, and you do not provide a pointer to the platform config file, `src/lmic/config.h` will define `CFG_eu868`.
+
+MCCI BSPs add menu entries to the Arduino IDE so you can select the target region interactively.
+
+The library changes configuration pretty substantially according to the region selected, and this affects the symbols in-scope in your sketches and cpp files. Some of the differences are listed below. This list is not comprehensive, and is subject to change in future major releases.
 
 #### eu868, as923, in866, kr920
 
@@ -1126,6 +1127,7 @@ function uflt12f(rawUflt12)
 
 - v3.0.99 (still in pre-release) adds the following changes. (This is not an exhaustive list.) Note that the behavior of the LMIC changes in important ways, as it now enforces the LoRaWAN mandated maximum frame size for a given data rate. For Class A devices, this may cause your device to go silent after join, if you're not able to handle the frame size dictated by the parameters downloaded to the device by the network during join. The library will attempt to find a data rate that will work, but there is no guarantee that the network has provided such a data rate.
 
+  - [#452](https://github.com/mcci-catena/arduino-lmic/pull/452) fixes a bug [#450](https://github.com/mcci-catena/arduino-lmic/issues/450) in `LMIC_clrTxData()` that would cause join hiccups if called while (1) a join was in progress and (2) a regular data packet was waiting to be uplinked after the join completes. Also fixes AS923- and AU915-specific bugs [#446](https://github.com/mcci-catena/arduino-lmic/issues/446), [#447](https://github.com/mcci-catena/arduino-lmic/issues/447), [#448](https://github.com/mcci-catena/arduino-lmic/issues/448). Version is `v3.0.99.5`.
   - [#443](https://github.com/mcci-catena/arduino-lmic/pull/443) addresses a number of problems found in cooperation with [RedwoodComm](https://redwoodcomm.com). They suggested a timing improvement to speed testing; this lead to the discovery of a number of problems. Some were in the compliance framework, but one corrects timing for very high spreading factors, several ([#442](https://github.com/mcci-catena/arduino-lmic/issues/442), [#436](https://github.com/mcci-catena/arduino-lmic/issues/438), [#435](https://github.com/mcci-catena/arduino-lmic/issues/435), [#434](https://github.com/mcci-catena/arduino-lmic/issues/434) fix glaring problems in FSK support; [#249](https://github.com/mcci-catena/arduino-lmic/issues/249) greatly enhances stability by making API calls much less likely to crash the LMIC if it's active. Version is v3.0.99.3.
   - [#388](https://github.com/mcci-catena/arduino-lmic/issues/388), [#389](https://github.com/mcci-catena/arduino-lmic/issues/390), [#390](https://github.com/mcci-catena/arduino-lmic/issues/390) change the LMIC to honor the maximum frame size for a given DR in the current region. This proves to be a breaking change for many applications, especially in the US, because DR0 in the US supports only an 11-byte payload, and many apps were ignoring this. Additional error codes were defined so that apps can detect and recover from this situation, but they must detect; otherwise they run the risk of being blocked from the network by the LMIC.  Because of this change, the next version of the LMIC will be V3.1 or higher, and the LMIC version for development is bumped to 3.0.99.0.
   - [#401](https://github.com/mcci-catena/arduino-lmic/issues/401) adds 865 MHz through 868 MHz to the "1%" band for EU.

examples/compliance-otaa-halconfig/compliance-otaa-halconfig.ino

@@ -314,7 +314,7 @@ void printFcnts(cEventQueue::eventnode_t &e) {
 }
 
 #if LMIC_ENABLE_event_logging
-// dump all the registers.  Must have printf setup.
+// dump all the registers.
 void printAllRegisters(void) {
     uint8_t regbuf[0x80];
     regbuf[0] = 0;
@@ -400,21 +400,21 @@ void eventPrint(cEventQueue::eventnode_t &e) {
                     u1_t nwkKey[16];
                     u1_t artKey[16];
                     LMIC_getSessionKeys(&netid, &devaddr, nwkKey, artKey);
-                    Serial.print("netid: ");
+                    Serial.print(F("netid: "));
                     Serial.println(netid, DEC);
-                    Serial.print("devaddr: ");
+                    Serial.print(F("devaddr: "));
                     Serial.println(devaddr, HEX);
-                    Serial.print("artKey: ");
-                    for (int i=0; i<sizeof(artKey); ++i) {
+                    Serial.print(F("artKey: "));
+                    for (size_t i=0; i<sizeof(artKey); ++i) {
                         if (i != 0)
-                            Serial.print("-");
+                            Serial.print('-');
                         printHex2(artKey[i]);
                     }
-                    Serial.println("");
-                    Serial.print("nwkKey: ");
-                    for (int i=0; i<sizeof(nwkKey); ++i) {
+                    printNl();
+                    Serial.print(F("nwkKey: "));
+                    for (size_t i=0; i<sizeof(nwkKey); ++i) {
                             if (i != 0)
-                                    Serial.print("-");
+                                    Serial.print('-');
                             printHex2(nwkKey[i]);
                     }
                 } while (0);
@@ -432,7 +432,6 @@ void eventPrint(cEventQueue::eventnode_t &e) {
                 printFreq(e.freq);
                 printRps(e.rps);
                 printOpmode(e.opmode);
-                printf(" irqLevel %u", hal_getIrqLevel());
 #if LMIC_ENABLE_event_logging
                 printAllRegisters();
 #endif

src/lmic/lmic.c

@@ -1597,7 +1597,7 @@ static bit_t processJoinAccept (void) {
     ASSERT((LMIC.opmode & (OP_JOINING|OP_REJOIN))!=0);
     //
     // XXX(tmm@mcci.com) OP_REJOIN confuses me, and I'm not sure why we're
-    // adjusting DRs here. We've just recevied a join accept, and the
+    // adjusting DRs here. We've just received a join accept, and the
     // datarate therefore shouldn't be in play.  In effect, we set the
     // initial data rate based on the number of times we tried to rejoin.
     //
@@ -1672,18 +1672,10 @@ static bit_t processJoinAccept_nojoinframe(void) {
         // claimed to return a delay but really returns 0 or 1.
         // Once we update as923 to return failed after dr2, we
         // can take out this #if.
-#if CFG_region != LMIC_REGION_as923
         os_setTimedCallback(&LMIC.osjob, os_getTime()+failed,
                             failed
                             ? FUNC_ADDR(onJoinFailed)      // one JOIN iteration done and failed
                             : FUNC_ADDR(runEngineUpdate)); // next step to be delayed
-#else
-       // in the join of AS923 v1.1 older, only DR2 is used. Therefore,
-       // not much improvement when it handles two different behavior;
-       // onJoinFailed or runEngineUpdate.
-        os_setTimedCallback(&LMIC.osjob, os_getTime()+failed,
-                            FUNC_ADDR(onJoinFailed));
-#endif
         // stop this join process.
         return 1;
 }
@@ -2752,9 +2744,20 @@ void LMIC_init (void) {
 
 
 void LMIC_clrTxData (void) {
-    bit_t const txActive = LMIC.opmode & OP_TXDATA;
-    LMIC.opmode &= ~(OP_TXDATA|OP_TXRXPEND|OP_POLL);
+    u2_t opmode = LMIC.opmode;
+    bit_t const txActive = opmode & OP_TXDATA;
+    if (! txActive) {
+        return;
+    }
     LMIC.pendTxLen = 0;
+    opmode &= ~(OP_TXDATA | OP_POLL);
+    if (! (opmode & OP_JOINING)) {
+        // in this case, we are joining, and the TX data
+        // is just pending.
+        opmode &= ~(OP_TXRXPEND);
+    }
+
+    LMIC.opmode = opmode;
 
     if (txActive)
         reportEventNoUpdate(EV_TXCANCELED);
@@ -2795,15 +2798,23 @@ dr_t LMIC_feasibleDataRateForFrame(dr_t dr, u1_t payloadSize) {
     return dr;
 }
 
-static void adjustDrForFrame(u1_t len) {
+static bit_t isTxPathBusy(void) {
+    return (LMIC.opmode & (OP_TXDATA|OP_JOINING)) != 0;
+}
+
+static bit_t adjustDrForFrameIfNotBusy(u1_t len) {
+    if (isTxPathBusy()) {
+        return 0;
+    }
     dr_t newDr = LMIC_feasibleDataRateForFrame(LMIC.datarate, len);
     if (newDr != LMIC.datarate) {
         setDrTxpow(DRCHG_FRAMESIZE, newDr, KEEP_TXPOW);
     }
+    return 1;
 }
 
 void LMIC_setTxData (void) {
-    adjustDrForFrame(LMIC.pendTxLen);
+    adjustDrForFrameIfNotBusy(LMIC.pendTxLen);
     LMIC_setTxData_strict();
 }
 
@@ -2820,7 +2831,7 @@ void LMIC_setTxData_strict (void) {
 
 // send a message, attempting to adjust TX data rate
 lmic_tx_error_t LMIC_setTxData2 (u1_t port, xref2u1_t data, u1_t dlen, u1_t confirmed) {
-    adjustDrForFrame(dlen);
+    adjustDrForFrameIfNotBusy(dlen);
     return LMIC_setTxData2_strict(port, data, dlen, confirmed);
 }
 
@@ -2854,7 +2865,7 @@ lmic_tx_error_t LMIC_sendWithCallback (
     u1_t port, xref2u1_t data, u1_t dlen, u1_t confirmed,
     lmic_txmessage_cb_t *pCb, void *pUserData
 ) {
-    adjustDrForFrame(dlen);
+    adjustDrForFrameIfNotBusy(dlen);
     return LMIC_sendWithCallback_strict(port, data, dlen, confirmed, pCb, pUserData);
 }
 

src/lmic/lmic.h

@@ -105,7 +105,7 @@ extern "C"{
 #define ARDUINO_LMIC_VERSION_CALC(major, minor, patch, local)	\
 	(((major) << 24ul) | ((minor) << 16ul) | ((patch) << 8ul) | ((local) << 0ul))
 
-#define	ARDUINO_LMIC_VERSION	ARDUINO_LMIC_VERSION_CALC(3, 0, 99, 3)	/* v3.0.99.3 */
+#define	ARDUINO_LMIC_VERSION	ARDUINO_LMIC_VERSION_CALC(3, 0, 99, 5)	/* v3.0.99.5 */
 
 #define	ARDUINO_LMIC_VERSION_GET_MAJOR(v)	\
 	(((v) >> 24u) & 0xFFu)

src/lmic/lmic_as923.c

@@ -77,15 +77,15 @@ static CONST_TABLE(u1_t, maxFrameLens_dwell1)[] = {
 static uint8_t
 LMICas923_getUplinkDwellBit(uint8_t mcmd_txparam) {
         if (mcmd_txparam == 0xFF)
-                return 0;
+                return AS923_INITIAL_TxParam_UplinkDwellTime;
 
         return (mcmd_txparam & MCMD_TxParam_TxDWELL_MASK) != 0;
 }
 
 static uint8_t
 LMICas923_getDownlinkDwellBit(uint8_t mcmd_txparam) {
         if (mcmd_txparam == 0xFF)
-                return 0;
+                return AS923_INITIAL_TxParam_DownlinkDwellTime;
 
         return (mcmd_txparam & MCMD_TxParam_RxDWELL_MASK) != 0;
 }
@@ -370,15 +370,28 @@ LMICas923_initJoinLoop(void) {
 void
 LMICas923_updateTx(ostime_t txbeg) {
         u4_t freq = LMIC.channelFreq[LMIC.txChnl];
+        u4_t dwellDelay;
+        u4_t globalDutyDelay;
+
         // Update global/band specific duty cycle stats
         ostime_t airtime = calcAirTime(LMIC.rps, LMIC.dataLen);
         // Update channel/global duty cycle stats
         xref2band_t band = &LMIC.bands[freq & 0x3];
         LMIC.freq = freq & ~(u4_t)3;
         LMIC.txpow = LMICas923_getMaxEIRP(LMIC.txParam);
         band->avail = txbeg + airtime * band->txcap;
-        if (LMIC.globalDutyRate != 0)
-                LMIC.globalDutyAvail = txbeg + (airtime << LMIC.globalDutyRate);
+        dwellDelay = globalDutyDelay = 0;
+        if (LMIC.globalDutyRate != 0) {
+                globalDutyDelay = (airtime << LMIC.globalDutyRate);
+        }
+        if (LMICas923_getUplinkDwellBit(LMIC.txParam)) {
+                dwellDelay = AS923_UPLINK_DWELL_TIME_osticks;
+        }
+        if (dwellDelay > globalDutyDelay) {
+                globalDutyDelay = dwellDelay;
+        }
+        if (globalDutyDelay != 0)
+                LMIC.globalDutyAvail = txbeg + globalDutyDelay;
 }
 
 

src/lmic/lmic_au921.c

@@ -67,7 +67,7 @@ static bit_t
 LMICau921_getUplinkDwellBit() {
         // if uninitialized, return default.
         if (LMIC.txParam == 0xFF) {
-                return 0;
+                return AU921_INITIAL_TxParam_UplinkDwellTime;
         }
         return (LMIC.txParam & MCMD_TxParam_TxDWELL_MASK) != 0;
 }
@@ -239,10 +239,23 @@ void LMICau921_updateTx(ostime_t txbeg) {
                 LMIC.freq = AU921_500kHz_UPFBASE + (chnl - 64)*AU921_500kHz_UPFSTEP;
         }
 
-        // Update global duty cycle stats
+        // Update global duty cycle stat and deal with dwell time.
+        u4_t dwellDelay;
+        u4_t globalDutyDelay;
+        dwellDelay = globalDutyDelay = 0;
+
         if (LMIC.globalDutyRate != 0) {
                 ostime_t airtime = calcAirTime(LMIC.rps, LMIC.dataLen);
-                LMIC.globalDutyAvail = txbeg + (airtime << LMIC.globalDutyRate);
+                globalDutyDelay = txbeg + (airtime << LMIC.globalDutyRate);
+        }
+        if (LMICau921_getUplinkDwellBit(LMIC.txParam)) {
+                dwellDelay = AU921_UPLINK_DWELL_TIME_osticks;
+        }
+        if (dwellDelay > globalDutyDelay) {
+                globalDutyDelay = dwellDelay;
+        }
+        if (globalDutyDelay != 0) {
+                LMIC.globalDutyAvail = txbeg + globalDutyDelay;
         }
 }
 

src/lmic/lmic_compliance.c

@@ -54,8 +54,9 @@ static lmic_event_cb_t lmicEventCb;
 static lmic_txmessage_cb_t sendUplinkCompleteCb;
 static osjobcbfn_t timerExpiredCb;
 
-/* this is declared global so the optimizer can chuck it without warnings */
+/* these are declared global so the optimizer can chuck them without warnings */
 const char *LMICcompliance_txSuccessToString(int fSuccess);
+const char * LMICcompliance_fsmstate_getName(lmic_compliance_fsmstate_t state);
 
 /****************************************************************************\
 |
@@ -386,8 +387,6 @@ Name:   evEchoCommand()
 
 */
 
-static lmic_txmessage_cb_t evEchoCommandCb;
-
 static void evEchoCommand(
     const uint8_t *pMessage,
     size_t nMessage
@@ -439,7 +438,7 @@ Name:   fsmEval()
 
 */
 
-static const char * lmic_compliance_fsmstate_Getname(lmic_compliance_fsmstate_t state) {
+const char * LMICcompliance_fsmstate_getName(lmic_compliance_fsmstate_t state) {
     const char * const names[] = { LMIC_COMPLIANCE_FSMSTATE__NAMES };
 
     if ((unsigned) state >= sizeof(names)/sizeof(names[0]))
@@ -497,8 +496,8 @@ static void fsmEval(void) {
             // state change!
             LMIC_COMPLIANCE_PRINTF("%s: change state %s(%u) => %s(%u)\n",
                 __func__,
-                lmic_compliance_fsmstate_Getname(oldState), (unsigned) oldState,
-                lmic_compliance_fsmstate_Getname(newState), (unsigned) newState
+                LMICcompliance_fsmstate_getName(oldState), (unsigned) oldState,
+                LMICcompliance_fsmstate_getName(newState), (unsigned) newState
                 );
             fNewState = true;
             LMIC_Compliance.fsmState = newState;

src/lmic/lmic_eu_like.c

@@ -174,24 +174,25 @@ ostime_t LMICeulike_nextJoinState(uint8_t nDefaultChannels) {
         if ((++LMIC.txCnt % nDefaultChannels) == 0) {
                 // Lower DR every nth try (having all default channels with same DR)
                 //
-                // TODO(tmm@mcci.com) add new DR_REGIN_JOIN_MIN instead of LORAWAN_DR0;
+                // TODO(tmm@mcci.com) add new DR_REGION_JOIN_MIN instead of LORAWAN_DR0;
                 // then we can eliminate the LMIC_REGION_as923 below because we'll set
                 // the failed flag here. This will cause the outer caller to take the
                 // appropriate join path. Or add new LMICeulike_GetLowestJoinDR()
                 //
+
+// TODO(tmm@mcci.com) - see above; please remove regional dependency from this file.
+#if CFG_region == LMIC_REGION_as923
+                // in the join of AS923 v1.1 or older, only DR2 is used.
+                // no need to change the DR.
+                LMIC.datarate = AS923_DR_SF10;
+                failed = 1;
+#else
                 if (LMIC.datarate == LORAWAN_DR0)
                         failed = 1; // we have tried all DR - signal EV_JOIN_FAILED
-                else
-                {
-// TODO(tmm@mcci.com) - see above; please remove regional dependency from this file.
-#if CFG_region != LMIC_REGION_as923
+                else {
                         LMICcore_setDrJoin(DRCHG_NOJACC, decDR((dr_t)LMIC.datarate));
-#else
-                        // in the join of AS923 v1.1 or older, only DR2 is used.
-                        // no need to change the DR.
-                        LMIC.datarate = AS923_DR_SF10;
-#endif
                 }
+#endif
         }
         // Clear NEXTCHNL because join state engine controls channel hopping
         LMIC.opmode &= ~OP_NEXTCHNL;

src/lmic/lorabase_as923.h

@@ -93,4 +93,13 @@ enum { AS923_V102_TX_CAP = 100 };		// v1.0.2 allows 100%
 # define AS923_TX_CAP	AS923_V102_TX_CAP
 #endif
 
+// TxParam defaults
+enum {
+        // initial value of UplinkDwellTime before TxParamSetupReq received.
+        AS923_INITIAL_TxParam_UplinkDwellTime = 1,
+        // initial value of DownlinkDwellTime before TxParamSetupReq received.
+        AS923_INITIAL_TxParam_DownlinkDwellTime = 1,
+        AS923_UPLINK_DWELL_TIME_osticks = sec2osticks(20),
+};
+
 #endif /* _lorabase_as923_h_ */

src/lmic/lorabase_au921.h

@@ -76,6 +76,11 @@ enum {
 enum {
         AU921_TX_EIRP_MAX_DBM = 30      // 30 dBm
 };
+enum {
+        // initial value of UplinkDwellTime before TxParamSetupReq received.
+        AU921_INITIAL_TxParam_UplinkDwellTime = 1,
+        AU921_UPLINK_DWELL_TIME_osticks = sec2osticks(20),
+};
 
 enum { DR_PAGE_AU921 = 0x10 * (LMIC_REGION_au921 - 1) };