From 6716194e47a1d85aa495eb7b913430edd7087713 Mon Sep 17 00:00:00 2001 From: Clyde Stubbs <2366188+clydebarrow@users.noreply.github.com> Date: Sat, 6 Dec 2025 09:59:29 +1100 Subject: [PATCH] [binary_sensor] Fix reporting of 'unknown' (#12296) Co-authored-by: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Co-authored-by: J. Nick Koston --- .../binary_sensor/binary_sensor.cpp | 13 +- .../components/binary_sensor/binary_sensor.h | 2 + esphome/core/entity_base.h | 16 +- tests/integration/README.md | 24 ++- .../binary_sensor_invalidate_state.yaml | 39 +++++ tests/integration/state_utils.py | 63 ++++++++ .../test_binary_sensor_invalidate_state.py | 138 ++++++++++++++++++ 7 files changed, 283 insertions(+), 12 deletions(-) create mode 100644 tests/integration/fixtures/binary_sensor_invalidate_state.yaml create mode 100644 tests/integration/test_binary_sensor_invalidate_state.py diff --git a/esphome/components/binary_sensor/binary_sensor.cpp b/esphome/components/binary_sensor/binary_sensor.cpp index 92b8db5c51..86b7350aa8 100644 --- a/esphome/components/binary_sensor/binary_sensor.cpp +++ b/esphome/components/binary_sensor/binary_sensor.cpp @@ -34,13 +34,20 @@ void BinarySensor::publish_initial_state(bool new_state) { void BinarySensor::send_state_internal(bool new_state) { // copy the new state to the visible property for backwards compatibility, before any callbacks this->state = new_state; - // Note that set_state_ de-dups and will only trigger callbacks if the state has actually changed - if (this->set_state_(new_state)) { - ESP_LOGD(TAG, "'%s': New state is %s", this->get_name().c_str(), ONOFF(new_state)); + // Note that set_new_state_ de-dups and will only trigger callbacks if the state has actually changed + this->set_new_state(new_state); +} + +bool BinarySensor::set_new_state(const optional &new_state) { + if (StatefulEntityBase::set_new_state(new_state)) { + // weirdly, this file could be compiled even without USE_BINARY_SENSOR defined #if defined(USE_BINARY_SENSOR) && defined(USE_CONTROLLER_REGISTRY) ControllerRegistry::notify_binary_sensor_update(this); #endif + ESP_LOGD(TAG, "'%s': %s", this->get_name().c_str(), ONOFFMAYBE(new_state)); + return true; } + return false; } void BinarySensor::add_filter(Filter *filter) { diff --git a/esphome/components/binary_sensor/binary_sensor.h b/esphome/components/binary_sensor/binary_sensor.h index 0dca3e1520..83c992bfed 100644 --- a/esphome/components/binary_sensor/binary_sensor.h +++ b/esphome/components/binary_sensor/binary_sensor.h @@ -61,6 +61,8 @@ class BinarySensor : public StatefulEntityBase, public EntityBase_DeviceCl protected: Filter *filter_list_{nullptr}; + + bool set_new_state(const optional &new_state) override; }; class BinarySensorInitiallyOff : public BinarySensor { diff --git a/esphome/core/entity_base.h b/esphome/core/entity_base.h index aa9b92877a..fdf3f6300a 100644 --- a/esphome/core/entity_base.h +++ b/esphome/core/entity_base.h @@ -205,7 +205,7 @@ template class StatefulEntityBase : public EntityBase { virtual bool has_state() const { return this->state_.has_value(); } virtual const T &get_state() const { return this->state_.value(); } virtual T get_state_default(T default_value) const { return this->state_.value_or(default_value); } - void invalidate_state() { this->set_state_({}); } + void invalidate_state() { this->set_new_state({}); } void add_full_state_callback(std::function previous, optional current)> &&callback) { if (this->full_state_callbacks_ == nullptr) @@ -227,20 +227,20 @@ template class StatefulEntityBase : public EntityBase { /** * Set a new state for this entity. This will trigger callbacks only if the new state is different from the previous. * - * @param state The new state. + * @param new_state The new state. * @return True if the state was changed, false if it was the same as before. */ - bool set_state_(const optional &state) { - if (this->state_ != state) { + virtual bool set_new_state(const optional &new_state) { + if (this->state_ != new_state) { // call the full state callbacks with the previous and new state if (this->full_state_callbacks_ != nullptr) - this->full_state_callbacks_->call(this->state_, state); + this->full_state_callbacks_->call(this->state_, new_state); // trigger legacy callbacks only if the new state is valid and either the trigger on initial state is enabled or // the previous state was valid auto had_state = this->has_state(); - this->state_ = state; - if (this->state_callbacks_ != nullptr && state.has_value() && (this->trigger_on_initial_state_ || had_state)) - this->state_callbacks_->call(state.value()); + this->state_ = new_state; + if (this->state_callbacks_ != nullptr && new_state.has_value() && (this->trigger_on_initial_state_ || had_state)) + this->state_callbacks_->call(new_state.value()); return true; } return false; diff --git a/tests/integration/README.md b/tests/integration/README.md index 2a6b6fe564..f99139db00 100644 --- a/tests/integration/README.md +++ b/tests/integration/README.md @@ -7,7 +7,7 @@ This directory contains end-to-end integration tests for ESPHome, focusing on te - `conftest.py` - Common fixtures and utilities - `const.py` - Constants used throughout the integration tests - `types.py` - Type definitions for fixtures and functions -- `state_utils.py` - State handling utilities (e.g., `InitialStateHelper`, `build_key_to_entity_mapping`) +- `state_utils.py` - State handling utilities (e.g., `InitialStateHelper`, `find_entity`, `require_entity`) - `fixtures/` - YAML configuration files for tests - `test_*.py` - Individual test files @@ -53,6 +53,28 @@ The `InitialStateHelper` class solves a common problem in integration tests: whe **Future work:** Consider converting existing integration tests to use `InitialStateHelper` for more reliable state tracking and to eliminate race conditions related to initial state broadcasts. +#### Entity Lookup Helpers (`state_utils.py`) + +Two helper functions simplify finding entities in test code: + +**`find_entity(entities, object_id_substring, entity_type=None)`** +- Finds an entity by searching for a substring in its `object_id` (case-insensitive) +- Optionally filters by entity type (e.g., `BinarySensorInfo`) +- Returns `None` if not found + +**`require_entity(entities, object_id_substring, entity_type=None, description=None)`** +- Same as `find_entity` but raises `AssertionError` if not found +- Use `description` parameter for clearer error messages + +```python +from aioesphomeapi import BinarySensorInfo +from .state_utils import require_entity + +# Find entities with clear error messages +binary_sensor = require_entity(entities, "test_sensor", BinarySensorInfo) +button = require_entity(entities, "set_true", description="Set True button") +``` + ### Writing Tests The simplest way to write a test is to use the `run_compiled` and `api_client_connected` fixtures: diff --git a/tests/integration/fixtures/binary_sensor_invalidate_state.yaml b/tests/integration/fixtures/binary_sensor_invalidate_state.yaml new file mode 100644 index 0000000000..4016cfe281 --- /dev/null +++ b/tests/integration/fixtures/binary_sensor_invalidate_state.yaml @@ -0,0 +1,39 @@ +esphome: + name: test-binary-sensor-invalidate + +host: +api: + batch_delay: 0ms # Disable batching to receive all state updates +logger: + level: DEBUG + +# Template binary sensor that we can control +binary_sensor: + - platform: template + name: "Test Binary Sensor" + id: test_binary_sensor + +# Buttons to control the binary sensor state +button: + - platform: template + name: "Set True" + id: set_true_button + on_press: + - binary_sensor.template.publish: + id: test_binary_sensor + state: true + + - platform: template + name: "Set False" + id: set_false_button + on_press: + - binary_sensor.template.publish: + id: test_binary_sensor + state: false + + - platform: template + name: "Invalidate State" + id: invalidate_button + on_press: + - binary_sensor.invalidate_state: + id: test_binary_sensor diff --git a/tests/integration/state_utils.py b/tests/integration/state_utils.py index 6434a41ddf..b649056f2b 100644 --- a/tests/integration/state_utils.py +++ b/tests/integration/state_utils.py @@ -4,11 +4,74 @@ from __future__ import annotations import asyncio import logging +from typing import TypeVar from aioesphomeapi import ButtonInfo, EntityInfo, EntityState _LOGGER = logging.getLogger(__name__) +T = TypeVar("T", bound=EntityInfo) + + +def find_entity( + entities: list[EntityInfo], + object_id_substring: str, + entity_type: type[T] | None = None, +) -> T | EntityInfo | None: + """Find an entity by object_id substring and optionally by type. + + Args: + entities: List of entity info objects from the API + object_id_substring: Substring to search for in object_id (case-insensitive) + entity_type: Optional entity type to filter by (e.g., BinarySensorInfo) + + Returns: + The first matching entity, or None if not found + + Example: + binary_sensor = find_entity(entities, "test_binary_sensor", BinarySensorInfo) + button = find_entity(entities, "set_true") # Any entity type + """ + substring_lower = object_id_substring.lower() + for entity in entities: + if substring_lower in entity.object_id.lower() and ( + entity_type is None or isinstance(entity, entity_type) + ): + return entity + return None + + +def require_entity( + entities: list[EntityInfo], + object_id_substring: str, + entity_type: type[T] | None = None, + description: str | None = None, +) -> T | EntityInfo: + """Find an entity or raise AssertionError if not found. + + Args: + entities: List of entity info objects from the API + object_id_substring: Substring to search for in object_id (case-insensitive) + entity_type: Optional entity type to filter by (e.g., BinarySensorInfo) + description: Human-readable description for error message + + Returns: + The first matching entity + + Raises: + AssertionError: If no matching entity is found + + Example: + binary_sensor = require_entity(entities, "test_sensor", BinarySensorInfo) + button = require_entity(entities, "set_true", description="Set True button") + """ + entity = find_entity(entities, object_id_substring, entity_type) + if entity is None: + desc = description or f"entity with '{object_id_substring}' in object_id" + type_info = f" of type {entity_type.__name__}" if entity_type else "" + raise AssertionError(f"{desc}{type_info} not found in entities") + return entity + def build_key_to_entity_mapping( entities: list[EntityInfo], entity_names: list[str] diff --git a/tests/integration/test_binary_sensor_invalidate_state.py b/tests/integration/test_binary_sensor_invalidate_state.py new file mode 100644 index 0000000000..ee9e57319c --- /dev/null +++ b/tests/integration/test_binary_sensor_invalidate_state.py @@ -0,0 +1,138 @@ +"""Integration test for binary_sensor.invalidate_state() functionality. + +This tests the fix in PR #12296 where invalidate_state() was not properly +reporting the 'unknown' state to the API. The binary sensor should report +missing_state=True when invalidated. + +Regression test for: https://github.com/esphome/esphome/issues/12252 +""" + +from __future__ import annotations + +import asyncio + +from aioesphomeapi import BinarySensorInfo, BinarySensorState, EntityState +import pytest + +from .state_utils import InitialStateHelper, require_entity +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_binary_sensor_invalidate_state( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test that binary_sensor.invalidate_state() reports unknown to the API. + + This verifies that: + 1. Binary sensor starts with missing_state=True (no initial state) + 2. Publishing true sets missing_state=False and state=True + 3. Publishing false sets missing_state=False and state=False + 4. Invalidating state sets missing_state=True (unknown state) + """ + loop = asyncio.get_running_loop() + + # Track state changes + states_received: list[BinarySensorState] = [] + state_future: asyncio.Future[BinarySensorState] = loop.create_future() + + def on_state(state: EntityState) -> None: + """Track binary sensor state changes.""" + if isinstance(state, BinarySensorState): + states_received.append(state) + if not state_future.done(): + state_future.set_result(state) + + async with ( + run_compiled(yaml_config), + api_client_connected() as client, + ): + # Verify device info + device_info = await client.device_info() + assert device_info is not None + assert device_info.name == "test-binary-sensor-invalidate" + + # Get entities + entities, _ = await client.list_entities_services() + + # Find our binary sensor and buttons using helper + binary_sensor = require_entity(entities, "test_binary_sensor", BinarySensorInfo) + set_true_button = require_entity( + entities, "set_true", description="Set True button" + ) + set_false_button = require_entity( + entities, "set_false", description="Set False button" + ) + invalidate_button = require_entity( + entities, "invalidate", description="Invalidate button" + ) + + # Set up initial state helper to handle the initial state broadcast + initial_state_helper = InitialStateHelper(entities) + client.subscribe_states(initial_state_helper.on_state_wrapper(on_state)) + + # Wait for initial states + try: + await initial_state_helper.wait_for_initial_states() + except TimeoutError: + pytest.fail("Timeout waiting for initial states") + + # Check initial state - should be missing (unknown) + initial_state = initial_state_helper.initial_states.get(binary_sensor.key) + assert initial_state is not None, "No initial state received for binary sensor" + assert isinstance(initial_state, BinarySensorState) + assert initial_state.missing_state is True, ( + f"Initial state should have missing_state=True, got {initial_state}" + ) + + # Test 1: Set state to true + states_received.clear() + state_future = loop.create_future() + client.button_command(set_true_button.key) + + try: + state = await asyncio.wait_for(state_future, timeout=5.0) + except TimeoutError: + pytest.fail("Timeout waiting for state=true") + + assert state.missing_state is False, ( + f"After setting true, missing_state should be False, got {state}" + ) + assert state.state is True, f"Expected state=True, got {state}" + + # Test 2: Set state to false + states_received.clear() + state_future = loop.create_future() + client.button_command(set_false_button.key) + + try: + state = await asyncio.wait_for(state_future, timeout=5.0) + except TimeoutError: + pytest.fail("Timeout waiting for state=false") + + assert state.missing_state is False, ( + f"After setting false, missing_state should be False, got {state}" + ) + assert state.state is False, f"Expected state=False, got {state}" + + # Test 3: Invalidate state (set to unknown) + # This is the critical test for the bug fix + states_received.clear() + state_future = loop.create_future() + client.button_command(invalidate_button.key) + + try: + state = await asyncio.wait_for(state_future, timeout=5.0) + except TimeoutError: + pytest.fail( + "Timeout waiting for invalidated state - " + "binary_sensor.invalidate_state() may not be reporting to the API. " + "See issue #12252." + ) + + assert state.missing_state is True, ( + f"After invalidate_state(), missing_state should be True (unknown), " + f"got {state}. This is the regression from issue #12252." + )