PhoneAPIs shouldn't register for messages until they have clients

docs/software/TODO.md

@@ -4,6 +4,8 @@ You probably don't care about this section - skip to the next one.
 
 ## before next release
 
+* turn off bluetooth interface ENTIRELY while using serial API (was python client times out on connect sometimes)
+* gps assistance from phone not working?
 * DONE test latest firmware update with is_router
 * DONE firmware OTA updates of is_router true nodes fails?
 * DONE add UI in android app to reset to defaults https://github.com/meshtastic/Meshtastic-Android/issues/263 

proto

@@ -1 +1 @@
-Subproject commit 820fa497dfde07e129cad6955bf2f4b2b9cecebc
+Subproject commit e8d2a96a00713608ba4a6a36c9bea4ce06886619

src/SerialConsole.cpp

@@ -29,7 +29,6 @@ SerialConsole::SerialConsole() : StreamAPI(&Port), RedirectablePrint(&Port)
                       // setDestination(&noopPrint); for testing, try turning off 'all' debug output and see what leaks
 
     Port.begin(SERIAL_BAUD);
-    StreamAPI::init();
     emitRebooted();
 }
 

src/mesh/PhoneAPI.cpp

@@ -17,34 +17,45 @@
 
 PhoneAPI::PhoneAPI() {}
 
-void PhoneAPI::init()
-{
-    observe(&service.fromNumChanged);
-}
-
 PhoneAPI::~PhoneAPI()
 {
     close();
 }
 
+void PhoneAPI::handleStartConfig()
+{
+    if (!isConnected()) {
+        onConnectionChanged(true);
+        observe(&service.fromNumChanged);
+    }
+
+    // even if we were already connected - restart our state machine
+    state = STATE_SEND_MY_INFO;
+
+    DEBUG_MSG("Reset nodeinfo read pointer\n");
+    nodeInfoForPhone = NULL;   // Don't keep returning old nodeinfos
+    nodeDB.resetReadPointer(); // FIXME, this read pointer should be moved out of nodeDB and into this class - because
+                               // this will break once we have multiple instances of PhoneAPI running independently
+}
+
 void PhoneAPI::close()
 {
-    unobserve();
-    state = STATE_SEND_NOTHING;
-    bool oldConnected = isConnected;
-    isConnected = false;
-    if (oldConnected != isConnected)
-        onConnectionChanged(isConnected);
+    if (state != STATE_SEND_NOTHING) {
+        state = STATE_SEND_NOTHING;
+
+        unobserve();
+        releasePhonePacket(); // Don't leak phone packets on shutdown
+
+        onConnectionChanged(false);
+    }
 }
 
 void PhoneAPI::checkConnectionTimeout()
 {
-    if (isConnected) {
+    if (isConnected()) {
         bool newConnected = (millis() - lastContactMsec < getPref_phone_timeout_secs() * 1000L);
-        if (!newConnected) {
-            isConnected = false;
-            onConnectionChanged(isConnected);
-        }
+        if (!newConnected)
+            close();
     }
 }
 
@@ -55,10 +66,7 @@ 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();
-    if (!isConnected) {
-        isConnected = true;
-        onConnectionChanged(isConnected);
-    }
+
     // return (lastContactMsec != 0) &&
 
     memset(&toRadioScratch, 0, sizeof(toRadioScratch));
@@ -70,12 +78,12 @@ bool PhoneAPI::handleToRadio(const uint8_t *buf, size_t bufLength)
         case ToRadio_want_config_id_tag:
             config_nonce = toRadioScratch.want_config_id;
             DEBUG_MSG("Client wants config, nonce=%u\n", config_nonce);
-            state = STATE_SEND_MY_INFO;
 
-            DEBUG_MSG("Reset nodeinfo read pointer\n");
-            nodeInfoForPhone = NULL;   // Don't keep returning old nodeinfos
-            nodeDB.resetReadPointer(); // FIXME, this read pointer should be moved out of nodeDB and into this class - because
-                                       // this will break once we have multiple instances of PhoneAPI running independently
+            handleStartConfig();
+            break;
+
+        case ToRadio_disconnect_tag:
+            close();
             break;
 
         default:
@@ -166,10 +174,8 @@ size_t PhoneAPI::getFromRadio(uint8_t *buf)
             // Encapsulate as a FromRadio packet
             fromRadioScratch.which_payloadVariant = FromRadio_packet_tag;
             fromRadioScratch.packet = *packetForPhone;
-
-            service.releaseToPool(packetForPhone); // we just copied the bytes, so don't need this buffer anymore
-            packetForPhone = NULL;
         }
+        releasePhonePacket();
         break;
 
     default:
@@ -188,6 +194,16 @@ size_t PhoneAPI::getFromRadio(uint8_t *buf)
     return 0;
 }
 
+void PhoneAPI::handleDisconnect() {}
+
+void PhoneAPI::releasePhonePacket()
+{
+    if (packetForPhone) {
+        service.releaseToPool(packetForPhone); // we just copied the bytes, so don't need this buffer anymore
+        packetForPhone = NULL;
+    }
+}
+
 /**
  * Return true if we have data available to send to the phone
  */

src/mesh/PhoneAPI.h

@@ -22,6 +22,7 @@ class PhoneAPI
     enum State {
         STATE_UNUSED,       // (no longer used) old default state - until Android apps are all updated, uses the old BLE API
         STATE_SEND_NOTHING, // (Eventual) Initial state, don't send anything until the client starts asking for config
+                            // (disconnected)
         STATE_SEND_MY_INFO, // send our my info record
         // STATE_SEND_RADIO, // in 1.2 we now send this as a regular mesh packet
         // STATE_SEND_OWNER, no need to send Owner specially, it is just part of the nodedb
@@ -58,9 +59,6 @@ class PhoneAPI
     /// Destructor - calls close()
     virtual ~PhoneAPI();
 
-    /// Do late init that can't happen at constructor time
-    virtual void init();
-
     // Call this when the client drops the connection, resets the state to STATE_SEND_NOTHING
     // Unregisters our observer.  A closed connection **can** be reopened by calling init again.
     virtual void close();
@@ -84,10 +82,9 @@ class PhoneAPI
      */
     bool available();
 
-  protected:
-    /// Are we currently connected to a client?
-    bool isConnected = false;
+    bool isConnected() { return state != STATE_SEND_NOTHING; }
 
+  protected:
     /// Our fromradio packet while it is being assembled
     FromRadio fromRadioScratch;
 
@@ -102,7 +99,17 @@ class PhoneAPI
      */
     virtual void onNowHasData(uint32_t fromRadioNum) {}
 
+    /**
+     * Subclasses can use this to find out when a client drops the link
+     */
+    virtual void handleDisconnect();
+
   private:
+    void releasePhonePacket();
+
+    /// begin a new connection
+    void handleStartConfig();
+
     /**
      * 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

src/mesh/generated/mesh.pb.h

@@ -191,6 +191,7 @@ typedef struct _ToRadio {
     union {
         MeshPacket packet;
         uint32_t want_config_id;
+        bool disconnect;
     };
 } ToRadio;
 
@@ -311,6 +312,7 @@ extern "C" {
 #define FromRadio_packet_tag                     11
 #define ToRadio_packet_tag                       2
 #define ToRadio_want_config_id_tag               100
+#define ToRadio_disconnect_tag                   104
 
 /* Struct field encoding specification for nanopb */
 #define Position_FIELDLIST(X, a) \
@@ -423,7 +425,8 @@ X(a, STATIC,   ONEOF,    MESSAGE,  (payloadVariant,packet,packet),  11)
 
 #define ToRadio_FIELDLIST(X, a) \
 X(a, STATIC,   ONEOF,    MESSAGE,  (payloadVariant,packet,packet),   2) \
-X(a, STATIC,   ONEOF,    UINT32,   (payloadVariant,want_config_id,want_config_id), 100)
+X(a, STATIC,   ONEOF,    UINT32,   (payloadVariant,want_config_id,want_config_id), 100) \
+X(a, STATIC,   ONEOF,    BOOL,     (payloadVariant,disconnect,disconnect), 104)
 #define ToRadio_CALLBACK NULL
 #define ToRadio_DEFAULT NULL
 #define ToRadio_payloadVariant_packet_MSGTYPE MeshPacket

src/nimble/BluetoothUtil.cpp

@@ -487,7 +487,6 @@ void reinitBluetooth()
     DEBUG_MSG("Starting bluetooth\n");
     if (isFirstTime) {
         bluetoothPhoneAPI = new BluetoothPhoneAPI();
-        bluetoothPhoneAPI->init();
     }
 
     // FIXME - if waking from light sleep, only esp_nimble_hci_init?