Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .github/workflows/label-prs.yml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
17 changes: 15 additions & 2 deletions custom_components/simple_pid_controller/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
20 changes: 16 additions & 4 deletions custom_components/simple_pid_controller/config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down
95 changes: 57 additions & 38 deletions custom_components/simple_pid_controller/sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -20,6 +24,7 @@
from typing import Any

from . import PIDDeviceHandle
from .const import DOMAIN
from .entity import BasePIDEntity
from .coordinator import PIDDataCoordinator

Expand All @@ -28,6 +33,8 @@

_LOGGER = logging.getLogger(__name__)

_NO_VALUE = (STATE_UNAVAILABLE, STATE_UNKNOWN)


async def async_setup_entry(
hass: HomeAssistant,
Expand Down Expand Up @@ -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(
Expand Down
141 changes: 107 additions & 34 deletions tests/test_sensor.py
Original file line number Diff line number Diff line change
@@ -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 (
Expand Down Expand Up @@ -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")
Expand Down
Loading
Loading