From 21b8efa4a330c2f79236e41b5a1c5d211b602d7b Mon Sep 17 00:00:00 2001
From: Mike Robbins <mrobbins@alum.mit.edu>
Date: Fri, 17 Oct 2025 18:31:53 -0400
Subject: [PATCH 01/11] Initial work to get NimbleBluetooth working reliably,
 and cross-task mutexes cleaned up

---
 src/mesh/PhoneAPI.cpp          |  24 ++-
 src/nimble/NimbleBluetooth.cpp | 297 +++++++++++++++++++++++++++------
 2 files changed, 261 insertions(+), 60 deletions(-)

diff --git a/src/mesh/PhoneAPI.cpp b/src/mesh/PhoneAPI.cpp
index 51a2bc14818..5cbc900dd54 100644
--- a/src/mesh/PhoneAPI.cpp
+++ b/src/mesh/PhoneAPI.cpp
@@ -71,7 +71,7 @@ void PhoneAPI::handleStartConfig()
     spiLock->unlock();
     LOG_DEBUG("Got %d files in manifest", filesManifest.size());
 
-    LOG_INFO("Start API client config");
+    LOG_INFO("Start API client config millis=%u", millis());
     // Protect against concurrent BLE callbacks: they run in NimBLE's FreeRTOS task and also touch nodeInfoQueue.
     {
         concurrency::LockGuard guard(&nodeInfoMutex);
@@ -453,7 +453,10 @@ size_t PhoneAPI::getFromRadio(uint8_t *buf)
         break;
 
     case STATE_SEND_OTHER_NODEINFOS: {
-        LOG_DEBUG("Send known nodes");
+        if (readIndex == 2) { //  readIndex==2 will be true for the first non-us node
+            LOG_INFO("Start sending nodeinfos millis=%u", millis());
+        }
+
         meshtastic_NodeInfo infoToSend = {};
         {
             concurrency::LockGuard guard(&nodeInfoMutex);
@@ -470,13 +473,22 @@ size_t PhoneAPI::getFromRadio(uint8_t *buf)
         if (infoToSend.num != 0) {
             // Just in case we stored a different user.id in the past, but should never happen going forward
             sprintf(infoToSend.user.id, "!%08x", infoToSend.num);
-            LOG_DEBUG("nodeinfo: num=0x%x, lastseen=%u, id=%s, name=%s", infoToSend.num, infoToSend.last_heard,
-                      infoToSend.user.id, infoToSend.user.long_name);
+
+            // Logging this really slows down sending nodes on initial connection because the serial console is so slow, so only
+            // uncomment if you really need to:
+            // LOG_INFO("nodeinfo: num=0x%x, lastseen=%u, id=%s, name=%s", nodeInfoForPhone.num, nodeInfoForPhone.last_heard,
+            // nodeInfoForPhone.user.id, nodeInfoForPhone.user.long_name);
+
+            // Occasional progress logging. (readIndex==2 will be true for the first non-us node)
+            if (readIndex == 2 || readIndex % 20 == 0) {
+                LOG_DEBUG("nodeinfo: %d/%d", readIndex, nodeDB->getNumMeshNodes());
+            }
+
             fromRadioScratch.which_payload_variant = meshtastic_FromRadio_node_info_tag;
             fromRadioScratch.node_info = infoToSend;
             prefetchNodeInfos();
         } else {
-            LOG_DEBUG("Done sending nodeinfo");
+            LOG_DEBUG("Done sending %d of %d nodeinfos millis=%u", readIndex, nodeDB->getNumMeshNodes(), millis());
             concurrency::LockGuard guard(&nodeInfoMutex);
             nodeInfoQueue.clear();
             state = STATE_SEND_FILEMANIFEST;
@@ -558,7 +570,7 @@ size_t PhoneAPI::getFromRadio(uint8_t *buf)
 
 void PhoneAPI::sendConfigComplete()
 {
-    LOG_INFO("Config Send Complete");
+    LOG_INFO("Config Send Complete millis=%u", millis());
     fromRadioScratch.which_payload_variant = meshtastic_FromRadio_config_complete_id_tag;
     fromRadioScratch.config_complete_id = config_nonce;
     config_nonce = 0;
diff --git a/src/nimble/NimbleBluetooth.cpp b/src/nimble/NimbleBluetooth.cpp
index eb1d909f107..ea5bc83a006 100644
--- a/src/nimble/NimbleBluetooth.cpp
+++ b/src/nimble/NimbleBluetooth.cpp
@@ -3,12 +3,15 @@
 #include "BluetoothCommon.h"
 #include "NimbleBluetooth.h"
 #include "PowerFSM.h"
+#include "StaticPointerQueue.h"
 
+#include "concurrency/OSThread.h"
 #include "main.h"
 #include "mesh/PhoneAPI.h"
 #include "mesh/mesh-pb-constants.h"
 #include "sleep.h"
 #include <NimBLEDevice.h>
+#include <atomic>
 #include <mutex>
 
 #ifdef NIMBLE_TWO
@@ -24,6 +27,11 @@
 #include "nimble/nimble/host/include/host/ble_gap.h"
 #endif
 
+#define DEBUG_NIMBLE_ON_READ_TIMING // uncomment to time onRead duration
+
+#define NIMBLE_BLUETOOTH_TO_PHONE_QUEUE_SIZE 3
+#define NIMBLE_BLUETOOTH_FROM_PHONE_QUEUE_SIZE 3
+
 namespace
 {
 constexpr uint16_t kPreferredBleMtu = 517;
@@ -42,35 +50,110 @@ static bool passkeyShowing;
 class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
 {
   public:
-    BluetoothPhoneAPI() : concurrency::OSThread("NimbleBluetooth") { nimble_queue.resize(3); }
-    std::vector<NimBLEAttValue> nimble_queue;
-    std::mutex nimble_mutex;
-    uint8_t queue_size = 0;
-    uint8_t fromRadioBytes[meshtastic_FromRadio_size] = {0};
-    size_t numBytes = 0;
-    bool hasChecked = false;
-    bool phoneWants = false;
+    BluetoothPhoneAPI() : concurrency::OSThread("NimbleBluetooth") {}
+
+    /* Packets from phone (BLE onWrite callback) */
+    std::mutex fromPhoneMutex;
+    std::atomic<size_t> fromPhoneQueueSize{0};
+    // We use array here (and pay the cost of memcpy) to avoid dynamic memory allocations and frees across FreeRTOS tasks.
+    std::array<NimBLEAttValue, NIMBLE_BLUETOOTH_FROM_PHONE_QUEUE_SIZE> fromPhoneQueue;
+
+    /* Packets to phone (BLE onRead callback) */
+    std::mutex toPhoneMutex;
+    std::atomic<size_t> toPhoneQueueSize{0};
+    // We use array here (and pay the cost of memcpy) to avoid dynamic memory allocations and frees across FreeRTOS tasks.
+    std::array<std::array<uint8_t, meshtastic_FromRadio_size>, NIMBLE_BLUETOOTH_TO_PHONE_QUEUE_SIZE> toPhoneQueue;
+    std::array<size_t, NIMBLE_BLUETOOTH_TO_PHONE_QUEUE_SIZE> toPhoneQueueByteSizes;
+    // The onReadCallbackIsWaitingForData flag provides synchronization between the NimBLE task's onRead callback and our main
+    // task's runOnce. It's only set by onRead, and only cleared by runOnce.
+    std::atomic<bool> onReadCallbackIsWaitingForData{false};
+
+    /* Statistics/logging helpers */
+    std::atomic<int32_t> readCount{0};
+    std::atomic<int32_t> notifyCount{0};
 
   protected:
+    bool runOnceHasWorkToDo()
+    {
+        // return true if the onRead callback is waiting for us, or if we have packets from the phone to handle.
+        return onReadCallbackIsWaitingForData || fromPhoneQueueSize > 0;
+    }
+
     virtual int32_t runOnce() override
     {
-        std::lock_guard<std::mutex> guard(nimble_mutex);
-        if (queue_size > 0) {
-            for (uint8_t i = 0; i < queue_size; i++) {
-                handleToRadio(nimble_queue.at(i).data(), nimble_queue.at(i).length());
+        // Stack buffer for getFromRadio packet
+        uint8_t fromRadioBytes[meshtastic_FromRadio_size] = {0};
+        size_t numBytes = 0;
+
+        while (runOnceHasWorkToDo()) {
+            // Service onRead first, because the onRead callback blocks NimBLE until we clear onReadCallbackIsWaitingForData.
+            if (onReadCallbackIsWaitingForData) {
+                numBytes = getFromRadio(fromRadioBytes);
+
+                if (numBytes == 0) {
+                    // Client expected a read, but we have nothing to send.
+                    // This is 100% OK, as we expect clients to do this regularly to make sure they have nothing else to read.
+                    // LOG_INFO("BLE getFromRadio returned numBytes=0");
+                }
+
+                // Push to toPhoneQueue, protected by toPhoneMutex. Hold the mutex as briefly as possible.
+                if (toPhoneQueueSize < NIMBLE_BLUETOOTH_TO_PHONE_QUEUE_SIZE) {
+                    // Note: the comparison above is safe without a mutex because we are the only method that *increases*
+                    // toPhoneQueueSize. (It's okay if toPhoneQueueSize *decreases* in the NimBLE task meanwhile.)
+
+                    { // scope for toPhoneMutex mutex
+                        std::lock_guard<std::mutex> guard(toPhoneMutex);
+                        size_t storeAtIndex = toPhoneQueueSize.load();
+                        memcpy(toPhoneQueue[storeAtIndex].data(), fromRadioBytes, numBytes);
+                        toPhoneQueueByteSizes[storeAtIndex] = numBytes;
+                        toPhoneQueueSize++;
+                    }
+                } else {
+                    // Shouldn't happen because the onRead callback shouldn't be waiting if the queue is full!
+                    LOG_ERROR("Shouldn't happen! Drop FromRadio packet, toPhoneQueue full (%u bytes)", numBytes);
+                }
+
+                onReadCallbackIsWaitingForData = false; // only clear this flag AFTER the push
+
+                // Return immediately after clearing onReadCallbackIsWaitingForData so that our onRead callback can proceed.
+                if (runOnceHasWorkToDo()) {
+                    // Allow a minimal delay so the NimBLE task's onRead callback can pick up this packet, and then come back here
+                    // ASAP to handle whatever work is next!
+                    return 0;
+                } else {
+                    // Nothing queued. We can wait for the next callback.
+                    return INT32_MAX;
+                }
+            }
+
+            // Handle packets we received from onWrite from the phone.
+            if (fromPhoneQueueSize > 0) {
+                // Note: the comparison above is safe without a mutex because we are the only method that *decreases*
+                // fromPhoneQueueSize. (It's okay if fromPhoneQueueSize *increases* in the NimBLE task meanwhile.)
+
+                LOG_DEBUG("NimbleBluetooth: handling ToRadio packet, fromPhoneQueueSize=%u", fromPhoneQueueSize.load());
+
+                // Pop the front of fromPhoneQueue, holding the mutex only briefly while we pop.
+                NimBLEAttValue val;
+                { // scope for fromPhoneMutex mutex
+                    std::lock_guard<std::mutex> guard(fromPhoneMutex);
+                    val = fromPhoneQueue[0];
+
+                    // Shift the rest of the queue down
+                    for (uint8_t i = 1; i < fromPhoneQueueSize; i++) {
+                        fromPhoneQueue[i - 1] = fromPhoneQueue[i];
+                    }
+                    fromPhoneQueueSize--;
+                }
+
+                handleToRadio(val.data(), val.length());
             }
-            LOG_DEBUG("Queue_size %u", queue_size);
-            queue_size = 0;
-        }
-        if (!hasChecked && phoneWants) {
-            // Pull fresh data while we're outside of the NimBLE callback context.
-            numBytes = getFromRadio(fromRadioBytes);
-            hasChecked = true;
         }
 
         // the run is triggered via NimbleBluetoothToRadioCallback and NimbleBluetoothFromRadioCallback
         return INT32_MAX;
     }
+
     /**
      * Subclasses can use this as a hook to provide custom notifications for their transport (i.e. bluetooth notifies)
      */
@@ -78,8 +161,12 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
     {
         PhoneAPI::onNowHasData(fromRadioNum);
 
+        int currentNotifyCount = notifyCount.fetch_add(1);
+
         uint8_t cc = bleServer->getConnectedCount();
-        LOG_DEBUG("BLE notify fromNum: %d connections: %d", fromRadioNum, cc);
+
+        // This logging slows things down when there are lots of packets going to the phone, like initial connection:
+        // LOG_DEBUG("BLE notify(%d) fromNum: %d connections: %d", currentNotifyCount, fromRadioNum, cc);
 
         uint8_t val[4];
         put_le32(val, fromRadioNum);
@@ -113,15 +200,29 @@ class NimbleBluetoothToRadioCallback : public NimBLECharacteristicCallbacks
 
 #endif
     {
+        // CAUTION: This callback runs in the NimBLE task!!! Don't do anything except communicate with the main task's runOnce.
+        // Assumption: onWrite is serialized by NimBLE, so we don't need to lock here against multiple concurrent onWrite calls.
+
         auto val = pCharacteristic->getValue();
 
         if (memcmp(lastToRadio, val.data(), val.length()) != 0) {
-            if (bluetoothPhoneAPI->queue_size < 3) {
+            if (bluetoothPhoneAPI->fromPhoneQueueSize < NIMBLE_BLUETOOTH_FROM_PHONE_QUEUE_SIZE) {
+                // Note: the comparison above is safe without a mutex because we are the only method that *increases*
+                // fromPhoneQueueSize. (It's okay if fromPhoneQueueSize *decreases* in the main task meanwhile.)
                 memcpy(lastToRadio, val.data(), val.length());
-                std::lock_guard<std::mutex> guard(bluetoothPhoneAPI->nimble_mutex);
-                bluetoothPhoneAPI->nimble_queue.at(bluetoothPhoneAPI->queue_size) = val;
-                bluetoothPhoneAPI->queue_size++;
+
+                { // scope for fromPhoneMutex mutex
+                    // Append to fromPhoneQueue, protected by fromPhoneMutex. Hold the mutex as briefly as possible.
+                    std::lock_guard<std::mutex> guard(bluetoothPhoneAPI->fromPhoneMutex);
+                    bluetoothPhoneAPI->fromPhoneQueue.at(bluetoothPhoneAPI->fromPhoneQueueSize) = val;
+                    bluetoothPhoneAPI->fromPhoneQueueSize++;
+                }
+
+                // After releasing the mutex, schedule immediate processing of the new packet.
                 bluetoothPhoneAPI->setIntervalFromNow(0);
+                concurrency::mainDelay.interrupt(); // wake up main loop if sleeping
+            } else {
+                LOG_WARN("Drop ToRadio packet, fromPhoneQueue full (%u bytes)", val.length());
             }
         } else {
             LOG_DEBUG("Drop duplicate ToRadio packet (%u bytes)", val.length());
@@ -137,32 +238,85 @@ class NimbleBluetoothFromRadioCallback : public NimBLECharacteristicCallbacks
     virtual void onRead(NimBLECharacteristic *pCharacteristic)
 #endif
     {
-        bluetoothPhoneAPI->phoneWants = true;
-        bluetoothPhoneAPI->setIntervalFromNow(0);
-        std::lock_guard<std::mutex> guard(bluetoothPhoneAPI->nimble_mutex); // BLE callbacks run in NimBLE task
-
-        if (!bluetoothPhoneAPI->hasChecked) {
-            // Fetch payload on demand; prefetch keeps this fast for the first read.
-            bluetoothPhoneAPI->numBytes = bluetoothPhoneAPI->getFromRadio(bluetoothPhoneAPI->fromRadioBytes);
-            bluetoothPhoneAPI->hasChecked = true;
-        }
+        // CAUTION: This callback runs in the NimBLE task!!! Don't do anything except communicate with the main task's runOnce.
 
-        pCharacteristic->setValue(bluetoothPhoneAPI->fromRadioBytes, bluetoothPhoneAPI->numBytes);
+        int currentReadCount = bluetoothPhoneAPI->readCount.fetch_add(1);
+        int tries = 0;
 
-        if (bluetoothPhoneAPI->numBytes != 0) {
-#ifdef NIMBLE_TWO
-            // Notify immediately so subscribed clients see the packet without an extra read.
-            pCharacteristic->notify(bluetoothPhoneAPI->fromRadioBytes, bluetoothPhoneAPI->numBytes, BLE_HS_CONN_HANDLE_NONE);
-#else
-            pCharacteristic->notify();
+#ifdef DEBUG_NIMBLE_ON_READ_TIMING
+        int startMillis = millis();
+        // LOG_DEBUG("BLE onRead(%d): start millis=%d", currentReadCount, startMillis);
 #endif
+
+        // Tell the main task that we'd like a packet.
+        bluetoothPhoneAPI->onReadCallbackIsWaitingForData = true;
+
+        while (bluetoothPhoneAPI->onReadCallbackIsWaitingForData && tries < 400) {
+            // Schedule the main task runOnce to run ASAP.
+            bluetoothPhoneAPI->setIntervalFromNow(0);
+            concurrency::mainDelay.interrupt(); // wake up main loop if sleeping
+
+            if (!bluetoothPhoneAPI->onReadCallbackIsWaitingForData) {
+                // we may be able to break even before a delay, if the call to interrupt woke up the main loop and it ran already
+#ifdef DEBUG_NIMBLE_ON_READ_TIMING
+                LOG_DEBUG("BLE onRead(%d): broke before delay after %u ms, %d tries", currentReadCount, millis() - startMillis,
+                          tries);
+#endif
+                break;
+            }
+
+            delay(tries < 10 ? 2 : 5);
+            tries++;
         }
 
-        if (bluetoothPhoneAPI->numBytes != 0) // if we did send something, queue it up right away to reload
+        // Pop from toPhoneQueue, protected by toPhoneMutex. Hold the mutex as briefly as possible.
+        uint8_t fromRadioBytes[meshtastic_FromRadio_size] = {0}; // Stack buffer for getFromRadio packet
+        size_t numBytes = 0;
+        { // scope for toPhoneMutex mutex
+            std::lock_guard<std::mutex> guard(bluetoothPhoneAPI->toPhoneMutex);
+            size_t toPhoneQueueSize = bluetoothPhoneAPI->toPhoneQueueSize.load();
+            if (toPhoneQueueSize > 0) {
+                // Copy from the front of the toPhoneQueue
+                memcpy(fromRadioBytes, bluetoothPhoneAPI->toPhoneQueue[0].data(), bluetoothPhoneAPI->toPhoneQueueByteSizes[0]);
+                numBytes = bluetoothPhoneAPI->toPhoneQueueByteSizes[0];
+
+                // Shift the rest of the queue down
+                for (uint8_t i = 1; i < toPhoneQueueSize; i++) {
+                    memcpy(bluetoothPhoneAPI->toPhoneQueue[i - 1].data(), bluetoothPhoneAPI->toPhoneQueue[i].data(),
+                           bluetoothPhoneAPI->toPhoneQueueByteSizes[i]);
+                    // The above line is similar to:
+                    //   bluetoothPhoneAPI->toPhoneQueue[i - 1] = bluetoothPhoneAPI->toPhoneQueue[i]
+                    // but is usually faster because it doesn't have to copy all the trailing bytes beyond
+                    // toPhoneQueueByteSizes[i].
+                    //
+                    // We deliberately use an array here (and pay the CPU cost of some memcpy) to avoid synchronizing dynamic
+                    // memory allocations and frees across FreeRTOS tasks.
+
+                    bluetoothPhoneAPI->toPhoneQueueByteSizes[i - 1] = bluetoothPhoneAPI->toPhoneQueueByteSizes[i];
+                }
+                bluetoothPhoneAPI->toPhoneQueueSize--;
+            } else {
+                // nothing in the toPhoneQueue; that's fine, and we'll just have numBytes=0.
+            }
+        }
+
+#ifdef DEBUG_NIMBLE_ON_READ_TIMING
+        int finishMillis = millis();
+        LOG_DEBUG("BLE onRead(%d): onReadCallbackIsWaitingForData took %u ms. numBytes=%d", currentReadCount,
+                  finishMillis - startMillis, numBytes);
+#endif
+
+        pCharacteristic->setValue(fromRadioBytes, numBytes);
+
+        bool sentSomething = false;
+        if (numBytes != 0)
+            sentSomething = true;
+
+        // If we did send something, wake up the main loop if it's sleeping in case there are more packets ready to send.
+        if (sentSomething) {
             bluetoothPhoneAPI->setIntervalFromNow(0);
-        bluetoothPhoneAPI->numBytes = 0;
-        bluetoothPhoneAPI->hasChecked = false;
-        bluetoothPhoneAPI->phoneWants = false;
+            concurrency::mainDelay.interrupt(); // wake up main loop if sleeping
+        }
     }
 };
 
@@ -244,6 +398,13 @@ class NimbleBluetoothServerCallback : public NimBLEServerCallbacks
             if (screen)
                 screen->endAlert();
         }
+
+        // Request high-throughput connection parameters for faster setup
+#ifdef NIMBLE_TWO
+        requestHighThroughputConnection(connInfo);
+#else
+        requestHighThroughputConnection(desc);
+#endif
     }
 
 #ifdef NIMBLE_TWO
@@ -290,12 +451,15 @@ class NimbleBluetoothServerCallback : public NimBLEServerCallbacks
         bluetoothStatus->updateStatus(&newStatus);
 
         if (bluetoothPhoneAPI) {
-            std::lock_guard<std::mutex> guard(bluetoothPhoneAPI->nimble_mutex);
             bluetoothPhoneAPI->close();
-            bluetoothPhoneAPI->numBytes = 0;
-            bluetoothPhoneAPI->queue_size = 0;
-            bluetoothPhoneAPI->hasChecked = false;
-            bluetoothPhoneAPI->phoneWants = false;
+
+            bluetoothPhoneAPI->fromPhoneQueueSize = 0;
+
+            bluetoothPhoneAPI->toPhoneQueueSize = 0;
+            bluetoothPhoneAPI->onReadCallbackIsWaitingForData = false;
+
+            bluetoothPhoneAPI->readCount = 0;
+            bluetoothPhoneAPI->notifyCount = 0;
         }
 
         // Clear the last ToRadio packet buffer to avoid rejecting first packet from new connection
@@ -314,6 +478,33 @@ class NimbleBluetoothServerCallback : public NimBLEServerCallbacks
         }
 #endif
     }
+
+#ifdef NIMBLE_TWO
+    void requestHighThroughputConnection(NimBLEConnInfo &connInfo)
+#else
+    void requestHighThroughputConnection(ble_gap_conn_desc *desc)
+#endif
+    {
+        /* Request a lower-latency, higher-throughput BLE connection.
+
+        This comes at the cost of higher power consumption, so we may want to only use this for initial setup, and then switch to
+        a slower mode.
+
+        See https://developer.apple.com/library/archive/qa/qa1931/_index.html for formulas to calculate values, iOS/macOS
+        constraints, and recommendations. (Android doesn't have specific constraints, but seems to be compatible with the Apple
+        recommendations.)
+
+        minInterval (units of 1.25ms): 7.5ms = 6 (lower than the Apple recommended minimum, but allows faster when the client
+        supports it.) maxInterval (units of 1.25ms): 15ms = 12 latency: 0 (don't allow peripheral to skip any connection events)
+        timeout (units of 10ms): 6 seconds = 600 (supervision timeout)
+        */
+        LOG_INFO("BLE requestHighThroughputConnection");
+#ifdef NIMBLE_TWO
+        bleServer->updateConnParams(connInfo.getConnHandle(), 6, 12, 0, 600);
+#else
+        bleServer->updateConnParams(desc->conn_handle, 6, 12, 0, 600);
+#endif
+    }
 };
 
 static NimbleBluetoothToRadioCallback *toRadioCallbacks;
@@ -436,17 +627,15 @@ void NimbleBluetooth::setupService()
     if (config.bluetooth.mode == meshtastic_Config_BluetoothConfig_PairingMode_NO_PIN) {
         ToRadioCharacteristic = bleService->createCharacteristic(TORADIO_UUID, NIMBLE_PROPERTY::WRITE);
         // Allow notifications so phones can stream FromRadio without polling.
-        FromRadioCharacteristic =
-            bleService->createCharacteristic(FROMRADIO_UUID, NIMBLE_PROPERTY::READ | NIMBLE_PROPERTY::NOTIFY);
+        FromRadioCharacteristic = bleService->createCharacteristic(FROMRADIO_UUID, NIMBLE_PROPERTY::READ);
         fromNumCharacteristic = bleService->createCharacteristic(FROMNUM_UUID, NIMBLE_PROPERTY::NOTIFY | NIMBLE_PROPERTY::READ);
         logRadioCharacteristic =
             bleService->createCharacteristic(LOGRADIO_UUID, NIMBLE_PROPERTY::NOTIFY | NIMBLE_PROPERTY::READ, 512U);
     } else {
         ToRadioCharacteristic = bleService->createCharacteristic(
             TORADIO_UUID, NIMBLE_PROPERTY::WRITE | NIMBLE_PROPERTY::WRITE_AUTHEN | NIMBLE_PROPERTY::WRITE_ENC);
-        FromRadioCharacteristic =
-            bleService->createCharacteristic(FROMRADIO_UUID, NIMBLE_PROPERTY::READ | NIMBLE_PROPERTY::READ_AUTHEN |
-                                                                 NIMBLE_PROPERTY::READ_ENC | NIMBLE_PROPERTY::NOTIFY);
+        FromRadioCharacteristic = bleService->createCharacteristic(
+            FROMRADIO_UUID, NIMBLE_PROPERTY::READ | NIMBLE_PROPERTY::READ_AUTHEN | NIMBLE_PROPERTY::READ_ENC);
         fromNumCharacteristic =
             bleService->createCharacteristic(FROMNUM_UUID, NIMBLE_PROPERTY::NOTIFY | NIMBLE_PROPERTY::READ |
                                                                NIMBLE_PROPERTY::READ_AUTHEN | NIMBLE_PROPERTY::READ_ENC);

From 007a92633caec545f63bb2b6a0f292b2bb4f34dc Mon Sep 17 00:00:00 2001
From: Mike Robbins <mrobbins@alum.mit.edu>
Date: Fri, 17 Oct 2025 19:33:26 -0400
Subject: [PATCH 02/11] Pre-fill toPhoneQueue when safe (during
 config/nodeinfo): runOnceToPhoneCanPreloadNextPacket

---
 src/mesh/PhoneAPI.h            |   1 +
 src/nimble/NimbleBluetooth.cpp | 179 +++++++++++++++++++++------------
 2 files changed, 118 insertions(+), 62 deletions(-)

diff --git a/src/mesh/PhoneAPI.h b/src/mesh/PhoneAPI.h
index a8d0faa28cf..d0ba91e72ad 100644
--- a/src/mesh/PhoneAPI.h
+++ b/src/mesh/PhoneAPI.h
@@ -136,6 +136,7 @@ class PhoneAPI
     bool available();
 
     bool isConnected() { return state != STATE_SEND_NOTHING; }
+    bool isSendingPackets() { return state == STATE_SEND_PACKETS; }
 
   protected:
     /// Our fromradio packet while it is being assembled
diff --git a/src/nimble/NimbleBluetooth.cpp b/src/nimble/NimbleBluetooth.cpp
index ea5bc83a006..94214eb37c0 100644
--- a/src/nimble/NimbleBluetooth.cpp
+++ b/src/nimble/NimbleBluetooth.cpp
@@ -27,7 +27,9 @@
 #include "nimble/nimble/host/include/host/ble_gap.h"
 #endif
 
+// Debugging options: careful, they slow things down quite a bit!
 #define DEBUG_NIMBLE_ON_READ_TIMING // uncomment to time onRead duration
+#define DEBUG_NIMBLE_NOTIFY         // uncomment to enable notify logging
 
 #define NIMBLE_BLUETOOTH_TO_PHONE_QUEUE_SIZE 3
 #define NIMBLE_BLUETOOTH_FROM_PHONE_QUEUE_SIZE 3
@@ -73,29 +75,62 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
     std::atomic<int32_t> notifyCount{0};
 
   protected:
-    bool runOnceHasWorkToDo()
+    virtual int32_t runOnce() override
     {
-        // return true if the onRead callback is waiting for us, or if we have packets from the phone to handle.
-        return onReadCallbackIsWaitingForData || fromPhoneQueueSize > 0;
+        while (runOnceHasWorkToDo()) {
+            // Important that we service onRead first, because the onRead callback blocks NimBLE until we clear
+            // onReadCallbackIsWaitingForData.
+            runOnceHandleToPhoneQueue();   // push data to onRead
+            runOnceHandleFromPhoneQueue(); // pull data from onWrite
+        }
+
+        // the run is triggered via NimbleBluetoothToRadioCallback and NimbleBluetoothFromRadioCallback
+        return INT32_MAX;
     }
 
-    virtual int32_t runOnce() override
+    bool runOnceHasWorkToDo() { return runOnceHasWorkToPhone() || runOnceHasWorkFromPhone(); }
+
+    bool runOnceHasWorkToPhone() { return onReadCallbackIsWaitingForData || runOnceToPhoneCanPreloadNextPacket(); }
+
+    bool runOnceToPhoneCanPreloadNextPacket()
+    {
+        /*
+         * PRELOADING getFromRadio RESPONSES:
+         *
+         * It's not safe to preload packets if we're in STATE_SEND_PACKETS, because there may be a while between the time we call
+         * getFromRadio and when the client actually reads it. If the connection drops in that time, we might lose that packet
+         * forever. In STATE_SEND_PACKETS, if we wait for onRead before we call getFromRadio, we minimize the time window where
+         * the client might disconnect before completing the read.
+         *
+         * However, if we're in the setup states (sending config, nodeinfo, etc), it's safe and beneficial to preload packets into
+         * toPhoneQueue because the client will just reconnect after a disconnect, losing nothing.
+         */
+
+        if (!isConnected()) {
+            return false;
+        } else if (isSendingPackets()) {
+            // If we're in STATE_SEND_PACKETS, we must wait for onRead before calling getFromRadio.
+            return false;
+        } else {
+            // In other states, we can preload as long as there's space in the toPhoneQueue.
+            return toPhoneQueueSize < NIMBLE_BLUETOOTH_TO_PHONE_QUEUE_SIZE;
+        }
+    }
+
+    void runOnceHandleToPhoneQueue()
     {
         // Stack buffer for getFromRadio packet
         uint8_t fromRadioBytes[meshtastic_FromRadio_size] = {0};
         size_t numBytes = 0;
 
-        while (runOnceHasWorkToDo()) {
-            // Service onRead first, because the onRead callback blocks NimBLE until we clear onReadCallbackIsWaitingForData.
-            if (onReadCallbackIsWaitingForData) {
-                numBytes = getFromRadio(fromRadioBytes);
-
-                if (numBytes == 0) {
-                    // Client expected a read, but we have nothing to send.
-                    // This is 100% OK, as we expect clients to do this regularly to make sure they have nothing else to read.
-                    // LOG_INFO("BLE getFromRadio returned numBytes=0");
-                }
+        if (onReadCallbackIsWaitingForData || runOnceToPhoneCanPreloadNextPacket()) {
+            numBytes = getFromRadio(fromRadioBytes);
 
+            if (numBytes == 0) {
+                // Client expected a read, but we have nothing to send.
+                // This is 100% OK, as we expect clients to do this regularly to make sure they have nothing else to read.
+                // LOG_INFO("BLE getFromRadio returned numBytes=0");
+            } else {
                 // Push to toPhoneQueue, protected by toPhoneMutex. Hold the mutex as briefly as possible.
                 if (toPhoneQueueSize < NIMBLE_BLUETOOTH_TO_PHONE_QUEUE_SIZE) {
                     // Note: the comparison above is safe without a mutex because we are the only method that *increases*
@@ -108,50 +143,47 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
                         toPhoneQueueByteSizes[storeAtIndex] = numBytes;
                         toPhoneQueueSize++;
                     }
+                    // LOG_DEBUG("BLE pushed toPhoneQueueSize=%u", toPhoneQueueSize.load());
                 } else {
                     // Shouldn't happen because the onRead callback shouldn't be waiting if the queue is full!
                     LOG_ERROR("Shouldn't happen! Drop FromRadio packet, toPhoneQueue full (%u bytes)", numBytes);
                 }
+            }
 
-                onReadCallbackIsWaitingForData = false; // only clear this flag AFTER the push
+            // Clear the onReadCallbackIsWaitingForData flag so onRead knows it can proceed.
+            onReadCallbackIsWaitingForData = false; // only clear this flag AFTER the push
+        }
+    }
 
-                // Return immediately after clearing onReadCallbackIsWaitingForData so that our onRead callback can proceed.
-                if (runOnceHasWorkToDo()) {
-                    // Allow a minimal delay so the NimBLE task's onRead callback can pick up this packet, and then come back here
-                    // ASAP to handle whatever work is next!
-                    return 0;
-                } else {
-                    // Nothing queued. We can wait for the next callback.
-                    return INT32_MAX;
-                }
-            }
+    bool runOnceHasWorkFromPhone() { return fromPhoneQueueSize > 0; }
 
-            // Handle packets we received from onWrite from the phone.
-            if (fromPhoneQueueSize > 0) {
-                // Note: the comparison above is safe without a mutex because we are the only method that *decreases*
-                // fromPhoneQueueSize. (It's okay if fromPhoneQueueSize *increases* in the NimBLE task meanwhile.)
+    void runOnceHandleFromPhoneQueue()
+    {
+        // Handle packets we received from onWrite from the phone.
+        if (fromPhoneQueueSize > 0) {
+            // Note: the comparison above is safe without a mutex because we are the only method that *decreases*
+            // fromPhoneQueueSize. (It's okay if fromPhoneQueueSize *increases* in the NimBLE task meanwhile.)
 
-                LOG_DEBUG("NimbleBluetooth: handling ToRadio packet, fromPhoneQueueSize=%u", fromPhoneQueueSize.load());
+            LOG_DEBUG("NimbleBluetooth: handling ToRadio packet, fromPhoneQueueSize=%u", fromPhoneQueueSize.load());
 
-                // Pop the front of fromPhoneQueue, holding the mutex only briefly while we pop.
-                NimBLEAttValue val;
-                { // scope for fromPhoneMutex mutex
-                    std::lock_guard<std::mutex> guard(fromPhoneMutex);
-                    val = fromPhoneQueue[0];
+            // Pop the front of fromPhoneQueue, holding the mutex only briefly while we pop.
+            NimBLEAttValue val;
+            { // scope for fromPhoneMutex mutex
+                std::lock_guard<std::mutex> guard(fromPhoneMutex);
+                val = fromPhoneQueue[0];
 
-                    // Shift the rest of the queue down
-                    for (uint8_t i = 1; i < fromPhoneQueueSize; i++) {
-                        fromPhoneQueue[i - 1] = fromPhoneQueue[i];
-                    }
-                    fromPhoneQueueSize--;
+                // Shift the rest of the queue down
+                for (uint8_t i = 1; i < fromPhoneQueueSize; i++) {
+                    fromPhoneQueue[i - 1] = fromPhoneQueue[i];
                 }
 
-                handleToRadio(val.data(), val.length());
+                // Safe decrement due to onDisconnect
+                if (fromPhoneQueueSize > 0)
+                    fromPhoneQueueSize--;
             }
-        }
 
-        // the run is triggered via NimbleBluetoothToRadioCallback and NimbleBluetoothFromRadioCallback
-        return INT32_MAX;
+            handleToRadio(val.data(), val.length());
+        }
     }
 
     /**
@@ -165,8 +197,10 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
 
         uint8_t cc = bleServer->getConnectedCount();
 
+#ifdef DEBUG_NIMBLE_NOTIFY
         // This logging slows things down when there are lots of packets going to the phone, like initial connection:
-        // LOG_DEBUG("BLE notify(%d) fromNum: %d connections: %d", currentNotifyCount, fromRadioNum, cc);
+        LOG_DEBUG("BLE notify(%d) fromNum: %d connections: %d", currentNotifyCount, fromRadioNum, cc);
+#endif
 
         uint8_t val[4];
         put_le32(val, fromRadioNum);
@@ -248,25 +282,37 @@ class NimbleBluetoothFromRadioCallback : public NimBLECharacteristicCallbacks
         // LOG_DEBUG("BLE onRead(%d): start millis=%d", currentReadCount, startMillis);
 #endif
 
-        // Tell the main task that we'd like a packet.
-        bluetoothPhoneAPI->onReadCallbackIsWaitingForData = true;
+        // Is there a packet ready to go, or do we have to ask the main task to get one for us?
+        if (bluetoothPhoneAPI->toPhoneQueueSize > 0) {
+            // Note: the comparison above is safe without a mutex because we are the only method that *decreases*
+            // toPhoneQueueSize. (It's okay if toPhoneQueueSize *increases* in the main task meanwhile.)
 
-        while (bluetoothPhoneAPI->onReadCallbackIsWaitingForData && tries < 400) {
-            // Schedule the main task runOnce to run ASAP.
-            bluetoothPhoneAPI->setIntervalFromNow(0);
-            concurrency::mainDelay.interrupt(); // wake up main loop if sleeping
+            // There's already a packet queued. Great! We don't need to wait for onReadCallbackIsWaitingForData.
+#ifdef DEBUG_NIMBLE_ON_READ_TIMING
+            LOG_DEBUG("BLE onRead(%d): packet already waiting, no need to set onReadCallbackIsWaitingForData", currentReadCount);
+#endif
+        } else {
+            // Tell the main task that we'd like a packet.
+            bluetoothPhoneAPI->onReadCallbackIsWaitingForData = true;
+
+            while (bluetoothPhoneAPI->onReadCallbackIsWaitingForData && tries < 400) {
+                // Schedule the main task runOnce to run ASAP.
+                bluetoothPhoneAPI->setIntervalFromNow(0);
+                concurrency::mainDelay.interrupt(); // wake up main loop if sleeping
 
-            if (!bluetoothPhoneAPI->onReadCallbackIsWaitingForData) {
-                // we may be able to break even before a delay, if the call to interrupt woke up the main loop and it ran already
+                if (!bluetoothPhoneAPI->onReadCallbackIsWaitingForData) {
+                    // we may be able to break even before a delay, if the call to interrupt woke up the main loop and it ran
+                    // already
 #ifdef DEBUG_NIMBLE_ON_READ_TIMING
-                LOG_DEBUG("BLE onRead(%d): broke before delay after %u ms, %d tries", currentReadCount, millis() - startMillis,
-                          tries);
+                    LOG_DEBUG("BLE onRead(%d): broke before delay after %u ms, %d tries", currentReadCount,
+                              millis() - startMillis, tries);
 #endif
-                break;
-            }
+                    break;
+                }
 
-            delay(tries < 10 ? 2 : 5);
-            tries++;
+                delay(tries < 10 ? 2 : 5);
+                tries++;
+            }
         }
 
         // Pop from toPhoneQueue, protected by toPhoneMutex. Hold the mutex as briefly as possible.
@@ -294,7 +340,10 @@ class NimbleBluetoothFromRadioCallback : public NimBLECharacteristicCallbacks
 
                     bluetoothPhoneAPI->toPhoneQueueByteSizes[i - 1] = bluetoothPhoneAPI->toPhoneQueueByteSizes[i];
                 }
-                bluetoothPhoneAPI->toPhoneQueueSize--;
+
+                // Safe decrement due to onDisconnect
+                if (bluetoothPhoneAPI->toPhoneQueueSize > 0)
+                    bluetoothPhoneAPI->toPhoneQueueSize--;
             } else {
                 // nothing in the toPhoneQueue; that's fine, and we'll just have numBytes=0.
             }
@@ -453,10 +502,16 @@ class NimbleBluetoothServerCallback : public NimBLEServerCallbacks
         if (bluetoothPhoneAPI) {
             bluetoothPhoneAPI->close();
 
-            bluetoothPhoneAPI->fromPhoneQueueSize = 0;
+            { // scope for fromPhoneMutex mutex
+                std::lock_guard<std::mutex> guard(bluetoothPhoneAPI->fromPhoneMutex);
+                bluetoothPhoneAPI->fromPhoneQueueSize = 0;
+            }
 
-            bluetoothPhoneAPI->toPhoneQueueSize = 0;
             bluetoothPhoneAPI->onReadCallbackIsWaitingForData = false;
+            { // scope for toPhoneMutex mutex
+                std::lock_guard<std::mutex> guard(bluetoothPhoneAPI->toPhoneMutex);
+                bluetoothPhoneAPI->toPhoneQueueSize = 0;
+            }
 
             bluetoothPhoneAPI->readCount = 0;
             bluetoothPhoneAPI->notifyCount = 0;

From 00b95704fa722e73936f9b3a330685ecf851e717 Mon Sep 17 00:00:00 2001
From: Mike Robbins <mrobbins@alum.mit.edu>
Date: Fri, 17 Oct 2025 21:18:03 -0400
Subject: [PATCH 03/11] Handle 0-byte responses breaking clients during initial
 config phases

---
 src/nimble/NimbleBluetooth.cpp | 84 +++++++++++++++++++++++++---------
 1 file changed, 63 insertions(+), 21 deletions(-)

diff --git a/src/nimble/NimbleBluetooth.cpp b/src/nimble/NimbleBluetooth.cpp
index 94214eb37c0..a58215a0c1c 100644
--- a/src/nimble/NimbleBluetooth.cpp
+++ b/src/nimble/NimbleBluetooth.cpp
@@ -28,8 +28,9 @@
 #endif
 
 // Debugging options: careful, they slow things down quite a bit!
-#define DEBUG_NIMBLE_ON_READ_TIMING // uncomment to time onRead duration
-#define DEBUG_NIMBLE_NOTIFY         // uncomment to enable notify logging
+// #define DEBUG_NIMBLE_ON_READ_TIMING  // uncomment to time onRead duration
+// #define DEBUG_NIMBLE_ON_WRITE_TIMING // uncomment to time onWrite duration
+// #define DEBUG_NIMBLE_NOTIFY          // uncomment to enable notify logging
 
 #define NIMBLE_BLUETOOTH_TO_PHONE_QUEUE_SIZE 3
 #define NIMBLE_BLUETOOTH_FROM_PHONE_QUEUE_SIZE 3
@@ -73,6 +74,7 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
     /* Statistics/logging helpers */
     std::atomic<int32_t> readCount{0};
     std::atomic<int32_t> notifyCount{0};
+    std::atomic<int32_t> writeCount{0};
 
   protected:
     virtual int32_t runOnce() override
@@ -80,8 +82,8 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
         while (runOnceHasWorkToDo()) {
             // Important that we service onRead first, because the onRead callback blocks NimBLE until we clear
             // onReadCallbackIsWaitingForData.
-            runOnceHandleToPhoneQueue();   // push data to onRead
-            runOnceHandleFromPhoneQueue(); // pull data from onWrite
+            runOnceHandleToPhoneQueue();   // push data from getFromRadio to onRead
+            runOnceHandleFromPhoneQueue(); // pull data from onWrite to handleToRadio
         }
 
         // the run is triggered via NimbleBluetoothToRadioCallback and NimbleBluetoothFromRadioCallback
@@ -128,8 +130,24 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
 
             if (numBytes == 0) {
                 // Client expected a read, but we have nothing to send.
-                // This is 100% OK, as we expect clients to do this regularly to make sure they have nothing else to read.
-                // LOG_INFO("BLE getFromRadio returned numBytes=0");
+                // Returning a 0-byte packet breaks clients during the config phase, so we have to block onRead until there's a
+                // packet ready.
+                if (isSendingPackets()) {
+                    // In STATE_SEND_PACKETS, it is 100% OK to return a 0-byte response, as we expect clients to do read beyond
+                    // notifies regularly, to make sure they have nothing else to read.
+#ifdef DEBUG_NIMBLE_ON_READ_TIMING
+                    LOG_DEBUG("BLE getFromRadio returned numBytes=0, but in STATE_SEND_PACKETS, so clearing "
+                              "onReadCallbackIsWaitingForData flag");
+#endif
+                } else {
+                    // In other states, this breaks clients.
+                    // Return early, leaving onReadCallbackIsWaitingForData==true so onRead knows to try again.
+                    // This gives runOnce a chance to handleToRadio and produce a response.
+#ifdef DEBUG_NIMBLE_ON_READ_TIMING
+                    LOG_DEBUG("BLE getFromRadio returned numBytes=0. Blocking onRead until we have data");
+#endif
+                    return;
+                }
             } else {
                 // Push to toPhoneQueue, protected by toPhoneMutex. Hold the mutex as briefly as possible.
                 if (toPhoneQueueSize < NIMBLE_BLUETOOTH_TO_PHONE_QUEUE_SIZE) {
@@ -143,7 +161,10 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
                         toPhoneQueueByteSizes[storeAtIndex] = numBytes;
                         toPhoneQueueSize++;
                     }
-                    // LOG_DEBUG("BLE pushed toPhoneQueueSize=%u", toPhoneQueueSize.load());
+#ifdef DEBUG_NIMBLE_ON_READ_TIMING
+                    LOG_DEBUG("BLE getFromRadio returned numBytes=%u, pushed toPhoneQueueSize=%u", numBytes,
+                              toPhoneQueueSize.load());
+#endif
                 } else {
                     // Shouldn't happen because the onRead callback shouldn't be waiting if the queue is full!
                     LOG_ERROR("Shouldn't happen! Drop FromRadio packet, toPhoneQueue full (%u bytes)", numBytes);
@@ -237,6 +258,13 @@ class NimbleBluetoothToRadioCallback : public NimBLECharacteristicCallbacks
         // CAUTION: This callback runs in the NimBLE task!!! Don't do anything except communicate with the main task's runOnce.
         // Assumption: onWrite is serialized by NimBLE, so we don't need to lock here against multiple concurrent onWrite calls.
 
+        int currentWriteCount = bluetoothPhoneAPI->writeCount.fetch_add(1);
+
+#ifdef DEBUG_NIMBLE_ON_WRITE_TIMING
+        int startMillis = millis();
+        LOG_DEBUG("BLE onWrite(%d): start millis=%d", currentWriteCount, startMillis);
+#endif
+
         auto val = pCharacteristic->getValue();
 
         if (memcmp(lastToRadio, val.data(), val.length()) != 0) {
@@ -255,11 +283,17 @@ class NimbleBluetoothToRadioCallback : public NimBLECharacteristicCallbacks
                 // After releasing the mutex, schedule immediate processing of the new packet.
                 bluetoothPhoneAPI->setIntervalFromNow(0);
                 concurrency::mainDelay.interrupt(); // wake up main loop if sleeping
+
+#ifdef DEBUG_NIMBLE_ON_WRITE_TIMING
+                int finishMillis = millis();
+                LOG_DEBUG("BLE onWrite(%d): append to fromPhoneQueue took %u ms. numBytes=%d", currentWriteCount,
+                          finishMillis - startMillis, val.length());
+#endif
             } else {
-                LOG_WARN("Drop ToRadio packet, fromPhoneQueue full (%u bytes)", val.length());
+                LOG_WARN("BLE onWrite(%d): Drop ToRadio packet, fromPhoneQueue full (%u bytes)", currentWriteCount, val.length());
             }
         } else {
-            LOG_DEBUG("Drop duplicate ToRadio packet (%u bytes)", val.length());
+            LOG_DEBUG("BLE onWrite(%d): Drop duplicate ToRadio packet (%u bytes)", currentWriteCount, val.length());
         }
     }
 };
@@ -276,10 +310,10 @@ class NimbleBluetoothFromRadioCallback : public NimBLECharacteristicCallbacks
 
         int currentReadCount = bluetoothPhoneAPI->readCount.fetch_add(1);
         int tries = 0;
+        int startMillis = millis();
 
 #ifdef DEBUG_NIMBLE_ON_READ_TIMING
-        int startMillis = millis();
-        // LOG_DEBUG("BLE onRead(%d): start millis=%d", currentReadCount, startMillis);
+        LOG_DEBUG("BLE onRead(%d): start millis=%d", currentReadCount, startMillis);
 #endif
 
         // Is there a packet ready to go, or do we have to ask the main task to get one for us?
@@ -295,7 +329,10 @@ class NimbleBluetoothFromRadioCallback : public NimBLECharacteristicCallbacks
             // Tell the main task that we'd like a packet.
             bluetoothPhoneAPI->onReadCallbackIsWaitingForData = true;
 
-            while (bluetoothPhoneAPI->onReadCallbackIsWaitingForData && tries < 400) {
+            // Wait for the main task to produce a packet for us, up to about 10 seconds.
+            // It normally takes just a few milliseconds, but at initial startup, etc, the main task can get blocked for longer
+            // doing various setup tasks.
+            while (bluetoothPhoneAPI->onReadCallbackIsWaitingForData && tries < 2000) {
                 // Schedule the main task runOnce to run ASAP.
                 bluetoothPhoneAPI->setIntervalFromNow(0);
                 concurrency::mainDelay.interrupt(); // wake up main loop if sleeping
@@ -310,8 +347,16 @@ class NimbleBluetoothFromRadioCallback : public NimBLECharacteristicCallbacks
                     break;
                 }
 
-                delay(tries < 10 ? 2 : 5);
+                // This delay happens in the NimBLE FreeRTOS task, which really can't do anything until we get a value back.
+                // No harm in polling pretty frequently.
+                delay(tries < 20 ? 1 : 5);
                 tries++;
+
+                if (tries == 2000) {
+                    LOG_WARN(
+                        "BLE onRead(%d): timeout waiting for data after %u ms, %d tries, giving up and returning 0-size response",
+                        currentReadCount, millis() - startMillis, tries);
+                }
             }
         }
 
@@ -351,18 +396,14 @@ class NimbleBluetoothFromRadioCallback : public NimBLECharacteristicCallbacks
 
 #ifdef DEBUG_NIMBLE_ON_READ_TIMING
         int finishMillis = millis();
-        LOG_DEBUG("BLE onRead(%d): onReadCallbackIsWaitingForData took %u ms. numBytes=%d", currentReadCount,
-                  finishMillis - startMillis, numBytes);
+        LOG_DEBUG("BLE onRead(%d): onReadCallbackIsWaitingForData took %u ms, %d tries. numBytes=%d", currentReadCount,
+                  finishMillis - startMillis, tries, numBytes);
 #endif
 
         pCharacteristic->setValue(fromRadioBytes, numBytes);
 
-        bool sentSomething = false;
-        if (numBytes != 0)
-            sentSomething = true;
-
-        // If we did send something, wake up the main loop if it's sleeping in case there are more packets ready to send.
-        if (sentSomething) {
+        // If we sent something, wake up the main loop if it's sleeping in case there are more packets ready to enqueue.
+        if (numBytes != 0) {
             bluetoothPhoneAPI->setIntervalFromNow(0);
             concurrency::mainDelay.interrupt(); // wake up main loop if sleeping
         }
@@ -515,6 +556,7 @@ class NimbleBluetoothServerCallback : public NimBLEServerCallbacks
 
             bluetoothPhoneAPI->readCount = 0;
             bluetoothPhoneAPI->notifyCount = 0;
+            bluetoothPhoneAPI->writeCount = 0;
         }
 
         // Clear the last ToRadio packet buffer to avoid rejecting first packet from new connection

From 492e2ae5e57b4387070cb3fb9b70e16917877a3b Mon Sep 17 00:00:00 2001
From: Mike Robbins <mrobbins@alum.mit.edu>
Date: Fri, 17 Oct 2025 21:43:03 -0400
Subject: [PATCH 04/11] requestLowerPowerConnection

---
 src/nimble/NimbleBluetooth.cpp | 43 +++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/src/nimble/NimbleBluetooth.cpp b/src/nimble/NimbleBluetooth.cpp
index a58215a0c1c..654a0eb8ec3 100644
--- a/src/nimble/NimbleBluetooth.cpp
+++ b/src/nimble/NimbleBluetooth.cpp
@@ -591,9 +591,15 @@ class NimbleBluetoothServerCallback : public NimBLEServerCallbacks
         constraints, and recommendations. (Android doesn't have specific constraints, but seems to be compatible with the Apple
         recommendations.)
 
-        minInterval (units of 1.25ms): 7.5ms = 6 (lower than the Apple recommended minimum, but allows faster when the client
-        supports it.) maxInterval (units of 1.25ms): 15ms = 12 latency: 0 (don't allow peripheral to skip any connection events)
-        timeout (units of 10ms): 6 seconds = 600 (supervision timeout)
+        Selected settings:
+            minInterval (units of 1.25ms): 7.5ms = 6 (lower than the Apple recommended minimum, but allows faster when the client
+        supports it.)
+            maxInterval (units of 1.25ms): 15ms = 12
+            latency: 0 (don't allow peripheral to skip any connection events)
+            timeout (units of 10ms): 6 seconds = 600 (supervision timeout)
+
+        These are intentionally aggressive to prioritize speed over power consumption, but are only used for a few seconds at
+        setup. Not worth adjusting much.
         */
         LOG_INFO("BLE requestHighThroughputConnection");
 #ifdef NIMBLE_TWO
@@ -602,6 +608,37 @@ class NimbleBluetoothServerCallback : public NimBLEServerCallbacks
         bleServer->updateConnParams(desc->conn_handle, 6, 12, 0, 600);
 #endif
     }
+
+#ifdef NIMBLE_TWO
+    void requestLowerPowerConnection(NimBLEConnInfo &connInfo)
+#else
+    void requestLowerPowerConnection(ble_gap_conn_desc *desc)
+#endif
+    {
+        /* Request a lower power consumption (but higher latency, lower throughput) BLE connection.
+
+        This is suitable for steady-state operation after initial setup is complete.
+
+        See https://developer.apple.com/library/archive/qa/qa1931/_index.html for formulas to calculate values, iOS/macOS
+        constraints, and recommendations. (Android doesn't have specific constraints, but seems to be compatible with the Apple
+        recommendations.)
+
+        Selected settings:
+            minInterval (units of 1.25ms): 30ms = 24
+            maxInterval (units of 1.25ms): 50ms = 40
+            latency: 2 (allow peripheral to skip up to 2 consecutive connection events to save power)
+            timeout (units of 10ms): 6 seconds = 600 (supervision timeout)
+
+        There's an opportunity for tuning here if anyone wants to do some power measurements, but these should allow 10-20 packets
+        per second.
+        */
+        LOG_INFO("BLE requestLowerPowerConnection");
+#ifdef NIMBLE_TWO
+        bleServer->updateConnParams(connInfo.getConnHandle(), 24, 40, 2, 600);
+#else
+        bleServer->updateConnParams(desc->conn_handle, 24, 40, 2, 600);
+#endif
+    }
 };
 
 static NimbleBluetoothToRadioCallback *toRadioCallbacks;

From a4785eed80f6ac9dc2e132c9b911cdd5b2aeefd1 Mon Sep 17 00:00:00 2001
From: Mike Robbins <mrobbins@alum.mit.edu>
Date: Fri, 17 Oct 2025 22:14:49 -0400
Subject: [PATCH 05/11] PhoneAPI: onConfigStart and onConfigComplete callbacks
 for subclasses

---
 src/mesh/PhoneAPI.cpp | 7 +++++++
 src/mesh/PhoneAPI.h   | 5 +++++
 2 files changed, 12 insertions(+)

diff --git a/src/mesh/PhoneAPI.cpp b/src/mesh/PhoneAPI.cpp
index 5cbc900dd54..d1e342c803d 100644
--- a/src/mesh/PhoneAPI.cpp
+++ b/src/mesh/PhoneAPI.cpp
@@ -57,6 +57,9 @@ void PhoneAPI::handleStartConfig()
 #endif
     }
 
+    // Allow subclasses to prepare for high-throughput config traffic
+    onConfigStart();
+
     // even if we were already connected - restart our state machine
     if (config_nonce == SPECIAL_NONCE_ONLY_NODES) {
         // If client only wants node info, jump directly to sending nodes
@@ -575,6 +578,10 @@ void PhoneAPI::sendConfigComplete()
     fromRadioScratch.config_complete_id = config_nonce;
     config_nonce = 0;
     state = STATE_SEND_PACKETS;
+
+    // Allow subclasses to know we've entered steady-state so they can lower power consumption
+    onConfigComplete();
+
     pauseBluetoothLogging = false;
 }
 
diff --git a/src/mesh/PhoneAPI.h b/src/mesh/PhoneAPI.h
index d0ba91e72ad..d6682684fc5 100644
--- a/src/mesh/PhoneAPI.h
+++ b/src/mesh/PhoneAPI.h
@@ -159,6 +159,11 @@ class PhoneAPI
      */
     virtual void onNowHasData(uint32_t fromRadioNum) {}
 
+    /// Subclasses can use these lifecycle hooks for transport-specific behavior around config/steady-state
+    /// (i.e. BLE connection params)
+    virtual void onConfigStart() {}
+    virtual void onConfigComplete() {}
+
     /// begin a new connection
     void handleStartConfig();
 

From cdb4d689b03c0fe54b0022cc438ad01d93942f8c Mon Sep 17 00:00:00 2001
From: Mike Robbins <mrobbins@alum.mit.edu>
Date: Fri, 17 Oct 2025 22:19:52 -0400
Subject: [PATCH 06/11] NimbleBluetooth: switch to high-throughput BLE mode
 during config, then lower-power BLE mode for steady-state

---
 src/nimble/NimbleBluetooth.cpp | 166 ++++++++++++++++++---------------
 1 file changed, 90 insertions(+), 76 deletions(-)

diff --git a/src/nimble/NimbleBluetooth.cpp b/src/nimble/NimbleBluetooth.cpp
index 654a0eb8ec3..0473f8e679d 100644
--- a/src/nimble/NimbleBluetooth.cpp
+++ b/src/nimble/NimbleBluetooth.cpp
@@ -27,14 +27,6 @@
 #include "nimble/nimble/host/include/host/ble_gap.h"
 #endif
 
-// Debugging options: careful, they slow things down quite a bit!
-// #define DEBUG_NIMBLE_ON_READ_TIMING  // uncomment to time onRead duration
-// #define DEBUG_NIMBLE_ON_WRITE_TIMING // uncomment to time onWrite duration
-// #define DEBUG_NIMBLE_NOTIFY          // uncomment to enable notify logging
-
-#define NIMBLE_BLUETOOTH_TO_PHONE_QUEUE_SIZE 3
-#define NIMBLE_BLUETOOTH_FROM_PHONE_QUEUE_SIZE 3
-
 namespace
 {
 constexpr uint16_t kPreferredBleMtu = 517;
@@ -43,12 +35,21 @@ constexpr uint16_t kPreferredBleTxTimeUs = (kPreferredBleTxOctets + 14) * 8;
 } // namespace
 #endif
 
+// Debugging options: careful, they slow things down quite a bit!
+// #define DEBUG_NIMBLE_ON_READ_TIMING  // uncomment to time onRead duration
+// #define DEBUG_NIMBLE_ON_WRITE_TIMING // uncomment to time onWrite duration
+// #define DEBUG_NIMBLE_NOTIFY          // uncomment to enable notify logging
+
+#define NIMBLE_BLUETOOTH_TO_PHONE_QUEUE_SIZE 3
+#define NIMBLE_BLUETOOTH_FROM_PHONE_QUEUE_SIZE 3
+
 NimBLECharacteristic *fromNumCharacteristic;
 NimBLECharacteristic *BatteryCharacteristic;
 NimBLECharacteristic *logRadioCharacteristic;
 NimBLEServer *bleServer;
 
 static bool passkeyShowing;
+static std::atomic<int32_t> nimbleBluetoothConnHandle{-1}; // actual handles are uint16_t, so -1 means "no connection"
 
 class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
 {
@@ -90,6 +91,32 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
         return INT32_MAX;
     }
 
+    virtual void onConfigStart() override
+    {
+        LOG_INFO("BLE onConfigStart");
+
+        // Prefer high throughput during config/setup, at the cost of high power consumption (for a few seconds)
+        if (bleServer && isConnected()) {
+            int32_t conn_handle = nimbleBluetoothConnHandle.load();
+            if (conn_handle != -1) {
+                requestHighThroughputConnection(static_cast<uint16_t>(conn_handle));
+            }
+        }
+    }
+
+    virtual void onConfigComplete() override
+    {
+        LOG_INFO("BLE onConfigComplete");
+
+        // Switch to lower power consumption BLE connection params for steady-state use after config/setup is complete
+        if (bleServer && isConnected()) {
+            int32_t conn_handle = nimbleBluetoothConnHandle.load();
+            if (conn_handle != -1) {
+                requestLowerPowerConnection(static_cast<uint16_t>(conn_handle));
+            }
+        }
+    }
+
     bool runOnceHasWorkToDo() { return runOnceHasWorkToPhone() || runOnceHasWorkFromPhone(); }
 
     bool runOnceHasWorkToPhone() { return onReadCallbackIsWaitingForData || runOnceToPhoneCanPreloadNextPacket(); }
@@ -236,6 +263,54 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
 
     /// Check the current underlying physical link to see if the client is currently connected
     virtual bool checkIsConnected() { return bleServer && bleServer->getConnectedCount() > 0; }
+
+    void requestHighThroughputConnection(uint16_t conn_handle)
+    {
+        /* Request a lower-latency, higher-throughput BLE connection.
+
+        This comes at the cost of higher power consumption, so we may want to only use this for initial setup, and then switch to
+        a slower mode.
+
+        See https://developer.apple.com/library/archive/qa/qa1931/_index.html for formulas to calculate values, iOS/macOS
+        constraints, and recommendations. (Android doesn't have specific constraints, but seems to be compatible with the Apple
+        recommendations.)
+
+        Selected settings:
+            minInterval (units of 1.25ms): 7.5ms = 6 (lower than the Apple recommended minimum, but allows faster when the client
+        supports it.)
+            maxInterval (units of 1.25ms): 15ms = 12
+            latency: 0 (don't allow peripheral to skip any connection events)
+            timeout (units of 10ms): 6 seconds = 600 (supervision timeout)
+
+        These are intentionally aggressive to prioritize speed over power consumption, but are only used for a few seconds at
+        setup. Not worth adjusting much.
+        */
+        LOG_INFO("BLE requestHighThroughputConnection");
+        bleServer->updateConnParams(conn_handle, 6, 12, 0, 600);
+    }
+
+    void requestLowerPowerConnection(uint16_t conn_handle)
+    {
+        /* Request a lower power consumption (but higher latency, lower throughput) BLE connection.
+
+        This is suitable for steady-state operation after initial setup is complete.
+
+        See https://developer.apple.com/library/archive/qa/qa1931/_index.html for formulas to calculate values, iOS/macOS
+        constraints, and recommendations. (Android doesn't have specific constraints, but seems to be compatible with the Apple
+        recommendations.)
+
+        Selected settings:
+            minInterval (units of 1.25ms): 30ms = 24
+            maxInterval (units of 1.25ms): 50ms = 40
+            latency: 2 (allow peripheral to skip up to 2 consecutive connection events to save power)
+            timeout (units of 10ms): 6 seconds = 600 (supervision timeout)
+
+        There's an opportunity for tuning here if anyone wants to do some power measurements, but these should allow 10-20 packets
+        per second.
+        */
+        LOG_INFO("BLE requestLowerPowerConnection");
+        bleServer->updateConnParams(conn_handle, 24, 40, 2, 600);
+    }
 };
 
 static BluetoothPhoneAPI *bluetoothPhoneAPI;
@@ -489,11 +564,11 @@ class NimbleBluetoothServerCallback : public NimBLEServerCallbacks
                 screen->endAlert();
         }
 
-        // Request high-throughput connection parameters for faster setup
+        // Store the connection handle for future use
 #ifdef NIMBLE_TWO
-        requestHighThroughputConnection(connInfo);
+        nimbleBluetoothConnHandle = connInfo.getConnHandle();
 #else
-        requestHighThroughputConnection(desc);
+        nimbleBluetoothConnHandle = desc->conn_handle;
 #endif
     }
 
@@ -561,6 +636,9 @@ class NimbleBluetoothServerCallback : public NimBLEServerCallbacks
 
         // Clear the last ToRadio packet buffer to avoid rejecting first packet from new connection
         memset(lastToRadio, 0, sizeof(lastToRadio));
+
+        nimbleBluetoothConnHandle = -1; // -1 means "no connection"
+
 #ifdef NIMBLE_TWO
         // Restart Advertising
         ble->startAdvertising();
@@ -575,70 +653,6 @@ class NimbleBluetoothServerCallback : public NimBLEServerCallbacks
         }
 #endif
     }
-
-#ifdef NIMBLE_TWO
-    void requestHighThroughputConnection(NimBLEConnInfo &connInfo)
-#else
-    void requestHighThroughputConnection(ble_gap_conn_desc *desc)
-#endif
-    {
-        /* Request a lower-latency, higher-throughput BLE connection.
-
-        This comes at the cost of higher power consumption, so we may want to only use this for initial setup, and then switch to
-        a slower mode.
-
-        See https://developer.apple.com/library/archive/qa/qa1931/_index.html for formulas to calculate values, iOS/macOS
-        constraints, and recommendations. (Android doesn't have specific constraints, but seems to be compatible with the Apple
-        recommendations.)
-
-        Selected settings:
-            minInterval (units of 1.25ms): 7.5ms = 6 (lower than the Apple recommended minimum, but allows faster when the client
-        supports it.)
-            maxInterval (units of 1.25ms): 15ms = 12
-            latency: 0 (don't allow peripheral to skip any connection events)
-            timeout (units of 10ms): 6 seconds = 600 (supervision timeout)
-
-        These are intentionally aggressive to prioritize speed over power consumption, but are only used for a few seconds at
-        setup. Not worth adjusting much.
-        */
-        LOG_INFO("BLE requestHighThroughputConnection");
-#ifdef NIMBLE_TWO
-        bleServer->updateConnParams(connInfo.getConnHandle(), 6, 12, 0, 600);
-#else
-        bleServer->updateConnParams(desc->conn_handle, 6, 12, 0, 600);
-#endif
-    }
-
-#ifdef NIMBLE_TWO
-    void requestLowerPowerConnection(NimBLEConnInfo &connInfo)
-#else
-    void requestLowerPowerConnection(ble_gap_conn_desc *desc)
-#endif
-    {
-        /* Request a lower power consumption (but higher latency, lower throughput) BLE connection.
-
-        This is suitable for steady-state operation after initial setup is complete.
-
-        See https://developer.apple.com/library/archive/qa/qa1931/_index.html for formulas to calculate values, iOS/macOS
-        constraints, and recommendations. (Android doesn't have specific constraints, but seems to be compatible with the Apple
-        recommendations.)
-
-        Selected settings:
-            minInterval (units of 1.25ms): 30ms = 24
-            maxInterval (units of 1.25ms): 50ms = 40
-            latency: 2 (allow peripheral to skip up to 2 consecutive connection events to save power)
-            timeout (units of 10ms): 6 seconds = 600 (supervision timeout)
-
-        There's an opportunity for tuning here if anyone wants to do some power measurements, but these should allow 10-20 packets
-        per second.
-        */
-        LOG_INFO("BLE requestLowerPowerConnection");
-#ifdef NIMBLE_TWO
-        bleServer->updateConnParams(connInfo.getConnHandle(), 24, 40, 2, 600);
-#else
-        bleServer->updateConnParams(desc->conn_handle, 24, 40, 2, 600);
-#endif
-    }
 };
 
 static NimbleBluetoothToRadioCallback *toRadioCallbacks;
@@ -879,4 +893,4 @@ void clearNVS()
     ESP.restart();
 #endif
 }
-#endif
+#endif
\ No newline at end of file

From 65ea474d9c57b5dc8675232b19c7d509522a2189 Mon Sep 17 00:00:00 2001
From: Mike Robbins <mrobbins@alum.mit.edu>
Date: Fri, 17 Oct 2025 22:50:09 -0400
Subject: [PATCH 07/11] Add some documentation to NimbleBluetooth.cpp

---
 src/nimble/NimbleBluetooth.cpp | 66 ++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/src/nimble/NimbleBluetooth.cpp b/src/nimble/NimbleBluetooth.cpp
index 0473f8e679d..e20dac508ac 100644
--- a/src/nimble/NimbleBluetooth.cpp
+++ b/src/nimble/NimbleBluetooth.cpp
@@ -53,6 +53,70 @@ static std::atomic<int32_t> nimbleBluetoothConnHandle{-1}; // actual handles are
 
 class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
 {
+    /*
+      CAUTION: There's a lot going on here and lots of room to break things.
+
+      This NimbleBluetooth.cpp file does some tricky synchronization between the NimBLE FreeRTOS task (which runs the onRead and
+      onWrite callbacks) and the main task (which runs runOnce and the rest of PhoneAPI).
+
+      The main idea is to add a little bit of synchronization here to make it so that the rest of the codebase doesn't have to
+      know about concurrency and mutexes, and can just run happily ever after as a cooperative multitasking OSThread system, where
+      locking isn't something that anyone has to worry about too much! :)
+
+      We achieve this by having some queues and mutexes in this file only, and ensuring that all calls to getFromRadio and
+      handleToRadio are only made from the main FreeRTOS task. This way, the rest of the codebase doesn't have to worry about
+      being run concurrently, which would make everything else much much much more complicated.
+
+      PHONE -> RADIO:
+        - [NimBLE FreeRTOS task:] onWrite callback holds fromPhoneMutex and pushes received packets into fromPhoneQueue.
+        - [Main task:] runOnceHandleFromPhoneQueue in main task holds fromPhoneMutex, pulls packets from fromPhoneQueue, and calls
+      handleToRadio **in main task**.
+
+      RADIO -> PHONE:
+        - [NimBLE FreeRTOS task:] onRead callback sets onReadCallbackIsWaitingForData flag and polls in a busy loop. (unless
+      there's already a packet waiting in toPhoneQueue)
+        - [Main task:] runOnceHandleToPhoneQueue sees onReadCallbackIsWaitingForData flag, calls getFromRadio **in main task** to
+      get packets from radio, holds toPhoneMutex, pushes the packet into toPhoneQueue, and clears the
+      onReadCallbackIsWaitingForData flag.
+        - [NimBLE FreeRTOS task:] onRead callback sees that the onReadCallbackIsWaitingForData flag cleared, holds toPhoneMutex,
+      pops the packet from toPhoneQueue, and returns it to NimBLE.
+
+      MUTEXES:
+        - fromPhoneMutex protects fromPhoneQueue and fromPhoneQueueSize
+        - toPhoneMutex protects toPhoneQueue, toPhoneQueueByteSizes, and toPhoneQueueSize
+
+      ATOMICS:
+        - fromPhoneQueueSize is only increased by onWrite, and only decreased by runOnceHandleFromPhoneQueue (or onDisconnect).
+        - toPhoneQueueSize is only increased by runOnceHandleToPhoneQueue, and only decreased by onRead (or onDisconnect).
+        - onReadCallbackIsWaitingForData is a flag. It's only set by onRead, and only cleared by runOnceHandleToPhoneQueue (or
+      onDisconnect).
+
+      PRELOADING: see comments in runOnceToPhoneCanPreloadNextPacket about when it's safe to preload packets from getFromRadio.
+
+      BLE CONNECTION PARAMS:
+        - During config, we request a high-throughput, low-latency BLE connection for speed.
+        - After config, we switch to a lower-power BLE connection for steady-state use to extend battery life.
+
+      MEMORY MANAGEMENT:
+        - We keep packets on the stack and do not allocate heap.
+        - We use std::array for fromPhoneQueue and toPhoneQueue to avoid mallocs and frees across FreeRTOS tasks.
+        - Yes, we have to do some copy operations on pop because of this, but it's worth it to avoid cross-task memory management.
+
+      NOTIFY IS BROKEN:
+        - Adding NIMBLE_PROPERTY::NOTIFY to FromRadioCharacteristic appears to break things. It is NOT backwards compatible.
+
+      ZERO-SIZE READS:
+        - Returning a zero-size read from onRead breaks some clients during the config phase. So we have to block onRead until we
+      have data.
+        - During the STATE_SEND_PACKETS phase, it's totally OK to return zero-size reads, as clients are expected to do reads
+      until they get a 0-byte response.
+
+      CROSS-TASK WAKEUP:
+        - If you call: bluetoothPhoneAPI->setIntervalFromNow(0); to schedule immediate processing of new data,
+        - Then you should also call: concurrency::mainDelay.interrupt(); to wake up the main loop if it's sleeping.
+        - Otherwise, you're going to wait ~100ms or so until the main loop wakes up from some other cause.
+    */
+
   public:
     BluetoothPhoneAPI() : concurrency::OSThread("NimbleBluetooth") {}
 
@@ -255,6 +319,8 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
 
         fromNumCharacteristic->setValue(val, sizeof(val));
 #ifdef NIMBLE_TWO
+        // NOTE: I don't have any NIMBLE_TWO devices, but this line makes me suspicious, and I suspect it needs to just be
+        // notify().
         fromNumCharacteristic->notify(val, sizeof(val), BLE_HS_CONN_HANDLE_NONE);
 #else
         fromNumCharacteristic->notify();

From 18af149cba003c394d43510e0f7c84daebf8eba5 Mon Sep 17 00:00:00 2001
From: Mike Robbins <mrobbins@alum.mit.edu>
Date: Fri, 17 Oct 2025 23:31:34 -0400
Subject: [PATCH 08/11] make cppcheck happier

---
 src/nimble/NimbleBluetooth.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/nimble/NimbleBluetooth.cpp b/src/nimble/NimbleBluetooth.cpp
index e20dac508ac..abc9d42bfb1 100644
--- a/src/nimble/NimbleBluetooth.cpp
+++ b/src/nimble/NimbleBluetooth.cpp
@@ -124,14 +124,14 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
     std::mutex fromPhoneMutex;
     std::atomic<size_t> fromPhoneQueueSize{0};
     // We use array here (and pay the cost of memcpy) to avoid dynamic memory allocations and frees across FreeRTOS tasks.
-    std::array<NimBLEAttValue, NIMBLE_BLUETOOTH_FROM_PHONE_QUEUE_SIZE> fromPhoneQueue;
+    std::array<NimBLEAttValue, NIMBLE_BLUETOOTH_FROM_PHONE_QUEUE_SIZE> fromPhoneQueue{};
 
     /* Packets to phone (BLE onRead callback) */
     std::mutex toPhoneMutex;
     std::atomic<size_t> toPhoneQueueSize{0};
     // We use array here (and pay the cost of memcpy) to avoid dynamic memory allocations and frees across FreeRTOS tasks.
-    std::array<std::array<uint8_t, meshtastic_FromRadio_size>, NIMBLE_BLUETOOTH_TO_PHONE_QUEUE_SIZE> toPhoneQueue;
-    std::array<size_t, NIMBLE_BLUETOOTH_TO_PHONE_QUEUE_SIZE> toPhoneQueueByteSizes;
+    std::array<std::array<uint8_t, meshtastic_FromRadio_size>, NIMBLE_BLUETOOTH_TO_PHONE_QUEUE_SIZE> toPhoneQueue{};
+    std::array<size_t, NIMBLE_BLUETOOTH_TO_PHONE_QUEUE_SIZE> toPhoneQueueByteSizes{};
     // The onReadCallbackIsWaitingForData flag provides synchronization between the NimBLE task's onRead callback and our main
     // task's runOnce. It's only set by onRead, and only cleared by runOnce.
     std::atomic<bool> onReadCallbackIsWaitingForData{false};
@@ -959,4 +959,4 @@ void clearNVS()
     ESP.restart();
 #endif
 }
-#endif
\ No newline at end of file
+#endif

From 4e820c76c825d04c0b560ecf5bb832a5b02339d6 Mon Sep 17 00:00:00 2001
From: Mike Robbins <mrobbins@alum.mit.edu>
Date: Sat, 18 Oct 2025 16:58:26 -0400
Subject: [PATCH 09/11] Allow runOnceHandleToPhoneQueue to tell runOnce to
 shouldBreakAndRetryLater, so we don't busy-loop forever in runOnce

---
 src/nimble/NimbleBluetooth.cpp | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/nimble/NimbleBluetooth.cpp b/src/nimble/NimbleBluetooth.cpp
index abc9d42bfb1..069ad98713b 100644
--- a/src/nimble/NimbleBluetooth.cpp
+++ b/src/nimble/NimbleBluetooth.cpp
@@ -144,11 +144,19 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
   protected:
     virtual int32_t runOnce() override
     {
+        bool shouldBreakAndRetryLater = false;
+
         while (runOnceHasWorkToDo()) {
             // Important that we service onRead first, because the onRead callback blocks NimBLE until we clear
             // onReadCallbackIsWaitingForData.
-            runOnceHandleToPhoneQueue();   // push data from getFromRadio to onRead
-            runOnceHandleFromPhoneQueue(); // pull data from onWrite to handleToRadio
+            shouldBreakAndRetryLater = runOnceHandleToPhoneQueue(); // push data from getFromRadio to onRead
+            runOnceHandleFromPhoneQueue();                          // pull data from onWrite to handleToRadio
+
+            if (shouldBreakAndRetryLater) {
+                // onRead still wants data, but it's not available yet. Return so we can try again when a packet may be ready.
+                LOG_INFO("BLE runOnce breaking to retry later (leaving onRead waiting)");
+                return 100; // try again in 100ms
+            }
         }
 
         // the run is triggered via NimbleBluetoothToRadioCallback and NimbleBluetoothFromRadioCallback
@@ -210,8 +218,12 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
         }
     }
 
-    void runOnceHandleToPhoneQueue()
+    bool runOnceHandleToPhoneQueue()
     {
+        // Returns false normally.
+        // Returns true if we should break out of runOnce and retry later, such as setup states where getFromRadio returns 0
+        // bytes.
+
         // Stack buffer for getFromRadio packet
         uint8_t fromRadioBytes[meshtastic_FromRadio_size] = {0};
         size_t numBytes = 0;
@@ -234,10 +246,11 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
                     // In other states, this breaks clients.
                     // Return early, leaving onReadCallbackIsWaitingForData==true so onRead knows to try again.
                     // This gives runOnce a chance to handleToRadio and produce a response.
-#ifdef DEBUG_NIMBLE_ON_READ_TIMING
                     LOG_DEBUG("BLE getFromRadio returned numBytes=0. Blocking onRead until we have data");
-#endif
-                    return;
+
+                    // Return true to tell runOnce to shouldBreakAndRetryLater, so we don't busy-loop in runOnce even though
+                    // onRead is still waiting!
+                    return true;
                 }
             } else {
                 // Push to toPhoneQueue, protected by toPhoneMutex. Hold the mutex as briefly as possible.
@@ -265,6 +278,8 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
             // Clear the onReadCallbackIsWaitingForData flag so onRead knows it can proceed.
             onReadCallbackIsWaitingForData = false; // only clear this flag AFTER the push
         }
+
+        return false;
     }
 
     bool runOnceHasWorkFromPhone() { return fromPhoneQueueSize > 0; }

From ab21e2b50d8c35e925ff90914c124a19aac09477 Mon Sep 17 00:00:00 2001
From: Mike Robbins <mrobbins@alum.mit.edu>
Date: Sat, 18 Oct 2025 17:10:10 -0400
Subject: [PATCH 10/11] Gating some logging behind DEBUG_NIMBLE_ON_READ_TIMING
 ifdef again; bump retry count

---
 src/nimble/NimbleBluetooth.cpp | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/nimble/NimbleBluetooth.cpp b/src/nimble/NimbleBluetooth.cpp
index 069ad98713b..beeeb63b49f 100644
--- a/src/nimble/NimbleBluetooth.cpp
+++ b/src/nimble/NimbleBluetooth.cpp
@@ -154,7 +154,9 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
 
             if (shouldBreakAndRetryLater) {
                 // onRead still wants data, but it's not available yet. Return so we can try again when a packet may be ready.
+#ifdef DEBUG_NIMBLE_ON_READ_TIMING
                 LOG_INFO("BLE runOnce breaking to retry later (leaving onRead waiting)");
+#endif
                 return 100; // try again in 100ms
             }
         }
@@ -246,7 +248,9 @@ class BluetoothPhoneAPI : public PhoneAPI, public concurrency::OSThread
                     // In other states, this breaks clients.
                     // Return early, leaving onReadCallbackIsWaitingForData==true so onRead knows to try again.
                     // This gives runOnce a chance to handleToRadio and produce a response.
+#ifdef DEBUG_NIMBLE_ON_READ_TIMING
                     LOG_DEBUG("BLE getFromRadio returned numBytes=0. Blocking onRead until we have data");
+#endif
 
                     // Return true to tell runOnce to shouldBreakAndRetryLater, so we don't busy-loop in runOnce even though
                     // onRead is still waiting!
@@ -485,10 +489,10 @@ class NimbleBluetoothFromRadioCallback : public NimBLECharacteristicCallbacks
             // Tell the main task that we'd like a packet.
             bluetoothPhoneAPI->onReadCallbackIsWaitingForData = true;
 
-            // Wait for the main task to produce a packet for us, up to about 10 seconds.
+            // Wait for the main task to produce a packet for us, up to about 20 seconds.
             // It normally takes just a few milliseconds, but at initial startup, etc, the main task can get blocked for longer
             // doing various setup tasks.
-            while (bluetoothPhoneAPI->onReadCallbackIsWaitingForData && tries < 2000) {
+            while (bluetoothPhoneAPI->onReadCallbackIsWaitingForData && tries < 4000) {
                 // Schedule the main task runOnce to run ASAP.
                 bluetoothPhoneAPI->setIntervalFromNow(0);
                 concurrency::mainDelay.interrupt(); // wake up main loop if sleeping
@@ -508,7 +512,7 @@ class NimbleBluetoothFromRadioCallback : public NimBLECharacteristicCallbacks
                 delay(tries < 20 ? 1 : 5);
                 tries++;
 
-                if (tries == 2000) {
+                if (tries == 4000) {
                     LOG_WARN(
                         "BLE onRead(%d): timeout waiting for data after %u ms, %d tries, giving up and returning 0-size response",
                         currentReadCount, millis() - startMillis, tries);

From 099ab06b9243d2427f3c406c275041b8f196aeb5 Mon Sep 17 00:00:00 2001
From: Jonathan Bennett <jbennett@incomsystems.biz>
Date: Sat, 18 Oct 2025 16:59:49 -0500
Subject: [PATCH 11/11] Add check for connected state in NimBLE onRead()

---
 src/nimble/NimbleBluetooth.cpp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/nimble/NimbleBluetooth.cpp b/src/nimble/NimbleBluetooth.cpp
index beeeb63b49f..9accf23c6f9 100644
--- a/src/nimble/NimbleBluetooth.cpp
+++ b/src/nimble/NimbleBluetooth.cpp
@@ -466,6 +466,10 @@ class NimbleBluetoothFromRadioCallback : public NimBLECharacteristicCallbacks
     virtual void onRead(NimBLECharacteristic *pCharacteristic)
 #endif
     {
+        // In some cases, it seems a new connection starts with a read.
+        // The API has no bytes to send, leading to a timeout. This short-circuits this problem.
+        if (!bluetoothPhoneAPI->isConnected())
+            return;
         // CAUTION: This callback runs in the NimBLE task!!! Don't do anything except communicate with the main task's runOnce.
 
         int currentReadCount = bluetoothPhoneAPI->readCount.fetch_add(1);