Merge pull request #44 from geeksville/master

various minor commits based on bugs I see while testing app

.clang-format

@@ -0,0 +1,6 @@
+Language: Cpp
+IndentWidth: 4
+ColumnLimit: 130
+PointerAlignment: Right
+BreakBeforeBraces: Linux
+AllowShortFunctionsOnASingleLine: Inline

.github/workflows/main.yml

@@ -1,5 +1,7 @@
 name: Continuous Integration
-on: push
+on:
+  - push
+  - pull_request
 
 jobs:
   main:

README.md

@@ -13,7 +13,7 @@ will optionally work with your phone, but no phone is required.
 
 Typical time between recharging the radios should be about eight days.
 
-This project is currently early-alpha, but if you have questions please join our chat [![Join the chat at https://gitter.im/Meshtastic/community](https://badges.gitter.im/Meshtastic/community.svg)](https://gitter.im/Meshtastic/community?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge).
+This project is currently early-alpha, but if you have questions please [join our discussion forum](https://meshtastic.discourse.group/).
 
 This software is 100% open source and developed by a group of hobbyist experimenters.  No warranty is provided, if you'd like to improve it - we'd love your help.  Please post in the chat.  
 
@@ -87,7 +87,7 @@ After our rate of change slows a bit, we will make beta builds available here (w
 
 # Development
 
-We'd love to have you join us on this merry little project.  Please see our [development documents](./docs/software/sw-design.md) and join us on the gitter chat above.
+We'd love to have you join us on this merry little project.  Please see our [development documents](./docs/software/sw-design.md) and [join us in our discussion forum](https://meshtastic.discourse.group/).
 
 # Credits
 

bin/exception_decoder.py

@@ -0,0 +1,329 @@
+#!/usr/bin/env python3
+
+"""ESP Exception Decoder
+
+github:  https://github.com/janLo/EspArduinoExceptionDecoder
+license: GPL v3
+author:  Jan Losinski
+
+Meshtastic notes:
+* original version is at: https://github.com/janLo/EspArduinoExceptionDecoder
+* version that's checked into meshtastic repo is based on: https://github.com/me21/EspArduinoExceptionDecoder
+  which adds in ESP32 Backtrace decoding.
+* this also updates the defaults to use ESP32, instead of ESP8266 and defaults to the built firmware.bin
+
+To use, copy the "Backtrace: 0x...." line to a file, e.g., backtrace.txt, then run:
+$ bin/exception_decoder.py backtrace.txt
+"""
+
+import argparse
+import re
+import subprocess
+from collections import namedtuple
+
+import sys
+
+import os
+
+EXCEPTIONS = [
+    "Illegal instruction",
+    "SYSCALL instruction",
+    "InstructionFetchError: Processor internal physical address or data error during instruction fetch",
+    "LoadStoreError: Processor internal physical address or data error during load or store",
+    "Level1Interrupt: Level-1 interrupt as indicated by set level-1 bits in the INTERRUPT register",
+    "Alloca: MOVSP instruction, if caller's registers are not in the register file",
+    "IntegerDivideByZero: QUOS, QUOU, REMS, or REMU divisor operand is zero",
+    "reserved",
+    "Privileged: Attempt to execute a privileged operation when CRING ? 0",
+    "LoadStoreAlignmentCause: Load or store to an unaligned address",
+    "reserved",
+    "reserved",
+    "InstrPIFDataError: PIF data error during instruction fetch",
+    "LoadStorePIFDataError: Synchronous PIF data error during LoadStore access",
+    "InstrPIFAddrError: PIF address error during instruction fetch",
+    "LoadStorePIFAddrError: Synchronous PIF address error during LoadStore access",
+    "InstTLBMiss: Error during Instruction TLB refill",
+    "InstTLBMultiHit: Multiple instruction TLB entries matched",
+    "InstFetchPrivilege: An instruction fetch referenced a virtual address at a ring level less than CRING",
+    "reserved",
+    "InstFetchProhibited: An instruction fetch referenced a page mapped with an attribute that does not permit instruction fetch",
+    "reserved",
+    "reserved",
+    "reserved",
+    "LoadStoreTLBMiss: Error during TLB refill for a load or store",
+    "LoadStoreTLBMultiHit: Multiple TLB entries matched for a load or store",
+    "LoadStorePrivilege: A load or store referenced a virtual address at a ring level less than CRING",
+    "reserved",
+    "LoadProhibited: A load referenced a page mapped with an attribute that does not permit loads",
+    "StoreProhibited: A store referenced a page mapped with an attribute that does not permit stores"
+]
+
+PLATFORMS = {
+    "ESP8266": "lx106",
+    "ESP32": "esp32"
+}
+
+BACKTRACE_REGEX = re.compile(r"(?:\s+(0x40[0-2](?:\d|[a-f]|[A-F]){5}):0x(?:\d|[a-f]|[A-F]){8})\b")
+EXCEPTION_REGEX = re.compile("^Exception \\((?P<exc>[0-9]*)\\):$")
+COUNTER_REGEX = re.compile('^epc1=(?P<epc1>0x[0-9a-f]+) epc2=(?P<epc2>0x[0-9a-f]+) epc3=(?P<epc3>0x[0-9a-f]+) '
+                           'excvaddr=(?P<excvaddr>0x[0-9a-f]+) depc=(?P<depc>0x[0-9a-f]+)$')
+CTX_REGEX = re.compile("^ctx: (?P<ctx>.+)$")
+POINTER_REGEX = re.compile('^sp: (?P<sp>[0-9a-f]+) end: (?P<end>[0-9a-f]+) offset: (?P<offset>[0-9a-f]+)$')
+STACK_BEGIN = '>>>stack>>>'
+STACK_END = '<<<stack<<<'
+STACK_REGEX = re.compile(
+    '^(?P<off>[0-9a-f]+):\W+(?P<c1>[0-9a-f]+) (?P<c2>[0-9a-f]+) (?P<c3>[0-9a-f]+) (?P<c4>[0-9a-f]+)(\W.*)?$')
+
+StackLine = namedtuple("StackLine", ["offset", "content"])
+
+
+class ExceptionDataParser(object):
+    def __init__(self):
+        self.exception = None
+
+        self.epc1 = None
+        self.epc2 = None
+        self.epc3 = None
+        self.excvaddr = None
+        self.depc = None
+
+        self.ctx = None
+
+        self.sp = None
+        self.end = None
+        self.offset = None
+
+        self.stack = []
+
+    def _parse_backtrace(self, line):
+        if line.startswith('Backtrace:'):
+            self.stack = [StackLine(offset=0, content=(addr,)) for addr in BACKTRACE_REGEX.findall(line)]
+            return None
+        return self._parse_backtrace
+
+    def _parse_exception(self, line):
+        match = EXCEPTION_REGEX.match(line)
+        if match is not None:
+            self.exception = int(match.group('exc'))
+            return self._parse_counters
+        return self._parse_exception
+
+    def _parse_counters(self, line):
+        match = COUNTER_REGEX.match(line)
+        if match is not None:
+            self.epc1 = match.group("epc1")
+            self.epc2 = match.group("epc2")
+            self.epc3 = match.group("epc3")
+            self.excvaddr = match.group("excvaddr")
+            self.depc = match.group("depc")
+            return self._parse_ctx
+        return self._parse_counters
+
+    def _parse_ctx(self, line):
+        match = CTX_REGEX.match(line)
+        if match is not None:
+            self.ctx = match.group("ctx")
+            return self._parse_pointers
+        return self._parse_ctx
+
+    def _parse_pointers(self, line):
+        match = POINTER_REGEX.match(line)
+        if match is not None:
+            self.sp = match.group("sp")
+            self.end = match.group("end")
+            self.offset = match.group("offset")
+            return self._parse_stack_begin
+        return self._parse_pointers
+
+    def _parse_stack_begin(self, line):
+        if line == STACK_BEGIN:
+            return self._parse_stack_line
+        return self._parse_stack_begin
+
+    def _parse_stack_line(self, line):
+        if line != STACK_END:
+            match = STACK_REGEX.match(line)
+            if match is not None:
+                self.stack.append(StackLine(offset=match.group("off"),
+                                            content=(match.group("c1"), match.group("c2"), match.group("c3"),
+                                                     match.group("c4"))))
+            return self._parse_stack_line
+        return None
+
+    def parse_file(self, file, platform, stack_only=False):
+        if platform == 'ESP32':
+            func = self._parse_backtrace
+        else:
+            func = self._parse_exception
+            if stack_only:
+                func = self._parse_stack_begin
+
+        for line in file:
+            func = func(line.strip())
+            if func is None:
+                break
+
+        if func is not None:
+            print("ERROR: Parser not complete!")
+            sys.exit(1)
+
+
+class AddressResolver(object):
+    def __init__(self, tool_path, elf_path):
+        self._tool = tool_path
+        self._elf = elf_path
+        self._address_map = {}
+
+    def _lookup(self, addresses):
+        cmd = [self._tool, "-aipfC", "-e", self._elf] + [addr for addr in addresses if addr is not None]
+
+        if sys.version_info[0] < 3:
+            output = subprocess.check_output(cmd)
+        else:
+            output = subprocess.check_output(cmd, encoding="utf-8")
+
+        line_regex = re.compile("^(?P<addr>[0-9a-fx]+): (?P<result>.+)$")
+
+        last = None
+        for line in output.splitlines():
+            line = line.strip()
+            match = line_regex.match(line)
+
+            if match is None:
+                if last is not None and line.startswith('(inlined by)'):
+                    line = line [12:].strip()
+                    self._address_map[last] += ("\n  \-> inlined by: " + line)
+                continue
+
+            if match.group("result") == '?? ??:0':
+                continue
+
+            self._address_map[match.group("addr")] = match.group("result")
+            last = match.group("addr")
+
+    def fill(self, parser):
+        addresses = [parser.epc1, parser.epc2, parser.epc3, parser.excvaddr, parser.sp, parser.end, parser.offset]
+        for line in parser.stack:
+            addresses.extend(line.content)
+
+        self._lookup(addresses)
+
+    def _sanitize_addr(self, addr):
+        if addr.startswith("0x"):
+            addr = addr[2:]
+
+        fill = "0" * (8 - len(addr))
+        return "0x" + fill + addr
+
+    def resolve_addr(self, addr):
+        out = self._sanitize_addr(addr)
+
+        if out in self._address_map:
+            out += ": " + self._address_map[out]
+
+        return out
+
+    def resolve_stack_addr(self, addr, full=True):
+        addr = self._sanitize_addr(addr)
+        if addr in self._address_map:
+            return addr + ": " + self._address_map[addr]
+
+        if full:
+            return "[DATA (0x" + addr + ")]"
+
+        return None
+
+
+def print_addr(name, value, resolver):
+    print("{}:{} {}".format(name, " " * (8 - len(name)), resolver.resolve_addr(value)))
+
+
+def print_stack_full(lines, resolver):
+    print("stack:")
+    for line in lines:
+        print(line.offset + ":")
+        for content in line.content:
+            print("  " + resolver.resolve_stack_addr(content))
+
+
+def print_stack(lines, resolver):
+    print("stack:")
+    for line in lines:
+        for content in line.content:
+            out = resolver.resolve_stack_addr(content, full=False)
+            if out is None:
+                continue
+            print(out)
+
+
+def print_result(parser, resolver, platform, full=True, stack_only=False):
+    if platform == 'ESP8266' and not stack_only:
+        print('Exception: {} ({})'.format(parser.exception, EXCEPTIONS[parser.exception]))
+
+        print("")
+        print_addr("epc1", parser.epc1, resolver)
+        print_addr("epc2", parser.epc2, resolver)
+        print_addr("epc3", parser.epc3, resolver)
+        print_addr("excvaddr", parser.excvaddr, resolver)
+        print_addr("depc", parser.depc, resolver)
+
+        print("")
+        print("ctx: " + parser.ctx)
+
+        print("")
+        print_addr("sp", parser.sp, resolver)
+        print_addr("end", parser.end, resolver)
+        print_addr("offset", parser.offset, resolver)
+
+        print("")
+    if full:
+        print_stack_full(parser.stack, resolver)
+    else:
+        print_stack(parser.stack, resolver)
+
+
+def parse_args():
+    parser = argparse.ArgumentParser(description="decode ESP Stacktraces.")
+
+    parser.add_argument("-p", "--platform", help="The platform to decode from", choices=PLATFORMS.keys(),
+                        default="ESP32")
+    parser.add_argument("-t", "--tool", help="Path to the xtensa toolchain",
+                        default="~/.platformio/packages/toolchain-xtensa32/")
+    parser.add_argument("-e", "--elf", help="path to elf file",
+                        default=".pio/build/esp32/firmware.elf")
+    parser.add_argument("-f", "--full", help="Print full stack dump", action="store_true")
+    parser.add_argument("-s", "--stack_only", help="Decode only a stractrace", action="store_true")
+    parser.add_argument("file", help="The file to read the exception data from ('-' for STDIN)", default="-")
+
+    return parser.parse_args()
+
+
+if __name__ == "__main__":
+    args = parse_args()
+
+    if args.file == "-":
+        file = sys.stdin
+    else:
+        if not os.path.exists(args.file):
+            print("ERROR: file " + args.file + " not found")
+            sys.exit(1)
+        file = open(args.file, "r")
+
+    addr2line = os.path.join(os.path.abspath(os.path.expanduser(args.tool)),
+                             "bin/xtensa-" + PLATFORMS[args.platform] + "-elf-addr2line")
+    if os.name == 'nt':
+        addr2line += '.exe'
+    if not os.path.exists(addr2line):
+        print("ERROR: addr2line not found (" + addr2line + ")")
+
+    elf_file = os.path.abspath(os.path.expanduser(args.elf))
+    if not os.path.exists(elf_file):
+        print("ERROR: elf file not found (" + elf_file + ")")
+
+    parser = ExceptionDataParser()
+    resolver = AddressResolver(addr2line, elf_file)
+
+    parser.parse_file(file, args.platform, args.stack_only)
+    resolver.fill(parser)
+
+    print_result(parser, resolver, args.platform, args.full, args.stack_only)

bin/program-release-tbeam.sh

@@ -0,0 +1,6 @@
+
+set -e
+
+source bin/version.sh
+
+esptool.py --baud 921600 write_flash 0x10000 release/latest/firmware-TBEAM-US-$VERSION.bin

bin/read-system-info.sh

@@ -0,0 +1 @@
+esptool.py --baud 921600 read_flash 0x1000 0xf000 system-info.img

bin/version.sh

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

docs/README.md

@@ -30,9 +30,9 @@ Not all of these features are fully implemented yet - see **important** disclaim
 * Eventually (within a couple of months) we should have a modified version of Signal that works with this project.
 * Very easy sharing of private secured channels.  Just share a special link or QR code with friends and they can join your encrypted mesh
  
-This project is currently in early alpha - if you have questions please join our chat [![Join the chat at https://gitter.im/Meshtastic/community](https://badges.gitter.im/Meshtastic/community.svg)](https://gitter.im/Meshtastic/community?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge).
+This project is currently in early alpha - if you have questions please [join our discussion forum](https://meshtastic.discourse.group/).
 
-This software is 100% open source and developed by a group of hobbyist experimenters.  No warranty is provided, if you'd like to improve it - we'd love your help.  Please post in the [chat](https://gitter.im/Meshtastic/community).  
+This software is 100% open source and developed by a group of hobbyist experimenters.  No warranty is provided, if you'd like to improve it - we'd love your help.  Please post in the [forum](https://meshtastic.discourse.group/).
 
 # Updates
 
@@ -52,7 +52,7 @@ But if you want the bleeding edge app now, we'd love to have your help testing.
 
 1. Join [this Google group](https://groups.google.com/forum/#!forum/meshtastic-alpha-testers) with the account you use in Google Play.
 2. Go to this [URL](https://play.google.com/apps/testing/com.geeksville.mesh) to opt-in to the alpha test.
-3. If you encounter any problems or have questions, post in our gitter chat and we'll help.
+3. If you encounter any problems or have questions, post in our [forum](https://meshtastic.discourse.group/) and we'll help.
 
 If you'd like to help with development, the source code is [on github](https://github.com/meshtastic/Meshtastic-Android).
 

docs/faq.md

@@ -15,7 +15,7 @@ For more details see the [device software TODO](https://github.com/meshtastic/Me
 
 # FAQ
 
-If you have a question missing from this faq, please ask on our gitter chat.  And if you are feeling extra generous send in a pull-request for this faq.md with whatever we answered ;-).
+If you have a question missing from this faq, please [ask in our discussion forum](https://meshtastic.discourse.group/).  And if you are feeling extra generous send in a pull-request for this faq.md with whatever we answered ;-).
 
 ## Q: Which of the various supported radios should I buy?
 

docs/software/TODO.md

@@ -7,6 +7,7 @@ being I have it set at 2 minutes to ensure enough time for a GPS lock from scrat
 
 * use gps sleep mode instead of killing its power (to allow fast position when we wake)
 * enable fast lock and low power inside the gps chip
+
 * remeasure wake time power draws now that we run CPU down at 80MHz
 * add a SF12 transmit option for _super_ long range
 
@@ -37,6 +38,7 @@ Items to complete before the first beta release.
 
 During the beta timeframe the following improvements 'would be nice' (and yeah - I guess some of these items count as features, but it is a hobby project ;-) )
 
+* figure out why this fixme is needed: "FIXME, disable wake due to PMU because it seems to fire all the time?"
 * If the phone doesn't read fromradio mailbox within X seconds, assume the phone is gone and we can stop queing location msgs 
 for it (because it will redownload the nodedb when it comes back)
 * Figure out why the RF95 ISR is never seeing RH_RF95_VALID_HEADER, so it is not protecting our rx packets from getting stomped on by sends
@@ -195,3 +197,7 @@ until the phone pulls those packets.  Ever so often power on bluetooth just so w
 * link to the kanban page
 * add a getting started page
 * finish mesh alg reeval
+* ublox gps parsing seems a little buggy (we shouldn't be sending out read solution commands, the device is already broadcasting them)
+* turn on gps https://github.com/sparkfun/SparkFun_Ublox_Arduino_Library/blob/master/examples/Example18_PowerSaveMode/Example18_PowerSaveMode.ino
+* switch gps to 38400 baud https://github.com/sparkfun/SparkFun_Ublox_Arduino_Library/blob/master/examples/Example11_ResetModule/Example2_FactoryDefaultsviaSerial/Example2_FactoryDefaultsviaSerial.ino
+* Use Neo-M8M API to put it in sleep mode

docs/software/build-instructions.md

@@ -14,3 +14,12 @@ in these instructions I describe use of their command line tool.
 5. Plug the radio into your USB port
 6. Type "pio run -t upload" (This command will fetch dependencies, build the project and install it on the board via USB)
 7. Platform IO also installs a very nice VisualStudio Code based IDE, see their [tutorial](https://docs.platformio.org/en/latest/tutorials/espressif32/arduino_debugging_unit_testing.html) if you'd like to use it
+
+
+## Decoding stack traces
+
+If you get a crash, you can decode the addresses from the `Backtrace:` line:
+1. Save the `Backtrace: 0x....` line to a file, e.g., `backtrace.txt`.
+2. Run `bin/exception_decoder.py backtrace.txt` (this uses symbols from the
+   last `firmware.elf`, so you must be running the same binary that's still in
+   your `.pio/build` directory).

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
+    // Remove our custom PIN request screen.
-    screen.setFrames();
+    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

platformio.ini

@@ -59,14 +59,14 @@ debug_init_break = tbreak setup
 ; names.
 lib_deps =
   https://github.com/meshtastic/RadioHead.git
-  1655 ; TinyGPSPlus
   https://github.com/meshtastic/esp8266-oled-ssd1306.git ; ESP8266_SSD1306 
   AXP202X_Library
   SPI
-  1260 ; OneButton
+  ; 1260 ; OneButton - not used yet
   1202 ; CRC32, explicitly needed because dependency is missing in the ble ota update lib
   Wire ; explicitly needed here because the AXP202 library forgets to add it
   https://github.com/meshtastic/arduino-fsm.git 
+  https://github.com/meshtastic/SparkFun_Ublox_Arduino_Library.git
 
 ;[env:tbeam] 
 ;board = ttgo-t-beam

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.3</string>
-</resources>

src/CustomRF95.cpp

@@ -21,6 +21,7 @@ CustomRF95::CustomRF95(MemoryPool<MeshPacket> &_pool, PointerQueue<MeshPacket> &
 bool CustomRF95::canSleep()
 {
     // We allow initializing mode, because sometimes while testing we don't ever call init() to turn on the hardware
+    DEBUG_MSG("canSleep, mode=%d, isRx=%d, txEmpty=%d, txGood=%d\n", _mode, _isReceiving, txQueue.isEmpty(), _txGood);
     return (_mode == RHModeInitialising || _mode == RHModeIdle || _mode == RHModeRx) && !_isReceiving && txQueue.isEmpty(); 
 }
 
@@ -125,8 +126,7 @@ void CustomRF95::handleInterrupt()
                 // parsing was successful, queue for our recipient
                 mp->has_payload = true;
 
-                int res = rxDest.enqueueFromISR(mp, &higherPriWoken); // NOWAIT - fixme, if queue is full, delete older messages
+                assert(rxDest.enqueueFromISR(mp, &higherPriWoken)); // NOWAIT - fixme, if queue is full, delete older messages
-                assert(res == pdTRUE);
             }
 
             clearRxBuf(); // This message accepted and cleared

src/GPS.cpp

@@ -1,30 +1,81 @@
 
 #include "GPS.h"
+#include "configuration.h"
 #include "time.h"
 #include <sys/time.h>
-#include "configuration.h"
 
-// stuff that really should be in in the instance instead...
 HardwareSerial _serial_gps(GPS_SERIAL_NUM);
-uint32_t timeStartMsec;  // Once we have a GPS lock, this is where we hold the initial msec clock that corresponds to that time
-uint64_t zeroOffsetSecs; // GPS based time in secs since 1970 - only updated once on initial lock
 
-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;
-bool hasValidLocation; // default to false, until we complete our first read
-bool wantNewLocation = true;
 
-GPS::GPS() : PeriodicTask()
+// 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 hasValidLocation; // default to false, until we complete our first read
+static bool wantNewLocation = true;
+
+GPS::GPS() : PeriodicTask() {}
 
 void GPS::setup()
 {
-    readFromRTC();
+    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);
+    // _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, 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)
+            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(2000);
+        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);
+    }
 #endif
 }
 
@@ -32,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);
@@ -45,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);
@@ -75,70 +124,83 @@ uint32_t GPS::getValidTime()
 /// 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 !wantNewLocation;
+    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()
 {
-    // discard all rx serial bytes so we don't try to parse them when we come back
+    if (isConnected)
-    while (_serial_gps.available())
+        ublox.powerOff();
-    {
-        _serial_gps.read();
-    }
-
-    // make the parser bail on whatever it was parsing
-    encode('\n');
 }
 
 void GPS::doTask()
 {
 #ifdef GPS_RX_PIN
+    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
 
-    while (_serial_gps.available())
+        // getPVT automatically calls checkUblox
-    {
+        ublox.checkUblox(); // See if new data is available. Process bytes as they come in.
-        encode(_serial_gps.read());
+
-        // DEBUG_MSG("Got GPS response\n");
+        // 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);
     }
 
-    if (!timeSetFromGPS && time.isValid() && date.isValid())
+    // 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;
 
-        DEBUG_MSG("Got time from GPS\n");
-
         /* 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). 
+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 = time.second();
+        t.tm_sec = ublox.getSecond();
-        t.tm_min = time.minute();
+        t.tm_min = ublox.getMinute();
-        t.tm_hour = time.hour();
+        t.tm_hour = ublox.getHour();
-        t.tm_mday = date.day();
+        t.tm_mday = ublox.getDay();
-        t.tm_mon = date.month() - 1;
+        t.tm_mon = ublox.getMonth() - 1;
-        t.tm_year = date.year() - 1900;
+        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);
     }
-#endif
 
-    if (location.isValid() && location.isUpdated())
+    if ((fixtype >= 3 && fixtype <= 4) && ublox.getP()) // rd fixes only
-    { // we only notify if position has changed
+    {
-        // DEBUG_MSG("new gps pos\n");
+        // we only notify if position has changed
-        hasValidLocation = true;
+        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
+    } 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 100ms 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
-    setPeriod(hasValidLocation && !wantNewLocation ? 30 * 1000 : 100);
+    // the serial
+    setPeriod(hasValidLocation && !wantNewLocation ? 30 * 1000 : 10 * 1000);
 }
 
 void GPS::startLock()
@@ -147,11 +209,3 @@ void GPS::startLock()
     wantNewLocation = true;
     setPeriod(1);
 }
-
-String GPS::getTimeStr()
-{
-    static char t[12]; // used to sprintf for Serial output
-
-    snprintf(t, sizeof(t), "%02d:%02d:%02d", time.hour(), time.minute(), time.second());
-    return t;
-}

src/GPS.h

@@ -1,18 +1,24 @@
 #pragma once
 
-#include <TinyGPS++.h>
 #include "PeriodicTask.h"
 #include "Observer.h"
 #include "sys/time.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 GPS : public PeriodicTask, public Observable, public TinyGPSPlus
+class GPS : public PeriodicTask, public Observable
 {
+    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
@@ -21,8 +27,6 @@ public:
     /// Return time since 1970 in secs.  If we don't have a GPS lock return zero
     uint32_t getValidTime();
 
-    String getTimeStr();
-
     void setup();
 
     virtual void loop();
@@ -46,3 +50,4 @@ private:
 };
 
 extern GPS gps;
+

src/MemoryPool.h

@@ -66,16 +66,14 @@ public:
     /// Return a buffer for use by others
     void release(T *p)
     {
-        int res = dead.enqueue(p, 0);
+        assert(dead.enqueue(p, 0));
-        assert(res == pdTRUE);
         assert(p >= buf && (p - buf) < maxElements); // sanity check to make sure a programmer didn't free something that didn't come from this pool
     }
 
     /// Return a buffer from an ISR, if higherPriWoken is set to true you have some work to do ;-)
     void releaseFromISR(T *p, BaseType_t *higherPriWoken)
     {
-        int res = dead.enqueueFromISR(p, higherPriWoken);
+        assert(dead.enqueueFromISR(p, higherPriWoken));
-        assert(res == pdTRUE);
         assert(p >= buf && (p - buf) < maxElements); // sanity check to make sure a programmer didn't free something that didn't come from this pool
     }
 };

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/MeshRadio.cpp

@@ -9,8 +9,6 @@
 #include "configuration.h"
 #include "NodeDB.h"
 
-#define DEFAULT_CHANNEL_NUM 3 // we randomly pick one
-
 /// 16 bytes of random PSK for our _public_ default channel that all devices power up on
 static const uint8_t defaultpsk[] = {0xd4, 0xf1, 0xbb, 0x3a, 0x20, 0x29, 0x07, 0x59, 0xf0, 0xbc, 0xff, 0xab, 0xcf, 0x4e, 0x69, 0xbf};
 
@@ -39,7 +37,6 @@ MeshRadio::MeshRadio(MemoryPool<MeshPacket> &_pool, PointerQueue<MeshPacket> &_r
   channelSettings.modem_config = ChannelSettings_ModemConfig_Bw125Cr48Sf4096; // slow and long range
 
   channelSettings.tx_power = 23;
-  channelSettings.channel_num = DEFAULT_CHANNEL_NUM;
   memcpy(&channelSettings.psk, &defaultpsk, sizeof(channelSettings.psk));
   strcpy(channelSettings.name, "Default");
   // Can't print strings this early - serial not setup yet
@@ -81,6 +78,22 @@ bool MeshRadio::init()
   return true;
 }
 
+/** hash a string into an integer
+ * 
+ * djb2 by Dan Bernstein.
+ * http://www.cse.yorku.ca/~oz/hash.html
+ */
+unsigned long hash(char *str)
+{
+  unsigned long hash = 5381;
+  int c;
+
+  while ((c = *str++) != 0)
+    hash = ((hash << 5) + hash) + (unsigned char)c; /* hash * 33 + c */
+
+  return hash;
+}
+
 void MeshRadio::reloadConfig()
 {
   rf95.setModeIdle(); // Need to be idle before doing init
@@ -91,10 +104,9 @@ void MeshRadio::reloadConfig()
                                                                                  //    setModemConfig(Bw125Cr48Sf4096); // slow and reliable?
   // rf95.setPreambleLength(8);           // Default is 8
 
-  assert(channelSettings.channel_num < NUM_CHANNELS); // If the phone tries to tell us to use an illegal channel then panic
-
   // Defaults after init are 434.0MHz, modulation GFSK_Rb250Fd250, +13dbM
-  float center_freq = CH0 + CH_SPACING * channelSettings.channel_num;
+  int channel_num = hash(channelSettings.name) % NUM_CHANNELS;
+  float center_freq = CH0 + CH_SPACING * channel_num;
   if (!rf95.setFrequency(center_freq))
   {
     DEBUG_MSG("setFrequency failed\n");
@@ -109,7 +121,7 @@ void MeshRadio::reloadConfig()
   // FIXME - can we do this?  It seems to be in the Heltec board.
   rf95.setTxPower(channelSettings.tx_power, false);
 
-  DEBUG_MSG("Set radio: name=%s. config=%u, ch=%d, txpower=%d\n", channelSettings.name, channelSettings.modem_config, channelSettings.channel_num, channelSettings.tx_power);
+  DEBUG_MSG("Set radio: name=%s. config=%u, ch=%d, txpower=%d\n", channelSettings.name, channelSettings.modem_config, channel_num, channelSettings.tx_power);
 
   // Done with init tell radio to start receiving
   rf95.setModeRx();

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;
@@ -173,7 +173,7 @@ void MeshService::handleFromRadio(MeshPacket *mp)
             if (d)
                 releaseToPool(d);
         }
-        assert(toPhoneQueue.enqueue(mp, 0) == pdTRUE); // FIXME, instead of failing for full queue, delete the oldest mssages
+        assert(toPhoneQueue.enqueue(mp, 0)); // FIXME, instead of failing for full queue, delete the oldest mssages
 
         if (mp->payload.want_response)
             sendNetworkPing(mp->from);
@@ -255,6 +255,9 @@ void MeshService::handleToRadio(std::string s)
             break;
         }
     }
+    else {
+        DEBUG_MSG("Error: ignoring malformed toradio\n");
+    }
 }
 
 void MeshService::sendToMesh(MeshPacket *p)
@@ -325,12 +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 (gps.altitude.isValid())
+    // !zero or !zero lat/long means valid
-        pos.altitude = gps.altitude.meters();
+    if(gps.latitude != 0 || gps.longitude != 0) {
-    pos.latitude = gps.location.lat();
+        if (gps.altitude != 0)
-    pos.longitude = gps.location.lng();
+            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;

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/PeriodicTask.cpp

@@ -1,19 +1,21 @@
 #include "PeriodicTask.h"
 #include "Periodic.h"
 
-PeriodicTask::PeriodicTask(uint32_t initialPeriod) : period(initialPeriod)
+PeriodicTask::PeriodicTask(uint32_t initialPeriod) : period(initialPeriod) {}
-{
-}
 
 /// call this from loop
 void PeriodicTask::loop()
 {
-    uint32_t now = millis();
-    if (period && (now - lastMsec) >= period)
     {
-        lastMsec = now;
+        meshtastic::LockGuard lg(&lock);
-        doTask();
+        uint32_t now = millis();
+        if (!period || (now - lastMsec) < period) {
+            return;
         }
+        lastMsec = now;
+    }
+    // Release the lock in case the task wants to change the period.
+    doTask();
 }
 
 void Periodic::doTask()

src/PeriodicTask.h

@@ -1,7 +1,8 @@
 #pragma once
 
-#include <Arduino.h>
+#include <cstdint>
-#include "configuration.h"
+
+#include "lock.h"
 
 /**
  * A base class for tasks that want their doTask() method invoked periodically
@@ -15,9 +16,10 @@ class PeriodicTask
     uint32_t lastMsec = 0;
     uint32_t period = 1; // call soon after creation
 
-public:
+    // Protects the above variables.
-    uint32_t periodMsec;
+    meshtastic::Lock lock;
 
+  public:
     virtual ~PeriodicTask() {}
 
     PeriodicTask(uint32_t initialPeriod = 1);
@@ -26,7 +28,11 @@ public:
     virtual void loop();
 
     /// Set a new period in msecs (can be called from doTask or elsewhere and the scheduler will cope)
-    void setPeriod(uint32_t p) { period = p; }
+    void setPeriod(uint32_t p)
+    {
+        meshtastic::LockGuard lg(&lock);
+        period = p;
+    }
 
   protected:
     virtual void doTask() = 0;

src/PointerQueue.h

@@ -18,7 +18,7 @@ public:
     {
         T *p;
 
-        return this->dequeue(&p, maxWait) == pdTRUE ? p : NULL;
+        return this->dequeue(&p, maxWait) ? p : nullptr;
     }
 
     // returns a ptr or null if the queue was empty
@@ -26,6 +26,6 @@ public:
     {
         T *p;
 
-        return this->dequeueFromISR(&p, higherPriWoken) == pdTRUE ? p : NULL;
+        return this->dequeueFromISR(&p, higherPriWoken) ? p : nullptr;
     }
 };

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,6 +25,7 @@ static void sdsEnter()
 
 static void lsEnter()
 {
+    DEBUG_MSG("lsEnter begin, ls_secs=%u\n", radioConfig.preferences.ls_secs);
     screen.setOn(false);
 
     while (!service.radio.rf95.canSleep())
@@ -32,18 +33,22 @@ 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
-        setGPSPower(false); // kill GPS power
+    // leave GPS on
+    //    setGPSPower(false); // kill GPS power
+
+    DEBUG_MSG("lsEnter end\n");
 }
 
 static void lsIdle()
 {
+    DEBUG_MSG("lsIdle begin ls_secs=%u\n", radioConfig.preferences.ls_secs);
+
     uint32_t secsSlept = 0;
     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;
 
@@ -62,21 +67,25 @@ 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);
+
+        if (!digitalRead(BUTTON_PIN)) // If we woke because of press, instead generate a PRESS event.
         {
-        // Regardless of why we woke just transition to NB (and that state will handle stuff like IRQs etc)
+            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);
         }
     }
+}
 
 static void lsExit()
 {
-    setGPSPower(true); // restore GPS power
+    // setGPSPower(true); // restore GPS power
     gps.startLock();
 }
 
@@ -97,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
+    // Note we don't really use this transition, because when we wake from light sleep we _always_ transition to NB and then it
-    // powerFSM.add_transition(&stateLS, &stateNB, EVENT_RECEIVED_PACKET, NULL, "Received packet");
+    // 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
+    // Handle press events
-    // powerFSM.add_transition(&stateLS, &stateON, EVENT_PRESS, NULL, "Press");
+    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
@@ -155,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

@@ -9,7 +9,7 @@
 #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/TypedQueue.h

@@ -1,15 +1,19 @@
 #pragma once
 
-#include <Arduino.h>
+#include <cassert>
-#include <assert.h>
+#include <type_traits>
+
+#include <freertos/FreeRTOS.h>
+#include <freertos/queue.h>
 
 /**
- * A wrapper for freertos queues.  Note: each element object must be quite small, so T should be only
+ * A wrapper for freertos queues.  Note: each element object should be small
- * pointer types or ints
+ * and POD (Plain Old Data type) as elements are memcpied by value.
  */
 template <class T>
 class TypedQueue
 {
+    static_assert(std::is_pod<T>::value, "T must be pod");
     QueueHandle_t h;
 
   public:
@@ -34,24 +38,22 @@ public:
         return uxQueueMessagesWaiting(h) == 0;
     }
 
-    // pdTRUE for success else failure
+    bool enqueue(T x, TickType_t maxWait = portMAX_DELAY)
-    BaseType_t enqueue(T x, TickType_t maxWait = portMAX_DELAY)
     {
-        return xQueueSendToBack(h, &x, maxWait);
+        return xQueueSendToBack(h, &x, maxWait) == pdTRUE;
     }
 
-    BaseType_t enqueueFromISR(T x, BaseType_t *higherPriWoken)
+    bool enqueueFromISR(T x, BaseType_t *higherPriWoken)
     {
-        return xQueueSendToBackFromISR(h, &x, higherPriWoken);
+        return xQueueSendToBackFromISR(h, &x, higherPriWoken) == pdTRUE;
     }
 
-    // pdTRUE for success else failure
+    bool dequeue(T *p, TickType_t maxWait = portMAX_DELAY)
-    BaseType_t dequeue(T *p, TickType_t maxWait = portMAX_DELAY)
     {
-        return xQueueReceive(h, p, maxWait);
+        return xQueueReceive(h, p, maxWait) == pdTRUE;
     }
 
-    BaseType_t dequeueFromISR(T *p, BaseType_t *higherPriWoken)
+    bool dequeueFromISR(T *p, BaseType_t *higherPriWoken)
     {
         return xQueueReceiveFromISR(h, p, higherPriWoken);
     }

src/configuration.h

@@ -84,14 +84,12 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #define GPS_SERIAL_NUM 1
 #define GPS_BAUDRATE 9600
 
-#if defined(T_BEAM_V10)
 #define GPS_RX_PIN 34
 #ifdef USE_JTAG
 #define GPS_TX_PIN -1
 #else
 #define GPS_TX_PIN 12
 #endif
-#endif
 
 // -----------------------------------------------------------------------------
 // LoRa SPI
@@ -108,6 +106,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 #define BICOLOR_DISPLAY // we have yellow at the top 16 lines
 
+// #define BUTTON_NEED_PULLUP // if set we need to turn on the internal CPU pullup during sleep
+
 #define I2C_SDA 21
 #define I2C_SCL 22
 

src/debug.cpp

@@ -0,0 +1,20 @@
+#include "debug.h"
+
+#include <cstdint>
+
+#include <freertos/FreeRTOS.h>
+#include <freertos/task.h>
+
+#include "configuration.h"
+
+namespace meshtastic
+{
+
+void printThreadInfo(const char *extra)
+{
+    uint32_t taskHandle = reinterpret_cast<uint32_t>(xTaskGetCurrentTaskHandle());
+    DEBUG_MSG("printThreadInfo(%s) task: %" PRIx32 " core id: %u min free stack: %u\n", extra, taskHandle, xPortGetCoreID(),
+              uxTaskGetStackHighWaterMark(nullptr));
+}
+
+} // namespace meshtastic

src/debug.h

@@ -0,0 +1,10 @@
+#pragma once
+
+namespace meshtastic
+{
+
+/// Dumps out which core we are running on, and min level of remaining stack
+/// seen.
+void printThreadInfo(const char *extra);
+
+} // namespace meshtastic

src/lock.cpp

@@ -0,0 +1,35 @@
+#include "lock.h"
+
+#include <cassert>
+
+namespace meshtastic
+{
+
+Lock::Lock()
+{
+    handle = xSemaphoreCreateBinary();
+    assert(handle);
+    assert(xSemaphoreGive(handle));
+}
+
+void Lock::lock()
+{
+    assert(xSemaphoreTake(handle, portMAX_DELAY));
+}
+
+void Lock::unlock()
+{
+    assert(xSemaphoreGive(handle));
+}
+
+LockGuard::LockGuard(Lock *lock) : lock(lock)
+{
+    lock->lock();
+}
+
+LockGuard::~LockGuard()
+{
+    lock->unlock();
+}
+
+} // namespace meshtastic

src/lock.h

@@ -0,0 +1,46 @@
+#pragma once
+
+#include <freertos/FreeRTOS.h>
+#include <freertos/semphr.h>
+
+namespace meshtastic
+{
+
+// Simple wrapper around FreeRTOS API for implementing a mutex lock.
+class Lock
+{
+public:
+  Lock();
+
+  Lock(const Lock &) = delete;
+  Lock &operator=(const Lock &) = delete;
+
+  /// Locks the lock.
+  //
+  // Must not be called from an ISR.
+  void lock();
+
+  // Unlocks the lock.
+  //
+  // Must not be called from an ISR.
+  void unlock();
+
+private:
+  SemaphoreHandle_t handle;
+};
+
+// RAII lock guard.
+class LockGuard
+{
+public:
+  LockGuard(Lock *lock);
+  ~LockGuard();
+
+  LockGuard(const LockGuard &) = delete;
+  LockGuard &operator=(const LockGuard &) = delete;
+
+private:
+  Lock *lock;
+};
+
+} // namespace meshtastic

src/main.cpp

@@ -24,7 +24,6 @@
 #include "configuration.h"
 #include "rom/rtc.h"
 #include <driver/rtc_io.h>
-#include <TinyGPS++.h>
 #include <Wire.h>
 #include "BluetoothUtil.h"
 #include "MeshBluetoothService.h"
@@ -45,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;
@@ -222,8 +229,6 @@ void setup()
   scanI2Cdevice();
 #endif
 
-  axp192Init();
-
   // Buttons & LED
 #ifdef BUTTON_PIN
   pinMode(BUTTON_PIN, INPUT_PULLUP);
@@ -241,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()
@@ -265,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
@@ -341,6 +356,9 @@ void loop()
   // axpDebugOutput.loop();
   loopBLE();
 
+  // for debug printing
+  // service.radio.rf95.canSleep();
+
 #ifdef T_BEAM_V10
   if (axp192_found)
   {
@@ -368,8 +386,7 @@ 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))
   {
@@ -381,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);
     }
   }
@@ -397,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;
+
+// Global Screen singleton.
+extern meshtastic::Screen screen;

src/mesh-pb-constants.cpp

@@ -30,7 +30,7 @@ bool pb_decode_from_bytes(const uint8_t *srcbuf, size_t srcbufsize, const pb_msg
     pb_istream_t stream = pb_istream_from_buffer(srcbuf, srcbufsize);
     if (!pb_decode(&stream, fields, dest_struct))
     {
-        DEBUG_MSG("Error: can't decode protobuf %s\n", PB_GET_ERROR(&stream));
+        DEBUG_MSG("Error: can't decode protobuf %s, pb_msgdesc 0x%p\n", PB_GET_ERROR(&stream), fields);
         return false;
     }
     else

src/mesh.pb.h

@@ -34,7 +34,6 @@ typedef enum _ChannelSettings_ModemConfig {
 /* Struct definitions */
 typedef struct _ChannelSettings {
     int32_t tx_power;
-    uint32_t channel_num;
     ChannelSettings_ModemConfig modem_config;
     pb_byte_t psk[16];
     char name[12];
@@ -173,7 +172,7 @@ typedef struct _ToRadio {
 #define User_init_default                        {"", "", "", {0}}
 #define SubPacket_init_default                   {0, {Position_init_default}, 0}
 #define MeshPacket_init_default                  {0, 0, false, SubPacket_init_default, 0}
-#define ChannelSettings_init_default             {0, 0, _ChannelSettings_ModemConfig_MIN, {0}, ""}
+#define ChannelSettings_init_default             {0, _ChannelSettings_ModemConfig_MIN, {0}, ""}
 #define RadioConfig_init_default                 {false, RadioConfig_UserPreferences_init_default, false, ChannelSettings_init_default}
 #define RadioConfig_UserPreferences_init_default {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
 #define NodeInfo_init_default                    {0, false, User_init_default, false, Position_init_default, 0, 0}
@@ -186,7 +185,7 @@ typedef struct _ToRadio {
 #define User_init_zero                           {"", "", "", {0}}
 #define SubPacket_init_zero                      {0, {Position_init_zero}, 0}
 #define MeshPacket_init_zero                     {0, 0, false, SubPacket_init_zero, 0}
-#define ChannelSettings_init_zero                {0, 0, _ChannelSettings_ModemConfig_MIN, {0}, ""}
+#define ChannelSettings_init_zero                {0, _ChannelSettings_ModemConfig_MIN, {0}, ""}
 #define RadioConfig_init_zero                    {false, RadioConfig_UserPreferences_init_zero, false, ChannelSettings_init_zero}
 #define RadioConfig_UserPreferences_init_zero    {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
 #define NodeInfo_init_zero                       {0, false, User_init_zero, false, Position_init_zero, 0, 0}
@@ -197,7 +196,6 @@ typedef struct _ToRadio {
 
 /* Field tags (for use in manual encoding/decoding) */
 #define ChannelSettings_tx_power_tag             1
-#define ChannelSettings_channel_num_tag          2
 #define ChannelSettings_modem_config_tag         3
 #define ChannelSettings_psk_tag                  4
 #define ChannelSettings_name_tag                 5
@@ -303,7 +301,6 @@ X(a, STATIC,   SINGULAR, UINT32,   rx_time,           4)
 
 #define ChannelSettings_FIELDLIST(X, a) \
 X(a, STATIC,   SINGULAR, INT32,    tx_power,          1) \
-X(a, STATIC,   SINGULAR, UINT32,   channel_num,       2) \
 X(a, STATIC,   SINGULAR, UENUM,    modem_config,      3) \
 X(a, STATIC,   SINGULAR, FIXED_LENGTH_BYTES, psk,               4) \
 X(a, STATIC,   SINGULAR, STRING,   name,              5)
@@ -421,12 +418,12 @@ extern const pb_msgdesc_t ToRadio_msg;
 #define User_size                                72
 #define SubPacket_size                           261
 #define MeshPacket_size                          292
-#define ChannelSettings_size                     50
+#define ChannelSettings_size                     44
-#define RadioConfig_size                         126
+#define RadioConfig_size                         120
 #define RadioConfig_UserPreferences_size         72
 #define NodeInfo_size                            155
 #define MyNodeInfo_size                          63
-#define DeviceState_size                         15064
+#define DeviceState_size                         15058
 #define FromRadio_size                           301
 #define ToRadio_size                             295
 

src/screen.cpp

@@ -20,54 +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 "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
-
+#define TRANSITION_FRAMERATE 30 // fps
-#ifdef I2C_SDA
+#define IDLE_FRAMERATE 10       // in fps
-SSD1306Wire dispdev(SSD1306_ADDRESS, I2C_SDA, I2C_SCL);
+#define COMPASS_DIAM 44
-#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 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;
+namespace meshtastic
-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)
 {
-    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
@@ -78,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";
+static void drawFrameBluetooth(OLEDDisplay *display, OLEDDisplayUiState *state, int16_t x, int16_t y)
-
-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");
@@ -98,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.
+    // with the third parameter you can define the width after which words will
-    // Currently only spaces and "-" are allowed for wrapping
+    // 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;
         }
@@ -182,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....
+        xo += SCREEN_WIDTH / COLUMNS;
-        if (xo >= SCREEN_WIDTH)
+        // Wrap to next row, if needed.
-        {
+        if (++col > COLUMNS) {
+            xo = x;
             yo += FONT_HEIGHT;
-            xo = 0;
+            col = 0;
         }
         f++;
     }
@@ -206,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?)
+/// Ported from my old java code, returns distance in meters along the globe
-float latLongToMeter(double lat_a, double lng_a, double lat_b, double lng_b)
+/// 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;
@@ -216,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 =
+    double t1 = cos_a1 * cos(a2) * cos_b1 * cos(b2);
-        cos_a1 * cos(a2) * cos_b1 * cos(b2);
+    double t2 = cos_a1 * sin(a2) * cos_b1 * sin(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))
@@ -228,18 +170,19 @@ 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.
+ * Computes the bearing in degrees between two points on Earth.  Ported from my
+ * old Gaggle android app.
  *
  * @param lat1
  * Latitude of the first point
@@ -252,7 +195,7 @@ inline double toDegrees(double r)
  * @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)
+static float bearing(double lat1, double lon1, double lat2, double lon2)
 {
     double lat1Rad = toRadians(lat1);
     double lat2Rad = toRadians(lat2);
@@ -262,6 +205,9 @@ float bearing(double lat1, double lon1, double lat2, double lon2)
     return atan2(y, x);
 }
 
+namespace
+{
+
 /// A basic 2D point class for drawing
 class Point
 {
@@ -273,10 +219,8 @@ public:
     /// Apply a rotation around zero (standard rotation matrix math)
     void rotate(float radian)
     {
-        float cos = cosf(radian),
+        float cos = cosf(radian), sin = sinf(radian);
-              sin = sinf(radian);
+        float rx = x * cos - y * sin, ry = x * sin + y * cos;
-        float rx = x * cos - y * sin,
-              ry = x * sin + y * cos;
 
         x = rx;
         y = ry;
@@ -295,7 +239,9 @@ 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);
 }
@@ -303,15 +249,15 @@ void drawLine(OLEDDisplay *d, const Point &p1, const Point &p2)
 /**
  * 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;
@@ -330,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
+/// Sometimes we will have Position objects that only have a time, so check for
-bool hasPosition(NodeInfo *n)
+/// 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();
         }
@@ -379,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)
@@ -394,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[] = {
+    const char *fields[] = {username, distStr, signalStr, lastStr, NULL};
-        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;
@@ -418,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);
@@ -431,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);
 
@@ -450,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[] = {
+    const char *fields[] = {batStr, gpsStr, usersStr, channelStr, NULL};
-        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()
 {
@@ -500,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 != screenOn) {
-    {
+        if (on) {
-        if (on)
-        {
             DEBUG_MSG("Turning on screen\n");
             dispdev.displayOn();
-            setPeriod(1); // redraw ASAP
+        } else {
-        }
-        else {
             DEBUG_MSG("Turning off screen\n");
             dispdev.displayOff();
         }
@@ -521,166 +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
+    if (!useDisplay)
-    // Display instance
+        return;
-    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
 
+// 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
+    // Add frames.
-#endif
+    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)
+    // Process incoming commands.
-    { // If we didn't just wake and the screen is still off, then stop updating until it is on again
+    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
+    // but we should only call setTargetFPS when framestate changes, because
-    // animations.
+    // otherwise that breaks animations.
-    if (targetFramerate != IDLE_FRAMERATE && ui.getUiState()->frameState == FIXED)
+    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
+    // While showing the bootscreen or Bluetooth pair screen all of our
-    if (!showingBluetooth)
+    // standard screen switching is stopped.
-    {
+    if (showingNormalScreen) {
-        // Once we finish showing the bootscreen, remove it from the loop
+        // TODO(girts): decouple nodeDB from screen.
-        if (showingBootScreen)
+        // standard screen loop handling ehre
-        {
-            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)
+        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);
+    // DEBUG_MSG("want fps %d, fixed=%d\n", targetFramerate,
-    // If we are scrolling we need to be called soon, otherwise just 1 fps (to save CPU)
+    // ui.getUiState()->frameState); If we are scrolling we need to be called
-    // We also ask to be called twice as fast as we really need so that any rounding errors still result
+    // soon, otherwise just 1 fps (to save CPU) We also ask to be called twice
-    // with the correct framerate
+    // 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)
@@ -691,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);
+    ui.setFrames(normalFrames, numframes);
-    showingBluetooth = false;
+    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::handleStartBluetoothPinScreen(uint32_t pin)
-void Screen::onPress()
+{
+    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();
 
@@ -723,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);
+namespace meshtastic
-void screen_print(const char * text, uint8_t x, uint8_t y, uint8_t alignment);
+{
 
-
+/// Deals with showing things on the screen of the device.
-// Show the bluetooth PIN screen
+//
-void screen_start_bluetooth(uint32_t pin); 
+// Other than setup(), this class is thread-safe. All state-changing calls are
-
+// queued and executed when the main loop calls us.
-// restore our regular frame list
-void screen_set_frames();
-
-
-/**
- * Slowly I'm moving screen crap into this class
- */
 class Screen : public PeriodicTask
 {
   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
+    /// Handles a button press.
-    void doWakeScreen();
+    void onPress() { enqueueCmd(CmdItem{.cmd = Cmd::ON_PRESS}); }
 
-    /// Is the screen currently on
+    /// Starts showing the Bluetooth PIN screen.
-    bool isOn();
+    //
+    // 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
+    /// Stops showing the bluetooth PIN screen.
-    void setOn(bool on);
+    void stopBluetoothPinScreen() { enqueueCmd(CmdItem{.cmd = Cmd::STOP_BLUETOOTH_PIN_SCREEN}); }
 
-    /// Handle a button press
+    /// Stops showing the boot screen.
-    void onPress();
+    void stopBootScreen() { enqueueCmd(CmdItem{.cmd = Cmd::STOP_BOOT_SCREEN}); }
+
+    /// 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;
 
-    /// Rebuilt our list of screens
-    void setFrames();
   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;
+        };
     };
 
-extern Screen screen;
+    /// 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;
+};
+
+} // namespace meshtastic

src/sleep.cpp

@@ -1,13 +1,11 @@
 #include "configuration.h"
 #include "rom/rtc.h"
 #include <driver/rtc_io.h>
-#include <TinyGPS++.h>
 #include <Wire.h>
 #include "BluetoothUtil.h"
 #include "MeshBluetoothService.h"
 #include "MeshService.h"
 #include "GPS.h"
-#include "screen.h"
 #include "NodeDB.h"
 #include "Periodic.h"
 #include "esp32/pm.h"
@@ -177,6 +175,10 @@ void doDeepSleep(uint64_t msecToWake)
   // Only GPIOs which are have RTC functionality can be used in this bit map: 0,2,4,12-15,25-27,32-39.
   uint64_t gpioMask = (1ULL << BUTTON_PIN);
 
+#ifdef BUTTON_NEED_PULLUP
+  gpio_pullup_en((gpio_num_t) BUTTON_PIN);
+#endif
+
   // Not needed because both of the current boards have external pullups
   // FIXME change polarity in hw so we can wake on ANY_HIGH instead - that would allow us to use all three buttons (instead of just the first)
   // gpio_pullup_en((gpio_num_t)BUTTON_PIN);
@@ -206,15 +208,20 @@ esp_sleep_wakeup_cause_t doLightSleep(uint64_t sleepMsec) // FIXME, use a more r
   // We want RTC peripherals to stay on
   esp_sleep_pd_config(ESP_PD_DOMAIN_RTC_PERIPH, ESP_PD_OPTION_ON);
 
+#ifdef BUTTON_NEED_PULLUP
+  gpio_pullup_en((gpio_num_t) BUTTON_PIN);
+#endif
+
   gpio_wakeup_enable((gpio_num_t)BUTTON_PIN, GPIO_INTR_LOW_LEVEL); // when user presses, this button goes low
   gpio_wakeup_enable((gpio_num_t)DIO0_GPIO, GPIO_INTR_HIGH_LEVEL); // RF95 interrupt, active high
 #ifdef PMU_IRQ
-  gpio_wakeup_enable((gpio_num_t)PMU_IRQ, GPIO_INTR_HIGH_LEVEL); // pmu irq
+  // FIXME, disable wake due to PMU because it seems to fire all the time?
+  // gpio_wakeup_enable((gpio_num_t)PMU_IRQ, GPIO_INTR_HIGH_LEVEL); // pmu irq
 #endif
   assert(esp_sleep_enable_gpio_wakeup() == ESP_OK);
   assert(esp_sleep_enable_timer_wakeup(sleepUsec) == ESP_OK);
   assert(esp_light_sleep_start() == ESP_OK);
-  //DEBUG_MSG("Exit light sleep\n");
+  //DEBUG_MSG("Exit light sleep b=%d, rf95=%d, pmu=%d\n", digitalRead(BUTTON_PIN), digitalRead(DIO0_GPIO), digitalRead(PMU_IRQ));
   return esp_sleep_get_wakeup_cause();
 }