Merge branch 'dev' into esp32-ble-conn-param-repeat-log

.github/workflows/ci.yml

@@ -462,7 +462,6 @@ jobs:
       - name: Register matcher
         run: echo "::add-matcher::.github/workflows/matchers/pytest.json"
       - name: Run integration tests
-        id: integration
         env:
           # JSON array of test paths; parsed into a bash array below to avoid
           # shell word-splitting / glob hazards.
@@ -478,32 +477,6 @@ jobs:
           # A hung test fails after 30 min: the 15 min shared build lock wait plus a cold compile
           pytest -vv --no-cov --tb=native --durations=30 -n auto --dist worksteal --timeout=1800 \
             --junitxml=junit-integration.xml "${test_files[@]}"
-      - name: Re-run the failed tests with debug symbols
-        # The first run builds without -g. A crash in the re-run prints a gdb
-        # backtrace of its core; exit 1 keeps the job red when the re-run passes.
-        if: failure() && steps.integration.outcome == 'failure'
-        timeout-minutes: 30
-        env:
-          ESPHOME_INTEGRATION_DEBUG_SYMBOLS: "1"
-          ESPHOME_INTEGRATION_CORE_DIR: /tmp/esphome-cores
-        run: |
-          . venv/bin/activate
-          # The image's package lists usually suffice; refresh them only if not
-          if ! command -v gdb >/dev/null &&
-            ! sudo DEBIAN_FRONTEND=noninteractive timeout -k 15 90 apt-get install -y -qq gdb; then
-            sudo timeout -k 15 300 apt-get update -qq &&
-              sudo DEBIAN_FRONTEND=noninteractive timeout -k 15 300 apt-get install -y -qq gdb ||
-              echo "::warning::gdb install failed; no backtraces"
-          fi
-          {
-            mkdir -p "$ESPHOME_INTEGRATION_CORE_DIR" &&
-              echo "$ESPHOME_INTEGRATION_CORE_DIR/core.%p" | sudo tee /proc/sys/kernel/core_pattern &&
-              ulimit -c unlimited
-          } || echo "::warning::core dumps not enabled; no backtraces"
-          # Under the step limit, so a hung test still reports before Actions kills the step
-          pytest -vv --no-cov --tb=native -n auto --dist worksteal --timeout=600 \
-            --lf --lfnf=none tests/integration || true
-          exit 1
       - name: Upload test artifacts
         # Tests that compare rendered output write the image they actually got here, so a
         # failure can be looked at without reproducing the whole build locally.

esphome/components/esp32_ble/ble.cpp

@@ -649,20 +649,30 @@ void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_pa
     // never answer an update, and without this the link silently stays on the
     // old parameters (visible only as unexplained supervision timeouts).
     case ESP_GAP_BLE_UPDATE_CONN_PARAMS_EVT: {
+      // A peer can keep asking for parameters it then rejects: warn once per peer, repeats are
+      // verbose until an update succeeds or it disconnects.
       if (param->update_conn_params.status != ESP_BT_STATUS_SUCCESS) {
         char mac_s[MAC_ADDRESS_PRETTY_BUFFER_SIZE];
         format_mac_addr_upper(param->update_conn_params.bda, mac_s);
-        ESP_LOGW(TAG, "[%s] Conn param update failed, status=%d", mac_s, param->update_conn_params.status);
-      }
+        if (global_ble->conn_param_failed_slot_(param->update_conn_params.bda) != nullptr) {
+          ESP_LOGV(TAG, "[%s] Conn param update failed again, status=%d", mac_s, param->update_conn_params.status);
+        } else {
+          ESP_LOGW(TAG, "[%s] Conn param update failed, status=%d", mac_s, param->update_conn_params.status);
+          static constexpr esp_bd_addr_t FREE{};
+          uint8_t *slot = global_ble->conn_param_failed_slot_(FREE);
+          if (slot != nullptr)
+            memcpy(slot, param->update_conn_params.bda, sizeof(esp_bd_addr_t));
+        }
+      } else {
+        global_ble->forget_conn_param_failure_(param->update_conn_params.bda);
 #if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERBOSE
-      else {
         char mac_s[MAC_ADDRESS_PRETTY_BUFFER_SIZE];
         format_mac_addr_upper(param->update_conn_params.bda, mac_s);
         ESP_LOGV(TAG, "[%s] Conn params updated: interval=%u (x1.25ms) latency=%u timeout=%u (x10ms)", mac_s,
                  param->update_conn_params.conn_int, param->update_conn_params.latency,
                  param->update_conn_params.timeout);
-      }
 #endif
+      }
       return;
     }
 
@@ -684,6 +694,8 @@ void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_pa
 #ifdef USE_ESP32_BLE_SERVER
 void ESP32BLE::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if,
                                    esp_ble_gatts_cb_param_t *param) {
+  if (event == ESP_GATTS_DISCONNECT_EVT)
+    global_ble->forget_conn_param_failure_(param->disconnect.remote_bda);
   enqueue_ble_event(event, gatts_if, param);
   // Wake up main loop to process GATT event immediately
   App.wake_loop_threadsafe();
@@ -693,12 +705,28 @@ void ESP32BLE::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t gat
 #ifdef USE_ESP32_BLE_CLIENT
 void ESP32BLE::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if,
                                    esp_ble_gattc_cb_param_t *param) {
+  if (event == ESP_GATTC_DISCONNECT_EVT)
+    global_ble->forget_conn_param_failure_(param->disconnect.remote_bda);
   enqueue_ble_event(event, gattc_if, param);
   // Wake up main loop to process GATT event immediately
   App.wake_loop_threadsafe();
 }
 #endif
 
+uint8_t *ESP32BLE::conn_param_failed_slot_(const uint8_t *bda) {
+  for (auto &slot : this->conn_param_failed_bdas_) {
+    if (memcmp(slot, bda, sizeof(esp_bd_addr_t)) == 0)
+      return slot;
+  }
+  return nullptr;
+}
+
+void ESP32BLE::forget_conn_param_failure_(const uint8_t *bda) {
+  uint8_t *slot = this->conn_param_failed_slot_(bda);
+  if (slot != nullptr)
+    memset(slot, 0, sizeof(esp_bd_addr_t));
+}
+
 void ESP32BLE::get_mac_msb_first(uint8_t out[MAC_ADDRESS_SIZE]) const {
   // The running stack owns the address (on hosted controllers it lives in
   // the remote chip's efuse); null before init becomes all-zero.

esphome/components/esp32_ble/ble.h

@@ -174,6 +174,9 @@ class ESP32BLE final : public Component {
   // Other non-ACTIVE states (e.g. OFF, DISABLED) are currently treated as no-ops.
   void __attribute__((noinline)) loop_handle_state_transition_not_active_();
 
+  uint8_t *conn_param_failed_slot_(const uint8_t *bda);
+  void forget_conn_param_failure_(const uint8_t *bda);
+
   bool ble_setup_();
   bool ble_dismantle_();
   void request_state_(bool enable);
@@ -244,6 +247,8 @@ class ESP32BLE final : public Component {
   esp_ble_io_cap_t io_cap_{ESP_IO_CAP_NONE};          // 1 byte (uint8_t typedef)
   BLEComponentState state_{BLE_COMPONENT_STATE_OFF};  // 1 byte (uint8_t enum)
   bool enable_on_boot_{};                             // 1 byte
+  // Peers already warned about a failed conn param update, one per connection; all-zero is free
+  esp_bd_addr_t conn_param_failed_bdas_[USE_ESP32_BLE_MAX_CONNECTIONS]{};
 #ifdef USE_ESP32_BLE_ADVERTISING
   uint8_t advertising_ref_count_{0};  // 1 byte, number of components requesting advertising
 #endif

tests/integration/conftest.py

@@ -16,7 +16,6 @@ import re
 import shutil
 import signal
 import socket
-import subprocess
 import sys
 import tempfile
 import time
@@ -29,7 +28,6 @@ import pytest_asyncio
 import esphome.config
 from esphome.core import CORE
 from esphome.helpers import (
-    get_bool_env,
     get_usable_cpu_count,
     read_file,
     rmtree,
@@ -41,7 +39,6 @@ from esphome.host.toolchain import get_elf_path
 from .const import (
     API_CONNECTION_TIMEOUT,
     DEFAULT_API_PORT,
-    GDB_TIMEOUT,
     LOCALHOST,
     PORT_POLL_INTERVAL,
     PORT_WAIT_TIMEOUT,
@@ -83,13 +80,6 @@ REPO_ROOT = Path(__file__).resolve().parent.parent.parent
 # CI caches parts of this path; keep in sync with ci.yml integration-tests.
 INTEGRATION_TESTS_ROOT = Path.home() / ".esphome-integration-tests"
 
-# Opt-in -g and gdb core backtraces; CI enables both only to re-run failed tests
-DEBUG_SYMBOLS = get_bool_env("ESPHOME_INTEGRATION_DEBUG_SYMBOLS")
-CORE_DIR = os.environ.get("ESPHOME_INTEGRATION_CORE_DIR")
-_CRASH_SIGNALS = frozenset(
-    {signal.SIGABRT, signal.SIGBUS, signal.SIGFPE, signal.SIGILL, signal.SIGSEGV}
-)
-
 
 def _get_build_env() -> dict[str, str]:
     """Environment for an ``esphome compile`` subprocess."""
@@ -237,12 +227,9 @@ async def yaml_config(request: pytest.FixtureRequest, unused_tcp_port: int) -> s
             "  build_flags:\n"
             '    - "-DDEBUG"  # Enable assert() statements\n'
             '    - "-DESPHOME_DEBUG"  # Enable ESPHOME_DEBUG_ASSERT checks\n'
-            '    - "-DESPHOME_DEBUG_API"  # Enable API protocol asserts'
-            + ('\n    - "-g"  # Add debug symbols' if DEBUG_SYMBOLS else ""),
+            '    - "-DESPHOME_DEBUG_API"  # Enable API protocol asserts\n'
+            '    - "-g"       # Add debug symbols',
         )
-    elif DEBUG_SYMBOLS and "  build_flags:\n" in content:
-        # A fixture with its own flags still gets symbols for a debug re-run
-        content = content.replace("  build_flags:\n", '  build_flags:\n    - "-g"\n', 1)
 
     # Replace external component path placeholder if present
     if "EXTERNAL_COMPONENT_PATH" in content:
@@ -785,46 +772,16 @@ async def _read_stream_lines(
             line_callback(decoded_line.rstrip())
 
 
-def _print_crash_backtrace(binary_path: Path, returncode: int, pid: int) -> None:
-    """Print a gdb backtrace from the core of a binary that died from a crash signal."""
-    if CORE_DIR is None or -returncode not in _CRASH_SIGNALS:
-        return
-    core = Path(CORE_DIR) / f"core.{pid}"
-    if not core.is_file() or (gdb := shutil.which("gdb")) is None:
-        print(f"No core or gdb for the crash of {binary_path} (looked for {core})")
-        return
-    try:
-        result = subprocess.run(
-            [gdb, "-batch", "-iex", "set debuginfod enabled off"]
-            + ["-ex", "thread apply all bt", str(binary_path), str(core)],
-            capture_output=True,
-            text=True,
-            errors="replace",
-            timeout=GDB_TIMEOUT,
-            check=False,
-        )
-    except (OSError, subprocess.TimeoutExpired) as err:
-        # Best effort: never replace the test's own failure
-        print(f"gdb could not read {core}: {err}")
-        return
-    print(f"--- Backtrace of {binary_path} ({core}) ---")
-    print(result.stdout + result.stderr)
-
-
 @asynccontextmanager
 async def run_binary(
     binary_path: Path,
     line_callback: Callable[[str], None] | None = None,
-    symbols_path: Path | None = None,
 ) -> AsyncGenerator[tuple[asyncio.subprocess.Process, list[str]]]:
     """Run a binary under a PTY, capture log output, and clean up on exit.
 
     Yields the running ``Process`` and a live list of captured log lines.
     No port wait -- callers that need that should use
-    ``run_binary_and_wait_for_port``.
-    ``symbols_path`` is the unstripped build when ``binary_path`` is a stripped
-    copy; a crash backtrace reads its symbols from it.
-    """
+    ``run_binary_and_wait_for_port``."""
     # Create a pseudo-terminal to make the binary think it's running interactively
     # This is needed because the ESPHome host logger checks isatty()
     controller_fd, device_fd = pty.openpty()
@@ -853,9 +810,6 @@ async def run_binary(
     )
 
     if process.returncode is not None:
-        _print_crash_backtrace(
-            symbols_path or binary_path, process.returncode, process.pid
-        )
         raise RuntimeError(
             f"Process died immediately with return code {process.returncode}. "
             "Ensure the binary is valid and can run successfully."
@@ -897,9 +851,6 @@ async def run_binary(
                     # Last resort: SIGKILL
                     process.kill()
                     await process.wait()
-        _print_crash_backtrace(
-            symbols_path or binary_path, process.returncode, process.pid
-        )
 
 
 @asynccontextmanager

tests/integration/const.py

@@ -19,4 +19,3 @@ KEY_ACTIVATION_DELAY = 0.5  # seconds
 # Process shutdown timeouts
 SIGINT_TIMEOUT = 5.0  # seconds
 SIGTERM_TIMEOUT = 2.0  # seconds
-GDB_TIMEOUT = 60.0  # seconds, reading a crashed binary's core

tests/integration/test_host_ota.py

@@ -141,10 +141,9 @@ async def _build(
     write_yaml_config: ConfigWriter,
     compile_esphome: CompileFunction,
     reserved_tcp_port: tuple[int, socket.socket],
-) -> tuple[int, int, Path, Path]:
+) -> tuple[int, int, Path]:
     """Reserve an OTA port, compile the fixture with it, and release both
-    ports right before the binary is started. Returns the stripped binary to
-    run and upload, then the unstripped build for crash backtraces."""
+    ports right before the binary is started."""
     api_port, api_socket = reserved_tcp_port
     with _reserve_port() as (ota_port, ota_socket):
         config_path = await write_yaml_config(
@@ -153,11 +152,11 @@ async def _build(
         binary_path = await compile_esphome(config_path)
         api_socket.close()
         ota_socket.close()
-    return api_port, ota_port, await _strip_debug(binary_path), binary_path
+    return api_port, ota_port, await _strip_debug(binary_path)
 
 
 async def _strip_debug(binary_path: Path) -> Path:
-    """A debug re-run builds with -g; every upload sends the image, so drop the
+    """The fixtures build with -g; every upload sends the image, so drop the
     debug info (GNU and Apple strip both take -S)."""
     if (strip := shutil.which("strip")) is None:
         _LOGGER.warning(
@@ -208,7 +207,6 @@ class _Device:
     api_port: int
     ota_port: int
     binary_path: Path
-    symbols_path: Path
     proc: asyncio.subprocess.Process | None = None
     waiter: LineWaiter = field(default_factory=LineWaiter)
 
@@ -304,9 +302,7 @@ async def test_host_ota_self_update(
             yaml_config, write_yaml_config, compile_esphome, reserved_tcp_port
         )
     )
-    async with run_binary(
-        dev.binary_path, symbols_path=dev.symbols_path, line_callback=dev.on_log
-    ) as (
+    async with run_binary(dev.binary_path, line_callback=dev.on_log) as (
         proc,
         _lines,
     ):
@@ -345,9 +341,7 @@ async def test_host_ota_deflate(
         """A stream that inflates past the size the client announced."""
         return _REAL_COMPRESS(data + bytes(8192), *args, **kwargs)
 
-    async with run_binary(
-        dev.binary_path, symbols_path=dev.symbols_path, line_callback=dev.on_log
-    ) as (
+    async with run_binary(dev.binary_path, line_callback=dev.on_log) as (
         proc,
         _lines,
     ):
@@ -428,9 +422,7 @@ async def test_host_ota_encrypted(
             yaml_config, write_yaml_config, compile_esphome, reserved_tcp_port
         )
     )
-    async with run_binary(
-        dev.binary_path, symbols_path=dev.symbols_path, line_callback=dev.on_log
-    ) as (proc, lines):
+    async with run_binary(dev.binary_path, line_callback=dev.on_log) as (proc, lines):
         dev.proc = proc
         await _wait_for_port(LOCALHOST, dev.api_port, PORT_WAIT_TIMEOUT)
         await asyncio.get_running_loop().run_in_executor(
@@ -466,9 +458,7 @@ async def test_host_ota_encrypted_safe_mode(
     )
     # The api port never opens in safe mode, so wait for the log line instead
     force_safe_mode(DEVICE_NAME)
-    async with run_binary(
-        dev.binary_path, symbols_path=dev.symbols_path, line_callback=dev.on_log
-    ) as (proc, lines):
+    async with run_binary(dev.binary_path, line_callback=dev.on_log) as (proc, lines):
         dev.proc = proc
         await _wait_for_line(lines, "SAFE MODE IS ACTIVE", PORT_WAIT_TIMEOUT)
         await _wait_for_port(LOCALHOST, dev.ota_port, PORT_WAIT_TIMEOUT)
@@ -495,9 +485,7 @@ async def test_host_ota_api_key_offer_with_password(
             yaml_config, write_yaml_config, compile_esphome, reserved_tcp_port
         )
     )
-    async with run_binary(
-        dev.binary_path, symbols_path=dev.symbols_path, line_callback=dev.on_log
-    ) as (proc, lines):
+    async with run_binary(dev.binary_path, line_callback=dev.on_log) as (proc, lines):
         dev.proc = proc
         await _wait_for_port(LOCALHOST, dev.api_port, PORT_WAIT_TIMEOUT)
         await _wait_for_line(lines, "Encryption: offered")
@@ -547,9 +535,7 @@ async def test_host_ota_provisioned_api_key(
             yaml_config, write_yaml_config, compile_esphome, reserved_tcp_port
         )
     )
-    async with run_binary(
-        dev.binary_path, symbols_path=dev.symbols_path, line_callback=dev.on_log
-    ) as (proc, lines):
+    async with run_binary(dev.binary_path, line_callback=dev.on_log) as (proc, lines):
         dev.proc = proc
         await _wait_for_port(LOCALHOST, dev.api_port, PORT_WAIT_TIMEOUT)
         await _wait_for_line(lines, "once the api key is provisioned")
@@ -584,9 +570,7 @@ async def test_host_ota_provisioned_api_key_safe_mode(
             yaml_config, write_yaml_config, compile_esphome, reserved_tcp_port
         )
     )
-    async with run_binary(
-        dev.binary_path, symbols_path=dev.symbols_path, line_callback=dev.on_log
-    ) as (proc, _lines):
+    async with run_binary(dev.binary_path, line_callback=dev.on_log) as (proc, _lines):
         dev.proc = proc
         await _wait_for_port(LOCALHOST, dev.api_port, PORT_WAIT_TIMEOUT)
         await _provision_key(dev, api_client_connected)
@@ -595,9 +579,7 @@ async def test_host_ota_provisioned_api_key_safe_mode(
     # exits, so safe mode takes a second start
     force_safe_mode(DEVICE_NAME)
     key = PROVISIONING_PSK.decode()
-    async with run_binary(
-        dev.binary_path, symbols_path=dev.symbols_path, line_callback=dev.on_log
-    ) as (proc, lines):
+    async with run_binary(dev.binary_path, line_callback=dev.on_log) as (proc, lines):
         dev.proc = proc
         await _wait_for_line(lines, "SAFE MODE IS ACTIVE", PORT_WAIT_TIMEOUT)
         await _wait_for_port(LOCALHOST, dev.ota_port, PORT_WAIT_TIMEOUT)
@@ -629,10 +611,7 @@ async def test_host_ota_rejects_garbage(
     bogus_path = integration_test_dir / "bogus.bin"
     bogus_path.write_bytes(b"NOT-AN-EXECUTABLE-AT-ALL" * 8)
 
-    async with run_binary(dev.binary_path, symbols_path=dev.symbols_path) as (
-        proc,
-        _lines,
-    ):
+    async with run_binary(dev.binary_path) as (proc, _lines):
         dev.proc = proc
         await _wait_for_port(LOCALHOST, dev.api_port, PORT_WAIT_TIMEOUT)
         pid_before = proc.pid