fix(ble): Fix notification timing

Author: lucasssvaz

libraries/BLE/src/BLECharacteristic.cpp

@@ -904,6 +904,52 @@ void BLECharacteristicCallbacks::onWrite(BLECharacteristic *pCharacteristic, esp
 
 #if defined(CONFIG_NIMBLE_ENABLED)
 
+/**
+ * @brief Process a deferred write callback.
+ *
+ * This function is called as a FreeRTOS task to execute the onWrite callback
+ * after the write response has been sent to the client. This maintains backwards
+ * compatibility with Bluedroid, where the write response is sent before the
+ * onWrite callback is invoked.
+ *
+ * The delay is based on the connection interval to ensure the write response
+ * packet has been transmitted over the air before the callback executes.
+ *
+ * See: https://github.com/espressif/arduino-esp32/issues/11938
+ */
+void BLECharacteristic::processDeferredWriteCallback(void *pvParameters) {
+  DeferredWriteCallback *pCallback = (DeferredWriteCallback *)pvParameters;
+
+  // Get connection parameters to calculate appropriate delay
+  ble_gap_conn_desc desc;
+  int rc = ble_gap_conn_find(pCallback->conn_handle, &desc);
+
+  if (rc == 0) {
+    // Connection interval is in units of 1.25ms
+    // Wait for at least one connection interval to ensure the write response
+    // has been transmitted. Add a small buffer for processing.
+    uint16_t intervalMs = (desc.conn_itvl * 125) / 100;  // Convert to milliseconds
+    uint16_t delayMs = intervalMs + 5;  // Add 5ms buffer
+
+    log_v("Deferring write callback by %dms (conn_interval=%d units, %dms)", delayMs, desc.conn_itvl, intervalMs);
+    vTaskDelay(pdMS_TO_TICKS(delayMs));
+  } else {
+    // If we can't get connection parameters, use a conservative default
+    // Most connections use 7.5-30ms intervals, so 50ms should be safe
+    log_w("Could not get connection parameters, using default 50ms delay");
+    vTaskDelay(pdMS_TO_TICKS(50));
+  }
+
+  // Call the onWrite callback now that the response has been transmitted
+  pCallback->pCharacteristic->m_pCallbacks->onWrite(pCallback->pCharacteristic, &pCallback->desc);
+
+  // Free the allocated memory
+  delete pCallback;
+
+  // Delete this one-shot task
+  vTaskDelete(NULL);
+}
+
 int BLECharacteristic::handleGATTServerEvent(uint16_t conn_handle, uint16_t attr_handle, struct ble_gatt_access_ctxt *ctxt, void *arg) {
   const ble_uuid_t *uuid;
   int rc;
@@ -955,7 +1001,29 @@ int BLECharacteristic::handleGATTServerEvent(uint16_t conn_handle, uint16_t attr
         rc = ble_gap_conn_find(conn_handle, &desc);
         assert(rc == 0);
         pCharacteristic->setValue(buf, len);
-        pCharacteristic->m_pCallbacks->onWrite(pCharacteristic, &desc);
+
+        // Defer the onWrite callback to maintain backwards compatibility with Bluedroid.
+        // In Bluedroid, the write response is sent BEFORE the onWrite callback is invoked.
+        // In NimBLE, the response is sent implicitly when this function returns.
+        // By deferring the callback to a separate task with a delay based on the connection
+        // interval, we ensure the response packet is transmitted before the callback executes.
+        // See: https://github.com/espressif/arduino-esp32/issues/11938
+        DeferredWriteCallback *pCallback = new DeferredWriteCallback();
+        pCallback->pCharacteristic = pCharacteristic;
+        pCallback->desc = desc;
+        pCallback->conn_handle = conn_handle;
+
+        // Create a one-shot task to execute the callback after the response is transmitted
+        // Using priority 1 (low priority) and sufficient stack for callback operations
+        // Note: Stack must be large enough to handle notify() calls from within onWrite()
+        xTaskCreate(
+          processDeferredWriteCallback,
+          "BLEWriteCB",
+          4096,  // Stack size - increased to handle notify() operations
+          pCallback,
+          1,     // Priority (low)
+          NULL   // Task handle (not needed for one-shot task)
+        );
 
         return 0;
       }

libraries/BLE/src/BLECharacteristic.h

@@ -47,6 +47,8 @@
 #include <host/ble_gatt.h>
 #include <host/ble_att.h>
 #include "BLEConnInfo.h"
+#include <freertos/FreeRTOS.h>
+#include <freertos/task.h>
 #define ESP_GATT_MAX_ATTR_LEN            BLE_ATT_ATTR_MAX_LEN
 #define ESP_GATT_CHAR_PROP_BIT_READ      BLE_GATT_CHR_PROP_READ
 #define ESP_GATT_CHAR_PROP_BIT_WRITE     BLE_GATT_CHR_PROP_WRITE
@@ -246,6 +248,13 @@ class BLECharacteristic {
   portMUX_TYPE m_readMux;
   uint8_t m_removed;
   std::vector<std::pair<uint16_t, uint16_t>> m_subscribedVec;
+
+  // Deferred callback support for maintaining backwards compatibility with Bluedroid timing
+  struct DeferredWriteCallback {
+    BLECharacteristic *pCharacteristic;
+    ble_gap_conn_desc desc;
+    uint16_t conn_handle;
+  };
 #endif
 
   /***************************************************************************
@@ -271,6 +280,7 @@ class BLECharacteristic {
 #if defined(CONFIG_NIMBLE_ENABLED)
   void setSubscribe(struct ble_gap_event *event);
   static int handleGATTServerEvent(uint16_t conn_handle, uint16_t attr_handle, struct ble_gatt_access_ctxt *ctxt, void *arg);
+  static void processDeferredWriteCallback(void *pvParameters);
 #endif
 };  // BLECharacteristic