diff --git a/homeassistant/components/solaredge_modbus/__init__.py b/homeassistant/components/solaredge_modbus/__init__.py index f192d41d8840..00aec9cd431e 100644 --- a/homeassistant/components/solaredge_modbus/__init__.py +++ b/homeassistant/components/solaredge_modbus/__init__.py @@ -7,8 +7,11 @@ the ``solaredged`` library. """ from collections.abc import Set as AbstractSet +from datetime import datetime +from functools import partial from typing import TYPE_CHECKING +from modbus_connection import ModbusUnit from solaredged import SolarEdge, SolarEdgeConnectionError, SolarEdgeError from homeassistant.components.modbus import async_get_unit @@ -20,8 +23,10 @@ from homeassistant.exceptions import ( HomeAssistantError, ) from homeassistant.helpers import device_registry as dr +from homeassistant.helpers.event import async_track_time_interval from .const import ( + ATTACHMENT_SCAN_INTERVAL, CONF_UNIT_ID, DOMAIN, LOGGER, @@ -140,6 +145,7 @@ async def async_setup_entry( settings=settings, device_info=device_info, inverter_device_id=inverter.id, + attachments=_attachment_identities(solaredge), ) if silent := solaredge.unresponsive_blocks & { @@ -157,9 +163,85 @@ async def async_setup_entry( await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) + # What is wired to the inverter is read while setting up, so a meter or + # battery added or removed later needs the entry to load again to be seen. + entry.async_on_unload( + async_track_time_interval( + hass, + partial(_async_reload_when_attachments_change, hass, entry, unit), + ATTACHMENT_SCAN_INTERVAL, + ) + ) + return True +def _attachment_identities(solaredge: SolarEdge) -> frozenset[str]: + """Return what the meters and batteries attached right now are known by.""" + return frozenset( + [ + *( + f"meter_{attachment_identity(meter, index)}" + for index, meter in enumerate(solaredge.meters, 1) + ), + *( + f"battery_{attachment_identity(battery, index)}" + for index, battery in enumerate(solaredge.batteries, 1) + ), + ] + ) + + +async def _async_reload_when_attachments_change( + hass: HomeAssistant, + entry: SolarEdgeModbusConfigEntry, + unit: ModbusUnit, + _now: datetime, +) -> None: + """Reload the entry when the hardware wired to the inverter changed.""" + solaredge = entry.runtime_data.solaredge + + # Swapping one meter for another leaves the count alone, but the polls have + # been reading the new one's serial number since it was wired in. + if _attachment_identities(solaredge) != entry.runtime_data.attachments: + LOGGER.info( + "%s: what is attached changed, reloading to pick that up", + entry.title, + ) + hass.config_entries.async_schedule_reload(entry.entry_id) + return + + try: + probed = await SolarEdge.async_probe(unit) + except SolarEdgeError as err: + # Nothing to conclude from a probe that did not finish; the coordinators + # report an inverter that stopped answering. + LOGGER.debug("%s: could not probe for attached hardware: %s", entry.title, err) + return + + for name, found, known in ( + (SUBSYSTEM_METERS, len(probed.meters), len(solaredge.meters)), + (SUBSYSTEM_BATTERIES, len(probed.batteries), len(solaredge.batteries)), + ): + if found == known: + continue + # A block that stayed silent is taken for absent, which is not the same + # as the inverter saying it is gone, and reloading on that would drop a + # device over one timeout. + if found < known and name in probed.unresponsive_blocks: + continue + + LOGGER.info( + "%s: %s went from %s to %s, reloading to pick that up", + entry.title, + name, + known, + found, + ) + hass.config_entries.async_schedule_reload(entry.entry_id) + return + + def _async_remove_stale_devices( hass: HomeAssistant, entry: SolarEdgeModbusConfigEntry, diff --git a/homeassistant/components/solaredge_modbus/const.py b/homeassistant/components/solaredge_modbus/const.py index 5453110499cf..076bf373cdfe 100644 --- a/homeassistant/components/solaredge_modbus/const.py +++ b/homeassistant/components/solaredge_modbus/const.py @@ -39,3 +39,7 @@ SCAN_INTERVAL: Final = timedelta(seconds=10) # The control blocks hold what the site was told to do; they only move when # something writes them, so they do not need a live measurement's cadence. SETTINGS_SCAN_INTERVAL: Final = timedelta(minutes=5) + +# Meters and batteries are wired to an inverter by hand, usually with the power +# off, so looking for a change now and then is often enough. +ATTACHMENT_SCAN_INTERVAL: Final = timedelta(minutes=15) diff --git a/homeassistant/components/solaredge_modbus/coordinator.py b/homeassistant/components/solaredge_modbus/coordinator.py index bb198e842858..ab62ab75b7c5 100644 --- a/homeassistant/components/solaredge_modbus/coordinator.py +++ b/homeassistant/components/solaredge_modbus/coordinator.py @@ -169,6 +169,9 @@ class SolarEdgeModbusRuntimeData: settings: SolarEdgeModbusDataUpdateCoordinator device_info: DeviceInfo inverter_device_id: str + # What was attached when this entry was built, to notice a swap: a meter + # replaced by another one leaves the count alone. + attachments: frozenset[str] # The export mode and its flags share one register, which the library # changes by taking its cached value, flipping bits and writing it back. diff --git a/homeassistant/components/solaredge_modbus/quality_scale.yaml b/homeassistant/components/solaredge_modbus/quality_scale.yaml index 332c05eaa4dc..7cb530fe3c04 100644 --- a/homeassistant/components/solaredge_modbus/quality_scale.yaml +++ b/homeassistant/components/solaredge_modbus/quality_scale.yaml @@ -61,7 +61,7 @@ rules: docs-supported-functions: todo docs-troubleshooting: todo docs-use-cases: todo - dynamic-devices: todo + dynamic-devices: done entity-category: done entity-device-class: done entity-disabled-by-default: done @@ -72,7 +72,7 @@ rules: repair-issues: status: exempt comment: No repairable issues are raised. - stale-devices: todo + stale-devices: done # Platinum async-dependency: done diff --git a/tests/components/solaredge_modbus/test_init.py b/tests/components/solaredge_modbus/test_init.py index 9da453c49d74..84cada46e63e 100644 --- a/tests/components/solaredge_modbus/test_init.py +++ b/tests/components/solaredge_modbus/test_init.py @@ -7,11 +7,12 @@ from freezegun.api import FrozenDateTimeFactory from modbus_connection import ( IllegalDataAddressError, ModbusTimeoutError, + ModbusUnit, ServerDeviceFailureError, ) from modbus_connection.mock import MockModbusConnection, MockModbusUnit import pytest -from solaredged import SolarEdgeConnectionError +from solaredged import SolarEdge, SolarEdgeConnectionError from homeassistant.components.select import ( ATTR_OPTION, @@ -19,6 +20,7 @@ from homeassistant.components.select import ( SERVICE_SELECT_OPTION, ) from homeassistant.components.solaredge_modbus.const import ( + ATTACHMENT_SCAN_INTERVAL, DOMAIN, SCAN_INTERVAL, SETTINGS_SCAN_INTERVAL, @@ -58,6 +60,9 @@ METER_MODEL_REGISTER = 40188 # An address inside the pooled storage and export control read. SITE_CONTROL_REGISTER = 57348 +# Where the first meter's serial number lives. +METER_SERIAL_REGISTER = 40171 + EXPORT_LIMITATION_ENTITY = "select.solaredge_se10000h_export_limitation" EXTERNAL_PRODUCTION_ENTITY = "switch.solaredge_se10000h_external_production" @@ -620,6 +625,205 @@ async def test_concurrent_control_writes_keep_both_changes( assert state.state == STATE_ON +async def _tick_attachment_check( + hass: HomeAssistant, freezer: FrozenDateTimeFactory +) -> int: + """Let the check for changed hardware run, and report what it cost. + + Setting up probes the device, so a check that reloads the entry probes + twice: once to look, once to build the entry again. + """ + probes = 0 + probe = SolarEdge.async_probe + + async def counting_probe(unit: ModbusUnit) -> SolarEdge: + nonlocal probes + probes += 1 + return await probe(unit) + + with patch.object(SolarEdge, "async_probe", counting_probe): + freezer.tick(ATTACHMENT_SCAN_INTERVAL) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + return probes + + +async def test_meter_added_later_is_picked_up( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + device_registry: dr.DeviceRegistry, + mock_config_entry: MockConfigEntry, + mock_modbus_unit: MockModbusUnit, +) -> None: + """A meter wired to a running installation appears without being asked. + + What is attached is read while the entry is set up, so the entry loads + again once a probe finds something that was not there before. + """ + mock_modbus_unit.fail_read(METER_MODEL_REGISTER, IllegalDataAddressError()) + await _setup(hass, mock_config_entry) + + meter = (DOMAIN, f"{SERIAL_NUMBER}_meter_{METER_SERIAL_NUMBER}") + assert ( + device_registry.async_get_device_by_identifier( + meter, mock_config_entry.entry_id + ) + is None + ) + + # The meter is wired in and answers from now on. + mock_modbus_unit.fail_read(METER_MODEL_REGISTER, None) + + await _tick_attachment_check(hass, freezer) + + assert ( + device_registry.async_get_device_by_identifier( + meter, mock_config_entry.entry_id + ) + is not None + ) + + +async def test_battery_removed_later_is_dropped( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + device_registry: dr.DeviceRegistry, + mock_config_entry: MockConfigEntry, + mock_modbus_unit: MockModbusUnit, +) -> None: + """A battery taken out of a running installation stops being a device.""" + await _setup(hass, mock_config_entry) + + battery = (DOMAIN, f"{SERIAL_NUMBER}_battery_{BATTERY_SERIAL_NUMBERS[0]}") + assert ( + device_registry.async_get_device_by_identifier( + battery, mock_config_entry.entry_id + ) + is not None + ) + + mock_modbus_unit.fail_read(BATTERY_RATED_ENERGY, IllegalDataAddressError()) + + await _tick_attachment_check(hass, freezer) + + assert ( + device_registry.async_get_device_by_identifier( + battery, mock_config_entry.entry_id + ) + is None + ) + + +async def test_replaced_meter_is_picked_up( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + device_registry: dr.DeviceRegistry, + mock_config_entry: MockConfigEntry, + mock_modbus_unit: MockModbusUnit, +) -> None: + """Another meter in the same place is another device, while running too. + + Swapping one meter for another leaves the count alone, so what gives it + away is the serial number the polls have been reading all along. + """ + await _setup(hass, mock_config_entry) + + replacement = "7E9C55A6" + padded = replacement.ljust(32, "\0").encode() + mock_modbus_unit.holding.update( + { + METER_SERIAL_REGISTER + index: (padded[index * 2] << 8) + | padded[index * 2 + 1] + for index in range(16) + } + ) + + # The swap is seen without probing, so the only probe here is the reload's. + assert await _tick_attachment_check(hass, freezer) == 1 + + assert ( + device_registry.async_get_device_by_identifier( + (DOMAIN, f"{SERIAL_NUMBER}_meter_{METER_SERIAL_NUMBER}"), + mock_config_entry.entry_id, + ) + is None + ) + assert ( + device_registry.async_get_device_by_identifier( + (DOMAIN, f"{SERIAL_NUMBER}_meter_{replacement}"), + mock_config_entry.entry_id, + ) + is not None + ) + + +async def test_silent_attachment_does_not_trigger_a_reload( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + device_registry: dr.DeviceRegistry, + mock_config_entry: MockConfigEntry, + mock_modbus_unit: MockModbusUnit, +) -> None: + """A meter that did not answer the probe is not a meter that was removed. + + Silence is taken for absence while probing, so reloading on it would drop a + device, and its history, over a single timeout. + """ + await _setup(hass, mock_config_entry) + + meter = (DOMAIN, f"{SERIAL_NUMBER}_meter_{METER_SERIAL_NUMBER}") + mock_modbus_unit.fail_read(METER_MODEL_REGISTER, ModbusTimeoutError("timed out")) + + assert await _tick_attachment_check(hass, freezer) == 1 + + assert ( + device_registry.async_get_device_by_identifier( + meter, mock_config_entry.entry_id + ) + is not None + ) + + +async def test_unchanged_attachments_leave_the_entry_alone( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + mock_config_entry: MockConfigEntry, +) -> None: + """Nothing changed means nothing happens, however often it is checked.""" + await _setup(hass, mock_config_entry) + + coordinator = mock_config_entry.runtime_data.readings + + assert await _tick_attachment_check(hass, freezer) == 1 + + assert mock_config_entry.state is ConfigEntryState.LOADED + # A reload would have built new coordinators. + assert mock_config_entry.runtime_data.readings is coordinator + + +async def test_a_dead_probe_leaves_the_entry_where_it_is( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + mock_config_entry: MockConfigEntry, + mock_modbus_unit: MockModbusUnit, +) -> None: + """An inverter that stops answering says nothing about what is wired to it. + + The coordinators already report an inverter gone quiet; reloading on top of + that would only take the entry down with it. + """ + await _setup(hass, mock_config_entry) + + coordinator = mock_config_entry.runtime_data.readings + mock_modbus_unit.fail_requests(ModbusTimeoutError("link died")) + + assert await _tick_attachment_check(hass, freezer) == 1 + + assert mock_config_entry.state is ConfigEntryState.LOADED + assert mock_config_entry.runtime_data.readings is coordinator + + async def test_setup_retry_when_device_unresponsive( hass: HomeAssistant, mock_config_entry: MockConfigEntry,