From 357c02079391d33ec0682ed75f3b57d52149f728 Mon Sep 17 00:00:00 2001 From: Alar Aun Date: Mon, 5 Oct 2026 22:42:36 +0300 Subject: [PATCH 1/4] fix: refresh on input changes without an executor job per state change The input listeners were plain functions, so Home Assistant ran each of the 11 as an executor job for every state_changed event in the system, and called hass.async_create_task from a worker thread. At shutdown the executor is gone and every state change logs 11 "Executor shutdown has been called" errors. They also never matched: they compared against number._, but entity_ids follow the device name, so a parameter change reached the controller only on the next interval tick. Use one callback listener with a callback event filter that matches the entry's input entities by unique_id, as PIDDeviceHandle already does. An entity being added, removed or passing through unavailable/unknown is not treated as a change. --- .../simple_pid_controller/sensor.py | 95 ++++++++----- tests/test_sensor.py | 134 +++++++++++++----- tests/test_unload_listeners.py | 2 +- 3 files changed, 158 insertions(+), 73 deletions(-) diff --git a/custom_components/simple_pid_controller/sensor.py b/custom_components/simple_pid_controller/sensor.py index 2495241..aae282d 100644 --- a/custom_components/simple_pid_controller/sensor.py +++ b/custom_components/simple_pid_controller/sensor.py @@ -4,12 +4,16 @@ import logging -from collections.abc import Callable - from homeassistant.components.sensor import SensorEntity, SensorStateClass from homeassistant.config_entries import ConfigEntry -from homeassistant.const import EntityCategory -from homeassistant.core import Event, HomeAssistant +from homeassistant.const import ( + EVENT_STATE_CHANGED, + STATE_UNAVAILABLE, + STATE_UNKNOWN, + EntityCategory, +) +from homeassistant.core import Event, EventStateChangedData, HomeAssistant, callback +from homeassistant.helpers import entity_registry as er from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.update_coordinator import CoordinatorEntity from homeassistant.helpers.restore_state import RestoreEntity @@ -20,6 +24,7 @@ from typing import Any from . import PIDDeviceHandle +from .const import DOMAIN from .entity import BasePIDEntity from .coordinator import PIDDataCoordinator @@ -28,6 +33,8 @@ _LOGGER = logging.getLogger(__name__) +_NO_VALUE = (STATE_UNAVAILABLE, STATE_UNKNOWN) + async def async_setup_entry( hass: HomeAssistant, @@ -209,44 +216,56 @@ async def start_refresh(_: Any) -> None: ] ) - # Put listeners on inputs - def make_listener(entity_id: str) -> Callable[[Event[Any]], None]: - def _listener(event: Event[Any]) -> None: - if event.data.get("entity_id") == entity_id: - _LOGGER.debug("Update detected on %s", entity_id) - # async_request_refresh is a coroutine: calling it from this - # synchronous listener without scheduling it left the refresh - # unawaited, so a change to one of the inputs below never - # actually reached the controller. - hass.async_create_task(coordinator.async_request_refresh()) - - return _listener - - for key in [ - "kp", - "ki", - "kd", - "setpoint", - "output_min", - "output_max", - "sample_time", - ]: - unsub = hass.bus.async_listen( - "state_changed", make_listener(f"number.{entry.entry_id}_{key}") + # Refresh when the value of one of this entry's inputs changes. An entity + # being added or removed, or passing through unavailable/unknown (restore + # at startup, unload), is not a change. The inputs' entity_ids follow the + # device name and can be renamed, so they are matched by unique_id. + watched = { + f"{entry.entry_id}_{key}" + for key in ( + "kp", + "ki", + "kd", + "setpoint", + "output_min", + "output_max", + "sample_time", + "auto_mode", + "proportional_on_measurement", + "windup_protection", + "start_mode", ) - entry.async_on_unload(unsub) - - for key in ["auto_mode", "proportional_on_measurement", "windup_protection"]: - unsub = hass.bus.async_listen( - "state_changed", make_listener(f"switch.{entry.entry_id}_{key}") + } + registry = er.async_get(hass) + + @callback + def _is_input(event_data: EventStateChangedData) -> bool: + old, new = event_data["old_state"], event_data["new_state"] + if ( + old is None + or new is None + or old.state in _NO_VALUE + or new.state in _NO_VALUE + or old.state == new.state + ): + return False + entity = registry.async_get(event_data["entity_id"]) + return ( + entity is not None + and entity.platform == DOMAIN + and entity.unique_id in watched ) - entry.async_on_unload(unsub) - for key in ["start_mode"]: - unsub = hass.bus.async_listen( - "state_changed", make_listener(f"select.{entry.entry_id}_{key}") + @callback + def _on_input_change(event: Event[EventStateChangedData]) -> None: + _LOGGER.debug("Update detected on %s", event.data["entity_id"]) + hass.async_create_task(coordinator.async_request_refresh()) + + entry.async_on_unload( + hass.bus.async_listen( + EVENT_STATE_CHANGED, _on_input_change, event_filter=_is_input ) - entry.async_on_unload(unsub) + ) class PIDOutputSensor( diff --git a/tests/test_sensor.py b/tests/test_sensor.py index a0e6ec7..84d8898 100644 --- a/tests/test_sensor.py +++ b/tests/test_sensor.py @@ -1,5 +1,8 @@ import pytest from datetime import timedelta +from homeassistant.const import EVENT_STATE_CHANGED +from homeassistant.core import is_callback +from homeassistant.helpers import entity_registry as er from homeassistant.util.dt import utcnow from pytest_homeassistant_custom_component.common import async_fire_time_changed from custom_components.simple_pid_controller.sensor import ( @@ -104,54 +107,117 @@ async def test_pid_contribution_native_value_rounding_and_none(hass, config_entr await coordinator.async_shutdown() -@pytest.mark.usefixtures("setup_integration") -@pytest.mark.asyncio -async def test_listeners_trigger_refresh_sensor(hass, config_entry, monkeypatch): - """Lines 131-132: coordinator.async_request_refresh called on sensor state change.""" - # Prepare handle - handle = config_entry.runtime_data.handle - handle.get_input_sensor_value = lambda: 0.0 - handle.get_number = lambda key: 0.0 - handle.get_switch = lambda key: True +def _count_refreshes(config_entry, monkeypatch) -> list[bool]: + """Replace the coordinator refresh with a coroutine that records each call.""" + called: list[bool] = [] + + # A coroutine function, so the test proves the scheduled refresh is awaited. + async def _fake_refresh(): + called.append(True) - # Capture listeners - listeners = [] monkeypatch.setattr( - type(hass.bus), - "async_listen", - lambda self, event, cb: listeners.append((event, cb)), + config_entry.runtime_data.coordinator, "async_request_refresh", _fake_refresh ) + return called + + +@pytest.mark.usefixtures("setup_integration") +@pytest.mark.parametrize( + ("domain", "service", "key", "data"), + [ + ("number", "set_value", "kp", {"value": 2.0}), + ("switch", "turn_off", "auto_mode", {}), + ("select", "select_option", "start_mode", {"option": "Last known value"}), + ], +) +async def test_input_change_requests_refresh( + hass, config_entry, monkeypatch, domain, service, key, data +): + """Changing one of the controller's own inputs requests a refresh.""" + called = _count_refreshes(config_entry, monkeypatch) + entity_id = config_entry.runtime_data.handle._get_entity_id(domain, key) + assert entity_id is not None + + await hass.services.async_call( + domain, service, {"entity_id": entity_id, **data}, blocking=True + ) + await hass.async_block_till_done() + + assert called - # Run setup to register listeners - entities = [] - await async_setup_entry(hass, config_entry, lambda ents: entities.extend(ents)) - coordinator = entities[0].coordinator - # Patch refresh method. It has to stay a coroutine function: the listener - # schedules it as a task, so a plain lambda would only prove that the - # listener fired, not that the refresh was ever awaited. +@pytest.mark.usefixtures("setup_integration") +async def test_unrelated_state_change_does_not_refresh(hass, config_entry, monkeypatch): + """State changes of other entities, and of this entry's outputs, are ignored.""" + called = _count_refreshes(config_entry, monkeypatch) + output = config_entry.runtime_data.handle._get_entity_id("sensor", "pid_output") + assert output is not None + + hass.states.async_set("sensor.unrelated", "1") + hass.states.async_set(output, "3.0") + await hass.async_block_till_done() + + assert not called + + +@pytest.mark.usefixtures("setup_integration") +async def test_renamed_input_still_requests_refresh(hass, config_entry, monkeypatch): + """Inputs are matched by unique_id, so a renamed entity_id keeps working.""" + called = _count_refreshes(config_entry, monkeypatch) + registry = er.async_get(hass) + old = config_entry.runtime_data.handle._get_entity_id("number", "kp") + registry.async_update_entity(old, new_entity_id="number.renamed_kp") + await hass.async_block_till_done() + + await hass.services.async_call( + "number", + "set_value", + {"entity_id": "number.renamed_kp", "value": 3.0}, + blocking=True, + ) + await hass.async_block_till_done() + + assert called + + +async def test_setup_and_unload_do_not_request_refresh(hass, config_entry, monkeypatch): + """Inputs being added, restored or unloaded do not request a refresh.""" called = [] - async def _fake_refresh(): + async def _fake_refresh(self): called.append(True) - monkeypatch.setattr(coordinator, "async_request_refresh", _fake_refresh) + monkeypatch.setattr(PIDDataCoordinator, "async_request_refresh", _fake_refresh) + assert await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + assert await hass.config_entries.async_unload(config_entry.entry_id) + await hass.async_block_till_done() + + assert not called + - # Simulate state change event for kp - entry_id = config_entry.entry_id - test_entity = f"number.{entry_id}_kp" - callback = next(cb for evt, cb in listeners if evt == "state_changed") +async def test_input_listener_runs_in_event_loop(hass, config_entry, monkeypatch): + """The state_changed listener and its filter are callbacks, not executor jobs.""" + captured = [] + original = type(hass.bus).async_listen - from types import SimpleNamespace + def _spy(self, event_type, listener, event_filter=None, **kwargs): + if event_type == EVENT_STATE_CHANGED and listener.__module__ == ( + sensor_module.__name__ + ): + captured.append((listener, event_filter)) + return original(self, event_type, listener, event_filter, **kwargs) - event = SimpleNamespace(data={"entity_id": test_entity}) - callback(event) + monkeypatch.setattr(type(hass.bus), "async_listen", _spy) + assert await hass.config_entries.async_setup(config_entry.entry_id) await hass.async_block_till_done() - assert called, ( - "Coordinator.async_request_refresh was not called on sensor state change" - ) - await async_unload_entry(hass, config_entry) + assert len(captured) == 1 + listener, event_filter = captured[0] + assert is_callback(listener) + assert event_filter is not None and is_callback(event_filter) + + await hass.config_entries.async_unload(config_entry.entry_id) @pytest.mark.usefixtures("setup_integration") diff --git a/tests/test_unload_listeners.py b/tests/test_unload_listeners.py index 2602a3d..0bcd8df 100644 --- a/tests/test_unload_listeners.py +++ b/tests/test_unload_listeners.py @@ -10,7 +10,7 @@ async def test_listeners_removed_after_unload(hass, config_entry, monkeypatch): created = [] called = [] - def fake_listen(self, event, callback): + def fake_listen(self, event, callback, event_filter=None, **kwargs): def unsub(): called.append(True) From 4b0976276c856083a32c535f4d5c3eb1267afaa2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 20:11:16 +0000 Subject: [PATCH 2/4] fix: use probatio schemas on Home Assistant 2026.10 and later Home Assistant 2026.10 replaced voluptuous with probatio and now types async_show_form's data_schema as probatio.Schema, so the lint job's mypy step fails on dev (scheduled run of 2026-10-05) and on every pull request. voluptuous is also no longer a Home Assistant dependency. Import probatio as vol from 2026.10 on and voluptuous before that. The choice goes by version, not by whether probatio imports: 2026.9 already installs probatio but still expects voluptuous schemas. The pre-commit mypy hook types against the latest stable release (2026.9) and the lint job against the one pytest-homeassistant-custom-component pins (2026.10), so the two config flow schemas are annotated Any. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012VjGcBURcFQJMzcFaFvciV --- .../simple_pid_controller/__init__.py | 17 ++++++++++++++-- .../simple_pid_controller/config_flow.py | 20 +++++++++++++++---- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/custom_components/simple_pid_controller/__init__.py b/custom_components/simple_pid_controller/__init__.py index 0221e37..bedf4b1 100644 --- a/custom_components/simple_pid_controller/__init__.py +++ b/custom_components/simple_pid_controller/__init__.py @@ -4,16 +4,29 @@ import logging from homeassistant.config_entries import ConfigEntry -from homeassistant.const import Platform, ATTR_ENTITY_ID +from homeassistant.const import ( + ATTR_ENTITY_ID, + MAJOR_VERSION, + MINOR_VERSION, + Platform, +) from homeassistant.core import HomeAssistant, ServiceCall from homeassistant.exceptions import ConfigEntryNotReady, HomeAssistantError from homeassistant.helpers import entity_registry as er from collections import deque from dataclasses import dataclass from simple_pid import PID -import voluptuous as vol import homeassistant.helpers.config_validation as cv +# Home Assistant 2026.10 replaced voluptuous with probatio, an API-compatible +# successor, and from then on expects probatio schemas. 2026.9 already ships +# probatio but still expects voluptuous ones, so the version decides, not +# whether probatio happens to be importable. +if (MAJOR_VERSION, MINOR_VERSION) >= (2026, 10): + import probatio as vol +else: + import voluptuous as vol # type: ignore[no-redef] + from .coordinator import PIDDataCoordinator from .const import ( diff --git a/custom_components/simple_pid_controller/config_flow.py b/custom_components/simple_pid_controller/config_flow.py index f2394ef..1f8a148 100644 --- a/custom_components/simple_pid_controller/config_flow.py +++ b/custom_components/simple_pid_controller/config_flow.py @@ -5,17 +5,25 @@ import logging from typing import Any -import voluptuous as vol - from homeassistant.config_entries import ( ConfigEntry, ConfigFlow, ConfigFlowResult, OptionsFlow, ) +from homeassistant.const import MAJOR_VERSION, MINOR_VERSION from homeassistant.core import callback from homeassistant.helpers.selector import selector +# Home Assistant 2026.10 replaced voluptuous with probatio, an API-compatible +# successor, and from then on expects probatio schemas. 2026.9 already ships +# probatio but still expects voluptuous ones, so the version decides, not +# whether probatio happens to be importable. +if (MAJOR_VERSION, MINOR_VERSION) >= (2026, 10): + import probatio as vol +else: + import voluptuous as vol # type: ignore[no-redef] + from .const import ( DOMAIN, CONF_NAME, @@ -54,7 +62,10 @@ async def async_step_user( ) -> ConfigFlowResult: """Handle the initial step.""" - schema = vol.Schema( + # Any: whether async_show_form takes a probatio or a voluptuous schema + # depends on the Home Assistant version (see the import above), and the + # pre-commit hook and the lint job type check against different ones. + schema: Any = vol.Schema( { vol.Required(CONF_NAME, default=DEFAULT_NAME): str, vol.Required(CONF_SENSOR_ENTITY_ID): selector( @@ -160,7 +171,8 @@ async def async_step_init( for key in DEFAULT_STEPS } - options_schema = vol.Schema( + # Any, for the reason given in async_step_user. + options_schema: Any = vol.Schema( { vol.Required( CONF_SENSOR_ENTITY_ID, From 005127bfdacaa7136ea87c665a7d27e9143c5616 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 20:11:47 +0000 Subject: [PATCH 3/4] test: make the unrelated-state-change test reach the unique_id check Both entities went from no state or unknown to a value, which the event filter rejects before it looks at the unique_id. The test therefore kept passing with the unique_id check replaced by 'return True'. Give them a value first, so the change under test is a real one. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012VjGcBURcFQJMzcFaFvciV --- tests/test_sensor.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/test_sensor.py b/tests/test_sensor.py index 84d8898..ae3b621 100644 --- a/tests/test_sensor.py +++ b/tests/test_sensor.py @@ -149,11 +149,18 @@ async def test_input_change_requests_refresh( @pytest.mark.usefixtures("setup_integration") async def test_unrelated_state_change_does_not_refresh(hass, config_entry, monkeypatch): """State changes of other entities, and of this entry's outputs, are ignored.""" - called = _count_refreshes(config_entry, monkeypatch) output = config_entry.runtime_data.handle._get_entity_id("sensor", "pid_output") assert output is not None + # Give both entities a real value first. A change from no state or from + # unknown is already filtered out before the unique_id check, so without + # this the test passes even if every entity counted as an input. hass.states.async_set("sensor.unrelated", "1") + hass.states.async_set(output, "2.0") + await hass.async_block_till_done() + called = _count_refreshes(config_entry, monkeypatch) + + hass.states.async_set("sensor.unrelated", "2") hass.states.async_set(output, "3.0") await hass.async_block_till_done() From b15c5270e19ba13739665ed8fc2719dd64810ec1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 01:21:07 +0000 Subject: [PATCH 4/4] ci: label pull requests from forks too For a pull request from a fork, the pull_request event gets a read-only token regardless of the permissions block, so the labeler failed with "Resource not accessible by integration" (seen on #158). Run on pull_request_target instead, which uses the base branch's workflow and a writable token. Neither step checks out or runs the pull request's code, which is what keeps that safe. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012VjGcBURcFQJMzcFaFvciV --- .github/workflows/label-prs.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/label-prs.yml b/.github/workflows/label-prs.yml index e040db3..565c9f9 100644 --- a/.github/workflows/label-prs.yml +++ b/.github/workflows/label-prs.yml @@ -1,7 +1,14 @@ name: Label PRs on: - pull_request: + # pull_request_target, not pull_request: for a pull request from a fork, + # pull_request gets a read-only token whatever the permissions block says, + # and adding the labels fails with "Resource not accessible by integration". + # This runs the workflow from the base branch with a writable token. That is + # safe only because no step checks out or runs the pull request's code: the + # labeler reads the changed file names through the API and the script below + # reads only the title. Keep it that way. + pull_request_target: types: [opened, synchronize, reopened] permissions: