Polish up changes for pull reguest #70

README.md

@@ -65,6 +65,7 @@ requires C99 mode to be enabled by default.
 - [Timing](#timing)
 - [Downlink datarate](#downlink-datarate)
 - [Release History](#release-history)
+- [Contributions](#contributions)
 - [Trademark Acknowledgements](#trademark-acknowledgements)
 - [License](#license)
 
@@ -685,6 +686,18 @@ When using OTAA, the network communicates the RX2 settings in the join accept me
 
 - V2.0.2 adds support for the extended bandplans.
 
+## Contributions
+
+This library started from the IBM V1.5 open-source code.
+
+- Thomas Telkamp and Matthijs Kooijman ported V1.5 to Arduino and did a lot of bug fixing.
+
+- Terry Moore, LeRoy Leslie, Frank Rose, and ChaeHee Won did a lot of work on US support.
+
+- Terry Moore added the AU921, AS923 and IN866 bandplans, and created the regionalization framework.
+
+- [@tanupoo](https://github.com/tanupoo) of the WIDE Project debugged AS923JP and LBT support.
+
 ## Trademark Acknowledgements
 
 LoRa is a registered trademark of the LoRa Alliance. LoRaWAN is a trademark of the LoRa Alliance.

src/lmic/lmic.c

@@ -1036,24 +1036,30 @@ static bit_t processJoinAccept (void) {
             return 1;
         }
         LMIC.opmode &= ~OP_TXRXPEND;
-        ostime_t delay = LMICbandplan_nextJoinState();
+        int failed = LMICbandplan_nextJoinState();
         EV(devCond, DEBUG, (e_.reason = EV::devCond_t::NO_JACC,
                             e_.eui    = MAIN::CDEV->getEui(),
                             e_.info   = LMIC.datarate|DR_PAGE,
-                            e_.info2  = osticks2ms(delay)));
+                            e_.info2  = failed));
         // Build next JOIN REQUEST with next engineUpdate call
         // Optionally, report join failed.
-        // Both after a random/chosen amount of ticks.
+        // Both after a random/chosen amount of ticks. That time
+	// is in LMIC.txend. The delay here is either zero or 1
+	// tick; onJoinFailed()/runEngineUpdate() are responsible
+	// for honoring that. XXX([email protected]) The IBM 1.6 code
+	// claimed to return a delay but really returns 0 or 1.
+	// Once we update as923 to return failed after dr2, we
+	// can take out this #if.
 #if CFG_region != LMIC_REGION_as923
-        os_setTimedCallback(&LMIC.osjob, os_getTime()+delay,
-                            (delay&1) != 0
+        os_setTimedCallback(&LMIC.osjob, os_getTime()+failed,
+                            failed
                             ? FUNC_ADDR(onJoinFailed)      // one JOIN iteration done and failed
                             : FUNC_ADDR(runEngineUpdate)); // next step to be delayed
 #else
        // in the join of AS923 v1.1 older, only DR2 is used. Therefore,
        // not much improvement when it handles two different behavior;
        // onJoinFailed or runEngineUpdate.
-        os_setTimedCallback(&LMIC.osjob, os_getTime()+delay,
+        os_setTimedCallback(&LMIC.osjob, os_getTime()+failed,
                             FUNC_ADDR(onJoinFailed));
 #endif
         return 1;
@@ -1121,8 +1127,14 @@ static bit_t processJoinAccept (void) {
                                       : EV::joininfo_t::ACCEPT)));
 
     ASSERT((LMIC.opmode & (OP_JOINING|OP_REJOIN))!=0);
+    //
+    // XXX([email protected]) OP_REJOIN confuses me, and I'm not sure why we're
+    // adjusting DRs here. We've just recevied a join accept, and the
+    // datarate therefore shouldn't be in play.
+    //
     if( (LMIC.opmode & OP_REJOIN) != 0 ) {
 #if CFG_region != LMIC_REGION_as923
+	// TODO([email protected]) regionalize
         // Lower DR every try below current UP DR
         LMIC.datarate = lowerDR(LMIC.datarate, LMIC.rejoinCnt);
 #else

src/lmic/lmic.h

@@ -34,14 +34,20 @@
 #include "oslmic.h"
 #include "lorabase.h"
 
-#if LMIC_DEBUG_LEVEL > 0
+#if LMIC_DEBUG_LEVEL > 0 || LMIC_X_DEBUG_LEVEL > 0
 # if defined(LMIC_DEBUG_INCLUDE)
 #   define LMIC_STRINGIFY_(x) #x
 #   define LMIC_STRINGIFY(x) LMIC_STRINGIFY_(x)
 #   include LMIC_STRINGIFY(LMIC_DEBUG_INCLUDE)
 # endif
+#  ifdef LMIC_DEBUG_PRINTF_FN
+     extern void LMIC_DEBUG_PRINTF_FN(const char *f, ...);
+#  endif // ndef LMIC_DEBUG_PRINTF_FN
+#endif
+
 // if LMIC_DEBUG_PRINTF is now defined, just use it. This lets you do anything
 // you like with a sufficiently crazy header file.
+#if LMIC_LEVEL_DEBUG > 0
 # ifndef LMIC_DEBUG_PRINTF
 //  otherwise, check whether someone configured a print-function to be used,
 //  and use it if so.
@@ -70,12 +76,14 @@
 # define LMIC_DEBUG_FLUSH()             do { ; } while (0)
 #endif // LMIC_DEBUG_LEVEL == 0
 
+//
+// LMIC_X_DEBUG_LEVEL enables additional, special print functions for debugging
+// RSSI features. This is used sparingly.
 #if LMIC_X_DEBUG_LEVEL > 0
 #  ifdef LMIC_DEBUG_PRINTF_FN
-     extern void LMIC_DEBUG_PRINTF_FN(const char *f, ...);
 #    define LMIC_X_DEBUG_PRINTF(f, ...) LMIC_DEBUG_PRINTF_FN(f, ## __VA_ARGS__)
 #  else
-#    error "LMIC_DEBUG_PRINTF_FN must be defined."
+#    error "LMIC_DEBUG_PRINTF_FN must be defined for LMIC_X_DEBUG_LEVEL > 0."
 #  endif
 #else
 #  define LMIC_X_DEBUG_PRINTF(f, ...)  do {;} while(0)

src/lmic/lmic_as923.c

@@ -184,7 +184,7 @@ void
 LMICas923_init(void) {
         // if this is japan, set LBT mode
         if (LMIC_COUNTRY_CODE == LMIC_COUNTRY_CODE_JP) {
-                LMIC.lbt_ticks = ms2osticks(AS923JP_LBT_5MS);
+                LMIC.lbt_ticks = us2osticks(AS923JP_LBT_US);
                 LMIC.lbt_dbmax = AS923JP_LBT_DB_MAX;
         }
 }
@@ -193,7 +193,7 @@ void
 LMICas923_resetDefaultChannels(void) {
         // if this is japan, set LBT mode
         if (LMIC_COUNTRY_CODE == LMIC_COUNTRY_CODE_JP) {
-                LMIC.lbt_ticks = ms2osticks(AS923JP_LBT_5MS);
+                LMIC.lbt_ticks = us2osticks(AS923JP_LBT_US);
                 LMIC.lbt_dbmax = AS923JP_LBT_DB_MAX;
         }
 }

src/lmic/lmic_eu_like.c

@@ -91,6 +91,16 @@ void LMICeulike_updateTx(ostime_t txbeg) {
 }
 
 #if !defined(DISABLE_JOIN)
+//
+// TODO([email protected]):
+//
+// The definition of this is a little strange. this seems to return a time, but
+// in reality it returns 0 if the caller should continue scanning through
+// channels, and 1 if the caller has scanned all channels on this session,
+// and therefore should reset to the beginning.  The IBM 1.6 code is the
+// same way, so apparently I just carried this across. We should declare
+// as bool_t and change callers to use the result clearly as a flag.
+//
 ostime_t LMICeulike_nextJoinState(uint8_t nDefaultChannels) {
         u1_t failed = 0;
 
@@ -100,10 +110,17 @@ ostime_t LMICeulike_nextJoinState(uint8_t nDefaultChannels) {
                 LMIC.txChnl = 0;
         if ((++LMIC.txCnt % nDefaultChannels) == 0) {
                 // Lower DR every nth try (having all default channels with same DR)
+		//
+		// TODO([email protected]) add new DR_REGIN_JOIN_MIN instead of LORAWAN_DR0;
+		// then we can eliminate the LMIC_REGION_as923 below because we'll set
+		// the failed flag here. This will cause the outer caller to take the
+		// appropriate join path. Or add new LMICeulike_GetLowestJoinDR()
+		//
                 if (LMIC.datarate == LORAWAN_DR0)
                         failed = 1; // we have tried all DR - signal EV_JOIN_FAILED
                 else
                 {
+// TODO([email protected]) - see above; please remove regional dependency from this file.
 #if CFG_region != LMIC_REGION_as923
                         LMICcore_setDrJoin(DRCHG_NOJACC, decDR((dr_t)LMIC.datarate));
 #else
@@ -118,12 +135,21 @@ ostime_t LMICeulike_nextJoinState(uint8_t nDefaultChannels) {
         // Move txend to randomize synchronized concurrent joins.
         // Duty cycle is based on txend.
         ostime_t const time = LMICbandplan_nextJoinTime(os_getTime());
+
+	// TODO([email protected]): change delay to (0:1) secs + a known t0, but randomized;
+        // starting adding a bias after 1 hour, 25 hours, etc.; and limit the duty
+        // cycle on power up. For testability, add a way to set the join start time
+        // externally (a test API) so we can check this feature.
+        // See https://github.com/mcci-catena/arduino-lmic/issues/2
+	// Current code doesn't match LoRaWAN 1.0.2 requirements.
+
         LMIC.txend = time +
                 (isTESTMODE()
                         // Avoid collision with JOIN ACCEPT @ SF12 being sent by GW (but we missed it)
                         ? DNW2_SAFETY_ZONE
                         // Otherwise: randomize join (street lamp case):
                         // SF12:255, SF11:127, .., SF7:8secs
+			//
                         : DNW2_SAFETY_ZONE + LMICcore_rndDelay(255 >> LMIC.datarate));
         // 1 - triggers EV_JOIN_FAILED event
         return failed;

src/lmic/lmic_us_like.c

@@ -193,6 +193,16 @@ void LMICuslike_initJoinLoop(void) {
 #endif // !DISABLE_JOIN
 
 #if !defined(DISABLE_JOIN)
+//
+// TODO([email protected]):
+//
+// The definition of this is a little strange. this seems to return a time, but
+// in reality it returns 0 if the caller should continue scanning through
+// channels, and 1 if the caller has scanned all channels on this session,
+// and therefore should reset to the beginning.  The IBM 1.6 code is the
+// same way, so apparently I just carried this across. We should declare
+// as bool_t and change callers to use the result clearly as a flag.
+//
 ostime_t LMICuslike_nextJoinState(void) {
         // Try the following:
         //   DR0 (SF10)  on a random channel 0..63
@@ -227,6 +237,8 @@ ostime_t LMICuslike_nextJoinState(void) {
         // cycle on power up. For testability, add a way to set the join start time
         // externally (a test API) so we can check this feature.
         // See https://github.com/mcci-catena/arduino-lmic/issues/2
+	// Current code doesn't match LoRaWAN 1.0.2 requirements.
+
         LMIC.txend = os_getTime() +
                 (isTESTMODE()
                         // Avoid collision with JOIN ACCEPT being sent by GW (but we missed it - GW is still busy)

src/lmic/lorabase_as923.h

@@ -74,12 +74,23 @@ enum { DR_PAGE_AS923 = 0x10 * (LMIC_REGION_as923 - 1) };
 
 enum { AS923_LMIC_REGION_EIRP = 1 };         // region uses EIRP
 
-enum { AS923JP_LBT_5MS = 5 };          // microseconds of LBT time
-enum { AS923JP_LBT_DB_MAX = -80 };      // maximum channel strength
+enum { AS923JP_LBT_US = 5000 };         // microseconds of LBT time -- 5000 ==>
+					// 5 ms. We use us rather than ms for
+					// future 128us support, and just for
+					// backward compatibility -- there
+					// is code that uses the _US constant,
+					// and it's awkward to break it.
 
-#if !defined(AS923_TX_CAP)
-// AS923 v1.1, all channels face a 1% duty cycle.
-# define AS923_TX_CAP 100
+enum { AS923JP_LBT_DB_MAX = -80 };      // maximum channel strength in dB; if TX
+					// we measure more than this, we don't tx.
+
+// AS923 v1.1, all channels face a 1% duty cycle. So this will have to change
+// in the future via a config. But this code base needs major changes for
+// v1.1 in any case.
+enum { AS923_V102_TX_CAP = 100 };		// v1.0.2 allows 100%
+
+#ifndef AS923_TX_CAP
+# define AS923_TX_CAP	AS923_V102_TX_CAP
 #endif
 
 #endif /* _lorabase_as923_h_ */

src/lmic/radio.c

@@ -25,10 +25,6 @@
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#if LMIC_X_DEBUG_LEVEL > 0
-#include <Arduino.h>
-#endif
-
 #define LMIC_DR_LEGACY 0
 
 #include "lmic.h"