Merge pull request #821 from mcci-catena/issue819

Fix corner cases for LinkADRReq; internal doc tweaks

Author: terrillmoore

README.md

@@ -120,7 +120,6 @@ The examples in this library (apart from the compliance sketch) are somewhat pri
 
 Although the wrappers in this library are designed to make the LMIC useful in the Arduino environment, the maintainers have tried to be careful to keep the core LMIC code generally useful. For example, I use this library without modification (but with wrappers) on a RISC-V platform in a non-Arduino environment.
 
-
 ## Installing
 
 To install this library:
@@ -854,8 +853,7 @@ The library observes and processes rising edges on the pins as part of `os_runlo
 This can be configured in one of two ways (see
 [Controlling use of interrupts](#controlling-use-of-interrupts)).  See [Interrupts and Arduino system timing](#interrupts-and-arduino-system-timing) for implementation details.
 
-By default, the library 
-polls the enabled pins to determine whether an event has occurred. This approach
+By default, the library polls the enabled pins to determine whether an event has occurred. This approach
 allows use of any CPU pin to sense the DIOs, and makes no assumptions about
 interrupts. However, it means that the end-of-transmit event is not observed
 (and time-stamped) until `os_runloop_once()` is called.

src/lmic/lmic.c

@@ -764,6 +764,9 @@ applyAdrRequests(
             p4     = opts[oidx+4];                  // ChMaskCtl, NbTrans
             u1_t chpage = p4 & MCMD_LinkADRReq_Redundancy_ChMaskCntl_MASK;     // channel page
 
+            // notice that we ignore map_ok except on the last setting.
+            // so LMICbandplan_mapChannels should report failure status, but do
+            // the work; if it fails, we'll back it out.
             map_ok = LMICbandplan_mapChannels(chpage, chmap);
             LMICOS_logEventUint32("applyAdrRequests: mapChannels", ((u4_t)chpage << 16)|(chmap << 0));
         }

src/lmic/lmic_us_like.c

@@ -99,6 +99,9 @@ bit_t LMICuslike_canMapChannels(u1_t chpage, u2_t chmap) {
 	|| channel map appllies to 500kHz (ch 64..71) and in addition
 	|| all channels 0..63 are turned off or on.  MCMC_LADR_CHP_BANK
 	|| is also special, in that it enables subbands.
+        ||
+        || TODO([email protected]) revise the 0xFF00 mask for regions with other than
+        || eight 500 kHz channels.
 	*/
 	if (chpage < MCMD_LinkADRReq_ChMaskCntl_USLIKE_SPECIAL) {
 		// operate on channels 0..15, 16..31, 32..47, 48..63, 64..71
@@ -111,17 +114,22 @@ bit_t LMICuslike_canMapChannels(u1_t chpage, u2_t chmap) {
 			return 1;
 		}
 	} else if (chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_BANK) {
-		if (chmap == 0 || (chmap & 0xFF00) != 0) {
-			// no bits set, or reserved bitsset , fail.
+		if ((chmap & 0xFF00) != 0) {
+			// Reserved bits set, fail.
 			return 0;
 		}
 	} else if (chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_125ON ||
 	           chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_125OFF) {
-                u1_t const en125 = chpage == MCMD_LinkADRReq_ChMaskCntl_USLIKE_125ON;
-
-		// if disabling all 125kHz chans, must have at least one 500kHz chan
-		// don't allow reserved bits to be set in chmap.
-		if ((! en125 && chmap == 0) || (chmap & 0xFF00) != 0)
+                //
+                // if disabling all 125kHz chans, you might think we must have
+                // at least one 500kHz chan; but that's a local conclusion.
+                // Some network servers will disable all (including 500kHz)
+                // then turn things back on in the next LinkADRReq. So
+                // we can't fail that here.
+                //
+                // But don't allow reserved bits to be set in chmap.
+                //
+                if ((chmap & 0xFF00) != 0)
 			return 0;
 	} else {
 		return 0;