Merge remote-tracking branch 'origin/dev' into HEAD

esphome/components/esp32_ble/__init__.py

@@ -590,6 +590,22 @@ async def to_code(config: ConfigType) -> None:
         _wrap_bluedroid_symbols("smp_check_auth_req", "btm_sec_clear_ble_keys")
         add_src_cmake(bluedroid_headers_cmake("bluedroid_bond_keys.c"))
 
+    # Bluedroid runs a queued MTU or execute write request that fails at once again until the BTU task stack
+    # overflows (see bluedroid_gattc_queue.c); every ESP-IDF release so far. Private headers, as above.
+    if (
+        CORE.using_toolchain_esp_idf
+        and not IS_WINDOWS
+        and _has_gatt_client(CORE.config)
+    ):
+        cg.add_define("USE_ESP32_BLE_BLUEDROID_GATTC_QUEUE")
+        _wrap_bluedroid_symbols(
+            "bta_gattc_cfg_mtu",
+            "bta_gattc_execute",
+            "GATTC_ConfigureMTU",
+            "GATTC_ExecuteWrite",
+        )
+        add_src_cmake(bluedroid_headers_cmake("bluedroid_gattc_queue.c"))
+
     # ESP-IDF 5.5.6 and 6.0.3 end GATT discovery on a response listing handles out of order
     # (see bluedroid_gattc_order.cpp). It needs no Bluedroid headers, so any toolchain works.
     if _has_gatt_client(CORE.config) and _has_strict_handle_order(idf_version()):
@@ -681,6 +697,7 @@ FILTER_SOURCE_FILES = filter_source_files_from_defines(
         "ble_advertising.cpp": "USE_ESP32_BLE_ADVERTISING",
         "bluedroid_hex_wraps.cpp": "USE_ESP32_BLE_BLUEDROID_HEX_WRAPS",
         "bluedroid_bond_keys.c": "USE_ESP32_BLE_BLUEDROID_BOND_KEYS",
+        "bluedroid_gattc_queue.c": "USE_ESP32_BLE_BLUEDROID_GATTC_QUEUE",
         "bluedroid_gattc_order.cpp": "USE_ESP32_BLE_GATTC_HANDLE_ORDER",
     }
 )

esphome/components/esp32_ble/bluedroid_gattc_queue.c

@@ -0,0 +1,67 @@
+// Linker wraps that stop Bluedroid running a failed queued MTU or execute write request again until the BTU task
+// stack overflows (esphome#17954). The same fix as removing the command from p_cmd_list in bta_gattc_act.c before
+// the failure path pops the queue; drop once an ESP-IDF release has it.
+
+// Compiled only with Bluedroid's private headers (bluedroid_headers_cmake); static analysis skips it
+#ifdef USE_ESP32_BLE_BLUEDROID_HEADERS
+
+#include <stddef.h>
+
+#include "bta_gattc_int.h"
+#include "osi/list.h"
+
+// The command bta_gattc_cfg_mtu() or bta_gattc_execute() is running, NULL outside them
+static tBTA_GATTC_CLCB *running_clcb = NULL;
+static tBTA_GATTC_DATA *running_cmd = NULL;
+
+void __real_bta_gattc_cfg_mtu(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data);
+void __real_bta_gattc_execute(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data);
+tGATT_STATUS __real_GATTC_ConfigureMTU(UINT16 conn_id);
+tGATT_STATUS __real_GATTC_ExecuteWrite(UINT16 conn_id, BOOLEAN is_execute);
+
+// Runs on the Bluetooth host task; the failure path pops the queue again, so this nests
+static void run_command(void (*real)(tBTA_GATTC_CLCB *, tBTA_GATTC_DATA *), tBTA_GATTC_CLCB *p_clcb,
+                        tBTA_GATTC_DATA *p_data) {
+  tBTA_GATTC_CLCB *const outer_clcb = running_clcb;
+  tBTA_GATTC_DATA *const outer_cmd = running_cmd;
+  running_clcb = p_clcb;
+  running_cmd = p_data;
+  real(p_clcb, p_data);
+  running_clcb = outer_clcb;
+  running_cmd = outer_cmd;
+}
+
+// The send failed, so the caller clears p_q_cmd and pops the queue. A command popped from p_cmd_list is still its
+// head and would run again; remove (and free) it first. A command not from the queue is not in it: a no-op.
+static void drop_failed_command(void) {
+  if (running_clcb != NULL && running_clcb->p_cmd_list != NULL && running_clcb->p_q_cmd == running_cmd) {
+    list_remove(running_clcb->p_cmd_list, running_cmd);
+  }
+}
+
+void __wrap_bta_gattc_cfg_mtu(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) {
+  run_command(__real_bta_gattc_cfg_mtu, p_clcb, p_data);
+}
+
+void __wrap_bta_gattc_execute(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) {
+  run_command(__real_bta_gattc_execute, p_clcb, p_data);
+}
+
+// The failure checks are the ones bta_gattc_cfg_mtu() and bta_gattc_execute() make
+tGATT_STATUS __wrap_GATTC_ConfigureMTU(UINT16 conn_id) {
+  const tGATT_STATUS status = __real_GATTC_ConfigureMTU(conn_id);
+  if (status != GATT_SUCCESS && status != GATT_CMD_STARTED) {
+    drop_failed_command();
+  }
+  return status;
+}
+
+tGATT_STATUS __wrap_GATTC_ExecuteWrite(UINT16 conn_id, BOOLEAN is_execute) {
+  const tGATT_STATUS status = __real_GATTC_ExecuteWrite(conn_id, is_execute);
+  if (status != BTA_GATT_OK) {
+    drop_failed_command();
+  }
+  return status;
+}
+
+#endif  // USE_ESP32_BLE_BLUEDROID_HEADERS

esphome/core/defines.h

@@ -538,6 +538,7 @@
 #define USE_WIFI_SCAN_RESULTS_LOCK
 #define USE_ESP32_BLE
 #define USE_ESP32_BLE_BLUEDROID_BOND_KEYS
+#define USE_ESP32_BLE_BLUEDROID_GATTC_QUEUE
 #define USE_ESP32_BLE_BLUEDROID_HEX_WRAPS
 #define USE_ESP32_BLE_GATTC_HANDLE_ORDER
 #define USE_ESP32_BLE_MAX_CONNECTIONS 3

tests/component_tests/esp32_ble/test_bluedroid_gattc_queue.py

@@ -0,0 +1,75 @@
+"""Tests for the Bluedroid wraps that drop a failed queued GATT client command."""
+
+from __future__ import annotations
+
+from collections.abc import Callable
+from pathlib import Path
+
+import pytest
+
+from esphome.components.esp32.const import KEY_ESP32, KEY_SRC_CMAKE
+from esphome.components.esp32_ble import bluedroid_headers_cmake
+from esphome.core import CORE
+
+_WRAPPED = (
+    "bta_gattc_cfg_mtu",
+    "bta_gattc_execute",
+    "GATTC_ConfigureMTU",
+    "GATTC_ExecuteWrite",
+)
+_WRAP_FLAGS = {f"-Wl,--wrap={s}" for s in _WRAPPED} | {
+    f"-Wl,--undefined=__wrap_{s}" for s in _WRAPPED
+}
+_TRACKER = "\nesp32_ble_tracker: {}\n"
+
+
+@pytest.mark.parametrize(
+    ("framework", "components", "windows", "expected"),
+    [
+        pytest.param(
+            "{type: esp-idf, version: 5.5.6}", _TRACKER, False, True, id="idf_5_5_6"
+        ),
+        pytest.param(
+            "{type: esp-idf, version: 6.0.1}", _TRACKER, False, True, id="idf_6"
+        ),
+        pytest.param(
+            "{type: arduino, version: 3.3.12}", _TRACKER, False, True, id="arduino"
+        ),
+        # Without the tracker or esp32_ble_client there is no GATT client
+        pytest.param(
+            "{type: esp-idf, version: 5.5.6}", "", False, False, id="no_gatt_client"
+        ),
+        pytest.param(
+            "{type: esp-idf, version: 5.5.6}", _TRACKER, True, False, id="windows"
+        ),
+        # PlatformIO builds cannot reach the private headers
+        pytest.param(
+            "{type: esp-idf, version: 5.5.6}\n  toolchain: platformio",
+            _TRACKER,
+            False,
+            False,
+            id="platformio",
+        ),
+    ],
+)
+def test_wraps_on_native_builds_with_a_gatt_client(
+    generate_main: Callable[[str | Path], str],
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+    framework: str,
+    components: str,
+    windows: bool,
+    expected: bool,
+) -> None:
+    monkeypatch.setattr("esphome.components.esp32_ble.IS_WINDOWS", windows)
+    config = tmp_path / "test.yaml"
+    config.write_text(
+        "esphome:\n  name: test\n\nesp32:\n  variant: esp32\n"
+        f"  framework: {framework}\n\nesp32_ble: {{}}\n{components}"
+    )
+    generate_main(config)
+    assert CORE.build_flags & _WRAP_FLAGS == (_WRAP_FLAGS if expected else set())
+    defines = {define.name for define in CORE.defines}
+    assert ("USE_ESP32_BLE_BLUEDROID_GATTC_QUEUE" in defines) is expected
+    headers = bluedroid_headers_cmake("bluedroid_gattc_queue.c")
+    assert (headers in CORE.data[KEY_ESP32][KEY_SRC_CMAKE]) is expected