move streamapi into a thread, saves power and increases responsiveness

src/SerialConsole.cpp

@@ -6,25 +6,28 @@
 
 #define Port Serial
 
-SerialConsole console;
+SerialConsole *console;
+
+void consoleInit()
+{
+    new SerialConsole(); // Must be dynamically allocated because we are now inheriting from thread
+}
 
 void consolePrintf(const char *format, ...)
 {
     va_list arg;
     va_start(arg, format);
-    console.vprintf(format, arg);
+    console->vprintf(format, arg);
     va_end(arg);
 }
 
 SerialConsole::SerialConsole() : StreamAPI(&Port), RedirectablePrint(&Port)
 {
+    assert(!console);
+    console = this;
     canWrite = false; // We don't send packets to our port until it has talked to us first
                       // setDestination(&noopPrint); for testing, try turning off 'all' debug output and see what leaks
-}
 
-/// Do late init that can't happen at constructor time
-void SerialConsole::init()
-{
     Port.begin(SERIAL_BAUD);
     StreamAPI::init();
     emitRebooted();
@@ -34,14 +37,14 @@ void SerialConsole::init()
  * we override this to notice when we've received a protobuf over the serial
  * stream.  Then we shunt off debug serial output.
  */
-void SerialConsole::handleToRadio(const uint8_t *buf, size_t len)
+bool SerialConsole::handleToRadio(const uint8_t *buf, size_t len)
 {
     // Turn off debug serial printing once the API is activated, because other threads could print and corrupt packets
     if (!radioConfig.preferences.debug_log_enabled)
         setDestination(&noopPrint);
     canWrite = true;
 
-    StreamAPI::handleToRadio(buf, len);
+    return StreamAPI::handleToRadio(buf, len);
 }
 
 /// Hookable to find out when connection changes

src/SerialConsole.h

@@ -11,14 +11,11 @@ class SerialConsole : public StreamAPI, public RedirectablePrint
   public:
     SerialConsole();
 
-    /// Do late init that can't happen at constructor time
-    virtual void init();
-
     /**
      * we override this to notice when we've received a protobuf over the serial stream.  Then we shunt off
      * debug serial output.
      */
-    virtual void handleToRadio(const uint8_t *buf, size_t len);
+    virtual bool handleToRadio(const uint8_t *buf, size_t len);
 
     virtual size_t write(uint8_t c)
     {
@@ -34,5 +31,6 @@ class SerialConsole : public StreamAPI, public RedirectablePrint
 
 // A simple wrapper to allow non class aware code write to the console
 void consolePrintf(const char *format, ...);
+void consoleInit();
 
-extern SerialConsole console;
+extern SerialConsole *console;

src/concurrency/BinarySemaphoreFreeRTOS.h

@@ -1,6 +1,5 @@
 #pragma once
 
-#include "configuration.h"
 #include "../freertosinc.h"
 
 namespace concurrency
@@ -28,4 +27,4 @@ class BinarySemaphoreFreeRTOS
 
 #endif
 
-} 
+} // namespace concurrency

src/configuration.h

@@ -457,7 +457,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 #include "SerialConsole.h"
 
-#define DEBUG_PORT console // Serial debug port
+#define DEBUG_PORT (*console) // Serial debug port
 
 // What platforms should use SEGGER?
 #ifdef NRF52_SERIES

src/main.cpp

@@ -177,6 +177,7 @@ class ButtonThread : public OSThread
     OneButton userButtonAlt;
 #endif
     static bool shutdown_on_long_stop;
+
   public:
     static uint32_t longPressTime;
 
@@ -315,9 +316,8 @@ void setup()
     SEGGER_RTT_ConfigUpBuffer(0, NULL, NULL, 0, SEGGER_RTT_MODE_NO_BLOCK_TRIM);
 #endif
 
-// Debug
 #ifdef DEBUG_PORT
-    DEBUG_PORT.init(); // Set serial baud rate and init our mesh console
+    consoleInit(); // Set serial baud rate and init our mesh console
 #endif
 
     initDeepSleep();
@@ -580,10 +580,6 @@ void loop()
 {
     // axpDebugOutput.loop();
 
-#ifdef DEBUG_PORT
-    DEBUG_PORT.loop(); // Send/receive protobufs over the serial port
-#endif
-
     // heap_caps_check_integrity_all(true); // FIXME - disable this expensive check
 
 #ifndef NO_ESP32

src/mesh/MeshService.cpp

@@ -176,8 +176,7 @@ void MeshService::sendNetworkPing(NodeNum dest, bool wantReplies)
             DEBUG_MSG("Sending position ping to 0x%x, wantReplies=%d\n", dest, wantReplies);
             positionPlugin->sendOurPosition(dest, wantReplies);
         }
-    }
+    } else {
-    else {
         if (nodeInfoPlugin) {
             DEBUG_MSG("Sending nodeinfo ping to 0x%x, wantReplies=%d\n", dest, wantReplies);
             nodeInfoPlugin->sendOurNodeInfo(dest, wantReplies);

src/mesh/MeshService.h

@@ -84,13 +84,12 @@ class MeshService
     NodeInfo *refreshMyNodeInfo();
 
   private:
-
     /// Called when our gps position has changed - updates nodedb and sends Location message out into the mesh
     /// returns 0 to allow futher processing
     int onGPSChanged(const meshtastic::GPSStatus *arg);
 
-    /// Handle a packet that just arrived from the radio.  This method does _ReliableRouternot_ free the provided packet.  If it needs
+    /// Handle a packet that just arrived from the radio.  This method does _ReliableRouternot_ free the provided packet.  If it
-    /// to keep the packet around it makes a copy
+    /// needs to keep the packet around it makes a copy
     int handleFromRadio(const MeshPacket *p);
     friend class RoutingPlugin;
 };

src/mesh/PhoneAPI.cpp

@@ -1,10 +1,10 @@
 #include "PhoneAPI.h"
+#include "Channels.h"
 #include "GPS.h"
 #include "MeshService.h"
 #include "NodeDB.h"
 #include "PowerFSM.h"
 #include "RadioInterface.h"
-#include "Channels.h"
 #include <assert.h>
 
 #if FromRadio_size > MAX_TO_FROM_RADIO_SIZE
@@ -22,11 +22,13 @@ void PhoneAPI::init()
     observe(&service.fromNumChanged);
 }
 
-PhoneAPI::~PhoneAPI() {
+PhoneAPI::~PhoneAPI()
+{
     close();
 }
 
-void PhoneAPI::close() {
+void PhoneAPI::close()
+{
     unobserve();
     state = STATE_SEND_NOTHING;
     bool oldConnected = isConnected;
@@ -49,7 +51,7 @@ void PhoneAPI::checkConnectionTimeout()
 /**
  * Handle a ToRadio protobuf
  */
-void PhoneAPI::handleToRadio(const uint8_t *buf, size_t bufLength)
+bool PhoneAPI::handleToRadio(const uint8_t *buf, size_t bufLength)
 {
     powerFSM.trigger(EVENT_CONTACT_FROM_PHONE); // As long as the phone keeps talking to us, don't let the radio go to sleep
     lastContactMsec = millis();
@@ -62,12 +64,9 @@ void PhoneAPI::handleToRadio(const uint8_t *buf, size_t bufLength)
     memset(&toRadioScratch, 0, sizeof(toRadioScratch));
     if (pb_decode_from_bytes(buf, bufLength, ToRadio_fields, &toRadioScratch)) {
         switch (toRadioScratch.which_payloadVariant) {
-        case ToRadio_packet_tag: {
+        case ToRadio_packet_tag:
-            MeshPacket &p = toRadioScratch.packet;
+            return handleToRadioPacket(toRadioScratch.packet);
-            printPacket("PACKET FROM PHONE", &p);
+
-            service.handleToRadio(p);
-            break;
-        }
         case ToRadio_want_config_id_tag:
             config_nonce = toRadioScratch.want_config_id;
             DEBUG_MSG("Client wants config, nonce=%u\n", config_nonce);
@@ -86,6 +85,8 @@ void PhoneAPI::handleToRadio(const uint8_t *buf, size_t bufLength)
     } else {
         DEBUG_MSG("Error: ignoring malformed toradio\n");
     }
+
+    return false;
 }
 
 /**
@@ -226,7 +227,13 @@ bool PhoneAPI::available()
 /**
  * Handle a packet that the phone wants us to send.  It is our responsibility to free the packet to the pool
  */
-void PhoneAPI::handleToRadioPacket(MeshPacket *p) {}
+bool PhoneAPI::handleToRadioPacket(MeshPacket &p)
+{
+    printPacket("PACKET FROM PHONE", &p);
+    service.handleToRadio(p);
+
+    return true;
+}
 
 /// If the mesh service tells us fromNum has changed, tell the phone
 int PhoneAPI::onNotify(uint32_t newValue)

src/mesh/PhoneAPI.h

@@ -67,8 +67,9 @@ class PhoneAPI
 
     /**
      * Handle a ToRadio protobuf
+     * @return true true if a packet was queued for sending (so that caller can yield)
      */
-    virtual void handleToRadio(const uint8_t *buf, size_t len);
+    virtual bool handleToRadio(const uint8_t *buf, size_t len);
 
     /**
      * Get the next packet we want to send to the phone
@@ -103,9 +104,10 @@ class PhoneAPI
 
   private:
     /**
-     * Handle a packet that the phone wants us to send.  It is our responsibility to free the packet to the pool
+     * Handle a packet that the phone wants us to send.  We can write to it but can not keep a reference to it
+     * @return true true if a packet was queued for sending
      */
-    void handleToRadioPacket(MeshPacket *p);
+    bool handleToRadioPacket(MeshPacket &p);
 
     /// If the mesh service tells us fromNum has changed, tell the phone
     virtual int onNotify(uint32_t newValue);

src/mesh/StreamAPI.cpp

@@ -5,18 +5,25 @@
 #define START2 0xc3
 #define HEADER_LEN 4
 
-void StreamAPI::loop()
+int32_t StreamAPI::runOnce()
 {
+    auto result = readStream();
     writeStream();
-    readStream();
     checkConnectionTimeout();
+    return result;
 }
 
 /**
  * Read any rx chars from the link and call handleToRadio
  */
-void StreamAPI::readStream()
+int32_t StreamAPI::readStream()
 {
+    uint32_t now = millis();
+    if (!stream->available()) {
+        // Nothing available this time, if the computer has talked to us recently, poll often, otherwise let CPU sleep a long time
+        bool recentRx = (now - lastRxMsec) < 2000;
+        return recentRx ? 5 : 250;
+    } else {
         while (stream->available()) { // Currently we never want to block
             uint8_t c = stream->read();
 
@@ -42,12 +49,19 @@ void StreamAPI::readStream()
                 }
 
                 if (rxPtr != 0 && ptr + 1 == len + HEADER_LEN) {
+                    rxPtr = 0; // start over again on the next packet
+
                     // If we didn't just fail the packet and we now have the right # of bytes, parse it
-                handleToRadio(rxBuf + HEADER_LEN, len);
+                    if (handleToRadio(rxBuf + HEADER_LEN, len))
-                rxPtr = 0; // start over again
+                        return 0; // we want to be called again ASAP because we still have more work to do
                 }
             }
         }
+
+        // we had packets available this time, so assume we might have them next time also
+        lastRxMsec = now;
+        return 0;
+    }
 }
 
 /**
@@ -71,7 +85,7 @@ void StreamAPI::writeStream()
 void StreamAPI::emitTxBuffer(size_t len)
 {
     if (len != 0) {
-        DEBUG_MSG("emit tx %d\n", len);
+        // DEBUG_MSG("emit tx %d\n", len);
         txBuf[0] = START1;
         txBuf[1] = START2;
         txBuf[2] = (len >> 8) & 0xff;
@@ -93,6 +107,6 @@ void StreamAPI::emitRebooted()
     fromRadioScratch.which_payloadVariant = FromRadio_rebooted_tag;
     fromRadioScratch.rebooted = true;
 
-    DEBUG_MSG("Emitting reboot packet for serial shell\n");
+    // DEBUG_MSG("Emitting reboot packet for serial shell\n");
     emitTxBuffer(pb_encode_to_bytes(txBuf + HEADER_LEN, FromRadio_size, FromRadio_fields, &fromRadioScratch));
 }

src/mesh/StreamAPI.h

@@ -2,6 +2,7 @@
 
 #include "PhoneAPI.h"
 #include "Stream.h"
+#include "concurrency/OSThread.h"
 
 // A To/FromRadio packet + our 32 bit header
 #define MAX_STREAM_BUF_SIZE (MAX_TO_FROM_RADIO_SIZE + sizeof(uint32_t))
@@ -27,7 +28,7 @@ valid utf8 encoding. This makes it a bit easier to start a device outputting reg
 after it has received a valid packet from the PC, turn off unencoded debug printing and switch to this packet encoding.
 
  */
-class StreamAPI : public PhoneAPI
+class StreamAPI : public PhoneAPI, protected concurrency::OSThread
 {
     /**
      * The stream we read/write from
@@ -37,21 +38,23 @@ class StreamAPI : public PhoneAPI
     uint8_t rxBuf[MAX_STREAM_BUF_SIZE];
     size_t rxPtr = 0;
 
+    /// time of last rx, used, to slow down our polling if we haven't heard from anyone
+    uint32_t lastRxMsec = 0;
+
   public:
-    StreamAPI(Stream *_stream) : stream(_stream) {}
+    StreamAPI(Stream *_stream) : concurrency::OSThread("StreamAPI"), stream(_stream) {}
 
     /**
      * Currently we require frequent invocation from loop() to check for arrived serial packets and to send new packets to the
      * phone.
-     * FIXME, to support better power behavior instead move to a thread and block on serial reads.
      */
-    void loop();
+    virtual int32_t runOnce();
 
   private:
     /**
      * Read any rx chars from the link and call handleToRadio
      */
-    void readStream();
+    int32_t readStream();
 
     /**
      * call getFromRadio() and deliver encapsulated packets to the Stream

src/mesh/wifi/WiFiServerAPI.cpp

@@ -40,18 +40,20 @@ void WiFiServerAPI::onConnectionChanged(bool connected)
 }
 
 /// override close to also shutdown the TCP link
-void WiFiServerAPI::close() {
+void WiFiServerAPI::close()
+{
     client.stop(); // drop tcp connection
     StreamAPI::close();
 }
 
-bool WiFiServerAPI::loop()
+int32_t WiFiServerAPI::runOnce()
 {
     if (client.connected()) {
-        StreamAPI::loop();
+        return StreamAPI::runOnce();
-        return true;
     } else {
-        return false;       
+        DEBUG_MSG("Client dropped connection, suspending API service\n");
+        enabled = false; // we no longer need to run
+        return 0;
     }
 }
 
@@ -78,18 +80,5 @@ int32_t WiFiServerPort::runOnce()
         openAPI = new WiFiServerAPI(client);
     }
 
-    if (openAPI) {
-        // Allow idle processing so the API can read from its incoming stream
-        if(!openAPI->loop()) {
-            // If our API link was up, shut it down
-
-            DEBUG_MSG("Client dropped connection, closing API client\n");
-            // Note: we can't call delete here because this object includes other state
-            // besides the stream API.  Instead kill it later when we start a new instance
-            delete openAPI;
-            openAPI = NULL;
-        }
-        return 0; // run fast while our API server is running
-    } else
     return 100; // only check occasionally for incoming connections
 }

src/mesh/wifi/WiFiServerAPI.h

@@ -1,7 +1,6 @@
 #pragma once
 
 #include "StreamAPI.h"
-#include "concurrency/OSThread.h"
 #include <WiFi.h>
 
 /**
@@ -18,15 +17,14 @@ class WiFiServerAPI : public StreamAPI
 
     virtual ~WiFiServerAPI();
 
-    /// @return true if we want to keep running, or false if we are ready to be destroyed
-    virtual bool loop(); // Check for dropped client connections
-
     /// override close to also shutdown the TCP link
     virtual void close();
 
   protected:
     /// Hookable to find out when connection changes
     virtual void onConnectionChanged(bool connected);
+
+    virtual int32_t runOnce(); // Check for dropped client connections
 };
 
 /**