From d45ba845aed9ae30421f26171dfe684f7d399694 Mon Sep 17 00:00:00 2001 From: Joost Lekkerkerker Date: Sat, 12 Sep 2026 10:34:15 +0200 Subject: [PATCH] Migrate SAJ to a coordinator (#174437) --- homeassistant/components/saj/__init__.py | 205 +------------------- homeassistant/components/saj/coordinator.py | 66 +++++++ homeassistant/components/saj/sensor.py | 85 ++------ tests/components/saj/conftest.py | 18 +- tests/components/saj/test_init.py | 31 ++- tests/components/saj/test_sensor.py | 19 +- 6 files changed, 110 insertions(+), 314 deletions(-) create mode 100644 homeassistant/components/saj/coordinator.py diff --git a/homeassistant/components/saj/__init__.py b/homeassistant/components/saj/__init__.py index d180982cc670..1a801425b2a8 100644 --- a/homeassistant/components/saj/__init__.py +++ b/homeassistant/components/saj/__init__.py @@ -1,198 +1,31 @@ """The saj component.""" -from collections.abc import Callable, Coroutine -from dataclasses import dataclass -from datetime import datetime -import logging from typing import Any import pysaj -from homeassistant.config_entries import ConfigEntry from homeassistant.const import ( CONF_HOST, CONF_PASSWORD, CONF_TYPE, CONF_USERNAME, - EVENT_HOMEASSISTANT_STOP, Platform, ) -from homeassistant.core import CALLBACK_TYPE, Event, HomeAssistant, callback -from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryNotReady -from homeassistant.helpers.event import async_call_later -from homeassistant.helpers.start import async_at_start +from homeassistant.core import HomeAssistant from .const import CONNECTION_TYPES - -_LOGGER = logging.getLogger(__name__) +from .coordinator import SAJConfigEntry, SAJDataUpdateCoordinator PLATFORMS: list[Platform] = [Platform.SENSOR] -MIN_INTERVAL_SEC = 5 -MAX_INTERVAL_SEC = 300 - - -@callback -def async_track_time_interval_backoff( - hass: HomeAssistant, action: Callable[[], Coroutine[Any, Any, bool]] -) -> CALLBACK_TYPE: - """Fire `action` on an interval; double the interval (capped) when it returns False.""" - remove: CALLBACK_TYPE | None = None - interval = MIN_INTERVAL_SEC - stopped = False - - async def interval_listener(_now: datetime | None = None) -> None: - nonlocal interval, remove, stopped - try: - if await action(): - interval = MIN_INTERVAL_SEC - else: - interval = min(interval * 2, MAX_INTERVAL_SEC) - finally: - if not stopped: - remove = async_call_later(hass, interval, interval_listener) - - hass.async_create_task(interval_listener()) - - def remove_listener() -> None: - nonlocal remove, stopped - stopped = True - if remove: - remove() - remove = None - - return remove_listener - - -class SAJPolling: - """Interval polling with backoff; entities register for per-poll callbacks.""" - - def __init__( - self, - hass: HomeAssistant, - entry: ConfigEntry, - saj: pysaj.SAJ, - sensor_def: pysaj.Sensors, - ) -> None: - """Initialize polling for one config entry.""" - self._hass = hass - self._entry = entry - self._saj = saj - self._sensor_def = sensor_def - self._listeners: list[Callable[[bool], None]] = [] - self._remove_backoff: CALLBACK_TYPE | None = None - self._cancel_at_start: CALLBACK_TYPE | None = None - self._unsub_stop: CALLBACK_TYPE | None = None - - @callback - def async_add_poll_listener( - self, target: Callable[[bool], None] - ) -> Callable[[], None]: - """Register to be called after each poll with the read success flag.""" - - @callback - def remove_listener() -> None: - self._listeners.remove(target) - if not self._listeners: - self._async_stop_backoff() - if self._cancel_at_start: - self._cancel_at_start() - self._cancel_at_start = None - - self._listeners.append(target) - if len(self._listeners) == 1: - self._schedule_polling_start() - return remove_listener - - def _schedule_polling_start(self) -> None: - @callback - def start(_hass: HomeAssistant) -> None: - self._cancel_at_start = None - if not self._listeners: - return - self._async_start_backoff() - - self._cancel_at_start = async_at_start(self._hass, start) - - @callback - def _async_start_backoff(self) -> None: - self._remove_backoff = async_track_time_interval_backoff( - self._hass, self._async_poll_with_notify - ) - - @callback - def stop_on_hass_stop(_event: Event) -> None: - self._async_stop_backoff() - - self._unsub_stop = self._hass.bus.async_listen( - EVENT_HOMEASSISTANT_STOP, stop_on_hass_stop - ) - - async def _async_poll_with_notify(self) -> bool: - success = False - try: - success = await self._saj.read(self._sensor_def) - except pysaj.UnauthorizedException: - _LOGGER.error( - "Username and/or password rejected during polling for %s", - self._entry.title, - ) - except pysaj.UnexpectedResponseException as err: - _LOGGER.error( - "Error in SAJ, please check host/ip address. Original error: %s", err - ) - except (TimeoutError, OSError) as err: - _LOGGER.error("Error communicating with SAJ: %s", err) - except Exception as err: # noqa: BLE001 - _LOGGER.error( - "Unexpected error polling SAJ inverter %s: %s", - self._entry.title, - err, - ) - - for listener in list(self._listeners): - listener(success) - return success - - @callback - def _async_stop_backoff(self) -> None: - if self._remove_backoff: - self._remove_backoff() - self._remove_backoff = None - if self._unsub_stop: - self._unsub_stop() - self._unsub_stop = None - - @callback - def async_shutdown(self) -> None: - """Cancel polling and any deferred start.""" - self._listeners.clear() - self._async_stop_backoff() - if self._cancel_at_start: - self._cancel_at_start() - self._cancel_at_start = None - - -@dataclass(frozen=True, slots=True) -class SAJRuntimeData: - """Runtime data attached to a SAJ config entry.""" - - saj: pysaj.SAJ - sensor_def: pysaj.Sensors - polling: SAJPolling - - -type SAJConfigEntry = ConfigEntry[SAJRuntimeData] - async def async_setup_entry(hass: HomeAssistant, entry: SAJConfigEntry) -> bool: """Set up SAJ from a config entry.""" host = entry.data[CONF_HOST] connection_type = entry.data[CONF_TYPE] - username = entry.data.get(CONF_USERNAME, None) - password = entry.data.get(CONF_PASSWORD, None) + username = entry.data.get(CONF_USERNAME) + password = entry.data.get(CONF_PASSWORD) - # Create SAJ connection kwargs: dict[str, Any] = {} wifi = connection_type == CONNECTION_TYPES[1] if wifi: @@ -202,33 +35,13 @@ async def async_setup_entry(hass: HomeAssistant, entry: SAJConfigEntry) -> bool: if password: kwargs["password"] = password - async def _async_connect() -> tuple[pysaj.SAJ, pysaj.Sensors]: - """Connect to SAJ and verify connection.""" - saj = pysaj.SAJ(host, **kwargs) - sensor_def = pysaj.Sensors(wifi) - done = await saj.read(sensor_def) - if not done: - raise ConfigEntryNotReady("Failed to read initial sensor data") - return saj, sensor_def + saj = pysaj.SAJ(host, **kwargs) + sensor_def = pysaj.Sensors(wifi) - try: - saj, sensor_def = await _async_connect() - except pysaj.UnauthorizedException as err: - if wifi: - raise ConfigEntryAuthFailed("Authentication failed") from err - raise ConfigEntryNotReady( - "Wrong connection type or device rejected connection" - ) from err - except pysaj.UnexpectedResponseException as err: - raise ConfigEntryNotReady(f"Connection error: {err}") from err - except TimeoutError as err: - raise ConfigEntryNotReady(f"Connection timeout: {err}") from err - except OSError as err: - raise ConfigEntryNotReady(f"Network error: {err}") from err + coordinator = SAJDataUpdateCoordinator(hass, entry, saj, sensor_def, wifi=wifi) + await coordinator.async_config_entry_first_refresh() - polling = SAJPolling(hass, entry, saj, sensor_def) - entry.runtime_data = SAJRuntimeData(saj=saj, sensor_def=sensor_def, polling=polling) - entry.async_on_unload(polling.async_shutdown) + entry.runtime_data = coordinator await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) diff --git a/homeassistant/components/saj/coordinator.py b/homeassistant/components/saj/coordinator.py new file mode 100644 index 000000000000..74e6a8040e20 --- /dev/null +++ b/homeassistant/components/saj/coordinator.py @@ -0,0 +1,66 @@ +"""DataUpdateCoordinator for the SAJ Solar Inverter integration.""" + +from datetime import timedelta +import logging +from typing import override + +import pysaj + +from homeassistant.config_entries import ConfigEntry +from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ConfigEntryAuthFailed +from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed + +from .const import DOMAIN + +_LOGGER = logging.getLogger(__name__) + +SCAN_INTERVAL = timedelta(seconds=60) + +type SAJConfigEntry = ConfigEntry[SAJDataUpdateCoordinator] + + +class SAJDataUpdateCoordinator(DataUpdateCoordinator[pysaj.Sensors]): + """Coordinator to poll a SAJ inverter and share data with all sensors.""" + + config_entry: SAJConfigEntry + + def __init__( + self, + hass: HomeAssistant, + config_entry: SAJConfigEntry, + saj: pysaj.SAJ, + sensor_def: pysaj.Sensors, + *, + wifi: bool, + ) -> None: + """Initialize the coordinator.""" + super().__init__( + hass, + _LOGGER, + config_entry=config_entry, + name=DOMAIN, + update_interval=SCAN_INTERVAL, + ) + self.saj = saj + self.sensor_def = sensor_def + self._wifi = wifi + + @override + async def _async_update_data(self) -> pysaj.Sensors: + """Fetch the latest data from the inverter.""" + try: + success = await self.saj.read(self.sensor_def) + except pysaj.UnauthorizedException as err: + # On ethernet an unauthorized response usually means a wrong + # connection type, which is a recoverable connection problem. + if self._wifi: + raise ConfigEntryAuthFailed("Authentication failed") from err + raise UpdateFailed("Wrong connection type or cannot connect") from err + except (pysaj.UnexpectedResponseException, TimeoutError, OSError) as err: + raise UpdateFailed(f"Error communicating with the inverter: {err}") from err + + if not success: + raise UpdateFailed("Failed to read sensor data from the inverter") + + return self.sensor_def diff --git a/homeassistant/components/saj/sensor.py b/homeassistant/components/saj/sensor.py index be0b81fa44e9..e7d26f8942d4 100644 --- a/homeassistant/components/saj/sensor.py +++ b/homeassistant/components/saj/sensor.py @@ -1,6 +1,5 @@ """SAJ solar inverter interface.""" -from datetime import date from typing import override import pysaj @@ -25,7 +24,7 @@ from homeassistant.const import ( UnitOfTemperature, UnitOfTime, ) -from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN, HomeAssistant, callback +from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN, HomeAssistant from homeassistant.data_entry_flow import FlowResultType from homeassistant.helpers import config_validation as cv, issue_registry as ir from homeassistant.helpers.entity_platform import ( @@ -33,10 +32,10 @@ from homeassistant.helpers.entity_platform import ( AddEntitiesCallback, ) from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType, StateType -from homeassistant.util import dt as dt_util +from homeassistant.helpers.update_coordinator import CoordinatorEntity -from . import SAJConfigEntry, SAJRuntimeData from .const import CONNECTION_TYPES, DOMAIN, INTEGRATION_TITLE +from .coordinator import SAJConfigEntry, SAJDataUpdateCoordinator SAJ_UNIT_MAPPINGS = { "": None, @@ -64,16 +63,13 @@ async def async_setup_entry( async_add_entities: AddConfigEntryEntitiesCallback, ) -> None: """Set up the SAJ sensors from a config entry.""" - runtime = entry.runtime_data - sensor_def = runtime.sensor_def + coordinator = entry.runtime_data - hass_sensors = [ - SAJsensor(runtime, entry.unique_id, sensor, inverter_name=None) - for sensor in sensor_def + async_add_entities( + SAJsensor(coordinator, entry.unique_id, sensor) + for sensor in coordinator.sensor_def if sensor.enabled - ] - - async_add_entities(hass_sensors) + ) async def async_setup_platform( @@ -123,25 +119,18 @@ async def async_setup_platform( ) -class SAJsensor(SensorEntity): +class SAJsensor(CoordinatorEntity[SAJDataUpdateCoordinator], SensorEntity): """Representation of a SAJ sensor.""" - _attr_should_poll = False - _state: StateType - def __init__( self, - runtime: SAJRuntimeData, + coordinator: SAJDataUpdateCoordinator, serialnumber: str | None, pysaj_sensor: pysaj.Sensor, - inverter_name: str | None = None, ) -> None: """Initialize the SAJ sensor.""" - self._runtime = runtime + super().__init__(coordinator) self._sensor = pysaj_sensor - self._inverter_name = inverter_name - self._serialnumber = serialnumber - self._state = self._sensor.value if pysaj_sensor.name in ("current_power", "temperature"): self._attr_state_class = SensorStateClass.MEASUREMENT @@ -151,10 +140,7 @@ class SAJsensor(SensorEntity): self._attr_unique_id = f"{serialnumber}_{pysaj_sensor.name}" native_uom = SAJ_UNIT_MAPPINGS[pysaj_sensor.unit] self._attr_native_unit_of_measurement = native_uom - if self._inverter_name: - self._attr_name = f"saj_{self._inverter_name}_{pysaj_sensor.name}" - else: - self._attr_name = f"saj_{pysaj_sensor.name}" + self._attr_name = f"saj_{pysaj_sensor.name}" if native_uom == UnitOfPower.WATT: self._attr_device_class = SensorDeviceClass.POWER if native_uom == UnitOfEnergy.KILO_WATT_HOUR: @@ -165,53 +151,8 @@ class SAJsensor(SensorEntity): ): self._attr_device_class = SensorDeviceClass.TEMPERATURE - @override - async def async_added_to_hass(self) -> None: - """Register for inverter poll updates.""" - await super().async_added_to_hass() - self.async_on_remove( - self._runtime.polling.async_add_poll_listener(self._on_poll_success) - ) - @property @override def native_value(self) -> StateType: """Return the state of the sensor.""" - return self._state - - @property - def per_day_basis(self) -> bool: - """Return if the sensors value is on daily basis or not.""" - return self._sensor.per_day_basis - - @property - def per_total_basis(self) -> bool: - """Return if the sensors value is cumulative or not.""" - return self._sensor.per_total_basis - - @property - def date_updated(self) -> date: - """Return the date when the sensor was last updated.""" - return self._sensor.date - - @callback - def _on_poll_success(self, success: bool) -> None: - """Update state from the inverter after a poll.""" - state_unknown = False - if not success and ( - (self.per_day_basis and dt_util.now().date() > self.date_updated) - or (not self.per_day_basis and not self.per_total_basis) - ): - state_unknown = True - - update = False - if self._sensor.value != self._state: - update = True - self._state = self._sensor.value - - if state_unknown and self._state is not None: - update = True - self._state = None - - if update: - self.async_write_ha_state() + return self._sensor.value diff --git a/tests/components/saj/conftest.py b/tests/components/saj/conftest.py index 772e46351d54..54ca658e07f1 100644 --- a/tests/components/saj/conftest.py +++ b/tests/components/saj/conftest.py @@ -1,7 +1,6 @@ """Fixtures for saj tests.""" from collections.abc import Generator -from datetime import date from typing import Any from unittest.mock import AsyncMock, MagicMock, patch @@ -56,9 +55,9 @@ def mock_setup_entry() -> Generator[AsyncMock]: def mock_pysaj_sensors() -> Generator[list[MagicMock]]: """Mock pysaj.Sensors across SAJ integration modules.""" sensors: list[MagicMock] = [] - for key, value, unit, per_day_basis, per_total_basis in ( - ("current_power", 5000.0, "W", False, False), - ("today_yield", 25.5, "kWh", True, False), + for key, value, unit in ( + ("current_power", 5000.0, "W"), + ("today_yield", 25.5, "kWh"), ): sensor = MagicMock() sensor.name = key @@ -66,9 +65,6 @@ def mock_pysaj_sensors() -> Generator[list[MagicMock]]: sensor.value = value sensor.unit = unit sensor.enabled = True - sensor.per_day_basis = per_day_basis - sensor.per_total_basis = per_total_basis - sensor.date = date.today() sensors.append(sensor) with ( @@ -81,10 +77,6 @@ def mock_pysaj_sensors() -> Generator[list[MagicMock]]: "homeassistant.components.saj.config_flow.pysaj.Sensors", new=sensors_cls, ), - patch( - "homeassistant.components.saj.sensor.pysaj.Sensors", - new=sensors_cls, - ), ): yield sensors @@ -106,9 +98,5 @@ def mock_pysaj_saj(mock_pysaj_sensors: list[MagicMock]) -> Generator[MagicMock]: "homeassistant.components.saj.config_flow.pysaj.SAJ", new=saj_cls, ), - patch( - "homeassistant.components.saj.sensor.pysaj.SAJ", - new=saj_cls, - ), ): yield saj_instance diff --git a/tests/components/saj/test_init.py b/tests/components/saj/test_init.py index 4bf9eb82b4f3..901660ec4364 100644 --- a/tests/components/saj/test_init.py +++ b/tests/components/saj/test_init.py @@ -49,36 +49,31 @@ async def test_setup_entry_auth_failed( assert entry.state is ConfigEntryState.SETUP_ERROR -async def test_setup_entry_ethernet_unauthorized_retries( - hass: HomeAssistant, - mock_config_entry: MockConfigEntry, - mock_pysaj_saj: MagicMock, -) -> None: - """Ethernet UnauthorizedException is treated as not ready (e.g. wrong type).""" - mock_pysaj_saj.read.side_effect = pysaj.UnauthorizedException("unexpected") - entry = await setup_integration(hass, mock_config_entry) - assert entry.state is ConfigEntryState.SETUP_RETRY - - @pytest.mark.parametrize( "exception", [ - Exception("Unexpected error"), - RuntimeError("Unexpected runtime error"), + pytest.param( + pysaj.UnauthorizedException("unexpected"), id="ethernet_unauthorized" + ), + pytest.param( + pysaj.UnexpectedResponseException("bad response"), id="unexpected_response" + ), + pytest.param(TimeoutError("timed out"), id="timeout"), + pytest.param(OSError("network unreachable"), id="os_error"), + pytest.param(Exception("Unexpected error"), id="unexpected"), + pytest.param(RuntimeError("Unexpected runtime error"), id="runtime_error"), ], ) -async def test_setup_entry_unexpected_error( +async def test_setup_entry_retries( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_pysaj_saj: MagicMock, exception: Exception, ) -> None: - """Test async_setup_entry handles unexpected errors.""" + """Test errors during setup result in a retry.""" mock_pysaj_saj.read.side_effect = exception entry = await setup_integration(hass, mock_config_entry) - # Truly unexpected exceptions should result in SETUP_ERROR - # so the actual error is visible rather than being hidden - assert entry.state is ConfigEntryState.SETUP_ERROR + assert entry.state is ConfigEntryState.SETUP_RETRY @pytest.mark.usefixtures("mock_pysaj_saj") diff --git a/tests/components/saj/test_sensor.py b/tests/components/saj/test_sensor.py index 674c84a95ff4..c9e84f8ed44b 100644 --- a/tests/components/saj/test_sensor.py +++ b/tests/components/saj/test_sensor.py @@ -1,6 +1,5 @@ """Test the saj sensor platform.""" -from datetime import timedelta from unittest.mock import AsyncMock, MagicMock from freezegun.api import FrozenDateTimeFactory @@ -8,10 +7,10 @@ import pysaj import pytest from syrupy.assertion import SnapshotAssertion -from homeassistant.components.saj import MIN_INTERVAL_SEC from homeassistant.components.saj.const import DOMAIN +from homeassistant.components.saj.coordinator import SCAN_INTERVAL from homeassistant.config_entries import ConfigEntryState -from homeassistant.const import CONF_HOST, STATE_UNKNOWN, Platform +from homeassistant.const import CONF_HOST, STATE_UNAVAILABLE, Platform from homeassistant.core import HomeAssistant from homeassistant.helpers import entity_registry as er, issue_registry as ir from homeassistant.setup import async_setup_component @@ -45,28 +44,22 @@ async def test_sensor_update_failure( mock_pysaj_saj: MagicMock, freezer: FrozenDateTimeFactory, ) -> None: - """Test sensor update handles failures.""" - # Setup read + initial scheduled poll succeed; next poll fails (unknown state). - mock_pysaj_saj.read = AsyncMock(side_effect=[True, True, False]) - + """Test sensors become unavailable when an update fails.""" entry = await setup_integration(hass, mock_config_entry) assert entry.state is ConfigEntryState.LOADED - await hass.async_block_till_done() - state = hass.states.get("sensor.saj_current_power") assert state is not None assert state.state == "5000.0" - assert mock_pysaj_saj.read.await_count == 2 - freezer.tick(timedelta(seconds=MIN_INTERVAL_SEC + 1)) + mock_pysaj_saj.read = AsyncMock(return_value=False) + freezer.tick(SCAN_INTERVAL) async_fire_time_changed(hass) await hass.async_block_till_done() - assert mock_pysaj_saj.read.await_count == 3 state = hass.states.get("sensor.saj_current_power") assert state is not None - assert state.state == STATE_UNKNOWN + assert state.state == STATE_UNAVAILABLE async def test_yaml_import_creates_deprecated_issue(