From 2e2b4d95b9ce4bcd26cd317c2e55199c13751857 Mon Sep 17 00:00:00 2001 From: Terry Moore Date: Sat, 24 Aug 2019 03:04:40 -0400 Subject: [PATCH] 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_