diff --git a/src/core/serialcmds.cpp b/src/core/serialcmds.cpp index edcbcd887..908c2c319 100644 --- a/src/core/serialcmds.cpp +++ b/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)); } } diff --git a/src/core/settings.cpp b/src/core/settings.cpp index aac16c058..9092168c5 100644 --- a/src/core/settings.cpp +++ b/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() { diff --git a/src/modules/ble_api/ble_api.cpp b/src/modules/ble_api/ble_api.cpp index 9d4c070dd..7f11682df 100644 --- a/src/modules/ble_api/ble_api.cpp +++ b/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(); diff --git a/src/modules/ble_api/ble_api.hpp b/src/modules/ble_api/ble_api.hpp index ef8912b18..e9de052ab 100644 --- a/src/modules/ble_api/ble_api.hpp +++ b/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; diff --git a/src/modules/ble_api/services/BLESerialService.cpp b/src/modules/ble_api/services/BLESerialService.cpp index 0911ecccb..6a33688eb 100644 --- a/src/modules/ble_api/services/BLESerialService.cpp +++ b/src/modules/ble_api/services/BLESerialService.cpp @@ -2,119 +2,231 @@ #include "BLESerialService.h" #include "modules/ble/ble_common.h" // bleNotifyRetry #include +#include -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(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(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(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(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(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(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(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(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(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(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(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(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 buf(size + 1); + vsnprintf(buf.data(), buf.size(), fmt, args); + queueTx(reinterpret_cast(buf.data()), static_cast(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; } diff --git a/src/modules/ble_api/services/BLESerialService.h b/src/modules/ble_api/services/BLESerialService.h index e1b87f50a..dc6717a99 100644 --- a/src/modules/ble_api/services/BLESerialService.h +++ b/src/modules/ble_api/services/BLESerialService.h @@ -3,15 +3,49 @@ #include "BruceBLEService.hpp" #include +#include +#include -#define BUFFER_SIZE 128 +// Nordic UART Service (NUS) - standard UUIDs so any generic BLE tooling +// (nRF Connect, the iOS companion app, ...) can discover and talk to Bruce. +#define NUS_SERVICE_UUID "6E400001-B5A3-F393-E0A9-E50E24DCCA9E" +#define NUS_RX_CHAR_UUID "6E400002-B5A3-F393-E0A9-E50E24DCCA9E" // app -> Bruce (write) +#define NUS_TX_CHAR_UUID "6E400003-B5A3-F393-E0A9-E50E24DCCA9E" // Bruce -> app (notify) class BLESerialCallbacks; class BLESerialService : public BruceBLEService, public SerialDevice { - NimBLECharacteristic *serial_char = nullptr; + NimBLECharacteristic *rx_char = nullptr; // written by the central (app) + NimBLECharacteristic *tx_char = nullptr; // notified to the central (app) BLESerialCallbacks *callbacks = nullptr; + // Bytes received from the central are queued here by the write callback + // (NimBLE host task) and consumed by the serial-commands task, so access + // is guarded by a mutex. + std::string rxBuffer; + SemaphoreHandle_t rxMutex = nullptr; + + // Outgoing bytes are coalesced here instead of emitting one notification per + // print()/println() call: a command that prints 40 lines used to cost 40 + // notifications, each waiting for its own connection event. Producers are the + // serial-commands task and the tft_logger async task, so this is mutex-guarded + // too (which also keeps a single write() atomic on the wire). + std::string txBuffer; + SemaphoreHandle_t txMutex = nullptr; + // millis() of the oldest buffered byte, 0 when the buffer is empty. + volatile uint32_t txPendingSince = 0; + // Set from onSubscribe on the TX characteristic. With nobody subscribed every + // notify() fails, so without this each chunk would burn the whole retry budget. + volatile bool txSubscribed = false; + + static constexpr uint32_t TX_FLUSH_INTERVAL_MS = 12; // max time a tail sits buffered + static constexpr uint8_t TX_NOTIFY_RETRIES = 25; // backpressure when the stack is full + + size_t txChunkSize() const; + // Both require txMutex to be held by the caller. + void drainTx(bool sendPartial); + void queueTx(const uint8_t *data, size_t len); + public: BLESerialService(); ~BLESerialService() override; @@ -27,9 +61,16 @@ class BLESerialService : public BruceBLEService, public SerialDevice { size_t println(uint32_t n) override; size_t write(uint8_t *str, size_t size) override; int read() override; - void flush() override {} + void flush() override; String readStringUntil(char terminator) override; int available() override; void setMTU(uint16_t mtu); + + // Called by the write callback to enqueue received bytes. + void feedRx(const uint8_t *data, size_t len); + // Called by the subscribe callback on the TX characteristic. + void setSubscribed(bool subscribed) { txSubscribed = subscribed; } + // Called when the central goes away: nothing can be delivered any more. + void onDisconnected(); }; #endif