Merge pull request #127 from geeksville/nema-124

0.6.3 - fix the problem of BLE message receiption being busted in 0.6.2

.vscode/settings.json

@@ -50,7 +50,10 @@
         "cassert": "cpp"
     },
     "cSpell.words": [
+        "Blox",
         "Meshtastic",
+        "NEMAGPS",
+        "Ublox",
         "descs",
         "protobufs"
     ]

bin/version.sh

@@ -1,3 +1,3 @@
 
 
-export VERSION=0.6.1
+export VERSION=0.6.3

docs/software/nrf52-TODO.md

@@ -8,8 +8,7 @@ Minimum items needed to make sure hardware is good.
 - use "variants" to get all gpio bindings
 - plug in correct variants for the real board
 - Use the PMU driver on real hardware
-- add a NEMA based GPS driver to test GPS
-- Use new radio driver on real hardware - possibly start with https://os.mbed.com/teams/Semtech/code/SX126xLib/
+- Use new radio driver on real hardware
 - Use UC1701 LCD driver on real hardware. Still need to create at startup and probe on SPI
 - test the LEDs
 - test the buttons
@@ -24,6 +23,7 @@ Minimum items needed to make sure hardware is good.
 
 Needed to be fully functional at least at the same level of the ESP32 boards. At this point users would probably want them.
 
+- stop polling for GPS characters, instead stay blocked on read in a thread
 - increase preamble length? - will break other clients? so all devices must update
 - enable BLE DFU somehow
 - set appversion/hwversion
@@ -100,6 +100,7 @@ Nice ideas worth considering someday...
 - DONE neg 7 error code from receive
 - DONE remove unused sx1262 lib from github
 - at boot we are starting our message IDs at 1, rather we should start them at a random number. also, seed random based on timer. this could be the cause of our first message not seen bug.
+- add a NEMA based GPS driver to test GPS
 
 ```
 

platformio.ini

@@ -31,7 +31,7 @@ board_build.partitions = partition-table.csv
 
 ; note: we add src to our include search path so that lmic_project_config can override
 ; FIXME: fix lib/BluetoothOTA dependency back on src/ so we can remove -Isrc
-build_flags = -Wno-missing-field-initializers -Isrc -Isrc/mesh -Ilib/nanopb/include -Os -Wl,-Map,.pio/build/output.map 
+build_flags = -Wno-missing-field-initializers -Isrc -Isrc/mesh -Isrc/gps -Ilib/nanopb/include -Os -Wl,-Map,.pio/build/output.map 
   -DAXP_DEBUG_PORT=Serial 
   -DHW_VERSION_${sysenv.COUNTRY}
   -DAPP_VERSION=${sysenv.APP_VERSION}
@@ -74,6 +74,7 @@ lib_deps =
   https://github.com/meshtastic/arduino-fsm.git 
   https://github.com/meshtastic/SparkFun_Ublox_Arduino_Library.git
   https://github.com/meshtastic/RadioLib.git
+  https://github.com/meshtastic/TinyGPSPlus.git 
   
 ; Common settings for ESP targes, mixin with extends = esp32_base
 [esp32_base]

src/GPS.cpp

@@ -1,218 +0,0 @@
-
-#include "GPS.h"
-#include "configuration.h"
-#include "time.h"
-#include <assert.h>
-#include <sys/time.h>
-
-#ifdef GPS_RX_PIN
-HardwareSerial _serial_gps(GPS_SERIAL_NUM);
-#else
-// Assume NRF52
-HardwareSerial &_serial_gps = Serial1;
-#endif
-
-bool timeSetFromGPS; // We try to set our time from GPS each time we wake from sleep
-
-GPS gps;
-
-// stuff that really should be in in the instance instead...
-static uint32_t
-    timeStartMsec; // Once we have a GPS lock, this is where we hold the initial msec clock that corresponds to that time
-static uint64_t zeroOffsetSecs; // GPS based time in secs since 1970 - only updated once on initial lock
-
-static bool wantNewLocation = true;
-
-GPS::GPS() : PeriodicTask() {}
-
-void GPS::setup()
-{
-    PeriodicTask::setup();
-
-    readFromRTC(); // read the main CPU RTC at first
-
-#ifdef GPS_RX_PIN
-    _serial_gps.begin(GPS_BAUDRATE, SERIAL_8N1, GPS_RX_PIN, GPS_TX_PIN);
-#else
-    _serial_gps.begin(GPS_BAUDRATE);
-#endif
-    // _serial_gps.setRxBufferSize(1024); // the default is 256
-    // ublox.enableDebugging(Serial);
-
-    // note: the lib's implementation has the wrong docs for what the return val is
-    // it is not a bool, it returns zero for success
-    isConnected = ublox.begin(_serial_gps);
-
-    // try a second time, the ublox lib serial parsing is buggy?
-    if (!isConnected)
-        isConnected = ublox.begin(_serial_gps);
-
-    if (isConnected) {
-        DEBUG_MSG("Connected to GPS successfully\n");
-
-        bool factoryReset = false;
-        bool ok;
-        if (factoryReset) {
-            // It is useful to force back into factory defaults (9600baud, NEMA to test the behavior of boards that don't have
-            // GPS_TX connected)
-            ublox.factoryReset();
-            delay(2000);
-            isConnected = ublox.begin(_serial_gps);
-            DEBUG_MSG("Factory reset success=%d\n", isConnected);
-            if (isConnected) {
-                ublox.assumeAutoPVT(true, true); // Just parse NEMA for now
-            }
-        } else {
-            ok = ublox.setUART1Output(COM_TYPE_UBX, 500); // Use native API
-            assert(ok);
-            ok = ublox.setNavigationFrequency(1, 500); // Produce 4x/sec to keep the amount of time we stall in getPVT low
-            assert(ok);
-            // ok = ublox.setAutoPVT(false); // Not implemented on NEO-6M
-            // assert(ok);
-            // ok = ublox.setDynamicModel(DYN_MODEL_BIKE); // probably PEDESTRIAN but just in case assume bike speeds
-            // assert(ok);
-            ok = ublox.powerSaveMode(); // use power save mode
-            assert(ok);
-        }
-        ok = ublox.saveConfiguration(3000);
-        assert(ok);
-    } else {
-        // Some boards might have only the TX line from the GPS connected, in that case, we can't configure it at all.  Just
-        // assume NEMA at 9600 baud.
-        DEBUG_MSG("ERROR: No bidirectional GPS found, hoping that it still might work\n");
-
-        // tell lib, we are expecting the module to send PVT messages by itself to our Rx pin
-        // you can set second parameter to "false" if you want to control the parsing and eviction of the data (need to call
-        // checkUblox cyclically)
-        ublox.assumeAutoPVT(true, true);
-    }
-}
-
-void GPS::readFromRTC()
-{
-    struct timeval tv; /* btw settimeofday() is helpfull here too*/
-
-    if (!gettimeofday(&tv, NULL)) {
-        uint32_t now = millis();
-
-        DEBUG_MSG("Read RTC time as %ld (cur millis %u) valid=%d\n", tv.tv_sec, now, timeSetFromGPS);
-        timeStartMsec = now;
-        zeroOffsetSecs = tv.tv_sec;
-    }
-}
-
-/// If we haven't yet set our RTC this boot, set it from a GPS derived time
-void GPS::perhapsSetRTC(const struct timeval *tv)
-{
-    if (!timeSetFromGPS) {
-        timeSetFromGPS = true;
-        DEBUG_MSG("Setting RTC %ld secs\n", tv->tv_sec);
-#ifndef NO_ESP32
-        settimeofday(tv, NULL);
-#else
-        DEBUG_MSG("ERROR TIME SETTING NOT IMPLEMENTED!\n");
-#endif
-        readFromRTC();
-    }
-}
-
-#include <time.h>
-
-uint32_t GPS::getTime()
-{
-    return ((millis() - timeStartMsec) / 1000) + zeroOffsetSecs;
-}
-
-uint32_t GPS::getValidTime()
-{
-    return timeSetFromGPS ? getTime() : 0;
-}
-
-/// Returns true if we think the board can enter deep or light sleep now (we might be trying to get a GPS lock)
-bool GPS::canSleep()
-{
-    return true; // we leave GPS on during sleep now, so sleep is okay !wantNewLocation;
-}
-
-/// Prepare the GPS for the cpu entering deep or light sleep, expect to be gone for at least 100s of msecs
-void GPS::prepareSleep()
-{
-    if (isConnected)
-        ublox.powerOff();
-}
-
-void GPS::doTask()
-{
-    uint8_t fixtype = 3; // If we are only using the RX pin, assume we have a 3d fix
-
-    if (isConnected) {
-        // Consume all characters that have arrived
-
-        // getPVT automatically calls checkUblox
-        ublox.checkUblox(); // See if new data is available. Process bytes as they come in.
-
-        // If we don't have a fix (a quick check), don't try waiting for a solution)
-        // Hmmm my fix type reading returns zeros for fix, which doesn't seem correct, because it is still sptting out positions
-        // turn off for now
-        // fixtype = ublox.getFixType();
-        DEBUG_MSG("fix type %d\n", fixtype);
-    }
-
-    // DEBUG_MSG("sec %d\n", ublox.getSecond());
-    // DEBUG_MSG("lat %d\n", ublox.getLatitude());
-
-    // any fix that has time
-    if (!timeSetFromGPS && ublox.getT()) {
-        struct timeval tv;
-
-        /* Convert to unix time
-The Unix epoch (or Unix time or POSIX time or Unix timestamp) is the number of seconds that have elapsed since January 1, 1970
-(midnight UTC/GMT), not counting leap seconds (in ISO 8601: 1970-01-01T00:00:00Z).
-*/
-        struct tm t;
-        t.tm_sec = ublox.getSecond();
-        t.tm_min = ublox.getMinute();
-        t.tm_hour = ublox.getHour();
-        t.tm_mday = ublox.getDay();
-        t.tm_mon = ublox.getMonth() - 1;
-        t.tm_year = ublox.getYear() - 1900;
-        t.tm_isdst = false;
-        time_t res = mktime(&t);
-        tv.tv_sec = res;
-        tv.tv_usec = 0; // time.centisecond() * (10 / 1000);
-
-        DEBUG_MSG("Got time from GPS month=%d, year=%d, unixtime=%ld\n", t.tm_mon, t.tm_year, tv.tv_sec);
-        if (t.tm_year < 0 || t.tm_year >= 300)
-            DEBUG_MSG("Ignoring invalid GPS time\n");
-        else
-            perhapsSetRTC(&tv);
-    }
-
-    if ((fixtype >= 3 && fixtype <= 4) && ublox.getP()) // rd fixes only
-    {
-        // we only notify if position has changed
-        latitude = ublox.getLatitude() * 1e-7;
-        longitude = ublox.getLongitude() * 1e-7;
-        altitude = ublox.getAltitude() / 1000; // in mm convert to meters
-        DEBUG_MSG("new gps pos lat=%f, lon=%f, alt=%d\n", latitude, longitude, altitude);
-
-        hasValidLocation = (latitude != 0) || (longitude != 0); // bogus lat lon is reported as 0,0
-        if (hasValidLocation) {
-            wantNewLocation = false;
-            notifyObservers(NULL);
-            // ublox.powerOff();
-        }
-    } else // we didn't get a location update, go back to sleep and hope the characters show up
-        wantNewLocation = true;
-
-    // Once we have sent a location once we only poll the GPS rarely, otherwise check back every 1s until we have something over
-    // the serial
-    setPeriod(hasValidLocation && !wantNewLocation ? 30 * 1000 : 10 * 1000);
-}
-
-void GPS::startLock()
-{
-    DEBUG_MSG("Looking for GPS lock\n");
-    wantNewLocation = true;
-    setPeriod(1);
-}

src/GPS.h

@@ -1,56 +0,0 @@
-#pragma once
-
-#include "Observer.h"
-#include "PeriodicTask.h"
-#include "SparkFun_Ublox_Arduino_Library.h"
-#include "sys/time.h"
-
-/**
- * A gps class that only reads from the GPS periodically (and FIXME - eventually keeps the gps powered down except when reading)
- *
- * When new data is available it will notify observers.
- */
-class GPS : public PeriodicTask, public Observable<void *>
-{
-    SFE_UBLOX_GPS ublox;
-
-  public:
-    double latitude, longitude;
-    uint32_t altitude;
-    bool isConnected; // Do we have a GPS we are talking to
-
-    GPS();
-
-    /// Return time since 1970 in secs.  Until we have a GPS lock we will be returning time based at zero
-    uint32_t getTime();
-
-    /// Return time since 1970 in secs.  If we don't have a GPS lock return zero
-    uint32_t getValidTime();
-
-    void setup();
-
-
-    virtual void doTask();
-
-    /// If we haven't yet set our RTC this boot, set it from a GPS derived time
-    void perhapsSetRTC(const struct timeval *tv);
-
-    /// Returns true if we think the board can enter deep or light sleep now (we might be trying to get a GPS lock)
-    bool canSleep();
-
-    /// Prepare the GPS for the cpu entering deep or light sleep, expect to be gone for at least 100s of msecs
-    void prepareSleep();
-
-    /// Restart our lock attempt - try to get and broadcast a GPS reading ASAP
-    void startLock();
-
-    /// Returns ture if we have acquired GPS lock.
-    bool hasLock() const { return hasValidLocation; }
-
-  private:
-    void readFromRTC();
-
-    bool hasValidLocation = false; // default to false, until we complete our first read
-};
-
-extern GPS gps;

src/OSTimer.cpp

@@ -0,0 +1,49 @@
+#include "OSTimer.h"
+#include "configuration.h"
+
+#ifdef NO_ESP32
+
+/**
+ * Schedule a callback to run.  The callback must _not_ block, though it is called from regular thread level (not ISR)
+ *
+ * NOTE! xTimerPend... seems to ignore the time passed in on ESP32 - I haven't checked on NRF52
+ *
+ * @return true if successful, false if the timer fifo is too full.
+ */
+bool scheduleOSCallback(PendableFunction callback, void *param1, uint32_t param2, uint32_t delayMsec)
+{
+    return xTimerPendFunctionCall(callback, param1, param2, pdMS_TO_TICKS(delayMsec));
+}
+
+#else
+
+// Super skanky quick hack to use hardware timers of the ESP32
+static hw_timer_t *timer;
+static PendableFunction tCallback;
+static void *tParam1;
+static uint32_t tParam2;
+
+static void IRAM_ATTR onTimer()
+{
+    (*tCallback)(tParam1, tParam2);
+}
+
+bool scheduleHWCallback(PendableFunction callback, void *param1, uint32_t param2, uint32_t delayMsec)
+{
+    if (!timer) {
+        timer = timerBegin(0, 80, true); // one usec per tick (main clock is 80MhZ on ESP32)
+        assert(timer);
+        timerAttachInterrupt(timer, &onTimer, true);
+    }
+
+    tCallback = callback;
+    tParam1 = param1;
+    tParam2 = param2;
+
+    timerAlarmWrite(timer, delayMsec * 1000L, false); // Do not reload, we want it to be a single shot timer
+    timerRestart(timer);
+    timerAlarmEnable(timer);
+    return true;
+}
+
+#endif

src/OSTimer.h

@@ -0,0 +1,18 @@
+#pragma once
+
+#include <Arduino.h>
+
+typedef void (*PendableFunction)(void *pvParameter1, uint32_t ulParameter2);
+
+/**
+ * Schedule a callback to run.  The callback must _not_ block, though it is called from regular thread level (not ISR)
+ *
+ * NOTE! ESP32 implementation is busted - always waits 0 ticks
+ * 
+ * @return true if successful, false if the timer fifo is too full.
+ */
+ bool scheduleOSCallback(PendableFunction callback, void *param1, uint32_t param2, uint32_t delayMsec);
+
+
+/// Uses a hardware timer, but calls the handler in _interrupt_ context
+bool scheduleHWCallback(PendableFunction callback, void *param1, uint32_t param2, uint32_t delayMsec);

src/PowerFSM.cpp

@@ -87,7 +87,7 @@ static void lsIdle()
 static void lsExit()
 {
     // setGPSPower(true); // restore GPS power
-    gps.startLock();
+    gps->startLock();
 }
 
 static void nbEnter()

src/SerialConsole.cpp

@@ -21,8 +21,8 @@ void SerialConsole::init()
 }
 
 /**
- * we override this to notice when we've received a protobuf over the serial stream.  Then we shunt off
- * debug serial output.
+ * we override this to notice when we've received a protobuf over the serial
+ * stream.  Then we shunt off debug serial output.
  */
 void SerialConsole::handleToRadio(const uint8_t *buf, size_t len)
 {

src/SerialConsole.h

@@ -19,6 +19,13 @@ class SerialConsole : public StreamAPI, public RedirectablePrint
      * debug serial output.
      */
     virtual void handleToRadio(const uint8_t *buf, size_t len);
+
+    virtual size_t write(uint8_t c)
+    {
+        if (c == '\n') // prefix any newlines with carriage return
+            RedirectablePrint::write('\r');
+        return RedirectablePrint::write(c);
+    }
 };
 
 extern SerialConsole console;

src/WorkerThread.cpp

@@ -39,6 +39,5 @@ void NotifiedWorkerThread::notifyFromISR(BaseType_t *highPriWoken, uint32_t v, e
 void NotifiedWorkerThread::block()
 {
     xTaskNotifyWait(0,                                          // don't clear notification on entry
-                    0,                             //  do not reset notification value on read
-                    &notification, portMAX_DELAY); // Wait forever
+                    clearOnRead, &notification, portMAX_DELAY); // Wait forever
 }

src/WorkerThread.h

@@ -70,6 +70,13 @@ class NotifiedWorkerThread : public WorkerThread
      */
     uint32_t notification = 0;
 
+    /**
+     * What notification bits should be cleared just after we read and return them in notification?
+     *
+     * Defaults to clear all of them.
+     */
+    uint32_t clearOnRead = UINT32_MAX;
+
     /**
      * A method that should block execution - either waiting ona queue/mutex or a "task notification"
      */

src/esp32/MeshBluetoothService.cpp

@@ -178,7 +178,7 @@ class MyNodeInfoCharacteristic : public ProtobufCharacteristic
     void onRead(BLECharacteristic *c)
     {
         // update gps connection state
-        myNodeInfo.has_gps = gps.isConnected;
+        myNodeInfo.has_gps = gps->isConnected;
 
         ProtobufCharacteristic::onRead(c);
 

src/gps/GPS.cpp

@@ -0,0 +1,81 @@
+
+#include "GPS.h"
+#include "configuration.h"
+#include "time.h"
+#include <assert.h>
+#include <sys/time.h>
+
+#ifdef GPS_RX_PIN
+HardwareSerial _serial_gps_real(GPS_SERIAL_NUM);
+HardwareSerial &GPS::_serial_gps = _serial_gps_real;
+#else
+// Assume NRF52
+HardwareSerial &GPS::_serial_gps = Serial1;
+#endif
+
+bool timeSetFromGPS; // We try to set our time from GPS each time we wake from sleep
+
+GPS *gps;
+
+// stuff that really should be in in the instance instead...
+static uint32_t
+    timeStartMsec; // Once we have a GPS lock, this is where we hold the initial msec clock that corresponds to that time
+static uint64_t zeroOffsetSecs; // GPS based time in secs since 1970 - only updated once on initial lock
+
+void readFromRTC()
+{
+    struct timeval tv; /* btw settimeofday() is helpfull here too*/
+
+    if (!gettimeofday(&tv, NULL)) {
+        uint32_t now = millis();
+
+        DEBUG_MSG("Read RTC time as %ld (cur millis %u) valid=%d\n", tv.tv_sec, now, timeSetFromGPS);
+        timeStartMsec = now;
+        zeroOffsetSecs = tv.tv_sec;
+    }
+}
+
+/// If we haven't yet set our RTC this boot, set it from a GPS derived time
+void perhapsSetRTC(const struct timeval *tv)
+{
+    if (!timeSetFromGPS) {
+        timeSetFromGPS = true;
+        DEBUG_MSG("Setting RTC %ld secs\n", tv->tv_sec);
+#ifndef NO_ESP32
+        settimeofday(tv, NULL);
+#else
+        DEBUG_MSG("ERROR TIME SETTING NOT IMPLEMENTED!\n");
+#endif
+        readFromRTC();
+    }
+}
+
+void perhapsSetRTC(struct tm &t)
+{
+    /* Convert to unix time
+    The Unix epoch (or Unix time or POSIX time or Unix timestamp) is the number of seconds that have elapsed since January 1, 1970
+    (midnight UTC/GMT), not counting leap seconds (in ISO 8601: 1970-01-01T00:00:00Z).
+    */
+    time_t res = mktime(&t);
+    struct timeval tv;
+    tv.tv_sec = res;
+    tv.tv_usec = 0; // time.centisecond() * (10 / 1000);
+
+    // DEBUG_MSG("Got time from GPS month=%d, year=%d, unixtime=%ld\n", t.tm_mon, t.tm_year, tv.tv_sec);
+    if (t.tm_year < 0 || t.tm_year >= 300)
+        DEBUG_MSG("Ignoring invalid GPS time\n");
+    else
+        perhapsSetRTC(&tv);
+}
+
+#include <time.h>
+
+uint32_t getTime()
+{
+    return ((millis() - timeStartMsec) / 1000) + zeroOffsetSecs;
+}
+
+uint32_t getValidTime()
+{
+    return timeSetFromGPS ? getTime() : 0;
+}

src/gps/GPS.h

@@ -0,0 +1,55 @@
+#pragma once
+
+#include "Observer.h"
+#include "PeriodicTask.h"
+#include "sys/time.h"
+
+/// If we haven't yet set our RTC this boot, set it from a GPS derived time
+void perhapsSetRTC(const struct timeval *tv);
+void perhapsSetRTC(struct tm &t);
+
+/// Return time since 1970 in secs.  Until we have a GPS lock we will be returning time based at zero
+uint32_t getTime();
+
+/// Return time since 1970 in secs.  If we don't have a GPS lock return zero
+uint32_t getValidTime();
+
+void readFromRTC();
+
+/**
+ * A gps class that only reads from the GPS periodically (and FIXME - eventually keeps the gps powered down except when reading)
+ *
+ * When new data is available it will notify observers.
+ */
+class GPS : public Observable<void *>
+{
+  protected:
+    bool hasValidLocation = false; // default to false, until we complete our first read
+
+    static HardwareSerial &_serial_gps;
+
+  public:
+    int32_t latitude = 0, longitude = 0; // as an int mult by 1e-7 to get value as double
+    int32_t altitude = 0;
+    bool isConnected = false; // Do we have a GPS we are talking to
+
+    virtual ~GPS() {}
+
+    /**
+     * Returns true if we succeeded
+     */
+    virtual bool setup() { return true; }
+
+    /// A loop callback for subclasses that need it.  FIXME, instead just block on serial reads
+    virtual void loop() {}
+
+    /// Returns ture if we have acquired GPS lock.
+    bool hasLock() const { return hasValidLocation; }
+
+    /**
+     * Restart our lock attempt - try to get and broadcast a GPS reading ASAP
+     * called after the CPU wakes from light-sleep state */
+    virtual void startLock() {}
+};
+
+extern GPS *gps;

src/gps/NEMAGPS.cpp

@@ -0,0 +1,65 @@
+#include "NEMAGPS.h"
+#include "configuration.h"
+
+static int32_t toDegInt(RawDegrees d)
+{
+    int32_t degMult = 10000000; // 1e7
+    int32_t r = d.deg * degMult + d.billionths / 100;
+    if (d.negative)
+        r *= -1;
+    return r;
+}
+
+void NEMAGPS::loop()
+{
+
+    while (_serial_gps.available() > 0) {
+        int c = _serial_gps.read();
+        // Serial.write(c);
+        reader.encode(c);
+    }
+
+    uint32_t now = millis();
+    if ((now - lastUpdateMsec) > 20 * 1000) { // Ugly hack for now - limit update checks to once every 20 secs (but still consume
+                                              // serial chars at whatever rate)
+        lastUpdateMsec = now;
+
+        auto ti = reader.time;
+        auto d = reader.date;
+        if (ti.isUpdated() && ti.isValid() && d.isValid()) {
+            /* Convert to unix time
+    The Unix epoch (or Unix time or POSIX time or Unix timestamp) is the number of seconds that have elapsed since January 1, 1970
+    (midnight UTC/GMT), not counting leap seconds (in ISO 8601: 1970-01-01T00:00:00Z).
+    */
+            struct tm t;
+            t.tm_sec = ti.second();
+            t.tm_min = ti.minute();
+            t.tm_hour = ti.hour();
+            t.tm_mday = d.day();
+            t.tm_mon = d.month() - 1;
+            t.tm_year = d.year() - 1900;
+            t.tm_isdst = false;
+            perhapsSetRTC(t);
+
+            isConnected = true; // we seem to have a real GPS (but not necessarily a lock)
+        }
+
+        if (reader.location.isUpdated()) {
+            if (reader.altitude.isValid())
+                altitude = reader.altitude.meters();
+
+            if (reader.location.isValid()) {
+                auto loc = reader.location.value();
+                latitude = toDegInt(loc.lat);
+                longitude = toDegInt(loc.lng);
+            }
+
+            // expect gps pos lat=37.520825, lon=-122.309162, alt=158
+            DEBUG_MSG("new NEMA GPS pos lat=%f, lon=%f, alt=%d\n", latitude * 1e-7, longitude * 1e-7, altitude);
+
+            hasValidLocation = (latitude != 0) || (longitude != 0); // bogus lat lon is reported as 0,0
+            if (hasValidLocation)
+                notifyObservers(NULL);
+        }
+    }
+}

src/gps/NEMAGPS.h

@@ -0,0 +1,21 @@
+#pragma once
+
+#include "GPS.h"
+#include "Observer.h"
+#include "PeriodicTask.h"
+#include "TinyGPS++.h"
+
+/**
+ * A gps class thatreads from a NEMA GPS stream (and FIXME - eventually keeps the gps powered down except when reading)
+ *
+ * When new data is available it will notify observers.
+ */
+class NEMAGPS : public GPS
+{
+    TinyGPSPlus reader;
+    
+    uint32_t lastUpdateMsec = 0;
+
+  public:
+    virtual void loop();
+};

src/gps/UBloxGPS.cpp

@@ -0,0 +1,139 @@
+#include "UBloxGPS.h"
+#include "sleep.h"
+#include <assert.h>
+
+UBloxGPS::UBloxGPS() : PeriodicTask()
+{
+    notifySleepObserver.observe(&notifySleep);
+}
+
+bool UBloxGPS::setup()
+{
+#ifdef GPS_RX_PIN
+    _serial_gps.begin(GPS_BAUDRATE, SERIAL_8N1, GPS_RX_PIN, GPS_TX_PIN);
+#else
+    _serial_gps.begin(GPS_BAUDRATE);
+#endif
+    // _serial_gps.setRxBufferSize(1024); // the default is 256
+    // ublox.enableDebugging(Serial);
+
+    // note: the lib's implementation has the wrong docs for what the return val is
+    // it is not a bool, it returns zero for success
+    isConnected = ublox.begin(_serial_gps);
+
+    // try a second time, the ublox lib serial parsing is buggy?
+    if (!isConnected)
+        isConnected = ublox.begin(_serial_gps);
+
+    if (isConnected) {
+        DEBUG_MSG("Connected to UBLOX GPS successfully\n");
+
+        bool factoryReset = false;
+        bool ok;
+        if (factoryReset) {
+            // It is useful to force back into factory defaults (9600baud, NEMA to test the behavior of boards that don't have
+            // GPS_TX connected)
+            ublox.factoryReset();
+            delay(3000);
+            isConnected = ublox.begin(_serial_gps);
+            DEBUG_MSG("Factory reset success=%d\n", isConnected);
+            ok = ublox.saveConfiguration(3000);
+            assert(ok);
+            return false;
+        } else {
+            ok = ublox.setUART1Output(COM_TYPE_UBX, 500); // Use native API
+            assert(ok);
+            ok = ublox.setNavigationFrequency(1, 500); // Produce 4x/sec to keep the amount of time we stall in getPVT low
+            assert(ok);
+            // ok = ublox.setAutoPVT(false); // Not implemented on NEO-6M
+            // assert(ok);
+            // ok = ublox.setDynamicModel(DYN_MODEL_BIKE); // probably PEDESTRIAN but just in case assume bike speeds
+            // assert(ok);
+            ok = ublox.powerSaveMode(); // use power save mode
+            assert(ok);
+        }
+        ok = ublox.saveConfiguration(3000);
+        assert(ok);
+
+        PeriodicTask::setup(); // We don't start our periodic task unless we actually found the device
+
+        return true;
+    } else {
+        return false;
+    }
+}
+
+/// Prepare the GPS for the cpu entering deep or light sleep, expect to be gone for at least 100s of msecs
+int UBloxGPS::prepareSleep(void *unused)
+{
+    if (isConnected)
+        ublox.powerOff();
+
+    return 0;
+}
+
+void UBloxGPS::doTask()
+{
+    uint8_t fixtype = 3; // If we are only using the RX pin, assume we have a 3d fix
+
+    assert(isConnected);
+
+    // Consume all characters that have arrived
+
+    // getPVT automatically calls checkUblox
+    ublox.checkUblox(); // See if new data is available. Process bytes as they come in.
+
+    // If we don't have a fix (a quick check), don't try waiting for a solution)
+    // Hmmm my fix type reading returns zeros for fix, which doesn't seem correct, because it is still sptting out positions
+    // turn off for now
+    // fixtype = ublox.getFixType();
+    DEBUG_MSG("fix type %d\n", fixtype);
+
+    // DEBUG_MSG("sec %d\n", ublox.getSecond());
+    // DEBUG_MSG("lat %d\n", ublox.getLatitude());
+
+    // any fix that has time
+    if (ublox.getT()) {
+        /* Convert to unix time
+The Unix epoch (or Unix time or POSIX time or Unix timestamp) is the number of seconds that have elapsed since January 1, 1970
+(midnight UTC/GMT), not counting leap seconds (in ISO 8601: 1970-01-01T00:00:00Z).
+*/
+        struct tm t;
+        t.tm_sec = ublox.getSecond();
+        t.tm_min = ublox.getMinute();
+        t.tm_hour = ublox.getHour();
+        t.tm_mday = ublox.getDay();
+        t.tm_mon = ublox.getMonth() - 1;
+        t.tm_year = ublox.getYear() - 1900;
+        t.tm_isdst = false;
+        perhapsSetRTC(t);
+    }
+
+    if ((fixtype >= 3 && fixtype <= 4) && ublox.getP()) // rd fixes only
+    {
+        // we only notify if position has changed
+        latitude = ublox.getLatitude();
+        longitude = ublox.getLongitude();
+        altitude = ublox.getAltitude() / 1000; // in mm convert to meters
+        DEBUG_MSG("new gps pos lat=%f, lon=%f, alt=%d\n", latitude * 1e-7, longitude * 1e-7, altitude);
+
+        hasValidLocation = (latitude != 0) || (longitude != 0); // bogus lat lon is reported as 0,0
+        if (hasValidLocation) {
+            wantNewLocation = false;
+            notifyObservers(NULL);
+            // ublox.powerOff();
+        }
+    } else // we didn't get a location update, go back to sleep and hope the characters show up
+        wantNewLocation = true;
+
+    // Once we have sent a location once we only poll the GPS rarely, otherwise check back every 1s until we have something over
+    // the serial
+    setPeriod(hasValidLocation && !wantNewLocation ? 30 * 1000 : 10 * 1000);
+}
+
+void UBloxGPS::startLock()
+{
+    DEBUG_MSG("Looking for GPS lock\n");
+    wantNewLocation = true;
+    setPeriod(1);
+}

src/gps/UBloxGPS.h

@@ -0,0 +1,41 @@
+#pragma once
+
+#include "GPS.h"
+#include "Observer.h"
+#include "PeriodicTask.h"
+#include "SparkFun_Ublox_Arduino_Library.h"
+
+/**
+ * A gps class that only reads from the GPS periodically (and FIXME - eventually keeps the gps powered down except when reading)
+ *
+ * When new data is available it will notify observers.
+ */
+class UBloxGPS : public GPS, public PeriodicTask
+{
+    SFE_UBLOX_GPS ublox;
+
+    bool wantNewLocation = true;
+
+    CallbackObserver<UBloxGPS, void *> notifySleepObserver = CallbackObserver<UBloxGPS, void *>(this, &UBloxGPS::prepareSleep);
+
+  public:
+    UBloxGPS();
+
+    /**
+     * Returns true if we succeeded
+     */
+    virtual bool setup();
+
+    virtual void doTask();
+
+    /**
+     * Restart our lock attempt - try to get and broadcast a GPS reading ASAP
+     * called after the CPU wakes from light-sleep state */
+    virtual void startLock();
+
+  private:
+
+    /// Prepare the GPS for the cpu entering deep or light sleep, expect to be gone for at least 100s of msecs
+    /// always returns 0 to indicate okay to sleep
+    int prepareSleep(void *unused);
+};

src/main.cpp

@@ -21,13 +21,14 @@
 
 */
 
-#include "GPS.h"
 #include "MeshRadio.h"
 #include "MeshService.h"
+#include "NEMAGPS.h"
 #include "NodeDB.h"
 #include "Periodic.h"
 #include "PowerFSM.h"
 #include "Router.h"
+#include "UBloxGPS.h"
 #include "configuration.h"
 #include "error.h"
 #include "power.h"
@@ -188,8 +189,18 @@ void setup()
 
     screen.print("Started...\n");
 
-    // Init GPS
-    gps.setup();
+    readFromRTC(); // read the main CPU RTC at first (in case we can't get GPS time)
+
+    // Init GPS - first try ublox
+    gps = new UBloxGPS();
+    if (!gps->setup()) {
+        // Some boards might have only the TX line from the GPS connected, in that case, we can't configure it at all.  Just
+        // assume NEMA at 9600 baud.
+        DEBUG_MSG("ERROR: No UBLOX GPS found, hoping that NEMA might work\n");
+        delete gps;
+        gps = new NEMAGPS();
+        gps->setup();
+    }
 
     service.init();
 
@@ -258,6 +269,7 @@ void loop()
 {
     uint32_t msecstosleep = 1000 * 30; // How long can we sleep before we again need to service the main loop?
 
+    gps->loop(); // FIXME, remove from main, instead block on read
     router.loop();
     powerFSM.run_machine();
     service.loop();
@@ -306,7 +318,7 @@ void loop()
     screen.debug()->setChannelNameStatus(channelSettings.name);
     screen.debug()->setPowerStatus(powerStatus);
     // TODO(#4): use something based on hdop to show GPS "signal" strength.
-    screen.debug()->setGPSStatus(gps.hasLock() ? "ok" : ":(");
+    screen.debug()->setGPSStatus(gps->hasLock() ? "good" : "bad");
 
     // No GPS lock yet, let the OS put the main CPU in low power mode for 100ms (or until another interrupt comes in)
     // i.e. don't just keep spinning in loop as fast as we can.

src/mesh/FloodingRouter.cpp

@@ -5,6 +5,8 @@
 /// We clear our old flood record five minute after we see the last of it
 #define FLOOD_EXPIRE_TIME (5 * 60 * 1000L)
 
+static bool supportFlooding = true; // Sometimes to simplify debugging we want jusT simple broadcast only
+
 FloodingRouter::FloodingRouter() : toResend(MAX_NUM_NODES)
 {
     recentBroadcasts.reserve(MAX_NUM_NODES); // Prealloc the worst case # of records - to prevent heap fragmentation
@@ -19,6 +21,7 @@ FloodingRouter::FloodingRouter() : toResend(MAX_NUM_NODES)
 ErrorCode FloodingRouter::send(MeshPacket *p)
 {
     // We update our table of recent broadcasts, even for messages we send
+    if (supportFlooding)
         wasSeenRecently(p);
 
     return Router::send(p);
@@ -30,6 +33,12 @@ uint32_t getRandomDelay()
     return random(200, 10 * 1000L); // between 200ms and 10s
 }
 
+/**
+ * Now that our generalized packet send code has a random delay - I don't think we need to wait here
+ * But I'm leaving this bool until I rip the code out for good.
+ */
+bool needDelay = false;
+
 /**
  * Called from loop()
  * Handle any packet that is received by an interface on this node.
@@ -39,20 +48,30 @@ uint32_t getRandomDelay()
  */
 void FloodingRouter::handleReceived(MeshPacket *p)
 {
+    if (supportFlooding) {
         if (wasSeenRecently(p)) {
             DEBUG_MSG("Ignoring incoming floodmsg, because we've already seen it\n");
             packetPool.release(p);
         } else {
             if (p->to == NODENUM_BROADCAST) {
                 if (p->id != 0) {
+                    MeshPacket *tosend = packetPool.allocCopy(*p); // keep a copy because we will be sending it
+
+                    if (needDelay) {
                         uint32_t delay = getRandomDelay();
 
-                DEBUG_MSG("Rebroadcasting received floodmsg to neighbors in %u msec, fr=0x%x,to=0x%x,id=%d\n", delay, p->from,
-                          p->to, p->id);
+                        DEBUG_MSG("Rebroadcasting received floodmsg to neighbors in %u msec, fr=0x%x,to=0x%x,id=%d\n", delay,
+                                  p->from, p->to, p->id);
 
-                MeshPacket *tosend = packetPool.allocCopy(*p);
                         toResend.enqueue(tosend);
                         setPeriod(delay); // This will work even if we were already waiting a random delay
+                    } else {
+                        DEBUG_MSG("Rebroadcasting received floodmsg to neighbors, fr=0x%x,to=0x%x,id=%d\n", p->from, p->to,
+                                  p->id);
+                        // Note: we are careful to resend using the original senders node id
+                        // We are careful not to call our hooked version of send() - because we don't want to check this again
+                        Router::send(tosend);
+                    }
                 } else {
                     DEBUG_MSG("Ignoring a simple (0 hop) broadcast\n");
                 }
@@ -61,6 +80,8 @@ void FloodingRouter::handleReceived(MeshPacket *p)
             // handle the packet as normal
             Router::handleReceived(p);
         }
+    } else
+        Router::handleReceived(p);
 }
 
 void FloodingRouter::doTask()

src/mesh/MeshService.cpp

@@ -84,7 +84,7 @@ void MeshService::init()
     sendOwnerPeriod.setup();
     nodeDB.init();
 
-    gpsObserver.observe(&gps);
+    gpsObserver.observe(gps);
     packetReceivedObserver.observe(&router.notifyPacketReceived);
 }
 
@@ -153,7 +153,7 @@ void MeshService::handleIncomingPosition(const MeshPacket *mp)
             tv.tv_sec = secs;
             tv.tv_usec = 0;
 
-            gps.perhapsSetRTC(&tv);
+            perhapsSetRTC(&tv);
         }
     } else {
         DEBUG_MSG("Ignoring incoming packet - not a position\n");
@@ -165,7 +165,7 @@ int MeshService::handleFromRadio(const MeshPacket *mp)
     powerFSM.trigger(EVENT_RECEIVED_PACKET); // Possibly keep the node from sleeping
 
     // If it is a position packet, perhaps set our clock (if we don't have a GPS of our own, otherwise wait for that to work)
-    if (!gps.isConnected)
+    if (!gps->isConnected)
         handleIncomingPosition(mp);
     else {
         DEBUG_MSG("Ignoring incoming time, because we have a GPS\n");
@@ -234,7 +234,7 @@ void MeshService::handleToRadio(MeshPacket &p)
     if (p.id == 0)
         p.id = generatePacketId(); // If the phone didn't supply one, then pick one
 
-    p.rx_time = gps.getValidTime(); // Record the time the packet arrived from the phone
+    p.rx_time = getValidTime(); // Record the time the packet arrived from the phone
                                 // (so we update our nodedb for the local node)
 
     // Send the packet into the mesh
@@ -258,7 +258,7 @@ void MeshService::sendToMesh(MeshPacket *p)
     // nodes shouldn't trust it anyways) Note: for now, we allow a device with a local GPS to include the time, so that gpsless
     // devices can get time.
     if (p->has_payload && p->payload.has_position) {
-        if (!gps.isConnected) {
+        if (!gps->isConnected) {
             DEBUG_MSG("Stripping time %u from position send\n", p->payload.position.time);
             p->payload.position.time = 0;
         } else
@@ -266,9 +266,10 @@ void MeshService::sendToMesh(MeshPacket *p)
     }
 
     // If the phone sent a packet just to us, don't send it out into the network
-    if (p->to == nodeDB.getNodeNum())
+    if (p->to == nodeDB.getNodeNum()) {
         DEBUG_MSG("Dropping locally processed message\n");
-    else {
+        releaseToPool(p);
+    } else {
         // Note: We might return !OK if our fifo was full, at that point the only option we have is to drop it
         if (router.send(p) != ERRNO_OK) {
             DEBUG_MSG("No radio was able to send packet, discarding...\n");
@@ -285,7 +286,7 @@ MeshPacket *MeshService::allocForSending()
     p->from = nodeDB.getNodeNum();
     p->to = NODENUM_BROADCAST;
     p->id = generatePacketId();
-    p->rx_time = gps.getValidTime(); // Just in case we process the packet locally - make sure it has a valid timestamp
+    p->rx_time = getValidTime(); // Just in case we process the packet locally - make sure it has a valid timestamp
 
     return p;
 }
@@ -314,7 +315,7 @@ void MeshService::sendOurPosition(NodeNum dest, bool wantReplies)
     p->payload.has_position = true;
     p->payload.position = node->position;
     p->payload.want_response = wantReplies;
-    p->payload.position.time = gps.getValidTime(); // This nodedb timestamp might be stale, so update it if our clock is valid.
+    p->payload.position.time = getValidTime(); // This nodedb timestamp might be stale, so update it if our clock is valid.
     sendToMesh(p);
 }
 
@@ -328,12 +329,12 @@ int MeshService::onGPSChanged(void *unused)
 
     Position &pos = p->payload.position;
     // !zero or !zero lat/long means valid
-    if (gps.latitude != 0 || gps.longitude != 0) {
-        if (gps.altitude != 0)
-            pos.altitude = gps.altitude;
-        pos.latitude = gps.latitude;
-        pos.longitude = gps.longitude;
-        pos.time = gps.getValidTime();
+    if (gps->latitude != 0 || gps->longitude != 0) {
+        if (gps->altitude != 0)
+            pos.altitude = gps->altitude;
+        pos.latitude_i = gps->latitude;
+        pos.longitude_i = gps->longitude;
+        pos.time = getValidTime();
     }
 
     // We limit our GPS broadcasts to a max rate

src/mesh/NodeDB.cpp

@@ -246,7 +246,7 @@ const NodeInfo *NodeDB::readNextInfo()
 /// Given a node, return how many seconds in the past (vs now) that we last heard from it
 uint32_t sinceLastSeen(const NodeInfo *n)
 {
-    uint32_t now = gps.getTime();
+    uint32_t now = getTime();
 
     uint32_t last_seen = n->position.time;
     int delta = (int)(now - last_seen);
@@ -303,7 +303,8 @@ void NodeDB::updateFrom(const MeshPacket &mp)
         if (p.has_data) {
             // Keep a copy of the most recent text message.
             if (p.data.typ == Data_Type_CLEAR_TEXT) {
-                DEBUG_MSG("Received text msg from=0%0x, msg=%.*s\n", mp.from, p.data.payload.size, p.data.payload.bytes);
+                DEBUG_MSG("Received text msg from=0x%0x, id=%d, msg=%.*s\n", mp.from, mp.id, p.data.payload.size,
+                          p.data.payload.bytes);
                 if (mp.to == NODENUM_BROADCAST || mp.to == nodeDB.getNodeNum()) {
                     // We only store/display messages destined for us.
                     devicestate.rx_text_message = mp;

src/mesh/PhoneAPI.h

@@ -88,7 +88,7 @@ class PhoneAPI
     /**
      * Subclasses can use this as a hook to provide custom notifications for their transport (i.e. bluetooth notifies)
      */
-    void onNowHasData(uint32_t fromRadioNum) {}
+    virtual void onNowHasData(uint32_t fromRadioNum) {}
 
   private:
     /**

src/mesh/RF95Interface.cpp

@@ -3,6 +3,9 @@
 #include "RadioLibRF95.h"
 #include <configuration.h>
 
+#define MAX_POWER 17
+// if we use 20 we are limited to 1% duty cycle or hw might overheat.  For continuous operation set a limit of 17
+
 RF95Interface::RF95Interface(RADIOLIB_PIN_TYPE cs, RADIOLIB_PIN_TYPE irq, RADIOLIB_PIN_TYPE rst, SPIClass &spi)
     : RadioLibInterface(cs, irq, rst, 0, spi)
 {
@@ -17,8 +20,8 @@ bool RF95Interface::init()
     RadioLibInterface::init();
 
     applyModemConfig();
-    if (power > 20) // This chip has lower power limits than some
-        power = 20;
+    if (power > MAX_POWER) // This chip has lower power limits than some
+        power = MAX_POWER;
 
     iface = lora = new RadioLibRF95(&module);
     int res = lora->begin(freq, bw, sf, cr, syncWord, power, currentLimit, preambleLength);
@@ -67,8 +70,8 @@ bool RF95Interface::reconfigure()
     err = lora->setFrequency(freq);
     assert(err == ERR_NONE);
 
-    if (power > 20) // This chip has lower power limits than some
-        power = 20;
+    if (power > MAX_POWER) // This chip has lower power limits than some
+        power = MAX_POWER;
     err = lora->setOutputPower(power);
     assert(err == ERR_NONE);
 

src/mesh/RF95Interface.h

@@ -50,6 +50,6 @@ class RF95Interface : public RadioLibInterface
      * Add SNR data to received messages
      */
     virtual void addReceiveMetadata(MeshPacket *mp);
-  private:
-    void setStandby();
+
+    virtual void setStandby();
 };

src/mesh/RadioInterface.cpp

@@ -17,7 +17,8 @@ RadioInterface::RadioInterface() : txQueue(MAX_TX_QUEUE)
 
 bool RadioInterface::init()
 {
-    start("radio", RADIO_STACK_SIZE); // Start our worker thread
+    // we want this thread to run at very high priority, because it is effectively running as a user space ISR 
+    start("radio", RADIO_STACK_SIZE, configMAX_PRIORITIES - 1); // Start our worker thread
 
     return true;
 }

src/mesh/RadioLibInterface.cpp

@@ -1,5 +1,6 @@
 #include "RadioLibInterface.h"
 #include "MeshTypes.h"
+#include "OSTimer.h"
 #include "mesh-pb-constants.h"
 #include <NodeDB.h> // FIXME, this class shouldn't need to look into nodedb
 #include <configuration.h>
@@ -24,13 +25,13 @@ RadioLibInterface::RadioLibInterface(RADIOLIB_PIN_TYPE cs, RADIOLIB_PIN_TYPE irq
 #define YIELD_FROM_ISR(x) portYIELD_FROM_ISR(x)
 #endif
 
-void INTERRUPT_ATTR RadioLibInterface::isrRxLevel0()
+void INTERRUPT_ATTR RadioLibInterface::isrLevel0Common(PendingISR cause)
 {
     instance->disableInterrupt();
 
-    instance->pending = ISR_RX;
+    instance->pending = cause;
     BaseType_t xHigherPriorityTaskWoken;
-    instance->notifyFromISR(&xHigherPriorityTaskWoken);
+    instance->notifyFromISR(&xHigherPriorityTaskWoken, cause, eSetValueWithOverwrite);
 
     /* Force a context switch if xHigherPriorityTaskWoken is now set to pdTRUE.
     The macro used to do this is dependent on the port and may be called
@@ -38,18 +39,14 @@ void INTERRUPT_ATTR RadioLibInterface::isrRxLevel0()
     YIELD_FROM_ISR(xHigherPriorityTaskWoken);
 }
 
+void INTERRUPT_ATTR RadioLibInterface::isrRxLevel0()
+{
+    isrLevel0Common(ISR_RX);
+}
+
 void INTERRUPT_ATTR RadioLibInterface::isrTxLevel0()
 {
-    instance->disableInterrupt();
-
-    instance->pending = ISR_TX;
-    BaseType_t xHigherPriorityTaskWoken;
-    instance->notifyFromISR(&xHigherPriorityTaskWoken);
-
-    /* Force a context switch if xHigherPriorityTaskWoken is now set to pdTRUE.
-    The macro used to do this is dependent on the port and may be called
-    portEND_SWITCHING_ISR. */
-    YIELD_FROM_ISR(xHigherPriorityTaskWoken);
+    isrLevel0Common(ISR_TX);
 }
 
 /** Our ISR code currently needs this to find our active instance
@@ -93,14 +90,17 @@ bool RadioLibInterface::canSendImmediately()
     // We wait _if_ we are partially though receiving a packet (rather than just merely waiting for one).
     // To do otherwise would be doubly bad because not only would we drop the packet that was on the way in,
     // we almost certainly guarantee no one outside will like the packet we are sending.
-    PendingISR isPending = pending;
     bool busyTx = sendingPacket != NULL;
     bool busyRx = isReceiving && isActivelyReceiving();
 
-    if (busyTx || busyRx || isPending)
-        DEBUG_MSG("Can not send yet, busyTx=%d, busyRx=%d, intPend=%d\n", busyTx, busyRx, isPending);
-
-    return !busyTx && !busyRx && !isPending;
+    if (busyTx || busyRx) {
+        if (busyTx)
+            DEBUG_MSG("Can not send yet, busyTx\n");
+        if (busyRx)
+            DEBUG_MSG("Can not send yet, busyRx\n");
+        return false;
+    } else
+        return true;
 }
 
 /// Send a packet (possibly by enquing in a private fifo).  This routine will
@@ -108,25 +108,20 @@ bool RadioLibInterface::canSendImmediately()
 /// bluetooth comms code.  If the txmit queue is empty it might return an error
 ErrorCode RadioLibInterface::send(MeshPacket *p)
 {
-    // We wait _if_ we are partially though receiving a packet (rather than just merely waiting for one).
-    // To do otherwise would be doubly bad because not only would we drop the packet that was on the way in,
-    // we almost certainly guarantee no one outside will like the packet we are sending.
-    if (canSendImmediately()) {
-        // if the radio is idle, we can send right away
-        DEBUG_MSG("immediate send on mesh fr=0x%x,to=0x%x,id=%d\n (txGood=%d,rxGood=%d,rxBad=%d)\n", p->from, p->to, p->id,
-                  txGood, rxGood, rxBad);
-
-        startSend(p);
-        return ERRNO_OK;
-    } else {
-        DEBUG_MSG("enqueuing packet for send from=0x%x, to=0x%x\n", p->from, p->to);
+    DEBUG_MSG("enqueuing for send on mesh fr=0x%x,to=0x%x,id=%d (txGood=%d,rxGood=%d,rxBad=%d)\n", p->from, p->to, p->id, txGood,
+              rxGood, rxBad);
     ErrorCode res = txQueue.enqueue(p, 0) ? ERRNO_OK : ERRNO_UNKNOWN;
 
-        if (res != ERRNO_OK) // we weren't able to queue it, so we must drop it to prevent leaks
+    if (res != ERRNO_OK) { // we weren't able to queue it, so we must drop it to prevent leaks
         packetPool.release(p);
-
         return res;
     }
+
+    // We want all sending/receiving to be done by our daemon thread, We use a delay here because this packet might have been sent
+    // in response to a packet we just received.  So we want to make sure the other side has had a chance to reconfigure its radio
+    startTransmitTimer(true);
+
+    return res;
 }
 
 bool RadioLibInterface::canSleep()
@@ -138,30 +133,105 @@ bool RadioLibInterface::canSleep()
     return res;
 }
 
+/** At the low end we want to pick a delay large enough that anyone who just completed sending (some other node)
+ * has had enough time to switch their radio back into receive mode.
+ */
+#define MIN_TX_WAIT_MSEC 100
+
+/**
+ * At the high end, this value is used to spread node attempts across time so when they are replying to a packet
+ * they don't both check that the airwaves are clear at the same moment.  As long as they are off by some amount
+ * one of the two will be first to start transmitting and the other will see that.  I bet 500ms is more than enough
+ * to guarantee this.
+ */
+#define MAX_TX_WAIT_MSEC 2000 // stress test would still fail occasionally with 1000
+
+/** radio helper thread callback.
+
+We never immediately transmit after any operation (either rx or tx).  Instead we should start receiving and
+wait a random delay of 50 to 200 ms to make sure we are not stomping on someone else.  The 50ms delay at the beginning ensures all
+possible listeners have had time to finish processing the previous packet and now have their radio in RX state.  The up to 200ms
+random delay gives a chance for all possible senders to have high odds of detecting that someone else started transmitting first
+and then they will wait until that packet finishes.
+
+NOTE: the large flood rebroadcast delay might still be needed even with this approach.  Because we might not be able to hear other
+transmitters that we are potentially stomping on.  Requires further thought.
+
+FIXME, the MIN_TX_WAIT_MSEC and MAX_TX_WAIT_MSEC values should be tuned via logic analyzer later.
+*/
 void RadioLibInterface::loop()
 {
-    PendingISR wasPending = pending;
     pending = ISR_NONE;
 
-    if (wasPending == ISR_TX)
+    switch (notification) {
+    case ISR_TX:
         handleTransmitInterrupt();
-    else if (wasPending == ISR_RX)
+        startReceive();
+        startTransmitTimer();
+        break;
+    case ISR_RX:
         handleReceiveInterrupt();
-    else
+        startReceive();
+        startTransmitTimer();
+        break;
+    case TRANSMIT_DELAY_COMPLETED:
+        // If we are not currently in receive mode, then restart the timer and try again later (this can happen if the main thread
+        // has placed the unit into standby)  FIXME, how will this work if the chipset is in sleep mode?
+        if (!txQueue.isEmpty()) {
+            if (!canSendImmediately()) {
+                startTransmitTimer(); // try again in a little while
+            } else {
+                // Send any outgoing packets we have ready
+                MeshPacket *txp = txQueue.dequeuePtr(0);
+                assert(txp);
+                startSend(txp);
+            }
+        }
+        break;
+    default:
         assert(0); // We expected to receive a valid notification from the ISR
-
-    startNextWork();
+    }
 }
 
-void RadioLibInterface::startNextWork()
+#ifndef NO_ESP32
+#define USE_HW_TIMER
+#endif
+
+void IRAM_ATTR RadioLibInterface::timerCallback(void *p1, uint32_t p2)
 {
-    // First send any outgoing packets we have ready
-    MeshPacket *txp = txQueue.dequeuePtr(0);
-    if (txp)
-        startSend(txp);
-    else {
-        // Nothing to send, let's switch back to receive mode
-        startReceive();
+    RadioLibInterface *t = (RadioLibInterface *)p1;
+
+    t->timerRunning = false;
+
+    // We use without overwrite, so that if there is already an interrupt pending to be handled, that gets handle properly (the
+    // ISR handler will restart our timer)
+#ifndef USE_HW_TIMER
+    t->notify(TRANSMIT_DELAY_COMPLETED, eSetValueWithoutOverwrite);
+#else
+    BaseType_t xHigherPriorityTaskWoken;
+    instance->notifyFromISR(&xHigherPriorityTaskWoken, TRANSMIT_DELAY_COMPLETED, eSetValueWithoutOverwrite);
+
+    /* Force a context switch if xHigherPriorityTaskWoken is now set to pdTRUE.
+    The macro used to do this is dependent on the port and may be called
+    portEND_SWITCHING_ISR. */
+    YIELD_FROM_ISR(xHigherPriorityTaskWoken);
+#endif
+}
+
+void RadioLibInterface::startTransmitTimer(bool withDelay)
+{
+    // If we have work to do and the timer wasn't already scheduled, schedule it now
+    if (!timerRunning && !txQueue.isEmpty()) {
+        timerRunning = true;
+        uint32_t delay =
+            !withDelay ? 0 : random(MIN_TX_WAIT_MSEC, MAX_TX_WAIT_MSEC); // See documentation for loop() wrt these values
+                                                                         // DEBUG_MSG("xmit timer %d\n", delay);
+#ifdef USE_HW_TIMER
+        bool okay = scheduleHWCallback(timerCallback, this, 0, delay);
+#else
+        bool okay = scheduleOSCallback(timerCallback, this, 0, delay);
+#endif
+        assert(okay);
     }
 }
 
@@ -211,6 +281,7 @@ void RadioLibInterface::handleReceiveInterrupt()
             const PacketHeader *h = (PacketHeader *)radiobuf;
             uint8_t ourAddr = nodeDB.getNodeNum();
 
+            rxGood++;
             if (h->to != 255 && h->to != ourAddr) {
                 DEBUG_MSG("ignoring packet not sent to us\n");
             } else {
@@ -230,7 +301,6 @@ void RadioLibInterface::handleReceiveInterrupt()
                 } else {
                     // parsing was successful, queue for our recipient
                     mp->has_payload = true;
-                    rxGood++;
                     DEBUG_MSG("Lora RX interrupt from=0x%x, id=%u\n", mp->from, mp->id);
 
                     deliverToReceiver(mp);
@@ -243,6 +313,9 @@ void RadioLibInterface::handleReceiveInterrupt()
 /** start an immediate transmit */
 void RadioLibInterface::startSend(MeshPacket *txp)
 {
+    DEBUG_MSG("Starting low level send from=0x%x, id=%u!\n", txp->from, txp->id);
+    setStandby(); // Cancel any already in process receives
+
     size_t numbytes = beginSending(txp);
 
     int res = iface->startTransmit(radiobuf, numbytes);

src/mesh/RadioLibInterface.h

@@ -14,9 +14,10 @@
 class RadioLibInterface : public RadioInterface
 {
     /// Used as our notification from the ISR
-    enum PendingISR { ISR_NONE = 0, ISR_RX, ISR_TX };
+    enum PendingISR { ISR_NONE = 0, ISR_RX, ISR_TX, TRANSMIT_DELAY_COMPLETED };
 
     volatile PendingISR pending = ISR_NONE;
+    volatile bool timerRunning = false;
 
     /** Our ISR code currently needs this to find our active instance
      */
@@ -25,7 +26,7 @@ class RadioLibInterface : public RadioInterface
     /**
      * Raw ISR handler that just calls our polymorphic method
      */
-    static void isrTxLevel0();
+    static void isrTxLevel0(), isrLevel0Common(PendingISR code);
 
     /**
      * Debugging counts
@@ -44,7 +45,7 @@ class RadioLibInterface : public RadioInterface
     uint8_t syncWord = SX126X_SYNC_WORD_PRIVATE;
 
     float currentLimit = 100;     // FIXME
-    uint16_t preambleLength = 8; // 8 is default, but FIXME use longer to increase the amount of sleep time when receiving
+    uint16_t preambleLength = 32; // 8 is default, but FIXME use longer to increase the amount of sleep time when receiving
 
     Module module; // The HW interface to the radio
 
@@ -83,12 +84,18 @@ class RadioLibInterface : public RadioInterface
     /** start an immediate transmit */
     void startSend(MeshPacket *txp);
 
-    /** start a queued transmit (if we have one), else start receiving */
-    void startNextWork();
+    /** if we have something waiting to send, start a short random timer so we can come check for collision before actually doing
+     * the transmit
+     *
+     * If the timer was already running, we just wait for that one to occur.
+     * */
+    void startTransmitTimer(bool withDelay = true);
 
     void handleTransmitInterrupt();
     void handleReceiveInterrupt();
 
+    static void timerCallback(void *p1, uint32_t p2);
+
   protected:
     /**
      * Convert our modemConfig enum into wf, sf, etc...
@@ -96,7 +103,7 @@ class RadioLibInterface : public RadioInterface
     void applyModemConfig();
 
     /** Could we send right now (i.e. either not actively receiving or transmitting)? */
-    bool canSendImmediately();
+    virtual bool canSendImmediately();
 
     /** are we actively receiving a packet (only called during receiving state) */
     virtual bool isActivelyReceiving() = 0;
@@ -121,4 +128,6 @@ class RadioLibInterface : public RadioInterface
     virtual void addReceiveMetadata(MeshPacket *mp) = 0;
 
     virtual void loop(); // Idle processing
+
+    virtual void setStandby() = 0;
 };

src/mesh/RadioLibRF95.cpp

@@ -56,8 +56,13 @@ int16_t RadioLibRF95::setFrequency(float freq)
 bool RadioLibRF95::isReceiving()
 {
     // 0x0b == Look for header info valid, signal synchronized or signal detected
-    uint8_t reg = _mod->SPIreadRegister(SX127X_REG_MODEM_STAT) & 0x1f;
+    uint8_t reg = readReg(SX127X_REG_MODEM_STAT);
     // Serial.printf("reg %x\n", reg);
     return (reg & (RH_RF95_MODEM_STATUS_SIGNAL_DETECTED | RH_RF95_MODEM_STATUS_SIGNAL_SYNCHRONIZED |
                    RH_RF95_MODEM_STATUS_HEADER_INFO_VALID)) != 0;
 }
+
+uint8_t RadioLibRF95::readReg(uint8_t addr)
+{
+    return _mod->SPIreadRegister(addr);
+}

src/mesh/RadioLibRF95.h

@@ -62,6 +62,9 @@ class RadioLibRF95: public SX1278 {
     // Return true if we are actively receiving a message currently
     bool isReceiving();
 
+    /// For debugging
+    uint8_t readReg(uint8_t addr); 
+
 #ifndef RADIOLIB_GODMODE
   private:
 #endif

src/mesh/Router.cpp

@@ -44,15 +44,16 @@ void Router::loop()
 /**
  * Send a packet on a suitable interface.  This routine will
  * later free() the packet to pool.  This routine is not allowed to stall.
- * If the txmit queue is full it might return an error
+ * If the txmit queue is full it might return an error.  
  */
 ErrorCode Router::send(MeshPacket *p)
 {
     if (iface) {
-        DEBUG_MSG("Sending packet via interface fr=0x%x,to=0x%x,id=%d\n", p->from, p->to, p->id);
+        // DEBUG_MSG("Sending packet via interface fr=0x%x,to=0x%x,id=%d\n", p->from, p->to, p->id);
         return iface->send(p);
     } else {
         DEBUG_MSG("Dropping packet - no interfaces - fr=0x%x,to=0x%x,id=%d\n", p->from, p->to, p->id);
+        packetPool.release(p);
         return ERRNO_NO_INTERFACES;
     }
 }
@@ -67,7 +68,7 @@ void Router::handleReceived(MeshPacket *p)
 {
     // FIXME, this class shouldn't EVER need to know about the GPS, move getValidTime() into a non gps dependent function
     // Also, we should set the time from the ISR and it should have msec level resolution
-    p->rx_time = gps.getValidTime(); // store the arrival timestamp for the phone
+    p->rx_time = getValidTime(); // store the arrival timestamp for the phone
 
     DEBUG_MSG("Notifying observers of received packet fr=0x%x,to=0x%x,id=%d\n", p->from, p->to, p->id);
     notifyPacketReceived.notifyObservers(p);

src/mesh/SX1262Interface.h

@@ -48,6 +48,8 @@ class SX1262Interface : public RadioLibInterface
      */
     virtual void addReceiveMetadata(MeshPacket *mp);
 
+    virtual void setStandby();
+
   private:
-    void setStandby();
+
 };

src/mesh/mesh.pb.c

@@ -55,11 +55,3 @@ PB_BIND(ToRadio, ToRadio, 2)
 
 
 
-#ifndef PB_CONVERT_DOUBLE_FLOAT
-/* On some platforms (such as AVR), double is really float.
- * To be able to encode/decode double on these platforms, you need.
- * to define PB_CONVERT_DOUBLE_FLOAT in pb.h or compiler command line.
- */
-PB_STATIC_ASSERT(sizeof(double) == 8, DOUBLE_MUST_BE_8_BYTES)
-#endif
-

src/mesh/mesh.pb.h

@@ -66,11 +66,11 @@ typedef struct _MyNodeInfo {
 } MyNodeInfo;
 
 typedef struct _Position {
-    double latitude;
-    double longitude;
     int32_t altitude;
     int32_t battery_level;
     uint32_t time;
+    int32_t latitude_i;
+    int32_t longitude_i;
 } Position;
 
 typedef struct _RadioConfig_UserPreferences {
@@ -237,8 +237,8 @@ typedef struct _ToRadio {
 #define MyNodeInfo_error_code_tag                7
 #define MyNodeInfo_error_address_tag             8
 #define MyNodeInfo_error_count_tag               9
-#define Position_latitude_tag                    1
-#define Position_longitude_tag                   2
+#define Position_latitude_i_tag                  7
+#define Position_longitude_i_tag                 8
 #define Position_altitude_tag                    3
 #define Position_battery_level_tag               4
 #define Position_time_tag                        6
@@ -297,11 +297,11 @@ typedef struct _ToRadio {
 
 /* Struct field encoding specification for nanopb */
 #define Position_FIELDLIST(X, a) \
-X(a, STATIC,   SINGULAR, DOUBLE,   latitude,          1) \
-X(a, STATIC,   SINGULAR, DOUBLE,   longitude,         2) \
 X(a, STATIC,   SINGULAR, INT32,    altitude,          3) \
 X(a, STATIC,   SINGULAR, INT32,    battery_level,     4) \
-X(a, STATIC,   SINGULAR, UINT32,   time,              6)
+X(a, STATIC,   SINGULAR, UINT32,   time,              6) \
+X(a, STATIC,   SINGULAR, SINT32,   latitude_i,        7) \
+X(a, STATIC,   SINGULAR, SINT32,   longitude_i,       8)
 #define Position_CALLBACK NULL
 #define Position_DEFAULT NULL
 
@@ -486,21 +486,21 @@ extern const pb_msgdesc_t ToRadio_msg;
 #define ToRadio_fields &ToRadio_msg
 
 /* Maximum encoded size of messages (where known) */
-#define Position_size                            46
+#define Position_size                            40
 #define Data_size                                256
 #define User_size                                72
 /* RouteDiscovery_size depends on runtime parameters */
-#define SubPacket_size                           383
-#define MeshPacket_size                          425
+#define SubPacket_size                           377
+#define MeshPacket_size                          419
 #define ChannelSettings_size                     44
 #define RadioConfig_size                         120
 #define RadioConfig_UserPreferences_size         72
-#define NodeInfo_size                            138
+#define NodeInfo_size                            132
 #define MyNodeInfo_size                          85
-#define DeviceState_size                         18925
+#define DeviceState_size                         18535
 #define DebugString_size                         258
-#define FromRadio_size                           434
-#define ToRadio_size                             428
+#define FromRadio_size                           428
+#define ToRadio_size                             422
 
 #ifdef __cplusplus
 } /* extern "C" */

src/screen.cpp

@@ -280,7 +280,7 @@ static float estimatedHeading(double lat, double lon)
 /// valid lat/lon
 static bool hasPosition(NodeInfo *n)
 {
-    return n->has_position && (n->position.latitude != 0 || n->position.longitude != 0);
+    return n->has_position && (n->position.latitude_i != 0 || n->position.longitude_i != 0);
 }
 
 /// We will skip one node - the one for us, so we just blindly loop over all
@@ -288,6 +288,9 @@ static bool hasPosition(NodeInfo *n)
 static size_t nodeIndex;
 static int8_t prevFrame = -1;
 
+/// Convert an integer GPS coords to a floating point
+#define DegD(i) (i * 1e-7)
+
 static void drawNodeInfo(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, int16_t y)
 {
     // We only advance our nodeIndex if the frame # has changed - because
@@ -334,7 +337,7 @@ static void drawNodeInfo(OLEDDisplay *display, OLEDDisplayUiState *state, int16_
     NodeInfo *ourNode = nodeDB.getNode(nodeDB.getNodeNum());
     if (ourNode && hasPosition(ourNode) && hasPosition(node)) {
         Position &op = ourNode->position, &p = node->position;
-        float d = latLongToMeter(p.latitude, p.longitude, op.latitude, op.longitude);
+        float d = latLongToMeter(DegD(p.latitude_i), DegD(p.longitude_i), DegD(op.latitude_i), DegD(op.longitude_i));
         if (d < 2000)
             snprintf(distStr, sizeof(distStr), "%.0f m", d);
         else
@@ -342,8 +345,8 @@ static void drawNodeInfo(OLEDDisplay *display, OLEDDisplayUiState *state, int16_
 
         // FIXME, also keep the guess at the operators heading and add/substract
         // it.  currently we don't do this and instead draw north up only.
-        float bearingToOther = bearing(p.latitude, p.longitude, op.latitude, op.longitude);
-        float myHeading = estimatedHeading(p.latitude, p.longitude);
+        float bearingToOther = bearing(DegD(p.latitude_i), DegD(p.longitude_i), DegD(op.latitude_i), DegD(op.longitude_i));
+        float myHeading = estimatedHeading(DegD(p.latitude_i), DegD(p.longitude_i));
         headingRadian = bearingToOther - myHeading;
     } else {
         // Debug info for gps lock errors

src/sleep.cpp

@@ -29,6 +29,7 @@ extern AXP20X_Class axp;
 Observable<void *> preflightSleep;
 
 /// Called to tell observers we are now entering sleep and you should prepare.  Must return 0
+/// notifySleep will be called for light or deep sleep, notifyDeepSleep is only called for deep sleep
 Observable<void *> notifySleep, notifyDeepSleep;
 
 // deep sleep support
@@ -125,12 +126,6 @@ static bool doPreflightSleep()
 /// Tell devices we are going to sleep and wait for them to handle things
 static void waitEnterSleep()
 {
-    /*
-    former hardwired code - now moved into notifySleep callbacks:
-    // Put radio in sleep mode (will still draw power but only 0.2uA)
-    service.radio.radioIf.sleep();
-    */
-
     uint32_t now = millis();
     while (!doPreflightSleep()) {
         delay(100); // Kinda yucky - wait until radio says say we can shutdown (finished in process sends/receives)
@@ -144,7 +139,6 @@ static void waitEnterSleep()
     // Code that still needs to be moved into notifyObservers
     Serial.flush();            // send all our characters before we stop cpu clock
     setBluetoothEnable(false); // has to be off before calling light sleep
-    gps.prepareSleep();        // abandon in-process parsing
 
     notifySleep.notifyObservers(NULL);
 }
@@ -157,6 +151,7 @@ void doDeepSleep(uint64_t msecToWake)
     // not using wifi yet, but once we are this is needed to shutoff the radio hw
     // esp_wifi_stop();
     waitEnterSleep();
+    notifySleep.notifyObservers(NULL); // also tell the regular sleep handlers
     notifyDeepSleep.notifyObservers(NULL);
 
     screen.setOn(false); // datasheet says this will draw only 10ua