From 0261c243e0a8cc4d75ca872223625bfae7ccbb87 Mon Sep 17 00:00:00 2001 From: Kevin Hester Date: Thu, 25 Mar 2021 07:51:54 +0800 Subject: [PATCH] PhoneAPIs shouldn't register for messages until they have clients --- docs/software/TODO.md | 2 + proto | 2 +- src/SerialConsole.cpp | 1 - src/mesh/PhoneAPI.cpp | 72 ++++++++++++++++++++++-------------- src/mesh/PhoneAPI.h | 19 +++++++--- src/mesh/generated/mesh.pb.h | 5 ++- src/nimble/BluetoothUtil.cpp | 1 - 7 files changed, 64 insertions(+), 38 deletions(-) diff --git a/docs/software/TODO.md b/docs/software/TODO.md index 2633d3bcf..39dfa69c7 100644 --- a/docs/software/TODO.md +++ b/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 diff --git a/proto b/proto index 820fa497d..e8d2a96a0 160000 --- a/proto +++ b/proto @@ -1 +1 @@ -Subproject commit 820fa497dfde07e129cad6955bf2f4b2b9cecebc +Subproject commit e8d2a96a00713608ba4a6a36c9bea4ce06886619 diff --git a/src/SerialConsole.cpp b/src/SerialConsole.cpp index 11534b1ee..137fbcf0d 100644 --- a/src/SerialConsole.cpp +++ b/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(); } diff --git a/src/mesh/PhoneAPI.cpp b/src/mesh/PhoneAPI.cpp index 3eaa60674..43259009a 100644 --- a/src/mesh/PhoneAPI.cpp +++ b/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 */ diff --git a/src/mesh/PhoneAPI.h b/src/mesh/PhoneAPI.h index dc7de3e18..02165aae3 100644 --- a/src/mesh/PhoneAPI.h +++ b/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 diff --git a/src/mesh/generated/mesh.pb.h b/src/mesh/generated/mesh.pb.h index 8ef7f16f7..566efc1f0 100644 --- a/src/mesh/generated/mesh.pb.h +++ b/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 diff --git a/src/nimble/BluetoothUtil.cpp b/src/nimble/BluetoothUtil.cpp index 1416453b1..0618c73e6 100644 --- a/src/nimble/BluetoothUtil.cpp +++ b/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?