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: 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, 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..ae3b621 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,124 @@ 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 - # 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") +@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 + + +@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.""" + 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() + + 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() - # 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") + assert not called - from types import SimpleNamespace - event = SimpleNamespace(data={"entity_id": test_entity}) - callback(event) +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 + + 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) + + 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)