Add Nordic UART Service BLE bridge for serial commands

src/core/serialcmds.cpp

@@ -63,7 +63,10 @@ void _serialCmdsTaskLoop(void *pvParameters) {
     Serial.begin(115200);
     while (1) {
         handleSerialCommands(serialCli);
-        vTaskDelay(pdMS_TO_TICKS(10));
+        // handleSerialCommands() runs one command per pass, so a burst queued by
+        // the BLE app used to pay the full idle tick between each one. Still always
+        // yields, just sooner when there is more input waiting.
+        vTaskDelay(pdMS_TO_TICKS(serialDevice->available() ? 1 : 10));
     }
 }
 

src/core/settings.cpp

@@ -1681,6 +1681,15 @@ void enableBLEAPI() {
     }
 
     ble_api_enabled = !ble_api_enabled;
+
+    // Give the user visual feedback about the new state, otherwise the toggle
+    // looks like it does nothing and gets pressed repeatedly (which cycles the
+    // BLE stack setup/teardown and can corrupt the GATT table).
+    if (ble_api_enabled) {
+        displayInfo("BLE API ON > Advertising as 'Bruce'", true);
+    } else {
+        displayInfo("BLE API OFF", true);
+    }
 }
 
 bool appStoreInstalled() {

src/modules/ble_api/ble_api.cpp

@@ -10,7 +10,14 @@ class BLEAPICallback : public NimBLEServerCallbacks {
     BLE_API *api;
 
     void onConnect(NimBLEServer *pServer, NimBLEConnInfo &connInfo) override {
-        pServer->updateConnParams(connInfo.getConnHandle(), 6, 24, 0, 400); // Improve latency
+        // 12..24 units = 15..30 ms. Apple rejects the whole request if the minimum
+        // is under 15 ms or if max < min + 15 ms, and a rejected request left us on
+        // whatever interval iOS picked by itself.
+        pServer->updateConnParams(connInfo.getConnHandle(), 12, 24, 0, 400);
+    };
+
+    void onDisconnect(NimBLEServer *pServer, NimBLEConnInfo &connInfo, int reason) override {
+        api->on_disconnect();
     };
 
     void onMTUChange(uint16_t MTU, NimBLEConnInfo &connInfo) override { api->update_mtu(MTU); };
@@ -23,6 +30,21 @@ void BLE_API::setup() {
     NimBLEDevice::init("Bruce");
     NimBLEDevice::setPower(ESP_PWR_LVL_P9); // 9 dBm, tweak if you want
 
+    // Ask for the largest standard ATT MTU. The central still decides the final
+    // value (iOS settles around 185); without this we advertised the NimBLE
+    // default and every notification was capped much lower than it had to be.
+    NimBLEDevice::setMTU(517);
+
+#if !defined(CONFIG_IDF_TARGET_ESP32)
+    // LE 2M PHY roughly doubles the raw throughput. The original ESP32 is BLE 4.2
+    // and has no 2M radio, so only offer it where the controller supports it; the
+    // peer keeps 1M if it does not.
+    NimBLEDevice::setDefaultPhy(
+        BLE_GAP_LE_PHY_1M_MASK | BLE_GAP_LE_PHY_2M_MASK,
+        BLE_GAP_LE_PHY_1M_MASK | BLE_GAP_LE_PHY_2M_MASK
+    );
+#endif
+
     pServer = NimBLEDevice::createServer();
     pServer->advertiseOnDisconnect(true);
     pServer->setCallbacks(new BLEAPICallback(this));
@@ -32,8 +54,12 @@ void BLE_API::setup() {
     serialDevice = &serial_service;
 
     BLEAdvertising *pAdvertising = pServer->getAdvertising();
-    pAdvertising->enableScanResponse(false); // Save some battery
-    pAdvertising->setName("Bruc");           // Bruce is too long for adv packet len
+    // The 128-bit NUS UUID + the 16-bit battery UUID + the name overflow the
+    // 31-byte advertising packet, so use the scan response for the extra data.
+    // This keeps the NUS service UUID discoverable, letting the companion app
+    // filter on it during scanning.
+    pAdvertising->enableScanResponse(true);
+    pAdvertising->setName("Bruce");
     pAdvertising->start();
 }
 
@@ -42,6 +68,8 @@ void BLE_API::update_mtu(uint16_t mtu) {
     serial_service.setMTU(mtu);
 }
 
+void BLE_API::on_disconnect() { serial_service.onDisconnected(); }
+
 void BLE_API::end() {
     battery_service.end();
     serial_service.end();

src/modules/ble_api/ble_api.hpp

@@ -10,6 +10,7 @@ class BLE_API {
     void setup();
     void end();
     void update_mtu(uint16_t mtu);
+    void on_disconnect();
 
 private:
     NimBLEServer *pServer;

src/modules/ble_api/services/BLESerialService.cpp

@@ -2,119 +2,231 @@
 #include "BLESerialService.h"
 #include "modules/ble/ble_common.h" // bleNotifyRetry
 #include <NimBLEDevice.h>
+#include <vector>
 
-BLESerialService::BLESerialService() : BruceBLEService() {}
-
-BLESerialService::~BLESerialService() {}
+BLESerialService::BLESerialService() : BruceBLEService() {
+    rxMutex = xSemaphoreCreateMutex();
+    txMutex = xSemaphoreCreateMutex();
+}
 
-static bool newValue = false;
+BLESerialService::~BLESerialService() {
+    if (rxMutex) vSemaphoreDelete(rxMutex);
+    if (txMutex) vSemaphoreDelete(txMutex);
+}
 
 class BLESerialCallbacks : public NimBLECharacteristicCallbacks {
+    BLESerialService *service;
+
+public:
+    explicit BLESerialCallbacks(BLESerialService *service) : service(service) {}
+
     void onWrite(NimBLECharacteristic *pCharacteristic, NimBLEConnInfo &connInfo) override {
-        newValue = true;
+        std::string value = pCharacteristic->getValue();
+        if (!value.empty())
+            service->feedRx(reinterpret_cast<const uint8_t *>(value.data()), value.size());
+    }
+
+    // Fires on the TX characteristic's CCCD. Bit 0 is "notifications enabled".
+    void onSubscribe(NimBLECharacteristic *pCharacteristic, NimBLEConnInfo &connInfo, uint16_t subValue)
+        override {
+        service->setSubscribed((subValue & 0x0001) != 0);
     }
 };
 
 void BLESerialService::setup(NimBLEServer *pServer) {
-    pService = pServer->createService("4371ec0b-3d43-49f9-b731-7c72a4a7bb91");
+    pService = pServer->createService(NUS_SERVICE_UUID);
 
-    serial_char = pService->createCharacteristic(
-        "d555ed97-bf2a-4f46-b3eb-d1fcdd7325e9", // Battery Level
-        NIMBLE_PROPERTY::READ | NIMBLE_PROPERTY::NOTIFY | NIMBLE_PROPERTY::WRITE
+    // App -> Bruce. WRITE and WRITE_NR so the app can use fast writeWithoutResponse.
+    rx_char = pService->createCharacteristic(
+        NUS_RX_CHAR_UUID, NIMBLE_PROPERTY::WRITE | NIMBLE_PROPERTY::WRITE_NR
     );
+    callbacks = new BLESerialCallbacks(this);
+    rx_char->setCallbacks(callbacks);
 
-    callbacks = new BLESerialCallbacks();
-    serial_char->setCallbacks(callbacks);
+    // Bruce -> app. Same callbacks object: onWrite never fires here (no write
+    // property), but onSubscribe does, which is what tells us anyone is listening.
+    tx_char = pService->createCharacteristic(NUS_TX_CHAR_UUID, NIMBLE_PROPERTY::NOTIFY);
+    tx_char->setCallbacks(callbacks);
 
     pService->start();
     pServer->getAdvertising()->addServiceUUID(pService->getUUID());
 }
 
-void BLESerialService::end() { delete callbacks; }
-
-int BLESerialService::available() {
-    if (!newValue) return 0;
-    newValue = false;
-
-    return serial_char->getValue().size();
+void BLESerialService::end() {
+    // Drop the characteristics first: BLEDevice::deinit() frees them right after
+    // this returns, and other tasks (tft_logger) may still be writing.
+    txSubscribed = false;
+    if (rx_char) rx_char->setCallbacks(nullptr);
+    if (tx_char) tx_char->setCallbacks(nullptr);
+    rx_char = nullptr;
+    tx_char = nullptr;
+
+    delete callbacks;
+    callbacks = nullptr;
+    if (rxMutex && xSemaphoreTake(rxMutex, portMAX_DELAY) == pdTRUE) {
+        rxBuffer.clear();
+        xSemaphoreGive(rxMutex);
+    }
+    if (txMutex && xSemaphoreTake(txMutex, portMAX_DELAY) == pdTRUE) {
+        txBuffer.clear();
+        txPendingSince = 0;
+        xSemaphoreGive(txMutex);
+    }
 }
 
-size_t BLESerialService::println(const String &s) {
-    String toSend = s + "\r\n";
-    bleNotifyRetry(serial_char, reinterpret_cast<const uint8_t *>(toSend.c_str()), toSend.length());
-    vTaskDelay(pdMS_TO_TICKS(10)); // Add some delay to ensure data is read by the client
-    return toSend.length();
+void BLESerialService::onDisconnected() {
+    txSubscribed = false;
+    mtu = 23; // renegotiated on the next connection
+    if (txMutex && xSemaphoreTake(txMutex, portMAX_DELAY) == pdTRUE) {
+        txBuffer.clear();
+        txPendingSince = 0;
+        xSemaphoreGive(txMutex);
+    }
 }
 
-size_t BLESerialService::print(const String &s) {
-    bleNotifyRetry(serial_char, reinterpret_cast<const uint8_t *>(s.c_str()), s.length());
-    vTaskDelay(pdMS_TO_TICKS(10));
-    return s.length();
+void BLESerialService::feedRx(const uint8_t *data, size_t len) {
+    if (!rxMutex) return;
+    if (xSemaphoreTake(rxMutex, portMAX_DELAY) == pdTRUE) {
+        rxBuffer.append(reinterpret_cast<const char *>(data), len);
+        xSemaphoreGive(rxMutex);
+    }
 }
 
-size_t BLESerialService::println(size_t n) {
-    String s = String(n);
-    return println(s);
+int BLESerialService::available() {
+    // The serial-commands task polls this continuously, so it doubles as the tick
+    // that pushes out a buffered tail nobody called flush() for. Reading
+    // txPendingSince unlocked is benign: worst case the flush happens one poll
+    // early or late.
+    uint32_t pendingSince = txPendingSince;
+    if (pendingSince != 0 && (millis() - pendingSince) >= TX_FLUSH_INTERVAL_MS) flush();
+
+    if (!rxMutex) return 0;
+    int result = 0;
+    if (xSemaphoreTake(rxMutex, portMAX_DELAY) == pdTRUE) {
+        // Only report data once a full line is buffered, so the command handler
+        // fires on complete commands and partial BLE writes accumulate.
+        size_t nl = rxBuffer.find('\n');
+        if (nl != std::string::npos) result = static_cast<int>(nl + 1);
+        xSemaphoreGive(rxMutex);
+    }
+    return result;
 }
 
-void BLESerialService::vprintf(const char *fmt, va_list args) {
-    int size = vsnprintf(NULL, 0, fmt, args) + 1;
-    char str[BUFFER_SIZE];
-    sprintf(str, fmt, args);
-
-    bleNotifyRetry(serial_char, reinterpret_cast<const uint8_t *>(str), size);
-    vTaskDelay(pdMS_TO_TICKS(10));
+int BLESerialService::read() {
+    if (!rxMutex) return -1;
+    int result = -1;
+    if (xSemaphoreTake(rxMutex, portMAX_DELAY) == pdTRUE) {
+        if (!rxBuffer.empty()) {
+            result = static_cast<unsigned char>(rxBuffer.front());
+            rxBuffer.erase(0, 1);
+        }
+        xSemaphoreGive(rxMutex);
+    }
+    return result;
 }
 
 String BLESerialService::readStringUntil(char terminator) {
-    Serial.println("readStringUntil");
+    if (!rxMutex) return String("");
     String result = "";
-    std::string value = serial_char->getValue();
-    for (char c : value) {
-        result += c;
-        if (c == terminator) break;
+    if (xSemaphoreTake(rxMutex, portMAX_DELAY) == pdTRUE) {
+        size_t pos = rxBuffer.find(terminator);
+        if (pos != std::string::npos) {
+            result = String(rxBuffer.substr(0, pos).c_str());
+            rxBuffer.erase(0, pos + 1); // consume the line including the terminator
+        } else {
+            result = String(rxBuffer.c_str());
+            rxBuffer.clear();
+        }
+        xSemaphoreGive(rxMutex);
     }
     return result;
 }
 
-size_t BLESerialService::println(const uint32_t n) {
-    String s = String(n);
-    return println(s);
+// Usable ATT payload = MTU - 3 (opcode + handle). Fall back to the safe 20B.
+size_t BLESerialService::txChunkSize() const {
+    return (mtu > 3) ? static_cast<size_t>(mtu - 3) : 20;
 }
 
-size_t BLESerialService::print(const int n, int format) {
-    String s = String(n, format);
-    return print(s);
+// Caller must hold txMutex. Emits full chunks; the trailing partial one only when
+// sendPartial is set, so a burst of small prints travels as few large packets.
+void BLESerialService::drainTx(bool sendPartial) {
+    const size_t chunk = txChunkSize();
+    while (!txBuffer.empty()) {
+        size_t n = txBuffer.size();
+        if (n < chunk) {
+            if (!sendPartial) break;
+        } else {
+            n = chunk;
+        }
+        if (!bleNotifyRetry(
+                tx_char, reinterpret_cast<const uint8_t *>(txBuffer.data()), n, TX_NOTIFY_RETRIES
+            )) {
+            // The link is stalled or gone. Dropping is what the previous code did
+            // anyway, and it keeps the calling task from blocking indefinitely.
+            txBuffer.clear();
+            break;
+        }
+        txBuffer.erase(0, n);
+    }
+    if (txBuffer.empty()) txPendingSince = 0;
 }
 
-size_t BLESerialService::println(const int n, int format) {
-    String s = String(n, format);
-    return println(s);
+void BLESerialService::queueTx(const uint8_t *data, size_t len) {
+    if (len == 0) return;
+    // No subscriber means every notify() would fail and burn the whole retry
+    // budget, which used to slow down all serial output while BLE was enabled.
+    if (tx_char == nullptr || !txSubscribed || !txMutex) return;
+    if (xSemaphoreTake(txMutex, portMAX_DELAY) != pdTRUE) return;
+    if (txBuffer.empty()) txPendingSince = millis();
+    txBuffer.append(reinterpret_cast<const char *>(data), len);
+    drainTx(false);
+    xSemaphoreGive(txMutex);
 }
 
-size_t BLESerialService::println() { return println(""); }
+void BLESerialService::flush() {
+    if (!txMutex) return;
+    if (xSemaphoreTake(txMutex, portMAX_DELAY) == pdTRUE) {
+        drainTx(true);
+        xSemaphoreGive(txMutex);
+    }
+}
 
-size_t BLESerialService::write(uint8_t *str, size_t size) {
-    bleNotifyRetry(serial_char, str, size);
-    vTaskDelay(pdMS_TO_TICKS(10));
-    return size;
+size_t BLESerialService::print(const String &s) {
+    queueTx(reinterpret_cast<const uint8_t *>(s.c_str()), s.length());
+    return s.length();
 }
 
-int BLESerialService::read() {
-    if (!available()) return -1;
+size_t BLESerialService::println(const String &s) {
+    String toSend = s + "\r\n";
+    queueTx(reinterpret_cast<const uint8_t *>(toSend.c_str()), toSend.length());
+    return toSend.length();
+}
 
-    std::string value = serial_char->getValue();
-    if (value.empty()) return -1;
+size_t BLESerialService::println(size_t n) { return println(String(n)); }
 
-    char firstChar = value[0];
-    // Remove the first character from the buffer
-    if (value.length() > 1) {
-        serial_char->setValue(value.substr(1));
-    } else {
-        serial_char->setValue("");
-    }
+size_t BLESerialService::println(const uint32_t n) { return println(String(n)); }
+
+size_t BLESerialService::print(const int n, int format) { return print(String(n, format)); }
+
+size_t BLESerialService::println(const int n, int format) { return println(String(n, format)); }
 
-    return (int)firstChar;
+size_t BLESerialService::println() { return println(String("")); }
+
+void BLESerialService::vprintf(const char *fmt, va_list args) {
+    va_list args_copy;
+    va_copy(args_copy, args);
+    int size = vsnprintf(nullptr, 0, fmt, args_copy);
+    va_end(args_copy);
+    if (size <= 0) return;
+
+    std::vector<char> buf(size + 1);
+    vsnprintf(buf.data(), buf.size(), fmt, args);
+    queueTx(reinterpret_cast<const uint8_t *>(buf.data()), static_cast<size_t>(size));
+}
+
+size_t BLESerialService::write(uint8_t *str, size_t size) {
+    queueTx(str, size);
+    return size;
 }
 
 void BLESerialService::setMTU(uint16_t mtu) { this->mtu = mtu; }