From 0b956dc4dfafde6330dce49706dff6961e8d664e Mon Sep 17 00:00:00 2001 From: Xavier Douville Date: Fri, 6 Nov 2026 17:29:15 -0500 Subject: [PATCH] Stop scanning and writing the global state machine AGENTS.md forbids async_all() scans during coordinator updates and async_set() on entities the integration does not own, yet every update did both: check_tarif called set_tarif on every entity in Home Assistant and relied on name filters to pick the Hilo tariff selects, and handle_unknown_power summed every entity ending in "_power" (including other integrations') and rewrote attributes of utility meter sensors through set_state. find_meter guessed the smart meter from entity IDs containing "meter" and "_power", and the challenge lock and high_times read sensor.defi_hilo by its entity ID, which breaks once a user renames it. The integration already holds all of this: the sensor platform now records the challenge sensor and the energy meter keys it creates, and unknown power is the smart meter reading minus the readings of the other power devices (skipping unavailable ones, models without a power sensor and null readings, which the entity sum also ignored). fix_utility_sensor only ran while unknown source tracking was enabled, and utility meters inherit device_class and unit from their source energy sensor, which declares them itself, so it is dropped along with set_state. Co-Authored-By: Claude Opus 5.5 --- custom_components/hilo/__init__.py | 241 ++++++++--------------------- custom_components/hilo/climate.py | 5 +- custom_components/hilo/sensor.py | 15 +- tests/test_tariff.py | 48 +++++- tests/test_unknown_power.py | 63 ++++++++ 5 files changed, 185 insertions(+), 187 deletions(-) create mode 100644 tests/test_unknown_power.py diff --git a/custom_components/hilo/__init__.py b/custom_components/hilo/__init__.py index 1a95b33c..fe73220c 100644 --- a/custom_components/hilo/__init__.py +++ b/custom_components/hilo/__init__.py @@ -3,9 +3,9 @@ from __future__ import annotations import asyncio -from collections import OrderedDict import contextlib from datetime import UTC, datetime, timedelta +from typing import TYPE_CHECKING from aiohttp import CookieJar, client_exceptions from homeassistant.components.select import ( @@ -13,11 +13,8 @@ DOMAIN as SELECT_DOMAIN, SERVICE_SELECT_OPTION, ) -from homeassistant.components.sensor import SensorDeviceClass from homeassistant.config_entries import ConfigEntry from homeassistant.const import ( - ATTR_DEVICE_CLASS, - ATTR_UNIT_OF_MEASUREMENT, CONF_SCAN_INTERVAL, CONF_USERNAME, EVENT_HOMEASSISTANT_STOP, @@ -39,6 +36,7 @@ from homeassistant.util import dt as dt_util from packaging.version import Version from pyhilo import API +from pyhilo.const import UNMONITORED_DEVICES from pyhilo.device import HiloDevice from pyhilo.devices import Devices from pyhilo.event import Event @@ -75,6 +73,9 @@ ) from .oauth2 import AuthCodeWithPKCEImplementation +if TYPE_CHECKING: + from .sensor import HiloChallengeSensor + DISPATCHER_TOPIC_SIGNALR_EVENT = "pyhilo_signalr_event" SIGNAL_UPDATE_ENTITY = "pyhilo_device_update_{}" COORDINATOR_AWARE_PLATFORMS = [Platform.SENSOR] @@ -400,7 +401,6 @@ def __init__(self, hass: HomeAssistant, entry: ConfigEntry, api: API) -> None: """Initialize.""" self._api = api self._hass = hass - self.find_meter(self._hass) self.entry = entry self.devices: Devices = Devices(api) self.graphql_helper: GraphQlHelper = GraphQlHelper(api, self.devices) @@ -427,6 +427,9 @@ def __init__(self, hass: HomeAssistant, entry: ConfigEntry, api: API) -> None: CONF_GENERATE_ENERGY_METERS, DEFAULT_GENERATE_ENERGY_METERS ) self.cost_sensors: dict = {} + # These get filled in by the sensor platform: + self.challenge_sensor: HiloChallengeSensor | None = None + self.energy_meter_keys: list[str] = [] # This will get filled in by async_init: self.coordinator: DataUpdateCoordinator | None = None self.unknown_tracker_device: HiloDevice | None = None @@ -995,76 +998,17 @@ async def async_update(self) -> None: if self.track_unknown_sources: self.handle_unknown_power() - def find_meter(self, hass): - """Find the smart meter entity in Home Assistant.""" - entity_registry_dict = {} - - registry = hass.data.get("entity_registry") - - if registry is None: - return entity_registry_dict - - # ic-dev21: Get names of all entities - for entity_id, entity_entry in registry.entities.items(): - entity_registry_dict[entity_id] = { - "name": entity_entry.entity_id, - } - - sorted_entity_registry_dict = OrderedDict(sorted(entity_registry_dict.items())) - LOG.debug("Entities Ordered dict is %s", sorted_entity_registry_dict) - - # ic-dev21: Let's grab the meter from our dict - filtered_names = [ - entity_data["name"] - for entity_data in sorted_entity_registry_dict.values() - if all( - substring in entity_data["name"] for substring in ["meter", "_power"] - ) - ] - - LOG.debug("Hilo Smart meter name is: %s", filtered_names) - - # Format output to use in check_tarif - return ", ".join(filtered_names) if filtered_names else "" - - def set_state(self, entity, state, new_attrs=None, keep_state=False, force=False): - """Set the state of an entity.""" - if new_attrs is None: - new_attrs = {} - params = f"{entity=} {state=} {new_attrs=} {keep_state=}" - current = self._hass.states.get(entity) - if not current: - if not force: - LOG.warning( - "Unable to set state because there's no current: %s", params - ) - return - attrs = {} - else: - attrs = dict(current.as_dict()["attributes"]) - attrs["last_update"] = datetime.now() - attrs["hilo_update"] = True - attrs = {**attrs, **new_attrs} - if keep_state and current: - state = current.state - if "Cost" in attrs: - attrs["Cost"] = state - LOG.debug("Setting state %s current=%s attrs=%s", params, current, attrs) - self._hass.states.async_set(entity, state, attrs, force_update=force) - @property - def high_times(self): + def high_times(self) -> bool: """Check if the current time is within high tariff periods.""" - challenge_sensor = self._hass.states.get("sensor.defi_hilo") - - if challenge_sensor is None: + if self.challenge_sensor is None: LOG.warning("high_times check tarif challenge sensor not found") return False LOG.debug( - "high_times check tarif challenge sensor is %s", challenge_sensor.state + "high_times check tarif challenge sensor is %s", self.challenge_sensor.state ) - return challenge_sensor.state == "reduction" + return self.challenge_sensor.state == "reduction" def check_season(self, current_time: datetime | None = None) -> bool: """Determine if we are using a winter or summer rate.""" @@ -1166,125 +1110,66 @@ def check_tarif(self): self.high_times, ) - # ic-dev21 : make sure the select for all meters still work by moving this here - for state in self._hass.states.async_all(): - entity = state.entity_id - self.set_tarif(entity, state.state, tarif) - - def handle_unknown_power(self): - """Take care of the unknown source meter.""" - known_power = 0 - smart_meter = self.find_meter(self._hass) - LOG.debug("Smart meter used currently is: %s", smart_meter) - unknown_source_tracker = "sensor.unknown_source_tracker_power" - for state in self._hass.states.async_all(): - entity = state.entity_id - if entity.endswith("hilo_rate_current"): + for meter_key in self.energy_meter_keys: + if self.untarificated_devices and meter_key != HILO_ENERGY_TOTAL: continue + self.set_tarif(f"select.{meter_key}", tarif) - if entity.endswith("_power") and entity not in [ - unknown_source_tracker, - smart_meter, - ]: - with contextlib.suppress(ValueError): - known_power += int(float(state.state)) - if not entity.endswith("_hilo_energy") or entity.endswith("_cost"): - continue - self.fix_utility_sensor(entity, state) - if self.track_unknown_sources: - total_power = self._hass.states.get(smart_meter) - try: - if known_power <= int(total_power.state): - unknown_power = int(total_power.state) - known_power - else: - unknown_power = 0 - except ValueError: - unknown_power = known_power - LOG.warning( - f"value of total_power ({total_power} not initialized correctly)" - ) - - self.devices.parse_values_received( - [ - { - "deviceId": 69420, - "locationId": self.devices.location_id, - "timeStampUTC": datetime.now(UTC).isoformat(), - "attribute": "Power", - "value": unknown_power, - "valueType": "Watt", - } - ] - ) - LOG.debug( - "Currently in use: Total: %s Known sources: %s Unknown sources: %s", - total_power.state, - known_power, - unknown_power, - ) - - @callback - def fix_utility_sensor(self, entity, state): - """Not sure why this doesn't get created with a proper device_class.""" - current_state = state.as_dict() - attrs = current_state.get("attributes", {}) - if entity.startswith("select.") or entity.find("hilo_rate") > 0: - return - if not attrs.get("source"): - LOG.debug("No source entity defined on %s: %s", entity, current_state) + def handle_unknown_power(self) -> None: + """Take care of the unknown source meter.""" + smart_meter = next((d for d in self.devices.all if d.type == "Meter"), None) + if smart_meter is None or not smart_meter.available: + LOG.debug("No smart meter reading available to track unknown sources") return - - parent_unit_state = self._hass.states.get(attrs.get("source")) - parent_unit = ( - "kWh" - if parent_unit_state is None - else parent_unit_state.attributes.get("unit_of_measurement") + known_power = sum( + int(device.get_value("power", 0) or 0) + for device in self.devices.all + if device.has_attribute("power") + and device.available + and device.model not in UNMONITORED_DEVICES + and device not in (smart_meter, self.unknown_tracker_device) + ) + total_power = int(smart_meter.get_value("power", 0) or 0) + unknown_power = max(total_power - known_power, 0) + + self.devices.parse_values_received( + [ + { + "deviceId": self.unknown_tracker_device.id, + "locationId": self.devices.location_id, + "timeStampUTC": datetime.now(UTC).isoformat(), + "attribute": "Power", + "value": unknown_power, + "valueType": "Watt", + } + ] + ) + LOG.debug( + "Currently in use: Total: %s Known sources: %s Unknown sources: %s", + total_power, + known_power, + unknown_power, ) - if not parent_unit: - LOG.warning("Unable to find state for parent unit: %s", current_state) - return - - new_attrs = { - ATTR_UNIT_OF_MEASUREMENT: parent_unit, # note ic-dev21: now uses parent_unit directly - ATTR_DEVICE_CLASS: SensorDeviceClass.ENERGY, - } - if not all(a in attrs for a in new_attrs): - LOG.warning( - f"Fixing utility sensor: {entity} {current_state} new_attrs: {new_attrs}" - ) - self.set_state(entity, None, new_attrs=new_attrs, keep_state=True) @callback - def set_tarif(self, entity, current, new): + def set_tarif(self, entity_id: str, new: str) -> None: """Set the tarif on the select entity if needed.""" - if self.untarificated_devices and entity != f"select.{HILO_ENERGY_TOTAL}": + if (current := self._hass.states.get(entity_id)) is None: return - if entity.startswith("select.hilo_energy") and current != new: - LOG.debug( - "check_tarif: Changing tarif of %s from %s to %s", entity, current, new - ) - context = Context() - data = {ATTR_OPTION: new, "entity_id": entity} - self._hass.async_create_task( - self._hass.services.async_call( - SELECT_DOMAIN, SERVICE_SELECT_OPTION, data, context=context - ) - ) - if ( - entity.startswith("select.") - and entity.endswith("_hilo_energy") - and current != new - ): - LOG.debug( - "check_tarif: Changing tarif of %s from %s to %s", entity, current, new - ) - context = Context() - data = {ATTR_OPTION: new, "entity_id": entity} - self._hass.async_create_task( - self._hass.services.async_call( - SELECT_DOMAIN, SERVICE_SELECT_OPTION, data, context=context - ) + if current.state == new: + return + LOG.debug( + "check_tarif: Changing tarif of %s from %s to %s", + entity_id, + current.state, + new, + ) + data = {ATTR_OPTION: new, "entity_id": entity_id} + self._hass.async_create_task( + self._hass.services.async_call( + SELECT_DOMAIN, SERVICE_SELECT_OPTION, data, context=Context() ) + ) @callback def async_migrate_unique_id( diff --git a/custom_components/hilo/climate.py b/custom_components/hilo/climate.py index 9aa74a40..fcf40f4f 100755 --- a/custom_components/hilo/climate.py +++ b/custom_components/hilo/climate.py @@ -265,10 +265,9 @@ def icon(self): def _check_challenge_lock(self) -> None: """Block setpoint changes during a Hilo challenge reduction phase.""" - if self._hilo.challenge_lock: - challenge = self._hilo._hass.states.get("sensor.defi_hilo") + if self._hilo.challenge_lock and self._hilo.challenge_sensor: validate_reduction_phase( - challenge.attributes.get("next_events", []), self._device._tag + self._hilo.challenge_sensor.next_events, self._device._tag ) async def async_set_hvac_mode(self, hvac_mode): diff --git a/custom_components/hilo/sensor.py b/custom_components/hilo/sensor.py index e7a1a58b..62897446 100755 --- a/custom_components/hilo/sensor.py +++ b/custom_components/hilo/sensor.py @@ -110,9 +110,8 @@ def generate_entities_from_device(device, hilo, scan_interval): """Generate the entities from the device description.""" entities = [] if device.type == "Gateway": - entities.append( - HiloChallengeSensor(hilo, device, scan_interval), - ) + hilo.challenge_sensor = HiloChallengeSensor(hilo, device, scan_interval) + entities.append(hilo.challenge_sensor) entities.append( HiloRewardSensor(hilo, device, scan_interval), ) @@ -171,6 +170,8 @@ async def async_setup_entry( energy_manager = await EnergyManager().init(hass, energy_meter_period) utility_manager = UtilityManager(hass, energy_meter_period, default_tariff_list) + hilo.energy_meter_keys = [] + def create_energy_entity(hilo, device): device._energy_entity = EnergySensor(hilo, device, hass) new_entities.append(device._energy_entity) @@ -187,6 +188,7 @@ def create_energy_entity(hilo, device): tariff_list = validate_tariff_list(tariff_config) net_consumption = device.net_consumption utility_manager.add_meter(energy_entity, tariff_list, net_consumption) + hilo.energy_meter_keys.append(energy_entity) for d in hilo.devices.all: LOG.debug("Adding device %s", d) @@ -1202,7 +1204,12 @@ def should_poll(self): @property def extra_state_attributes(self): """Return the next events attribute.""" - return {"next_events": self._next_events} + return {"next_events": self.next_events} + + @property + def next_events(self) -> list[dict]: + """Return the scheduled and ongoing challenges.""" + return self._next_events async def async_added_to_hass(self): """Handle entity about to be added to hass event.""" diff --git a/tests/test_tariff.py b/tests/test_tariff.py index 7a530a3d..56d4957c 100644 --- a/tests/test_tariff.py +++ b/tests/test_tariff.py @@ -3,10 +3,17 @@ from datetime import datetime from unittest.mock import AsyncMock, MagicMock, patch +from homeassistant.components.select import ( + DOMAIN as SELECT_DOMAIN, + SERVICE_SELECT_OPTION, +) from homeassistant.const import STATE_UNAVAILABLE, STATE_UNKNOWN from homeassistant.core import HomeAssistant import pytest -from pytest_homeassistant_custom_component.common import MockConfigEntry +from pytest_homeassistant_custom_component.common import ( + MockConfigEntry, + async_mock_service, +) from custom_components.hilo import Hilo from custom_components.hilo.const import ( @@ -37,6 +44,8 @@ def create_mock_hilo(hass: HomeAssistant, hq_plan_name: str = "flex d") -> Hilo: hilo.generate_energy_meters = True hilo.untarificated_devices = False hilo.track_unknown_sources = False + hilo.energy_meter_keys = [] + hilo.challenge_sensor = None hilo.cost_sensors = { "low": DummyCostSensor(CONF_TARIFF["flex d"]["low"]), "medium": DummyCostSensor(CONF_TARIFF["flex d"]["medium"]), @@ -148,7 +157,7 @@ def test_check_tarif_low_medium_selection( def test_check_tarif_high_tier_during_challenge(hass: HomeAssistant): """Test check_tarif selects high during a challenge reduction period.""" hilo = create_mock_hilo(hass, "flex d") - hass.states.async_set("sensor.defi_hilo", "reduction") + hilo.challenge_sensor = MagicMock(state="reduction") with patch.object(hilo, "check_season", return_value=True): hilo.check_tarif() @@ -156,6 +165,41 @@ def test_check_tarif_high_tier_during_challenge(hass: HomeAssistant): assert hilo.cost_sensors["current"]._cost == CONF_TARIFF["flex d"]["high"] +@pytest.mark.parametrize( + ("untarificated_devices", "expected_selects"), + [ + pytest.param( + False, + [f"select.{HILO_ENERGY_TOTAL}", "select.smart_plug_hilo_energy"], + id="all_meters", + ), + pytest.param(True, [f"select.{HILO_ENERGY_TOTAL}"], id="untarificated"), + ], +) +async def test_check_tarif_switches_own_meters( + hass: HomeAssistant, untarificated_devices: bool, expected_selects: list[str] +) -> None: + """Test check_tarif only switches the tariff selects of the meters it created.""" + hilo = create_mock_hilo(hass, "flex d") + hilo.untarificated_devices = untarificated_devices + hilo.energy_meter_keys = [HILO_ENERGY_TOTAL, "smart_plug_hilo_energy"] + for select in ( + f"select.{HILO_ENERGY_TOTAL}", + "select.smart_plug_hilo_energy", + "select.unrelated_hilo_energy", + ): + hass.states.async_set(select, "medium") + calls = async_mock_service(hass, SELECT_DOMAIN, SERVICE_SELECT_OPTION) + + with patch.object(hilo, "check_season", return_value=False): + hilo.check_tarif() + await hass.async_block_till_done() + + assert [call.data for call in calls] == [ + {"entity_id": select, "option": "low"} for select in expected_selects + ] + + async def test_check_tarif_season_change_transition(hass: HomeAssistant): """Test dynamic rate transition when season changes from winter to summer.""" hilo = create_mock_hilo(hass, "flex d") diff --git a/tests/test_unknown_power.py b/tests/test_unknown_power.py new file mode 100644 index 00000000..988f71f7 --- /dev/null +++ b/tests/test_unknown_power.py @@ -0,0 +1,63 @@ +"""Tests for the unknown source power tracking.""" + +from unittest.mock import MagicMock + +import pytest + +from custom_components.hilo import Hilo + + +def power_device( + device_type: str, power: int | None, available: bool = True, model: str = "X" +) -> MagicMock: + """Return a device reporting the given power.""" + device = MagicMock(type=device_type, available=available, model=model) + device.has_attribute.side_effect = lambda attribute: attribute == "power" + device.get_value.return_value = power + return device + + +@pytest.mark.parametrize( + "ignored_device", + [ + pytest.param( + power_device("Thermostat", 900, available=False), id="unavailable" + ), + pytest.param( + power_device("Thermostat", 900, model="TH6250WF"), id="unmonitored" + ), + pytest.param(power_device("Thermostat", None), id="no_reading_value"), + ], +) +def test_unknown_power_is_meter_minus_known_devices( + ignored_device: MagicMock, +) -> None: + """Test unknown power is the smart meter reading minus the known devices.""" + hilo = Hilo.__new__(Hilo) + hilo.unknown_tracker_device = power_device("Tracker", 77) + hilo.devices = MagicMock( + all=[ + power_device("Meter", 5000), + power_device("Outlet", 1200), + ignored_device, + hilo.unknown_tracker_device, + ] + ) + + hilo.handle_unknown_power() + + [readings] = hilo.devices.parse_values_received.call_args.args + assert readings[0]["value"] == 3800 + + +def test_unknown_power_skipped_without_meter_reading() -> None: + """Test nothing is reported while the smart meter is unavailable.""" + hilo = Hilo.__new__(Hilo) + hilo.unknown_tracker_device = power_device("Tracker", 77) + hilo.devices = MagicMock( + all=[power_device("Meter", 5000, available=False), power_device("Outlet", 1)] + ) + + hilo.handle_unknown_power() + + hilo.devices.parse_values_received.assert_not_called()