Skip to content

Commit

Permalink
Merge pull request #391 from mcci-catena/issue385
Browse files Browse the repository at this point in the history
Fix #385: correct TxSetupParamReq processing
  • Loading branch information
terrillmoore authored Aug 24, 2019
2 parents dd23e15 + 624d2d8 commit 6427a03
Show file tree
Hide file tree
Showing 9 changed files with 128 additions and 68 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
52 changes: 29 additions & 23 deletions examples/compliance-otaa-halconfig/compliance-otaa-halconfig.ino
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 = ',') {
Expand All @@ -261,28 +268,30 @@ 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;
hal_spi_read(1, regbuf + 1, sizeof(regbuf) - 1);

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.
Expand All @@ -297,7 +306,7 @@ void printAllRegisters(void) {
}

void printNl(void) {
Serial.println("");
Serial.println();
}

void eventPrint(cEventQueue::eventnode_t &e) {
Expand All @@ -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 {
Expand Down Expand Up @@ -360,15 +369,15 @@ void eventPrint(cEventQueue::eventnode_t &e) {
Serial.print("artKey: ");
for (int i=0; i<sizeof(artKey); ++i) {
if (i != 0)
Serial.print("-");
Serial.print(artKey[i], HEX);
Serial.print("-");
printHex2(artKey[i]);
}
Serial.println("");
Serial.print("nwkKey: ");
for (int i=0; i<sizeof(nwkKey); ++i) {
if (i != 0)
Serial.print("-");
Serial.print(nwkKey[i], HEX);
printHex2(nwkKey[i]);
}
} while (0);
break;
Expand Down Expand Up @@ -503,7 +512,7 @@ void myRxMessageCb(
if (port == LORAWAN_PORT_COMPLIANCE) {
Serial.print(F("Received test packet 0x"));
if (nMessage > 0)
Serial.print(pMessage[0], HEX);
printHex2(pMessage[0]);
Serial.print(F(" length "));
Serial.println((unsigned) nMessage);
}
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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();
Expand All @@ -665,7 +671,7 @@ void setup_printSignOn()

void setupForNetwork(bool preJoin) {
#if defined(CFG_us915)
LMIC_selectSubBand(1);
LMIC_selectSubBand(0);

if (! preJoin) {
// LMIC_setLinkCheckMode(0);
Expand Down
13 changes: 11 additions & 2 deletions src/lmic/lmic.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/lmic/lmic.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions src/lmic/lmic_bandplan.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
//
Expand Down
9 changes: 9 additions & 0 deletions src/lmic/lmic_eu_like.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/lmic/lmic_eu_like.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_
106 changes: 65 additions & 41 deletions src/lmic/lmic_us_like.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -149,60 +147,86 @@ 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
// but also do the channel hopping dance.
ostime_t LMICuslike_nextTx(ostime_t now) {
// TODO([email protected]): 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
Expand Down
3 changes: 3 additions & 0 deletions src/lmic/lmic_us_like.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_

0 comments on commit 6427a03

Please sign in to comment.