From 2e2b4d95b9ce4bcd26cd317c2e55199c13751857 Mon Sep 17 00:00:00 2001 From: Terry Moore Date: Sat, 24 Aug 2019 03:04:40 -0400 Subject: [PATCH 1/4] Fix #385: correct TxParamSetupReq processing --- src/lmic/lmic.c | 13 ++++- src/lmic/lmic_bandplan.h | 4 ++ src/lmic/lmic_eu_like.c | 9 ++++ src/lmic/lmic_eu_like.h | 4 ++ src/lmic/lmic_us_like.c | 106 ++++++++++++++++++++++++--------------- src/lmic/lmic_us_like.h | 3 ++ 6 files changed, 96 insertions(+), 43 deletions(-) diff --git a/src/lmic/lmic.c b/src/lmic/lmic.c index af730591..dfc22db8 100644 --- a/src/lmic/lmic.c +++ b/src/lmic/lmic.c @@ -761,6 +761,17 @@ applyAdrRequests( if (! map_ok) { adrAns &= ~MCMD_LinkADRAns_ChannelACK; + } + + // p1 now has txpow + DR. DR must be feasible. + dr_t dr = (dr_t)(p1>>MCMD_LinkADRReq_DR_SHIFT); + + if (adrAns == (MCMD_LinkADRAns_PowerACK | MCMD_LinkADRAns_DataRateACK | MCMD_LinkADRAns_ChannelACK) && ! LMICbandplan_isDataRateFeasible(dr)) { + adrAns &= ~MCMD_LinkADRAns_DataRateACK; + LMICOS_logEventUint32("applyAdrRequests: final DR not feasible", dr); + } + + if (adrAns != (MCMD_LinkADRAns_PowerACK | MCMD_LinkADRAns_DataRateACK | MCMD_LinkADRAns_ChannelACK)) { LMICbandplan_restoreAdrState(&initialState); } @@ -786,8 +797,6 @@ applyAdrRequests( changes = 1; } - dr_t dr = (dr_t)(p1>>MCMD_LinkADRReq_DR_SHIFT); - LMICOS_logEventUint32("applyAdrRequests: setDrTxPow", (adrAns << 16)|(dr << 8)|(p1 << 0)); // handle power changes here, too. diff --git a/src/lmic/lmic_bandplan.h b/src/lmic/lmic_bandplan.h index b4c552c6..7332c84c 100644 --- a/src/lmic/lmic_bandplan.h +++ b/src/lmic/lmic_bandplan.h @@ -162,6 +162,10 @@ # error "LMICbandplan_restoreAdrState() not defined by bandplan" #endif +#if !defined(LMICbandplan_isDataRateFeasible) +# error "LMICbandplan_isDataRateFeasible() not defined by bandplan" +#endif + // // Things common to lmic.c code // diff --git a/src/lmic/lmic_eu_like.c b/src/lmic/lmic_eu_like.c index 00acbe92..7614f056 100644 --- a/src/lmic/lmic_eu_like.c +++ b/src/lmic/lmic_eu_like.c @@ -108,6 +108,15 @@ bit_t LMICeulike_mapChannels(u1_t chpage, u2_t chmap) { return LMIC.channelMap != 0; } +bit_t LMICeulike_isDataRateFeasible(dr_t dr) { + for (u1_t chnl = 0; chnl < MAX_CHANNELS; ++chnl) { + if ((LMIC.channelMap & (1 << chnl)) != 0 && // channel enabled + (LMIC.channelDrMap[chnl] & (1 << dr)) != 0) + return 1; + } + return 0; +} + #if !defined(DISABLE_JOIN) void LMICeulike_initJoinLoop(uint8_t nDefaultChannels, s1_t adrTxPow) { #if CFG_TxContinuousMode diff --git a/src/lmic/lmic_eu_like.h b/src/lmic/lmic_eu_like.h index 16f2679e..624fa78f 100644 --- a/src/lmic/lmic_eu_like.h +++ b/src/lmic/lmic_eu_like.h @@ -107,4 +107,8 @@ void LMICeulike_restoreAdrState(const lmic_saved_adr_state_t *pStateBuffer); // set Rx1 frequency (might be different than uplink). void LMICeulike_setRx1Freq(void); +bit_t LMICeulike_isDataRateFeasible(dr_t dr); +#define LMICbandplan_isDataRateFeasible(dr) LMICeulike_isDataRateFeasible(dr) + + #endif // _lmic_eu_like_h_ diff --git a/src/lmic/lmic_us_like.c b/src/lmic/lmic_us_like.c index 90d8c92b..c6b075fe 100644 --- a/src/lmic/lmic_us_like.c +++ b/src/lmic/lmic_us_like.c @@ -138,8 +138,6 @@ bit_t LMICuslike_mapChannels(u1_t chpage, u2_t chmap) { || all channels 0..63 are turned off or on. MCMC_LADR_CHP_BANK || is also special, in that it enables subbands. */ - u1_t base, top; - if (chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_BANK) { // each bit enables a bank of channels for (u1_t subband = 0; subband < 8; ++subband, chmap >>= 1) { @@ -149,43 +147,47 @@ bit_t LMICuslike_mapChannels(u1_t chpage, u2_t chmap) { LMIC_disableSubBand(subband); } } + } else { + u1_t base, top; + + if (chpage < MCMD_LinkADRReq_ChMaskCntl_USLIKE_SPECIAL) { + // operate on channels 0..15, 16..31, 32..47, 48..63 + // note that the chpage hasn't been shifted right, so + // it's really the base. + base = chpage; + top = base + 16; + if (base == 64) { + top = 72; + } + } else /* if (chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_125ON || + chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_125OFF) */ { + u1_t const en125 = chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_125ON; + + // enable or disable all 125kHz channels + for (u1_t chnl = 0; chnl < 64; ++chnl) { + if (en125) + LMIC_enableChannel(chnl); + else + LMIC_disableChannel(chnl); + } + + // then apply mask to top 8 channels. + base = 64; + top = 72; + } - return LMIC.activeChannels125khz || LMIC.activeChannels500khz; - } - - if (chpage < MCMD_LinkADRReq_ChMaskCntl_USLIKE_SPECIAL) { - // operate on channels 0..15, 16..31, 32..47, 48..63 - base = chpage << 4; - top = base + 16; - if (base == 64) { - top = 72; - } - } else /* if (chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_125ON || - chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_125OFF) */ { - u1_t const en125 = chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_125ON; - - // enable or disable all 125kHz channels - for (u1_t chnl = 0; chnl < 64; ++chnl) { - if (en125) - LMIC_enableChannel(chnl); - else - LMIC_disableChannel(chnl); - } - - // then apply mask to top 8 channels. - base = 64; - top = 72; - } - - // apply chmap to channels in [base..top-1]. - // Use enable/disable channel to keep activeChannel counts in sync. - for (u1_t chnl = base; chnl < top; ++chnl, chmap >>= 1) { - if (chmap & 0x0001) - LMIC_enableChannel(chnl); - else - LMIC_disableChannel(chnl); + // apply chmap to channels in [base..top-1]. + // Use enable/disable channel to keep activeChannel counts in sync. + for (u1_t chnl = base; chnl < top; ++chnl, chmap >>= 1) { + if (chmap & 0x0001) + LMIC_enableChannel(chnl); + else + LMIC_disableChannel(chnl); + } } - return LMIC.activeChannels125khz || LMIC.activeChannels500khz; + + LMICOS_logEventUint32("LMICuslike_mapChannels", (LMIC.activeChannels125khz << 16u)|(LMIC.activeChannels500khz << 0u)); + return (LMIC.activeChannels125khz > 0) || (LMIC.activeChannels500khz > 0); } // US does not have duty cycling - return now as earliest TX time @@ -193,16 +195,38 @@ bit_t LMICuslike_mapChannels(u1_t chpage, u2_t chmap) { ostime_t LMICuslike_nextTx(ostime_t now) { // TODO(tmm@mcci.com): use a static const for US-like if (LMIC.datarate >= LMICuslike_getFirst500kHzDR()) { // 500kHz - ASSERT(LMIC.activeChannels500khz>0); - setNextChannel(64, 64 + 8, LMIC.activeChannels500khz); + if (LMIC.activeChannels500khz > 0) { + setNextChannel(64, 64 + 8, LMIC.activeChannels500khz); + } else if (LMIC.activeChannels125khz > 0) { + LMIC.datarate = lowerDR(LMICuslike_getFirst500kHzDR(), 1); + setNextChannel(0, 64, LMIC.activeChannels125khz); + LMICOS_logEvent("LMICuslike_nextTx: no 500k, choose 125k"); + } else { + LMICOS_logEvent("LMICuslike_nextTx: no channels at all (500)"); + } } else { // 125kHz - ASSERT(LMIC.activeChannels125khz>0); - setNextChannel(0, 64, LMIC.activeChannels125khz); + if (LMIC.activeChannels125khz > 0) { + setNextChannel(0, 64, LMIC.activeChannels125khz); + } else if (LMIC.activeChannels500khz > 0) { + LMIC.datarate = LMICuslike_getFirst500kHzDR(); + setNextChannel(64, 64 + 8, LMIC.activeChannels500khz); + LMICOS_logEvent("LMICuslike_nextTx: no 125k, choose 500k"); + } else { + LMICOS_logEvent("LMICuslike_nextTx: no channels at all (125)"); + } } return now; } +bit_t LMICuslike_isDataRateFeasible(dr_t dr) { + if (dr >= LMICuslike_getFirst500kHzDR()) { // 500kHz + return LMIC.activeChannels500khz > 0; + } else { + return LMIC.activeChannels125khz > 6; + } +} + #if !defined(DISABLE_JOIN) void LMICuslike_initJoinLoop(void) { // set an initial condition so that setNextChannel()'s preconds are met diff --git a/src/lmic/lmic_us_like.h b/src/lmic/lmic_us_like.h index b0dc88fe..62812369 100644 --- a/src/lmic/lmic_us_like.h +++ b/src/lmic/lmic_us_like.h @@ -109,4 +109,7 @@ bit_t LMICuslike_compareAdrState(const lmic_saved_adr_state_t *pStateBuffer); void LMICuslike_restoreAdrState(const lmic_saved_adr_state_t *pStateBuffer); #define LMICbandplan_restoreAdrState(pState) LMICuslike_restoreAdrState(pState) +bit_t LMICuslike_isDataRateFeasible(dr_t dr); +#define LMICbandplan_isDataRateFeasible(dr) LMICuslike_isDataRateFeasible(dr) + #endif // _lmic_us_like_h_ From e24a67486c0522d927381dbeb63d7eba6157c946 Mon Sep 17 00:00:00 2001 From: Terry Moore Date: Sat, 24 Aug 2019 03:07:43 -0400 Subject: [PATCH 2/4] Use subband 0 for testing --- .../compliance-otaa-halconfig/compliance-otaa-halconfig.ino | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/compliance-otaa-halconfig/compliance-otaa-halconfig.ino b/examples/compliance-otaa-halconfig/compliance-otaa-halconfig.ino index 31ac7afe..b686b70d 100644 --- a/examples/compliance-otaa-halconfig/compliance-otaa-halconfig.ino +++ b/examples/compliance-otaa-halconfig/compliance-otaa-halconfig.ino @@ -665,7 +665,7 @@ void setup_printSignOn() void setupForNetwork(bool preJoin) { #if defined(CFG_us915) - LMIC_selectSubBand(1); + LMIC_selectSubBand(0); if (! preJoin) { // LMIC_setLinkCheckMode(0); From 5cc5262771ef37f3f5e849c27e6478d122346e4b Mon Sep 17 00:00:00 2001 From: Terry Moore Date: Sat, 24 Aug 2019 12:26:18 -0400 Subject: [PATCH 3/4] Update documentation; version 2.3.2.71 --- README.md | 3 ++- src/lmic/lmic.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index be5ed6f1..3db3b5c1 100644 --- a/README.md +++ b/README.md @@ -1098,7 +1098,8 @@ function uflt12f(rawUflt12) - HEAD adds the following changes. - - [#378](https://github.com/mcci-catena/arduino-lmic/pull/378) completely reworks MAC downlink handling. Resulting code passes the LoRaWAN V1.5 EU certification test. (v2.32.2.70) + - [#385](https://github.com/mcci-catena/arduino-lmic/issues/385) corrects an error handling data rate selection for `TxParamSetupReq`, found in US-915 certification testing. (v2.3.2.71) + - [#378](https://github.com/mcci-catena/arduino-lmic/pull/378) completely reworks MAC downlink handling. Resulting code passes the LoRaWAN V1.5 EU certification test. (v2.3.2.70) - [#360](https://github.com/mcci-catena/arduino-lmic/pull/360) adds support for the KR-920 regional plan. - v2.3.2 is a patch release. It incorporates two pull requests. diff --git a/src/lmic/lmic.h b/src/lmic/lmic.h index cd06d952..fa185feb 100644 --- a/src/lmic/lmic.h +++ b/src/lmic/lmic.h @@ -105,7 +105,7 @@ extern "C"{ #define ARDUINO_LMIC_VERSION_CALC(major, minor, patch, local) \ (((major) << 24u) | ((minor) << 16u) | ((patch) << 8u) | (local)) -#define ARDUINO_LMIC_VERSION ARDUINO_LMIC_VERSION_CALC(2, 3, 2, 70) /* v2.3.2.70 */ +#define ARDUINO_LMIC_VERSION ARDUINO_LMIC_VERSION_CALC(2, 3, 2, 71) /* v2.3.2.71 */ #define ARDUINO_LMIC_VERSION_GET_MAJOR(v) \ (((v) >> 24u) & 0xFFu) From 624d2d82ebc9478512b1e85322776e56f75f2577 Mon Sep 17 00:00:00 2001 From: Terry Moore Date: Sat, 24 Aug 2019 13:24:34 -0400 Subject: [PATCH 4/4] Bug #386: slim down the compliance test for AVR --- .../compliance-otaa-halconfig.ino | 50 +++++++++++-------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/examples/compliance-otaa-halconfig/compliance-otaa-halconfig.ino b/examples/compliance-otaa-halconfig/compliance-otaa-halconfig.ino index b686b70d..339e80b2 100644 --- a/examples/compliance-otaa-halconfig/compliance-otaa-halconfig.ino +++ b/examples/compliance-otaa-halconfig/compliance-otaa-halconfig.ino @@ -225,6 +225,13 @@ const char *getCrcName(rps_t rps) { return getNocrc(rps) ? "NoCrc" : "Crc"; } +void printHex2(unsigned v) { + v &= 0xff; + if (v < 16) + Serial.print('0'); + Serial.print(v, HEX); +} + void printFreq(u4_t freq) { Serial.print(F(": freq=")); Serial.print(freq / 1000000); @@ -233,13 +240,13 @@ void printFreq(u4_t freq) { } void printRps(rps_t rps) { - Serial.print(F(" rps=0x")); Serial.print(unsigned(rps), HEX); + Serial.print(F(" rps=0x")); printHex2(rps); Serial.print(F(" (")); Serial.print(getSfName(rps)); Serial.print(F(" ")); Serial.print(getBwName(rps)); Serial.print(F(" ")); Serial.print(getCrName(rps)); Serial.print(F(" ")); Serial.print(getCrcName(rps)); Serial.print(F(" IH=")); Serial.print(unsigned(getIh(rps))); - Serial.print(F(")")); + Serial.print(')'); } void printOpmode(uint16_t opmode, char sep = ',') { @@ -261,18 +268,18 @@ void printDatarate(u1_t datarate) { Serial.print(F(", datarate=")); Serial.print(unsigned(datarate)); } -void printTxrxflags(u2_t txrxFlags) { - Serial.print(F(", txrxFlags=0x")); Serial.print(unsigned(txrxFlags), HEX); +void printTxrxflags(u1_t txrxFlags) { + Serial.print(F(", txrxFlags=0x")); printHex2(txrxFlags); if (txrxFlags & TXRX_ACK) Serial.print(F("; Received ack")); } void printSaveIrqFlags(u1_t saveIrqFlags) { Serial.print(F(", saveIrqFlags 0x")); - Serial.print(unsigned(saveIrqFlags), HEX); + printHex2(saveIrqFlags); } -// dump all the registers. +// dump all the registers. Must have printf setup. void printAllRegisters(void) { uint8_t regbuf[0x80]; regbuf[0] = 0; @@ -280,9 +287,11 @@ void printAllRegisters(void) { for (unsigned i = 0; i < sizeof(regbuf); ++i) { if (i % 16 == 0) { - printf("\r\n%02x:", i); + printNl(); + printHex2(i); } - printf("%s%02x", ((i % 16) == 8) ? " - " : " ", regbuf[i]); + printHex2(regbuf[i]); + Serial.print(((i % 16) == 8) ? F(" - ") : F(" ")); } // reset the radio, just in case the register dump caused issues. @@ -297,7 +306,7 @@ void printAllRegisters(void) { } void printNl(void) { - Serial.println(""); + Serial.println(); } void eventPrint(cEventQueue::eventnode_t &e) { @@ -322,7 +331,7 @@ void eventPrint(cEventQueue::eventnode_t &e) { printTxChnl(e.txChnl); printRps(e.rps); printOpmode(e.opmode); - printTxrxflags(e.opmode); + printTxrxflags(e.txrxFlags); printSaveIrqFlags(e.saveIrqFlags); printAllRegisters(); } else { @@ -360,15 +369,15 @@ void eventPrint(cEventQueue::eventnode_t &e) { Serial.print("artKey: "); for (int i=0; i 0) - Serial.print(pMessage[0], HEX); + printHex2(pMessage[0]); Serial.print(F(" length ")); Serial.println((unsigned) nMessage); } @@ -607,14 +616,11 @@ void setup() { do_send(&sendjob); } -void setup_printSignOnDashes(void) +void setup_printSignOnDashLine(void) { - Serial.print(F("------------------------------------")); - } -void setup_printSignOnDashLine() - { - setup_printSignOnDashes(); - setup_printSignOnDashes(); + for (unsigned i = 0; i < 78; ++i) + Serial.print('-'); + printNl(); } @@ -656,7 +662,7 @@ void setup_printSignOn() printVersion(ARDUINO_LMIC_VERSION); Serial.print(F(" configured for region ")); Serial.print(CFG_region); - Serial.println(F(".")); + Serial.println('.'); Serial.println(F("Remember to select 'Line Ending: Newline' at the bottom of the monitor window.")); setup_printSignOnDashLine();