Merge pull request #44 from geeksville/master

various minor commits based on bugs I see while testing app

bin/version.sh

@@ -1,3 +1,3 @@
 
 
-export VERSION=0.1.7
+export VERSION=0.1.8

lib/BluetoothOTA/src/BluetoothUtil.cpp

@@ -5,7 +5,6 @@
 #include <Arduino.h>
 #include <Update.h>
 #include "configuration.h"
-#include "screen.h"
 
 SimpleAllocator btPool;
 
@@ -173,7 +172,7 @@ uint32_t getValue32(BLECharacteristic *c, uint32_t defaultValue)
 
 class MySecurity : public BLESecurityCallbacks
 {
-
+ protected:
   bool onConfirmPIN(uint32_t pin)
   {
     Serial.printf("onConfirmPIN %u\n", pin);
@@ -189,7 +188,7 @@ class MySecurity : public BLESecurityCallbacks
   void onPassKeyNotify(uint32_t pass_key)
   {
     Serial.printf("onPassKeyNotify %u\n", pass_key);
-    screen_start_bluetooth(pass_key);
+    startCb(pass_key);
   }
 
   bool onSecurityRequest()
@@ -211,9 +210,13 @@ class MySecurity : public BLESecurityCallbacks
       Serial.printf("onAuthenticationComplete -> fail %d\n", cmpl.fail_reason);
     }
 
-    // Remove our custom screen
-    screen.setFrames();
+    // Remove our custom PIN request screen.
+    stopCb();
   }
+
+ public:
+  StartBluetoothPinScreenCallback startCb;
+  StopBluetoothPinScreenCallback stopCb;
 };
 
 BLEServer *pServer;
@@ -255,7 +258,10 @@ void deinitBLE()
   btPool.reset();
 }
 
-BLEServer *initBLE(std::string deviceName, std::string hwVendor, std::string swVersion, std::string hwVersion)
+BLEServer *initBLE(
+    StartBluetoothPinScreenCallback startBtPinScreen,
+    StopBluetoothPinScreenCallback stopBtPinScreen,
+    std::string deviceName, std::string hwVendor, std::string swVersion, std::string hwVersion)
 {
   BLEDevice::init(deviceName);
   BLEDevice::setEncryptionLevel(ESP_BLE_SEC_ENCRYPT);
@@ -264,6 +270,8 @@ BLEServer *initBLE(std::string deviceName, std::string hwVendor, std::string swV
    * Required in authentication process to provide displaying and/or input passkey or yes/no butttons confirmation
    */
   static MySecurity mySecurity;
+  mySecurity.startCb = startBtPinScreen;
+  mySecurity.stopCb = stopBtPinScreen;
   BLEDevice::setSecurityCallbacks(&mySecurity);
 
   // Create the BLE Server

lib/BluetoothOTA/src/BluetoothUtil.h

@@ -1,5 +1,7 @@
 #pragma once
 
+#include <functional>
+
 #include <Arduino.h>
 #include <BLEDevice.h>
 #include <BLEServer.h>
@@ -17,8 +19,14 @@ void dumpCharacteristic(BLECharacteristic *c);
 /** converting endianness pull out a 32 bit value */
 uint32_t getValue32(BLECharacteristic *c, uint32_t defaultValue);
 
+// TODO(girts): create a class for the bluetooth utils helpers?
+using StartBluetoothPinScreenCallback = std::function<void(uint32_t pass_key)>;
+using StopBluetoothPinScreenCallback = std::function<void(void)>;
+
 void loopBLE();
-BLEServer *initBLE(std::string devName, std::string hwVendor, std::string swVersion, std::string hwVersion = "");
+BLEServer *initBLE(
+    StartBluetoothPinScreenCallback startBtPinScreen, StopBluetoothPinScreenCallback stopBtPinScreen,
+    std::string devName, std::string hwVendor, std::string swVersion, std::string hwVersion = "");
 void deinitBLE();
 
 /// Add a characteristic that we will delete when we restart

release/latest/.gitignore

@@ -0,0 +1 @@
+curfirmwareversion.xml

release/latest/curfirmwareversion.xml

@@ -1,9 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-
-<!-- This file is kept in source control because it reflects the last stable
-release.  It is used by the android app for forcing software updates.  Do not edit.
-Generated by bin/buildall.sh -->
-
-<resources>
-    <string name="cur_firmware_version">0.1.7</string>
-</resources>

src/GPS.cpp

@@ -1,25 +1,25 @@
 
 #include "GPS.h"
+#include "configuration.h"
 #include "time.h"
 #include <sys/time.h>
-#include "configuration.h"
 
 HardwareSerial _serial_gps(GPS_SERIAL_NUM);
 
-RTC_DATA_ATTR bool timeSetFromGPS; // We only reset our time once per _boot_ after that point just run from the internal clock (even across sleeps)
+RTC_DATA_ATTR bool timeSetFromGPS; // We only reset our time once per _boot_ after that point just run from the internal clock
+                                   // (even across sleeps)
 
 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 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 hasValidLocation; // default to false, until we complete our first read
 static bool wantNewLocation = true;
 
-GPS::GPS() : PeriodicTask()
-{
-}
+GPS::GPS() : PeriodicTask() {}
 
 void GPS::setup()
 {
@@ -35,50 +35,45 @@ void GPS::setup()
     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) isConnected = ublox.begin(_serial_gps);
 
-    if (isConnected)
-    {
+    if (isConnected) {
         DEBUG_MSG("Connected to GPS successfully, TXpin=%d\n", GPS_TX_PIN);
 
         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)
+        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)
-            {
+            if (isConnected) {
                 ublox.assumeAutoPVT(true, true); // Just parse NEMA for now
             }
-        }
-        else
-        {
+        } 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
+            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
+            // 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(2000);
         assert(ok);
-    }
-    else
-    {
+    } 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)
+        // 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);
     }
 #endif
@@ -88,8 +83,7 @@ void GPS::readFromRTC()
 {
     struct timeval tv; /* btw settimeofday() is helpfull here too*/
 
-    if (!gettimeofday(&tv, NULL))
-    {
+    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);
@@ -101,8 +95,7 @@ void GPS::readFromRTC()
 /// 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)
-    {
+    if (!timeSetFromGPS) {
         timeSetFromGPS = true;
         DEBUG_MSG("Setting RTC %ld secs\n", tv->tv_sec);
         settimeofday(tv, NULL);
@@ -144,67 +137,69 @@ void GPS::prepareSleep()
 void GPS::doTask()
 {
 #ifdef GPS_RX_PIN
-    if (isConnected)
-    {
+    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.
-
-        // DEBUG_MSG("sec %d\n", ublox.getSecond());
-        // DEBUG_MSG("lat %d\n", ublox.getLatitude());
+        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)
-        uint8_t fixtype = ublox.getFixType();
+        // 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);
-
-        // any fix that has time
-        if ((fixtype >= 2 && fixtype <= 5) && !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);
-
-            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();
-                //ublox.powerOff();
-            }
-        }
-        else // we didn't get a location update, go back to sleep and hope the characters show up
-            wantNewLocation = true;
     }
+
+    // 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);
+
+        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();
+            // ublox.powerOff();
+        }
+    } else // we didn't get a location update, go back to sleep and hope the characters show up
+        wantNewLocation = true;
 #endif
 
-    // 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
+    // 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);
 }
 

src/MeshBluetoothService.cpp

@@ -13,6 +13,8 @@
 #include "PowerFSM.h"
 #include "CallbackCharacteristic.h"
 
+#include "GPS.h"
+
 // This scratch buffer is used for various bluetooth reads/writes - but it is safe because only one bt operation can be in proccess at once
 static uint8_t trBytes[_max(_max(_max(_max(ToRadio_size, RadioConfig_size), User_size), MyNodeInfo_size), FromRadio_size)];
 
@@ -93,6 +95,7 @@ public:
     }
 };
 
+
 // wrap our protobuf version with something that forces the service to reload the config
 class RadioCharacteristic : public ProtobufCharacteristic
 {
@@ -102,6 +105,16 @@ public:
     {
     }
 
+    void onRead(BLECharacteristic *c)
+    {
+        DEBUG_MSG("Reading radio config\n");
+
+        // update gps connection state
+        devicestate.has_radio = gps.isConnected;
+
+        BLEKeepAliveCallbacks::onRead(c);
+    }
+
     void onWrite(BLECharacteristic *c)
     {
         ProtobufCharacteristic::onWrite(c);

src/MeshService.cpp

@@ -2,12 +2,12 @@
 #include <Arduino.h>
 #include <assert.h>
 
+#include "main.h"
 #include "mesh-pb-constants.h"
 #include "MeshService.h"
 #include "MeshBluetoothService.h"
 #include "NodeDB.h"
 #include "GPS.h"
-#include "screen.h"
 #include "Periodic.h"
 #include "PowerFSM.h"
 
@@ -118,7 +118,7 @@ MeshPacket *MeshService::handleFromRadioUser(MeshPacket *mp)
         sendOurOwner(mp->from);
 
         String lcd = String("Joined: ") + mp->payload.variant.user.long_name + "\n";
-        screen_print(lcd.c_str());
+        screen.print(lcd.c_str());
     }
 
     return mp;
@@ -328,14 +328,16 @@ void MeshService::onGPSChanged()
     // Update our local node info with our position (even if we don't decide to update anyone else)
     MeshPacket *p = allocForSending();
     p->payload.which_variant = SubPacket_position_tag;
+
     Position &pos = p->payload.variant.position;
-#if 0
-    if (gps.altitude.isValid())
-        pos.altitude = gps.altitude.meters();
-    pos.latitude = gps.location.lat();
-    pos.longitude = gps.location.lng();
-    pos.time = gps.getValidTime();
-#endif
+    // !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();
+    }
 
     // We limit our GPS broadcasts to a max rate
     static uint32_t lastGpsSend;
@@ -360,4 +362,4 @@ void MeshService::onNotify(Observable *o)
 {
     DEBUG_MSG("got gps notify\n");
     onGPSChanged();
-}
+}

src/NodeDB.cpp

@@ -26,7 +26,7 @@ DeviceState versions used to be defined in the .proto file but really only this
 #define here.
 */
 
-#define DEVICESTATE_CUR_VER 2
+#define DEVICESTATE_CUR_VER 6
 #define DEVICESTATE_MIN_VER DEVICESTATE_CUR_VER
 
 #define FS SPIFFS
@@ -52,7 +52,7 @@ void NodeDB::init()
     devicestate.has_my_node = true;
     devicestate.has_radio = true;
     devicestate.has_owner = true;
-    devicestate.has_radio = true;
+    devicestate.has_radio = false;
     devicestate.radio.has_channel_settings = true;
     devicestate.radio.has_preferences = true;
     devicestate.node_db_count = 0;
@@ -159,6 +159,7 @@ void NodeDB::loadFromDisk()
                 DEBUG_MSG("Warn: devicestate is old, discarding\n");
             else
             {
+                DEBUG_MSG("Loaded saved preferences version %d\n", scratch.version);
                 devicestate = scratch;
             }
 

src/PowerFSM.cpp

@@ -1,12 +1,12 @@
 
-#include "sleep.h"
+#include "PowerFSM.h"
+#include "GPS.h"
 #include "MeshService.h"
 #include "NodeDB.h"
 #include "configuration.h"
-#include "screen.h"
-#include "PowerFSM.h"
-#include "GPS.h"
 #include "main.h"
+#include "screen.h"
+#include "sleep.h"
 
 static void sdsEnter()
 {
@@ -25,7 +25,7 @@ static void sdsEnter()
 
 static void lsEnter()
 {
-    DEBUG_MSG("lsEnter begin\n");
+    DEBUG_MSG("lsEnter begin, ls_secs=%u\n", radioConfig.preferences.ls_secs);
     screen.setOn(false);
 
     while (!service.radio.rf95.canSleep())
@@ -33,7 +33,8 @@ static void lsEnter()
 
     gps.prepareSleep(); // abandon in-process parsing
 
-    //if (!isUSBPowered)      // FIXME - temp hack until we can put gps in sleep mode, if we have AC when we go to sleep then leave GPS on
+    // if (!isUSBPowered)      // FIXME - temp hack until we can put gps in sleep mode, if we have AC when we go to sleep then
+    // leave GPS on
     //    setGPSPower(false); // kill GPS power
 
     DEBUG_MSG("lsEnter end\n");
@@ -47,8 +48,7 @@ static void lsIdle()
     esp_sleep_source_t wakeCause = ESP_SLEEP_WAKEUP_UNDEFINED;
     bool reached_ls_secs = false;
 
-    while (!reached_ls_secs)
-    {
+    while (!reached_ls_secs) {
         // Briefly come out of sleep long enough to blink the led once every few seconds
         uint32_t sleepTime = 5;
 
@@ -67,17 +67,19 @@ static void lsIdle()
     }
     setLed(false);
 
-    if (reached_ls_secs)
-    {
+    if (reached_ls_secs) {
         // stay in LS mode but let loop check whatever it wants
         DEBUG_MSG("reached ls_secs, servicing loop()\n");
-    }
-    else
-    {
+    } else {
         DEBUG_MSG("wakeCause %d\n", wakeCause);
 
-        // Regardless of why we woke just transition to NB (and that state will handle stuff like IRQs etc)
-        powerFSM.trigger(EVENT_WAKE_TIMER);
+        if (!digitalRead(BUTTON_PIN)) // If we woke because of press, instead generate a PRESS event.
+        {
+            powerFSM.trigger(EVENT_PRESS);
+        } else {
+            // Otherwise let the NB state handle the IRQ (and that state will handle stuff like IRQs etc)
+            powerFSM.trigger(EVENT_WAKE_TIMER);
+        }
     }
 }
 
@@ -104,38 +106,50 @@ static void onEnter()
 {
     screen.setOn(true);
     setBluetoothEnable(true);
+
+    static uint32_t lastPingMs;
+
+    uint32_t now = millis();
+
+    if (now - lastPingMs > 60 * 1000) { // if more than a minute since our last press, ask other nodes to update their state
+        service.sendNetworkPing();
+        lastPingMs = now;
+    }
 }
 
-
-static void wakeForPing()
-{
-}
+static void wakeForPing() {}
 
 static void screenPress()
 {
     screen.onPress();
 }
 
+
+static void bootEnter() {
+}
+
 State stateSDS(sdsEnter, NULL, NULL, "SDS");
 State stateLS(lsEnter, lsIdle, lsExit, "LS");
 State stateNB(nbEnter, NULL, NULL, "NB");
 State stateDARK(darkEnter, NULL, NULL, "DARK");
+State stateBOOT(bootEnter , NULL,  NULL, "BOOT");
 State stateON(onEnter, NULL, NULL, "ON");
-Fsm powerFSM(&stateDARK);
+Fsm powerFSM(&stateBOOT);
 
 void PowerFSM_setup()
 {
-    powerFSM.add_transition(&stateDARK, &stateON, EVENT_BOOT, NULL, "Boot");
+    powerFSM.add_timed_transition(&stateBOOT, &stateON, 3 * 1000, NULL,
+                                  "boot timeout");
+
     powerFSM.add_transition(&stateLS, &stateDARK, EVENT_WAKE_TIMER, wakeForPing, "Wake timer");
 
-    // Note we don't really use this transition, because when we wake from light sleep we _always_ transition to NB and then it handles things
-    // powerFSM.add_transition(&stateLS, &stateNB, EVENT_RECEIVED_PACKET, NULL, "Received packet");
+    // Note we don't really use this transition, because when we wake from light sleep we _always_ transition to NB and then it
+    // handles things powerFSM.add_transition(&stateLS, &stateNB, EVENT_RECEIVED_PACKET, NULL, "Received packet");
 
     powerFSM.add_transition(&stateNB, &stateNB, EVENT_RECEIVED_PACKET, NULL, "Received packet, resetting win wake");
 
-    // Note we don't really use this transition, because when we wake from light sleep we _always_ transition to NB and then it handles things
-    // powerFSM.add_transition(&stateLS, &stateON, EVENT_PRESS, NULL, "Press");
-
+    // Handle press events
+    powerFSM.add_transition(&stateLS, &stateON, EVENT_PRESS, NULL, "Press");
     powerFSM.add_transition(&stateNB, &stateON, EVENT_PRESS, NULL, "Press");
     powerFSM.add_transition(&stateDARK, &stateON, EVENT_PRESS, NULL, "Press");
     powerFSM.add_transition(&stateON, &stateON, EVENT_PRESS, screenPress, "Press"); // reenter On to restart our timers
@@ -162,11 +176,14 @@ void PowerFSM_setup()
 
     powerFSM.add_timed_transition(&stateNB, &stateLS, radioConfig.preferences.min_wake_secs * 1000, NULL, "Min wake timeout");
 
-    powerFSM.add_timed_transition(&stateDARK, &stateLS, radioConfig.preferences.wait_bluetooth_secs * 1000, NULL, "Bluetooth timeout");
+    powerFSM.add_timed_transition(&stateDARK, &stateLS, radioConfig.preferences.wait_bluetooth_secs * 1000, NULL,
+                                  "Bluetooth timeout");
 
-    powerFSM.add_timed_transition(&stateLS, &stateSDS, radioConfig.preferences.mesh_sds_timeout_secs * 1000, NULL, "mesh timeout");
+    powerFSM.add_timed_transition(&stateLS, &stateSDS, radioConfig.preferences.mesh_sds_timeout_secs * 1000, NULL,
+                                  "mesh timeout");
     // removing for now, because some users don't even have phones
-    // powerFSM.add_timed_transition(&stateLS, &stateSDS, radioConfig.preferences.phone_sds_timeout_sec * 1000, NULL, "phone timeout");
+    // powerFSM.add_timed_transition(&stateLS, &stateSDS, radioConfig.preferences.phone_sds_timeout_sec * 1000, NULL, "phone
+    // timeout");
 
     powerFSM.run_machine(); // run one interation of the state machine, so we run our on enter tasks for the initial DARK state
-}
+}

src/PowerFSM.h

@@ -4,12 +4,12 @@
 
 // See sw-design.md for documentation
 
-#define EVENT_PRESS 1
+#define EVENT_PRESS 1 
 #define EVENT_WAKE_TIMER 2
 #define EVENT_RECEIVED_PACKET 3
 #define EVENT_PACKET_FOR_PHONE 4
 #define EVENT_RECEIVED_TEXT_MSG 5
-#define EVENT_BOOT 6
+// #define EVENT_BOOT 6 // now done with a timed transition
 #define EVENT_BLUETOOTH_PAIR 7
 #define EVENT_NODEDB_UPDATED 8 // NodeDB has a big enough change that we think you should turn on the screen
 #define EVENT_CONTACT_FROM_PHONE 9 // the phone just talked to us over bluetooth

src/main.cpp

@@ -44,6 +44,14 @@ AXP20X_Class axp;
 bool pmu_irq = false;
 #endif
 
+// Global Screen singleton
+#ifdef I2C_SDA
+meshtastic::Screen screen(SSD1306_ADDRESS, I2C_SDA, I2C_SCL);
+#else
+// Fake values for pins to keep build happy, we won't ever initialize it.
+meshtastic::Screen screen(SSD1306_ADDRESS, 0, 0);
+#endif
+
 // these flags are all in bss so they default false
 bool isCharging;
 bool isUSBPowered;
@@ -221,8 +229,6 @@ void setup()
   scanI2Cdevice();
 #endif
 
-  axp192Init();
-
   // Buttons & LED
 #ifdef BUTTON_PIN
   pinMode(BUTTON_PIN, INPUT_PULLUP);
@@ -240,21 +246,24 @@ void setup()
   if (wakeCause == ESP_SLEEP_WAKEUP_TIMER)
     ssd1306_found = false; // forget we even have the hardware
 
+  // Initialize the screen first so we can show the logo while we start up everything else.
   if (ssd1306_found)
     screen.setup();
 
+  axp192Init();
+
+  screen.print("Started...\n");
+
   // Init GPS
   gps.setup();
 
-  screen_print("Started...\n");
-
   service.init();
 
+  // This must be _after_ service.init because we need our preferences loaded from flash to have proper timeout values
+  PowerFSM_setup(); // we will transition to ON in a couple of seconds, FIXME, only do this for cold boots, not waking from SDS
+
   // setBluetoothEnable(false); we now don't start bluetooth until we enter the proper state
   setCPUFast(false); // 80MHz is fine for our slow peripherals
-
-  PowerFSM_setup();
-  powerFSM.trigger(EVENT_BOOT); // transition to ON, FIXME, only do this for cold boots, not waking from SDS
 }
 
 void initBluetooth()
@@ -264,7 +273,14 @@ void initBluetooth()
   // FIXME - we are leaking like crazy
   // AllocatorScope scope(btPool);
 
-  BLEServer *serve = initBLE(getDeviceName(), HW_VENDOR, xstr(APP_VERSION), xstr(HW_VERSION)); // FIXME, use a real name based on the macaddr
+  // Note: these callbacks might be coming in from a different thread.
+  BLEServer *serve = initBLE(
+      [](uint8_t pin) {
+          powerFSM.trigger(EVENT_BLUETOOTH_PAIR);
+          screen.startBluetoothPinScreen(pin);
+      },
+      []() { screen.stopBluetoothPinScreen(); },
+      getDeviceName(), HW_VENDOR, xstr(APP_VERSION), xstr(HW_VERSION)); // FIXME, use a real name based on the macaddr
   createMeshBluetoothService(serve);
 
   // Start advertising - this must be done _after_ creating all services
@@ -370,9 +386,8 @@ void loop()
 #ifdef BUTTON_PIN
   // if user presses button for more than 3 secs, discard our network prefs and reboot (FIXME, use a debounce lib instead of this boilerplate)
   static bool wasPressed = false;
-  static uint32_t minPressMs; // what tick should we call this press long enough
-  static uint32_t lastPingMs;
 
+ 
   if (!digitalRead(BUTTON_PIN))
   {
     if (!wasPressed)
@@ -383,15 +398,6 @@ void loop()
       // esp_pm_dump_locks(stdout); // FIXME, do this someplace better
       wasPressed = true;
 
-      uint32_t now = millis();
-      minPressMs = now + 3000;
-
-      if (now - lastPingMs > 60 * 1000)
-      { // if more than a minute since our last press, ask other nodes to update their state
-        service.sendNetworkPing();
-        lastPingMs = now;
-      }
-
       powerFSM.trigger(EVENT_PRESS);
     }
   }
@@ -399,16 +405,17 @@ void loop()
   {
     // we just did a release
     wasPressed = false;
-    if (millis() > minPressMs)
-    {
-      // held long enough
-      screen_print("Erasing prefs");
-      delay(5000); // Give some time to read the screen
-      // ESP.restart();
-    }
   }
 #endif
 
+  // Show boot screen for first 3 seconds, then switch to normal operation.
+  static bool showingBootScreen = true;
+  if (showingBootScreen && (millis() > 3000))
+  {
+      screen.stopBootScreen();
+      showingBootScreen = false;
+  }
+
   // 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.
   //DEBUG_MSG("msecs %d\n", msecstosleep);

src/main.h

@@ -1,6 +1,11 @@
 #pragma once
 
+#include "screen.h"
+
 extern bool axp192_found;
 extern bool ssd1306_found;
 extern bool isCharging;
-extern bool isUSBPowered;
+extern bool isUSBPowered;
+
+// Global Screen singleton.
+extern meshtastic::Screen screen;

src/screen.cpp

@@ -20,55 +20,37 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 */
 
+#include <OLEDDisplay.h>
 #include <Wire.h>
-#include "SSD1306Wire.h"
-#include "OLEDDisplay.h"
-#include "images.h"
-#include "fonts.h"
+
 #include "GPS.h"
-#include "OLEDDisplayUi.h"
-#include "screen.h"
-#include "mesh-pb-constants.h"
 #include "NodeDB.h"
-#include "main.h"
 #include "configuration.h"
+#include "fonts.h"
+#include "images.h"
+#include "main.h"
+#include "mesh-pb-constants.h"
+#include "screen.h"
 
 #define FONT_HEIGHT 14 // actually 13 for "ariel 10" but want a little extra space
 #define FONT_HEIGHT_16 (ArialMT_Plain_16[1] + 1)
 #define SCREEN_WIDTH 128
 #define SCREEN_HEIGHT 64
-
-#ifdef I2C_SDA
-SSD1306Wire dispdev(SSD1306_ADDRESS, I2C_SDA, I2C_SCL);
-#else
-SSD1306Wire dispdev(SSD1306_ADDRESS, 0, 0); // fake values to keep build happy, we won't ever init
-#endif
-
-bool disp;     // true if we are using display
-bool screenOn; // true if the display is currently powered
-
-OLEDDisplayUi ui(&dispdev);
+#define TRANSITION_FRAMERATE 30 // fps
+#define IDLE_FRAMERATE 10       // in fps
+#define COMPASS_DIAM 44
 
 #define NUM_EXTRA_FRAMES 2 // text message and debug frame
-// A text message frame + debug frame + all the node infos
-FrameCallback nonBootFrames[MAX_NUM_NODES + NUM_EXTRA_FRAMES];
 
-Screen screen;
-static bool showingBluetooth;
-
-/// If set to true (possibly from an ISR), we should turn on the screen the next time our idle loop runs.
-static bool showingBootScreen = true; // start by showing the bootscreen
-
-bool Screen::isOn() { return screenOn; }
-
-void msOverlay(OLEDDisplay *display, OLEDDisplayUiState *state)
+namespace meshtastic
 {
-    display->setTextAlignment(TEXT_ALIGN_RIGHT);
-    display->setFont(ArialMT_Plain_10);
-    display->drawString(128, 0, String(millis()));
-}
 
-void drawBootScreen(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, int16_t y)
+// A text message frame + debug frame + all the node infos
+static FrameCallback normalFrames[MAX_NUM_NODES + NUM_EXTRA_FRAMES];
+static uint32_t targetFramerate = IDLE_FRAMERATE;
+static char btPIN[16] = "888888";
+
+static void drawBootScreen(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, int16_t y)
 {
     // draw an xbm image.
     // Please note that everything that should be transitioned
@@ -79,16 +61,10 @@ void drawBootScreen(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x,
     display->setFont(ArialMT_Plain_16);
     display->setTextAlignment(TEXT_ALIGN_CENTER);
     display->drawString(64 + x, SCREEN_HEIGHT - FONT_HEIGHT_16, "meshtastic.org");
-
-    ui.disableIndicator();
 }
 
-static char btPIN[16] = "888888";
-
-void drawFrameBluetooth(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, int16_t y)
+static void drawFrameBluetooth(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, int16_t y)
 {
-    // Demonstrates the 3 included default sizes. The fonts come from SSD1306Fonts.h file
-    // Besides the default fonts there will be a program to convert TrueType fonts into this format
     display->setTextAlignment(TEXT_ALIGN_CENTER);
     display->setFont(ArialMT_Plain_16);
     display->drawString(64 + x, 2 + y, "Bluetooth");
@@ -99,81 +75,45 @@ void drawFrameBluetooth(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t
     display->setTextAlignment(TEXT_ALIGN_CENTER);
     display->setFont(ArialMT_Plain_24);
     display->drawString(64 + x, 22 + y, btPIN);
-
-    ui.disableIndicator();
-}
-
-void drawFrame2(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, int16_t y)
-{
-    // Demonstrates the 3 included default sizes. The fonts come from SSD1306Fonts.h file
-    // Besides the default fonts there will be a program to convert TrueType fonts into this format
-    display->setTextAlignment(TEXT_ALIGN_LEFT);
-    display->setFont(ArialMT_Plain_10);
-    display->drawString(0 + x, 10 + y, "Arial 10");
-
-    display->setFont(ArialMT_Plain_16);
-    display->drawString(0 + x, 20 + y, "Arial 16");
-
-    display->setFont(ArialMT_Plain_24);
-    display->drawString(0 + x, 34 + y, "Arial 24");
-}
-
-void drawFrame3(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, int16_t y)
-{
-    // Text alignment demo
-    display->setFont(ArialMT_Plain_10);
-
-    // The coordinates define the left starting point of the text
-    display->setTextAlignment(TEXT_ALIGN_LEFT);
-    display->drawString(0 + x, 11 + y, "Left aligned (0,10)");
-
-    // The coordinates define the center of the text
-    display->setTextAlignment(TEXT_ALIGN_CENTER);
-    display->drawString(64 + x, 22 + y, "Center aligned (64,22)");
-
-    // The coordinates define the right end of the text
-    display->setTextAlignment(TEXT_ALIGN_RIGHT);
-    display->drawString(128 + x, 33 + y, "Right aligned (128,33)");
 }
 
 /// Draw the last text message we received
-void drawTextMessageFrame(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, int16_t y)
+static void drawTextMessageFrame(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, int16_t y)
 {
     MeshPacket &mp = devicestate.rx_text_message;
     NodeInfo *node = nodeDB.getNode(mp.from);
-    // DEBUG_MSG("drawing text message from 0x%x: %s\n", mp.from, mp.payload.variant.data.payload.bytes);
+    // DEBUG_MSG("drawing text message from 0x%x: %s\n", mp.from,
+    // mp.payload.variant.data.payload.bytes);
 
     // Demo for drawStringMaxWidth:
-    // with the third parameter you can define the width after which words will be wrapped.
-    // Currently only spaces and "-" are allowed for wrapping
+    // with the third parameter you can define the width after which words will
+    // be wrapped. Currently only spaces and "-" are allowed for wrapping
     display->setTextAlignment(TEXT_ALIGN_LEFT);
     display->setFont(ArialMT_Plain_16);
     String sender = (node && node->has_user) ? node->user.short_name : "???";
     display->drawString(0 + x, 0 + y, sender);
     display->setFont(ArialMT_Plain_10);
 
+    // the max length of this buffer is much longer than we can possibly print
     static char tempBuf[96];
-    snprintf(tempBuf, sizeof(tempBuf), "         %s", mp.payload.variant.data.payload.bytes); // the max length of this buffer is much longer than we can possibly print
+    snprintf(tempBuf, sizeof(tempBuf), "         %s",
+             mp.payload.variant.data.payload.bytes);
 
     display->drawStringMaxWidth(4 + x, 10 + y, 128, tempBuf);
-
-    // ui.disableIndicator();
 }
 
 /// Draw a series of fields in a column, wrapping to multiple colums if needed
-void drawColumns(OLEDDisplay *display, int16_t x, int16_t y, const char **fields)
+static void drawColumns(OLEDDisplay *display, int16_t x, int16_t y, const char **fields)
 {
     // The coordinates define the left starting point of the text
     display->setTextAlignment(TEXT_ALIGN_LEFT);
 
     const char **f = fields;
     int xo = x, yo = y;
-    while (*f)
-    {
+    while (*f) {
         display->drawString(xo, yo, *f);
         yo += FONT_HEIGHT;
-        if (yo > SCREEN_HEIGHT - FONT_HEIGHT)
-        {
+        if (yo > SCREEN_HEIGHT - FONT_HEIGHT) {
             xo += SCREEN_WIDTH / 2;
             yo = 0;
         }
@@ -183,21 +123,23 @@ void drawColumns(OLEDDisplay *display, int16_t x, int16_t y, const char **fields
 
 /// Draw a series of fields in a row, wrapping to multiple rows if needed
 /// @return the max y we ended up printing to
-uint32_t drawRows(OLEDDisplay *display, int16_t x, int16_t y, const char **fields)
+static uint32_t drawRows(OLEDDisplay *display, int16_t x, int16_t y, const char **fields)
 {
     // The coordinates define the left starting point of the text
     display->setTextAlignment(TEXT_ALIGN_LEFT);
 
     const char **f = fields;
     int xo = x, yo = y;
-    while (*f)
-    {
+    const int COLUMNS = 2; // hardwired for two columns per row....
+    int col = 0;           // track which column we are on
+    while (*f) {
         display->drawString(xo, yo, *f);
-        xo += SCREEN_WIDTH / 2; // hardwired for two columns per row....
-        if (xo >= SCREEN_WIDTH)
-        {
+        xo += SCREEN_WIDTH / COLUMNS;
+        // Wrap to next row, if needed.
+        if (++col > COLUMNS) {
+            xo = x;
             yo += FONT_HEIGHT;
-            xo = 0;
+            col = 0;
         }
         f++;
     }
@@ -207,8 +149,9 @@ uint32_t drawRows(OLEDDisplay *display, int16_t x, int16_t y, const char **field
     return yo;
 }
 
-/// Ported from my old java code, returns distance in meters along the globe surface (by magic?)
-float latLongToMeter(double lat_a, double lng_a, double lat_b, double lng_b)
+/// Ported from my old java code, returns distance in meters along the globe
+/// surface (by magic?)
+static float latLongToMeter(double lat_a, double lng_a, double lat_b, double lng_b)
 {
     double pk = (180 / 3.14169);
     double a1 = lat_a / pk;
@@ -217,10 +160,8 @@ float latLongToMeter(double lat_a, double lng_a, double lat_b, double lng_b)
     double b2 = lng_b / pk;
     double cos_b1 = cos(b1);
     double cos_a1 = cos(a1);
-    double t1 =
-        cos_a1 * cos(a2) * cos_b1 * cos(b2);
-    double t2 =
-        cos_a1 * sin(a2) * cos_b1 * sin(b2);
+    double t1 = cos_a1 * cos(a2) * cos_b1 * cos(b2);
+    double t2 = cos_a1 * sin(a2) * cos_b1 * sin(b2);
     double t3 = sin(a1) * sin(b1);
     double tt = acos(t1 + t2 + t3);
     if (isnan(tt))
@@ -229,31 +170,32 @@ float latLongToMeter(double lat_a, double lng_a, double lat_b, double lng_b)
     return (float)(6366000 * tt);
 }
 
-inline double toRadians(double deg)
+static inline double toRadians(double deg)
 {
     return deg * PI / 180;
 }
 
-inline double toDegrees(double r)
+static inline double toDegrees(double r)
 {
     return r * 180 / PI;
 }
 
 /**
-     * Computes the bearing in degrees between two points on Earth.  Ported from my old Gaggle android app.
-     *
-     * @param lat1
-     * Latitude of the first point
-     * @param lon1
-     * Longitude of the first point
-     * @param lat2
-     * Latitude of the second point
-     * @param lon2
-     * Longitude of the second point
-     * @return Bearing between the two points in radians. A value of 0 means due
-     * north.
-     */
-float bearing(double lat1, double lon1, double lat2, double lon2)
+ * Computes the bearing in degrees between two points on Earth.  Ported from my
+ * old Gaggle android app.
+ *
+ * @param lat1
+ * Latitude of the first point
+ * @param lon1
+ * Longitude of the first point
+ * @param lat2
+ * Latitude of the second point
+ * @param lon2
+ * Longitude of the second point
+ * @return Bearing between the two points in radians. A value of 0 means due
+ * north.
+ */
+static float bearing(double lat1, double lon1, double lat2, double lon2)
 {
     double lat1Rad = toRadians(lat1);
     double lat2Rad = toRadians(lat2);
@@ -263,10 +205,13 @@ float bearing(double lat1, double lon1, double lat2, double lon2)
     return atan2(y, x);
 }
 
+namespace
+{
+
 /// A basic 2D point class for drawing
 class Point
 {
-public:
+  public:
     float x, y;
 
     Point(float _x, float _y) : x(_x), y(_y) {}
@@ -274,10 +219,8 @@ public:
     /// Apply a rotation around zero (standard rotation matrix math)
     void rotate(float radian)
     {
-        float cos = cosf(radian),
-              sin = sinf(radian);
-        float rx = x * cos - y * sin,
-              ry = x * sin + y * cos;
+        float cos = cosf(radian), sin = sinf(radian);
+        float rx = x * cos - y * sin, ry = x * sin + y * cos;
 
         x = rx;
         y = ry;
@@ -296,23 +239,25 @@ public:
     }
 };
 
-void drawLine(OLEDDisplay *d, const Point &p1, const Point &p2)
+} // namespace
+
+static void drawLine(OLEDDisplay *d, const Point &p1, const Point &p2)
 {
     d->drawLine(p1.x, p1.y, p2.x, p2.y);
 }
 
 /**
  * Given a recent lat/lon return a guess of the heading the user is walking on.
- * 
- * We keep a series of "after you've gone 10 meters, what is your heading since the last reference point?"
+ *
+ * We keep a series of "after you've gone 10 meters, what is your heading since
+ * the last reference point?"
  */
-float estimatedHeading(double lat, double lon)
+static float estimatedHeading(double lat, double lon)
 {
     static double oldLat, oldLon;
     static float b;
 
-    if (oldLat == 0)
-    {
+    if (oldLat == 0) {
         // just prepare for next time
         oldLat = lat;
         oldLon = lon;
@@ -331,28 +276,28 @@ float estimatedHeading(double lat, double lon)
     return b;
 }
 
-/// Sometimes we will have Position objects that only have a time, so check for valid lat/lon
-bool hasPosition(NodeInfo *n)
+/// Sometimes we will have Position objects that only have a time, so check for
+/// valid lat/lon
+static bool hasPosition(NodeInfo *n)
 {
     return n->has_position && (n->position.latitude != 0 || n->position.longitude != 0);
 }
-#define COMPASS_DIAM 44
 
-/// We will skip one node - the one for us, so we just blindly loop over all nodes
+/// We will skip one node - the one for us, so we just blindly loop over all
+/// nodes
 static size_t nodeIndex;
 static int8_t prevFrame = -1;
 
-void drawNodeInfo(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, int16_t y)
+static void drawNodeInfo(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, int16_t y)
 {
-    // We only advance our nodeIndex if the frame # has changed - because drawNodeInfo will be called repeatedly while the frame is shown
-    if (state->currentFrame != prevFrame)
-    {
+    // We only advance our nodeIndex if the frame # has changed - because
+    // drawNodeInfo will be called repeatedly while the frame is shown
+    if (state->currentFrame != prevFrame) {
         prevFrame = state->currentFrame;
 
         nodeIndex = (nodeIndex + 1) % nodeDB.getNumNodes();
         NodeInfo *n = nodeDB.getNodeByIndex(nodeIndex);
-        if (n->num == nodeDB.getNodeNum())
-        {
+        if (n->num == nodeDB.getNodeNum()) {
             // Don't show our node, just skip to next
             nodeIndex = (nodeIndex + 1) % nodeDB.getNumNodes();
         }
@@ -380,14 +325,14 @@ void drawNodeInfo(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, in
         snprintf(lastStr, sizeof(lastStr), "%d hours ago", agoSecs / 60 / 60);
 
     static float simRadian;
-    simRadian += 0.1; // For testing, have the compass spin unless both locations are valid
+    simRadian += 0.1; // For testing, have the compass spin unless both
+                      // locations are valid
 
     static char distStr[20];
     *distStr = 0; // might not have location data
     float headingRadian = simRadian;
     NodeInfo *ourNode = nodeDB.getNode(nodeDB.getNodeNum());
-    if (ourNode && hasPosition(ourNode) && hasPosition(node))
-    {
+    if (ourNode && hasPosition(ourNode) && hasPosition(node)) {
         Position &op = ourNode->position, &p = node->position;
         float d = latLongToMeter(p.latitude, p.longitude, op.latitude, op.longitude);
         if (d < 2000)
@@ -395,23 +340,20 @@ void drawNodeInfo(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, in
         else
             snprintf(distStr, sizeof(distStr), "%.1f km", d / 1000);
 
-        // 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.
+        // 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);
         headingRadian = bearingToOther - myHeading;
     }
 
-    const char *fields[] = {
-        username,
-        distStr,
-        signalStr,
-        lastStr,
-        NULL};
+    const char *fields[] = {username, distStr, signalStr, lastStr, NULL};
     drawColumns(display, x, y, fields);
 
     // coordinates for the center of the compass
     int16_t compassX = x + SCREEN_WIDTH - COMPASS_DIAM / 2 - 1, compassY = y + SCREEN_HEIGHT / 2;
-    // display->drawXbm(compassX, compassY, compass_width, compass_height, (const uint8_t *)compass_bits);
+    // display->drawXbm(compassX, compassY, compass_width, compass_height,
+    // (const uint8_t *)compass_bits);
 
     Point tip(0.0f, 0.5f), tail(0.0f, -0.5f); // pointing up initially
     float arrowOffsetX = 0.2f, arrowOffsetY = 0.2f;
@@ -419,8 +361,7 @@ void drawNodeInfo(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, in
 
     Point *points[] = {&tip, &tail, &leftArrow, &rightArrow};
 
-    for (int i = 0; i < 4; i++)
-    {
+    for (int i = 0; i < 4; i++) {
         points[i]->rotate(headingRadian);
         points[i]->scale(COMPASS_DIAM * 0.6);
         points[i]->translate(compassX, compassY);
@@ -432,7 +373,7 @@ void drawNodeInfo(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, in
     display->drawCircle(compassX, compassY, COMPASS_DIAM / 2);
 }
 
-void drawDebugInfo(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, int16_t y)
+static void drawDebugInfo(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, int16_t y)
 {
     display->setFont(ArialMT_Plain_10);
 
@@ -451,32 +392,17 @@ void drawDebugInfo(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, i
 
     static char gpsStr[20];
     if (myNodeInfo.has_gps)
-        snprintf(gpsStr, sizeof(gpsStr), "GPS %d%%", 75); // FIXME, use something based on hdop
+        snprintf(gpsStr, sizeof(gpsStr), "GPS %d%%",
+                 75); // FIXME, use something based on hdop
     else
         gpsStr[0] = '\0'; // Just show emptystring
 
-    const char *fields[] = {
-        batStr,
-        gpsStr,
-        usersStr,
-        channelStr,
-        NULL};
+    const char *fields[] = {batStr, gpsStr, usersStr, channelStr, NULL};
     uint32_t yo = drawRows(display, x, y, fields);
 
     display->drawLogBuffer(x, yo);
 }
 
-// This array keeps function pointers to all frames
-// frames are the single views that slide in
-FrameCallback bootFrames[] = {drawBootScreen};
-
-// Overlays are statically drawn on top of a frame eg. a clock
-OverlayCallback overlays[] = {/* msOverlay */};
-
-// how many frames are there?
-const int bootFrameCount = sizeof(bootFrames) / sizeof(bootFrames[0]);
-const int overlaysCount = sizeof(overlays) / sizeof(overlays[0]);
-
 #if 0
 void _screen_header()
 {
@@ -501,20 +427,21 @@ void _screen_header()
 }
 #endif
 
-void Screen::setOn(bool on)
+Screen::Screen(uint8_t address, uint8_t sda, uint8_t scl)
+    : cmdQueue(32), useDisplay(sda || scl), dispdev(address, sda, scl), ui(&dispdev)
 {
-    if (!disp)
+}
+
+void Screen::handleSetOn(bool on)
+{
+    if (!useDisplay)
         return;
 
-    if (on != screenOn)
-    {
-        if (on)
-        {
+    if (on != screenOn) {
+        if (on) {
             DEBUG_MSG("Turning on screen\n");
             dispdev.displayOn();
-            setPeriod(1); // redraw ASAP
-        }
-        else {
+        } else {
             DEBUG_MSG("Turning off screen\n");
             dispdev.displayOff();
         }
@@ -522,167 +449,134 @@ void Screen::setOn(bool on)
     }
 }
 
-void screen_print(const char *text, uint8_t x, uint8_t y, uint8_t alignment)
-{
-    DEBUG_MSG(text);
-
-    if (!disp)
-        return;
-
-    dispdev.setTextAlignment((OLEDDISPLAY_TEXT_ALIGNMENT)alignment);
-    dispdev.drawString(x, y, text);
-}
-
-void screen_print(const char *text)
-{
-    DEBUG_MSG("Screen: %s", text);
-    if (!disp)
-        return;
-
-    dispdev.print(text);
-    // ui.update();
-}
-
 void Screen::setup()
 {
-#ifdef I2C_SDA
-    // Display instance
-    disp = true;
-
-    // The ESP is capable of rendering 60fps in 80Mhz mode
-    // but that won't give you much time for anything else
-    // run it in 160Mhz mode or just set it to 30 fps
-    // We do this now in loop()
-    // ui.setTargetFPS(30);
-
-    // Customize the active and inactive symbol
-    //ui.setActiveSymbol(activeSymbol);
-    //ui.setInactiveSymbol(inactiveSymbol);
-
-    ui.setTimePerTransition(300); // msecs
-
-    // You can change this to
-    // TOP, LEFT, BOTTOM, RIGHT
-    ui.setIndicatorPosition(BOTTOM);
-
-    // Defines where the first frame is located in the bar.
-    ui.setIndicatorDirection(LEFT_RIGHT);
-
-    // You can change the transition that is used
-    // SLIDE_LEFT, SLIDE_RIGHT, SLIDE_UP, SLIDE_DOWN
-    ui.setFrameAnimation(SLIDE_LEFT);
-
-    // Add frames - we subtract one from the framecount so there won't be a visual glitch when we take the boot screen out of the sequence.
-    ui.setFrames(bootFrames, bootFrameCount);
-
-    // Add overlays
-    ui.setOverlays(overlays, overlaysCount);
-
-    // Initialising the UI will init the display too.
-    ui.init();
-
-    // Scroll buffer
-    dispdev.setLogBuffer(3, 32);
-
-    setOn(true); // update our screenOn bool
+    if (!useDisplay)
+        return;
 
+// TODO(girts): how many of the devices come with the bicolor displays? With
+// this enabled, the logo looklooks nice, but the regular screens look a bit
+// wacky as the text crosses the color boundary and there's a 1px gap.
 #ifdef BICOLOR_DISPLAY
     dispdev.flipScreenVertically(); // looks better without this on lora32
 #endif
 
-    // dispdev.setFont(Custom_ArialMT_Plain_10);
+    // Initialising the UI will init the display too.
+    ui.init();
+    ui.setTimePerTransition(300); // msecs
+    ui.setIndicatorPosition(BOTTOM);
+    // Defines where the first frame is located in the bar.
+    ui.setIndicatorDirection(LEFT_RIGHT);
+    ui.setFrameAnimation(SLIDE_LEFT);
+    // Don't show the page swipe dots while in boot screen.
+    ui.disableAllIndicators();
 
-    ui.disableAutoTransition(); // we now require presses
-    ui.update(); // force an immediate draw of the bootscreen, because on some ssd1306 clones, the first draw command is discarded
-#endif
+    // Add frames.
+    static FrameCallback bootFrames[] = {drawBootScreen};
+    static const int bootFrameCount = sizeof(bootFrames) / sizeof(bootFrames[0]);
+    ui.setFrames(bootFrames, bootFrameCount);
+    // No overlays.
+    ui.setOverlays(nullptr, 0);
+
+    // Require presses to switch between frames.
+    ui.disableAutoTransition();
+
+    // Set up a log buffer with 3 lines, 32 chars each.
+    dispdev.setLogBuffer(3, 32);
+
+    // Turn on the display hardware.
+    handleSetOn(true);
+
+    // On some ssd1306 clones, the first draw command is discarded, so draw it
+    // twice initially.
+    ui.update();
+    ui.update();
 }
 
-#define TRANSITION_FRAMERATE 30 // fps
-#define IDLE_FRAMERATE 10       // in fps
-
-static uint32_t targetFramerate = IDLE_FRAMERATE;
-
 void Screen::doTask()
 {
-    if (!disp)
-    { // If we don't have a screen, don't ever spend any CPU for us
+    // If we don't have a screen, don't ever spend any CPU for us.
+    if (!useDisplay) {
         setPeriod(0);
         return;
     }
 
-    if (!screenOn)
-    { // If we didn't just wake and the screen is still off, then stop updating until it is on again
+    // Process incoming commands.
+    for (;;) {
+        CmdItem cmd;
+        if (!cmdQueue.dequeue(&cmd, 0)) {
+            break;
+        }
+        switch (cmd.cmd) {
+        case Cmd::SET_ON:
+            handleSetOn(true);
+            break;
+        case Cmd::SET_OFF:
+            handleSetOn(false);
+            break;
+        case Cmd::ON_PRESS:
+            handleOnPress();
+            break;
+        case Cmd::START_BLUETOOTH_PIN_SCREEN:
+            handleStartBluetoothPinScreen(cmd.bluetooth_pin);
+            break;
+        case Cmd::STOP_BLUETOOTH_PIN_SCREEN:
+        case Cmd::STOP_BOOT_SCREEN:
+            setFrames();
+            break;
+        case Cmd::PRINT:
+            handlePrint(cmd.print_text);
+            free(cmd.print_text);
+            break;
+        default:
+            DEBUG_MSG("BUG: invalid cmd");
+        }
+    }
+
+    if (!screenOn) { // If we didn't just wake and the screen is still off, then
+                     // stop updating until it is on again
         setPeriod(0);
         return;
     }
 
     // Switch to a low framerate (to save CPU) when we are not in transition
-    // but we should only call setTargetFPS when framestate changes, because otherwise that breaks
-    // animations.
-    if (targetFramerate != IDLE_FRAMERATE && ui.getUiState()->frameState == FIXED)
-    {
+    // but we should only call setTargetFPS when framestate changes, because
+    // otherwise that breaks animations.
+    if (targetFramerate != IDLE_FRAMERATE && ui.getUiState()->frameState == FIXED) {
         // oldFrameState = ui.getUiState()->frameState;
         DEBUG_MSG("Setting idle framerate\n");
         targetFramerate = IDLE_FRAMERATE;
         ui.setTargetFPS(targetFramerate);
     }
 
-    // While showing the bluetooth pair screen all of our standard screen switching is stopped
-    if (!showingBluetooth)
-    {
-        // Once we finish showing the bootscreen, remove it from the loop
-        if (showingBootScreen)
-        {
-            if (millis() > 3 * 1000) // we show the boot screen for a few seconds only
-            {
-                showingBootScreen = false;
-                setFrames();
-            }
-        }
-        else // standard screen loop handling ehre
-        {
-            // If the # nodes changes, we need to regen our list of screens
-            if (nodeDB.updateGUI || nodeDB.updateTextMessage)
-            {
-                setFrames();
-                nodeDB.updateGUI = false;
-                nodeDB.updateTextMessage = false;
-            }
+    // While showing the bootscreen or Bluetooth pair screen all of our
+    // standard screen switching is stopped.
+    if (showingNormalScreen) {
+        // TODO(girts): decouple nodeDB from screen.
+        // standard screen loop handling ehre
+        // If the # nodes changes, we need to regen our list of screens
+        if (nodeDB.updateGUI || nodeDB.updateTextMessage) {
+            setFrames();
+            nodeDB.updateGUI = false;
+            nodeDB.updateTextMessage = false;
         }
     }
 
-    // This must be after we possibly do screen_set_frames() to ensure we draw the new data
     ui.update();
 
-    // DEBUG_MSG("want fps %d, fixed=%d\n", targetFramerate, ui.getUiState()->frameState);
-    // If we are scrolling we need to be called soon, otherwise just 1 fps (to save CPU)
-    // We also ask to be called twice as fast as we really need so that any rounding errors still result
-    // with the correct framerate
+    // DEBUG_MSG("want fps %d, fixed=%d\n", targetFramerate,
+    // ui.getUiState()->frameState); If we are scrolling we need to be called
+    // soon, otherwise just 1 fps (to save CPU) We also ask to be called twice
+    // as fast as we really need so that any rounding errors still result with
+    // the correct framerate
     setPeriod(1000 / targetFramerate);
 }
 
-#include "PowerFSM.h"
-
-// Show the bluetooth PIN screen
-void screen_start_bluetooth(uint32_t pin)
-{
-    static FrameCallback btFrames[] = {drawFrameBluetooth};
-
-    snprintf(btPIN, sizeof(btPIN), "%06d", pin);
-
-    DEBUG_MSG("showing bluetooth screen\n");
-    showingBluetooth = true;
-    powerFSM.trigger(EVENT_BLUETOOTH_PAIR);
-
-    ui.setFrames(btFrames, 1); // Just show the bluetooth frame
-    // we rely on our main loop to show this screen (because we are invoked deep inside of bluetooth callbacks)
-    // ui.update(); // manually draw once, because I'm not sure if loop is getting called
-}
-
 // restore our regular frame list
 void Screen::setFrames()
 {
     DEBUG_MSG("showing standard frames\n");
+    showingNormalScreen = true;
 
     size_t numnodes = nodeDB.getNumNodes();
     // We don't show the node info our our node (if we have it yet - we should)
@@ -693,28 +587,49 @@ void Screen::setFrames()
 
     // If we have a text message - show it first
     if (devicestate.has_rx_text_message)
-        nonBootFrames[numframes++] = drawTextMessageFrame;
+        normalFrames[numframes++] = drawTextMessageFrame;
 
     // then all the nodes
     for (size_t i = 0; i < numnodes; i++)
-        nonBootFrames[numframes++] = drawNodeInfo;
+        normalFrames[numframes++] = drawNodeInfo;
 
     // then the debug info
-    nonBootFrames[numframes++] = drawDebugInfo;
+    normalFrames[numframes++] = drawDebugInfo;
 
-    ui.setFrames(nonBootFrames, numframes);
-    showingBluetooth = false;
+    ui.setFrames(normalFrames, numframes);
+    ui.enableAllIndicators();
 
-    prevFrame = -1; // Force drawNodeInfo to pick a new node (because our list just changed)
+    prevFrame = -1; // Force drawNodeInfo to pick a new node (because our list
+                    // just changed)
 }
 
-/// handle press of the button
-void Screen::onPress()
+void Screen::handleStartBluetoothPinScreen(uint32_t pin)
+{
+    DEBUG_MSG("showing bluetooth screen\n");
+    showingNormalScreen = false;
+
+    static FrameCallback btFrames[] = {drawFrameBluetooth};
+
+    snprintf(btPIN, sizeof(btPIN), "%06d", pin);
+
+    ui.disableAllIndicators();
+    ui.setFrames(btFrames, 1);
+}
+
+void Screen::handlePrint(const char *text)
+{
+    DEBUG_MSG("Screen: %s", text);
+    if (!useDisplay)
+        return;
+
+    dispdev.print(text);
+}
+
+void Screen::handleOnPress()
 {
     // If screen was off, just wake it, otherwise advance to next frame
     // If we are in a transition, the press must have bounced, drop it.
-    if (ui.getUiState()->frameState == FIXED)
-    {
+    if (ui.getUiState()->frameState == FIXED) {
         setPeriod(1); // redraw ASAP
         ui.nextFrame();
 
@@ -725,3 +640,5 @@ void Screen::onPress()
         ui.setTargetFPS(targetFramerate);
     }
 }
+
+} // namespace meshtastic

src/screen.h

@@ -1,44 +1,127 @@
 #pragma once
 
+#include <cstring>
+
+#include <OLEDDisplayUi.h>
+#include <SSD1306Wire.h>
+
 #include "PeriodicTask.h"
+#include "TypedQueue.h"
 
-void screen_print(const char * text);
-void screen_print(const char * text, uint8_t x, uint8_t y, uint8_t alignment);
+namespace meshtastic
+{
 
-
-// Show the bluetooth PIN screen
-void screen_start_bluetooth(uint32_t pin); 
-
-// restore our regular frame list
-void screen_set_frames();
-
-
-/**
- * Slowly I'm moving screen crap into this class
- */
+/// Deals with showing things on the screen of the device.
+//
+// Other than setup(), this class is thread-safe. All state-changing calls are
+// queued and executed when the main loop calls us.
 class Screen : public PeriodicTask
 {
-public:
+  public:
+    Screen(uint8_t address, uint8_t sda, uint8_t scl);
 
+    Screen(const Screen &) = delete;
+    Screen &operator=(const Screen &) = delete;
+
+    /// Initializes the UI, turns on the display, starts showing boot screen.
+    //
+    // Not thread safe - must be called before any other methods are called.
     void setup();
 
-    virtual void doTask();
+    /// Turns the screen on/off.
+    void setOn(bool on) { enqueueCmd(CmdItem{.cmd = on ? Cmd::SET_ON : Cmd::SET_OFF}); }
 
-    /// Turn on the screen asap
-    void doWakeScreen();
+    /// Handles a button press.
+    void onPress() { enqueueCmd(CmdItem{.cmd = Cmd::ON_PRESS}); }
 
-    /// Is the screen currently on
-    bool isOn();
+    /// Starts showing the Bluetooth PIN screen.
+    //
+    // Switches over to a static frame showing the Bluetooth pairing screen
+    // with the PIN.
+    void startBluetoothPinScreen(uint32_t pin)
+    {
+        CmdItem cmd;
+        cmd.cmd = Cmd::START_BLUETOOTH_PIN_SCREEN;
+        cmd.bluetooth_pin = pin;
+        enqueueCmd(cmd);
+    }
 
-    /// Turn the screen on/off
-    void setOn(bool on);
+    /// Stops showing the bluetooth PIN screen.
+    void stopBluetoothPinScreen() { enqueueCmd(CmdItem{.cmd = Cmd::STOP_BLUETOOTH_PIN_SCREEN}); }
 
-    /// Handle a button press
-    void onPress();
+    /// Stops showing the boot screen.
+    void stopBootScreen() { enqueueCmd(CmdItem{.cmd = Cmd::STOP_BOOT_SCREEN}); }
 
-    /// Rebuilt our list of screens
+    /// Writes a string to the screen.
+    void print(const char *text)
+    {
+        CmdItem cmd;
+        cmd.cmd = Cmd::PRINT;
+        // TODO(girts): strdup() here is scary, but we can't use std::string as
+        // FreeRTOS queue is just dumbly copying memory contents. It would be
+        // nice if we had a queue that could copy objects by value.
+        cmd.print_text = strdup(text);
+        if (!enqueueCmd(cmd)) {
+            free(cmd.print_text);
+        }
+    }
+
+  protected:
+    /// Updates the UI.
+    //
+    // Called periodically from the main loop.
+    void doTask() final;
+
+  private:
+    enum class Cmd {
+        INVALID,
+        SET_ON,
+        SET_OFF,
+        ON_PRESS,
+        START_BLUETOOTH_PIN_SCREEN,
+        STOP_BLUETOOTH_PIN_SCREEN,
+        STOP_BOOT_SCREEN,
+        PRINT,
+    };
+    struct CmdItem {
+        Cmd cmd;
+        union {
+            uint32_t bluetooth_pin;
+            char *print_text;
+        };
+    };
+
+    /// Enques given command item to be processed by main loop().
+    bool enqueueCmd(const CmdItem &cmd)
+    {
+        bool success = cmdQueue.enqueue(cmd, 0);
+        setPeriod(1); // handle ASAP
+        return success;
+    }
+
+    // Implementations of various commands, called from doTask().
+    void handleSetOn(bool on);
+    void handleOnPress();
+    void handleStartBluetoothPinScreen(uint32_t pin);
+    void handlePrint(const char *text);
+
+    /// Rebuilds our list of frames (screens) to default ones.
     void setFrames();
+
 private:
+    /// Queue of commands to execute in doTask.
+    TypedQueue<CmdItem> cmdQueue;
+    /// Whether we are using a display
+    bool useDisplay = false;
+    /// Whether the display is currently powered
+    bool screenOn = false;
+    // Whether we are showing the regular screen (as opposed to booth screen or
+    // Bluetooth PIN screen)
+    bool showingNormalScreen = false;
+    /// Display device
+    SSD1306Wire dispdev;
+    /// UI helper for rendering to frames and switching between them
+    OLEDDisplayUi ui;
 };
 
-extern Screen screen;
+} // namespace meshtastic

src/sleep.cpp

@@ -6,7 +6,6 @@
 #include "MeshBluetoothService.h"
 #include "MeshService.h"
 #include "GPS.h"
-#include "screen.h"
 #include "NodeDB.h"
 #include "Periodic.h"
 #include "esp32/pm.h"