Skip to content

Commit 6427a03

Browse files
terrillmooreterrillmoore
authored
Merge pull request #391 from mcci-catena/issue385
Fix #385: correct TxSetupParamReq processing
2 parents dd23e15 + 624d2d8 commit 6427a03

File tree

9 files changed

+128
-68
lines changed

9 files changed

+128
-68
lines changed

‎README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1098,7 +1098,8 @@ function uflt12f(rawUflt12)
10981098

10991099
- HEAD adds the following changes.
11001100

1101-
- [#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)
1101+
- [#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)
1102+
- [#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)
11021103
- [#360](https://github.com/mcci-catena/arduino-lmic/pull/360) adds support for the KR-920 regional plan.
11031104

11041105
- v2.3.2 is a patch release. It incorporates two pull requests.

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

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,13 @@ const char *getCrcName(rps_t rps) {
225225
return getNocrc(rps) ? "NoCrc" : "Crc";
226226
}
227227

228+
void printHex2(unsigned v) {
229+
v &= 0xff;
230+
if (v < 16)
231+
Serial.print('0');
232+
Serial.print(v, HEX);
233+
}
234+
228235
void printFreq(u4_t freq) {
229236
Serial.print(F(": freq="));
230237
Serial.print(freq / 1000000);
@@ -233,13 +240,13 @@ void printFreq(u4_t freq) {
233240
}
234241

235242
void printRps(rps_t rps) {
236-
Serial.print(F(" rps=0x")); Serial.print(unsigned(rps), HEX);
243+
Serial.print(F(" rps=0x")); printHex2(rps);
237244
Serial.print(F(" (")); Serial.print(getSfName(rps));
238245
Serial.print(F(" ")); Serial.print(getBwName(rps));
239246
Serial.print(F(" ")); Serial.print(getCrName(rps));
240247
Serial.print(F(" ")); Serial.print(getCrcName(rps));
241248
Serial.print(F(" IH=")); Serial.print(unsigned(getIh(rps)));
242-
Serial.print(F(")"));
249+
Serial.print(')');
243250
}
244251

245252
void printOpmode(uint16_t opmode, char sep = ',') {
@@ -261,28 +268,30 @@ void printDatarate(u1_t datarate) {
261268
Serial.print(F(", datarate=")); Serial.print(unsigned(datarate));
262269
}
263270

264-
void printTxrxflags(u2_t txrxFlags) {
265-
Serial.print(F(", txrxFlags=0x")); Serial.print(unsigned(txrxFlags), HEX);
271+
void printTxrxflags(u1_t txrxFlags) {
272+
Serial.print(F(", txrxFlags=0x")); printHex2(txrxFlags);
266273
if (txrxFlags & TXRX_ACK)
267274
Serial.print(F("; Received ack"));
268275
}
269276

270277
void printSaveIrqFlags(u1_t saveIrqFlags) {
271278
Serial.print(F(", saveIrqFlags 0x"));
272-
Serial.print(unsigned(saveIrqFlags), HEX);
279+
printHex2(saveIrqFlags);
273280
}
274281

275-
// dump all the registers.
282+
// dump all the registers. Must have printf setup.
276283
void printAllRegisters(void) {
277284
uint8_t regbuf[0x80];
278285
regbuf[0] = 0;
279286
hal_spi_read(1, regbuf + 1, sizeof(regbuf) - 1);
280287

281288
for (unsigned i = 0; i < sizeof(regbuf); ++i) {
282289
if (i % 16 == 0) {
283-
printf("\r\n%02x:", i);
290+
printNl();
291+
printHex2(i);
284292
}
285-
printf("%s%02x", ((i % 16) == 8) ? " - " : " ", regbuf[i]);
293+
printHex2(regbuf[i]);
294+
Serial.print(((i % 16) == 8) ? F(" - ") : F(" "));
286295
}
287296

288297
// reset the radio, just in case the register dump caused issues.
@@ -297,7 +306,7 @@ void printAllRegisters(void) {
297306
}
298307

299308
void printNl(void) {
300-
Serial.println("");
309+
Serial.println();
301310
}
302311

303312
void eventPrint(cEventQueue::eventnode_t &e) {
@@ -322,7 +331,7 @@ void eventPrint(cEventQueue::eventnode_t &e) {
322331
printTxChnl(e.txChnl);
323332
printRps(e.rps);
324333
printOpmode(e.opmode);
325-
printTxrxflags(e.opmode);
334+
printTxrxflags(e.txrxFlags);
326335
printSaveIrqFlags(e.saveIrqFlags);
327336
printAllRegisters();
328337
} else {
@@ -360,15 +369,15 @@ void eventPrint(cEventQueue::eventnode_t &e) {
360369
Serial.print("artKey: ");
361370
for (int i=0; i<sizeof(artKey); ++i) {
362371
if (i != 0)
363-
Serial.print("-");
364-
Serial.print(artKey[i], HEX);
372+
Serial.print("-");
373+
printHex2(artKey[i]);
365374
}
366375
Serial.println("");
367376
Serial.print("nwkKey: ");
368377
for (int i=0; i<sizeof(nwkKey); ++i) {
369378
if (i != 0)
370379
Serial.print("-");
371-
Serial.print(nwkKey[i], HEX);
380+
printHex2(nwkKey[i]);
372381
}
373382
} while (0);
374383
break;
@@ -503,7 +512,7 @@ void myRxMessageCb(
503512
if (port == LORAWAN_PORT_COMPLIANCE) {
504513
Serial.print(F("Received test packet 0x"));
505514
if (nMessage > 0)
506-
Serial.print(pMessage[0], HEX);
515+
printHex2(pMessage[0]);
507516
Serial.print(F(" length "));
508517
Serial.println((unsigned) nMessage);
509518
}
@@ -607,14 +616,11 @@ void setup() {
607616
do_send(&sendjob);
608617
}
609618

610-
void setup_printSignOnDashes(void)
619+
void setup_printSignOnDashLine(void)
611620
{
612-
Serial.print(F("------------------------------------"));
613-
}
614-
void setup_printSignOnDashLine()
615-
{
616-
setup_printSignOnDashes();
617-
setup_printSignOnDashes();
621+
for (unsigned i = 0; i < 78; ++i)
622+
Serial.print('-');
623+
618624
printNl();
619625
}
620626

@@ -656,7 +662,7 @@ void setup_printSignOn()
656662
printVersion(ARDUINO_LMIC_VERSION);
657663
Serial.print(F(" configured for region "));
658664
Serial.print(CFG_region);
659-
Serial.println(F("."));
665+
Serial.println('.');
660666
Serial.println(F("Remember to select 'Line Ending: Newline' at the bottom of the monitor window."));
661667

662668
setup_printSignOnDashLine();
@@ -665,7 +671,7 @@ void setup_printSignOn()
665671

666672
void setupForNetwork(bool preJoin) {
667673
#if defined(CFG_us915)
668-
LMIC_selectSubBand(1);
674+
LMIC_selectSubBand(0);
669675

670676
if (! preJoin) {
671677
// LMIC_setLinkCheckMode(0);

‎src/lmic/lmic.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,17 @@ applyAdrRequests(
761761

762762
if (! map_ok) {
763763
adrAns &= ~MCMD_LinkADRAns_ChannelACK;
764+
}
765+
766+
// p1 now has txpow + DR. DR must be feasible.
767+
dr_t dr = (dr_t)(p1>>MCMD_LinkADRReq_DR_SHIFT);
768+
769+
if (adrAns == (MCMD_LinkADRAns_PowerACK | MCMD_LinkADRAns_DataRateACK | MCMD_LinkADRAns_ChannelACK) && ! LMICbandplan_isDataRateFeasible(dr)) {
770+
adrAns &= ~MCMD_LinkADRAns_DataRateACK;
771+
LMICOS_logEventUint32("applyAdrRequests: final DR not feasible", dr);
772+
}
773+
774+
if (adrAns != (MCMD_LinkADRAns_PowerACK | MCMD_LinkADRAns_DataRateACK | MCMD_LinkADRAns_ChannelACK)) {
764775
LMICbandplan_restoreAdrState(&initialState);
765776
}
766777

@@ -786,8 +797,6 @@ applyAdrRequests(
786797
changes = 1;
787798
}
788799

789-
dr_t dr = (dr_t)(p1>>MCMD_LinkADRReq_DR_SHIFT);
790-
791800
LMICOS_logEventUint32("applyAdrRequests: setDrTxPow", (adrAns << 16)|(dr << 8)|(p1 << 0));
792801

793802
// handle power changes here, too.

‎src/lmic/lmic.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ extern "C"{
105105
#define ARDUINO_LMIC_VERSION_CALC(major, minor, patch, local) \
106106
(((major) << 24u) | ((minor) << 16u) | ((patch) << 8u) | (local))
107107

108-
#define ARDUINO_LMIC_VERSION ARDUINO_LMIC_VERSION_CALC(2, 3, 2, 70) /* v2.3.2.70 */
108+
#define ARDUINO_LMIC_VERSION ARDUINO_LMIC_VERSION_CALC(2, 3, 2, 71) /* v2.3.2.71 */
109109

110110
#define ARDUINO_LMIC_VERSION_GET_MAJOR(v) \
111111
(((v) >> 24u) & 0xFFu)

‎src/lmic/lmic_bandplan.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,10 @@
162162
# error "LMICbandplan_restoreAdrState() not defined by bandplan"
163163
#endif
164164

165+
#if !defined(LMICbandplan_isDataRateFeasible)
166+
# error "LMICbandplan_isDataRateFeasible() not defined by bandplan"
167+
#endif
168+
165169
//
166170
// Things common to lmic.c code
167171
//

‎src/lmic/lmic_eu_like.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,15 @@ bit_t LMICeulike_mapChannels(u1_t chpage, u2_t chmap) {
108108
return LMIC.channelMap != 0;
109109
}
110110

111+
bit_t LMICeulike_isDataRateFeasible(dr_t dr) {
112+
for (u1_t chnl = 0; chnl < MAX_CHANNELS; ++chnl) {
113+
if ((LMIC.channelMap & (1 << chnl)) != 0 && // channel enabled
114+
(LMIC.channelDrMap[chnl] & (1 << dr)) != 0)
115+
return 1;
116+
}
117+
return 0;
118+
}
119+
111120
#if !defined(DISABLE_JOIN)
112121
void LMICeulike_initJoinLoop(uint8_t nDefaultChannels, s1_t adrTxPow) {
113122
#if CFG_TxContinuousMode

‎src/lmic/lmic_eu_like.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,4 +107,8 @@ void LMICeulike_restoreAdrState(const lmic_saved_adr_state_t *pStateBuffer);
107107
// set Rx1 frequency (might be different than uplink).
108108
void LMICeulike_setRx1Freq(void);
109109

110+
bit_t LMICeulike_isDataRateFeasible(dr_t dr);
111+
#define LMICbandplan_isDataRateFeasible(dr) LMICeulike_isDataRateFeasible(dr)
112+
113+
110114
#endif // _lmic_eu_like_h_

‎src/lmic/lmic_us_like.c

Lines changed: 65 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,6 @@ bit_t LMICuslike_mapChannels(u1_t chpage, u2_t chmap) {
138138
|| all channels 0..63 are turned off or on. MCMC_LADR_CHP_BANK
139139
|| is also special, in that it enables subbands.
140140
*/
141-
u1_t base, top;
142-
143141
if (chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_BANK) {
144142
// each bit enables a bank of channels
145143
for (u1_t subband = 0; subband < 8; ++subband, chmap >>= 1) {
@@ -149,60 +147,86 @@ bit_t LMICuslike_mapChannels(u1_t chpage, u2_t chmap) {
149147
LMIC_disableSubBand(subband);
150148
}
151149
}
150+
} else {
151+
u1_t base, top;
152+
153+
if (chpage < MCMD_LinkADRReq_ChMaskCntl_USLIKE_SPECIAL) {
154+
// operate on channels 0..15, 16..31, 32..47, 48..63
155+
// note that the chpage hasn't been shifted right, so
156+
// it's really the base.
157+
base = chpage;
158+
top = base + 16;
159+
if (base == 64) {
160+
top = 72;
161+
}
162+
} else /* if (chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_125ON ||
163+
chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_125OFF) */ {
164+
u1_t const en125 = chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_125ON;
165+
166+
// enable or disable all 125kHz channels
167+
for (u1_t chnl = 0; chnl < 64; ++chnl) {
168+
if (en125)
169+
LMIC_enableChannel(chnl);
170+
else
171+
LMIC_disableChannel(chnl);
172+
}
173+
174+
// then apply mask to top 8 channels.
175+
base = 64;
176+
top = 72;
177+
}
152178

153-
return LMIC.activeChannels125khz || LMIC.activeChannels500khz;
154-
}
155-
156-
if (chpage < MCMD_LinkADRReq_ChMaskCntl_USLIKE_SPECIAL) {
157-
// operate on channels 0..15, 16..31, 32..47, 48..63
158-
base = chpage << 4;
159-
top = base + 16;
160-
if (base == 64) {
161-
top = 72;
162-
}
163-
} else /* if (chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_125ON ||
164-
chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_125OFF) */ {
165-
u1_t const en125 = chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_125ON;
166-
167-
// enable or disable all 125kHz channels
168-
for (u1_t chnl = 0; chnl < 64; ++chnl) {
169-
if (en125)
170-
LMIC_enableChannel(chnl);
171-
else
172-
LMIC_disableChannel(chnl);
173-
}
174-
175-
// then apply mask to top 8 channels.
176-
base = 64;
177-
top = 72;
178-
}
179-
180-
// apply chmap to channels in [base..top-1].
181-
// Use enable/disable channel to keep activeChannel counts in sync.
182-
for (u1_t chnl = base; chnl < top; ++chnl, chmap >>= 1) {
183-
if (chmap & 0x0001)
184-
LMIC_enableChannel(chnl);
185-
else
186-
LMIC_disableChannel(chnl);
179+
// apply chmap to channels in [base..top-1].
180+
// Use enable/disable channel to keep activeChannel counts in sync.
181+
for (u1_t chnl = base; chnl < top; ++chnl, chmap >>= 1) {
182+
if (chmap & 0x0001)
183+
LMIC_enableChannel(chnl);
184+
else
185+
LMIC_disableChannel(chnl);
186+
}
187187
}
188-
return LMIC.activeChannels125khz || LMIC.activeChannels500khz;
188+
189+
LMICOS_logEventUint32("LMICuslike_mapChannels", (LMIC.activeChannels125khz << 16u)|(LMIC.activeChannels500khz << 0u));
190+
return (LMIC.activeChannels125khz > 0) || (LMIC.activeChannels500khz > 0);
189191
}
190192

191193
// US does not have duty cycling - return now as earliest TX time
192194
// but also do the channel hopping dance.
193195
ostime_t LMICuslike_nextTx(ostime_t now) {
194196
// TODO(tmm@mcci.com): use a static const for US-like
195197
if (LMIC.datarate >= LMICuslike_getFirst500kHzDR()) { // 500kHz
196-
ASSERT(LMIC.activeChannels500khz>0);
197-
setNextChannel(64, 64 + 8, LMIC.activeChannels500khz);
198+
if (LMIC.activeChannels500khz > 0) {
199+
setNextChannel(64, 64 + 8, LMIC.activeChannels500khz);
200+
} else if (LMIC.activeChannels125khz > 0) {
201+
LMIC.datarate = lowerDR(LMICuslike_getFirst500kHzDR(), 1);
202+
setNextChannel(0, 64, LMIC.activeChannels125khz);
203+
LMICOS_logEvent("LMICuslike_nextTx: no 500k, choose 125k");
204+
} else {
205+
LMICOS_logEvent("LMICuslike_nextTx: no channels at all (500)");
206+
}
198207
}
199208
else { // 125kHz
200-
ASSERT(LMIC.activeChannels125khz>0);
201-
setNextChannel(0, 64, LMIC.activeChannels125khz);
209+
if (LMIC.activeChannels125khz > 0) {
210+
setNextChannel(0, 64, LMIC.activeChannels125khz);
211+
} else if (LMIC.activeChannels500khz > 0) {
212+
LMIC.datarate = LMICuslike_getFirst500kHzDR();
213+
setNextChannel(64, 64 + 8, LMIC.activeChannels500khz);
214+
LMICOS_logEvent("LMICuslike_nextTx: no 125k, choose 500k");
215+
} else {
216+
LMICOS_logEvent("LMICuslike_nextTx: no channels at all (125)");
217+
}
202218
}
203219
return now;
204220
}
205221

222+
bit_t LMICuslike_isDataRateFeasible(dr_t dr) {
223+
if (dr >= LMICuslike_getFirst500kHzDR()) { // 500kHz
224+
return LMIC.activeChannels500khz > 0;
225+
} else {
226+
return LMIC.activeChannels125khz > 6;
227+
}
228+
}
229+
206230
#if !defined(DISABLE_JOIN)
207231
void LMICuslike_initJoinLoop(void) {
208232
// set an initial condition so that setNextChannel()'s preconds are met

‎src/lmic/lmic_us_like.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,4 +109,7 @@ bit_t LMICuslike_compareAdrState(const lmic_saved_adr_state_t *pStateBuffer);
109109
void LMICuslike_restoreAdrState(const lmic_saved_adr_state_t *pStateBuffer);
110110
#define LMICbandplan_restoreAdrState(pState) LMICuslike_restoreAdrState(pState)
111111

112+
bit_t LMICuslike_isDataRateFeasible(dr_t dr);
113+
#define LMICbandplan_isDataRateFeasible(dr) LMICuslike_isDataRateFeasible(dr)
114+
112115
#endif // _lmic_us_like_h_

0 commit comments

Comments
 (0)