diff --git a/homeassistant/components/fronius/__init__.py b/homeassistant/components/fronius/__init__.py index 2f65c6bb799c50..d2413f9934deca 100644 --- a/homeassistant/components/fronius/__init__.py +++ b/homeassistant/components/fronius/__init__.py @@ -18,9 +18,9 @@ from homeassistant.components.modbus import async_get_unit from homeassistant.config_entries import ConfigEntry, ConfigEntryState from homeassistant.const import ATTR_MODEL, ATTR_SW_VERSION, CONF_HOST, Platform -from homeassistant.core import HomeAssistant +from homeassistant.core import HomeAssistant, callback from homeassistant.exceptions import ConfigEntryNotReady, HomeAssistantError -from homeassistant.helpers import device_registry as dr +from homeassistant.helpers import device_registry as dr, entity_registry as er from homeassistant.helpers.aiohttp_client import async_get_clientsession from homeassistant.helpers.device_registry import DeviceInfo from homeassistant.helpers.dispatcher import async_dispatcher_send @@ -30,23 +30,25 @@ CONF_MODBUS_PORT, DEFAULT_MODBUS_PORT, DOMAIN, - SOLAR_NET_DISCOVERY_NEW, SOLAR_NET_ID_SYSTEM, SOLAR_NET_RESCAN_TIMER, FroniusDeviceInfo, SolarNetId, + discovery_signal, ) from .coordinator import ( FroniusCoordinatorBase, FroniusInverterUpdateCoordinator, FroniusLoggerUpdateCoordinator, FroniusMeterUpdateCoordinator, + FroniusModbusCoordinatorBase, FroniusModbusInverterUpdateCoordinator, FroniusModbusSettingsUpdateCoordinator, FroniusOhmpilotUpdateCoordinator, FroniusPowerFlowUpdateCoordinator, FroniusStorageUpdateCoordinator, ) +from .sensor import MODBUS_INVERTER_ENTITY_DESCRIPTIONS _LOGGER: Final = logging.getLogger(__name__) PLATFORMS: Final = [ @@ -58,9 +60,38 @@ type FroniusConfigEntry = ConfigEntry[FroniusSolarNet] +MODBUS_SENSOR_KEYS: Final = { + description.key for description in MODBUS_INVERTER_ENTITY_DESCRIPTIONS +} + + +@callback +def _async_fix_modbus_sensor_unique_ids( + hass: HomeAssistant, entry: FroniusConfigEntry +) -> None: + """Move sensors that were registered with the SolarAPI unique ID format. + + A Modbus coordinator found by a re-scan reached the sensor platform + before it could be told from a SolarAPI one, so 2026.9 built its entities + as SolarAPI ones: `-` instead of `-modbus-`. + The keys of the two are distinct, so a Modbus key without the marker can + only come from that. + """ + registry = er.async_get(hass) + for entity in er.async_entries_for_config_entry(registry, entry.entry_id): + inverter_id, _, key = entity.unique_id.rpartition("-") + if key not in MODBUS_SENSOR_KEYS or inverter_id.endswith("-modbus"): + continue + unique_id = f"{inverter_id}-modbus-{key}" + if registry.async_get_entity_id(entity.domain, DOMAIN, unique_id): + continue + _LOGGER.debug("Migrating unique ID of %s to %s", entity.entity_id, unique_id) + registry.async_update_entity(entity.entity_id, new_unique_id=unique_id) + async def async_setup_entry(hass: HomeAssistant, entry: FroniusConfigEntry) -> bool: """Set up fronius from a config entry.""" + _async_fix_modbus_sensor_unique_ids(hass, entry) host = entry.data[CONF_HOST] fronius = Fronius( async_get_clientsession( @@ -260,7 +291,11 @@ async def _init_devices_inverter(self, _now: datetime | None = None) -> None: # Only for re-scans. Initial setup adds entities # through sensor.async_setup_entry if self.config_entry.state is ConfigEntryState.LOADED: - async_dispatcher_send(self.hass, SOLAR_NET_DISCOVERY_NEW, _coordinator) + async_dispatcher_send( + self.hass, + discovery_signal(self.config_entry.entry_id), + _coordinator, + ) _LOGGER.debug( "New inverter added (UID: %s)", @@ -336,10 +371,17 @@ def _modbus_params(self) -> ModbusTcpParams | None: async def _init_modbus_inverter(self, inverter_info: FroniusDeviceInfo) -> None: """Set up a Modbus coordinator for an inverter exposing SunSpec MPPT data.""" - if inverter_info.solar_net_id in [ + # each coordinator is retried on its own: a device may answer for one + # of them and not the other, and recover on a later re-scan + needs_readings = inverter_info.solar_net_id not in { coordinator.inverter_info.solar_net_id for coordinator in self.modbus_inverter_coordinators - ]: + } + needs_settings = inverter_info.solar_net_id not in { + coordinator.inverter_info.solar_net_id + for coordinator in self.modbus_settings_coordinators + } + if not needs_readings and not needs_settings: return if (unit_id := self._modbus_unit_id(inverter_info.solar_net_id)) is None: return @@ -367,7 +409,7 @@ async def _init_modbus_inverter(self, inverter_info: FroniusDeviceInfo) -> None: err, ) return - if modbus_inverter.mppt is not None: + if needs_readings and modbus_inverter.mppt is not None: readings = FroniusModbusInverterUpdateCoordinator( hass=self.hass, solar_net=self, @@ -377,16 +419,19 @@ async def _init_modbus_inverter(self, inverter_info: FroniusDeviceInfo) -> None: modbus_inverter=modbus_inverter, config_entry=self.config_entry, ) - if await self._start_modbus_coordinator(readings): - self.modbus_inverter_coordinators.append(readings) - else: + await self._start_modbus_coordinator( + readings, self.modbus_inverter_coordinators + ) + elif needs_readings: _LOGGER.debug( "No MPPT model exposed by inverter %s at Modbus unit %s", inverter_info.solar_net_id, unit_id, ) - if await self._modbus_control_allowed(modbus_inverter, unit_id): + if needs_settings and await self._modbus_control_allowed( + modbus_inverter, unit_id + ): settings = FroniusModbusSettingsUpdateCoordinator( hass=self.hass, solar_net=self, @@ -396,8 +441,9 @@ async def _init_modbus_inverter(self, inverter_info: FroniusDeviceInfo) -> None: modbus_inverter=modbus_inverter, config_entry=self.config_entry, ) - if await self._start_modbus_coordinator(settings): - self.modbus_settings_coordinators.append(settings) + await self._start_modbus_coordinator( + settings, self.modbus_settings_coordinators + ) _LOGGER.debug( "Modbus enabled for inverter %s (UID: %s, unit ID: %s)", @@ -406,8 +452,10 @@ async def _init_modbus_inverter(self, inverter_info: FroniusDeviceInfo) -> None: unit_id, ) - async def _start_modbus_coordinator( - self, coordinator: FroniusCoordinatorBase + async def _start_modbus_coordinator[ + _ModbusCoordinatorT: FroniusModbusCoordinatorBase + ]( + self, coordinator: _ModbusCoordinatorT, coordinators: list[_ModbusCoordinatorT] ) -> bool: """Do the first refresh of a Modbus coordinator, reporting success. @@ -418,10 +466,15 @@ async def _start_modbus_coordinator( await coordinator.async_refresh() if not coordinator.last_update_success: return False + # the platforms tell the coordinators apart by the list they are in, + # so it is kept before they are told about this one + coordinators.append(coordinator) # Only for re-scans. Initial setup adds entities through the # platforms' async_setup_entry. if self.config_entry.state is ConfigEntryState.LOADED: - async_dispatcher_send(self.hass, SOLAR_NET_DISCOVERY_NEW, coordinator) + async_dispatcher_send( + self.hass, discovery_signal(self.config_entry.entry_id), coordinator + ) return True async def _modbus_control_allowed( diff --git a/homeassistant/components/fronius/const.py b/homeassistant/components/fronius/const.py index 22934207d1c5e7..00e27916cc402f 100644 --- a/homeassistant/components/fronius/const.py +++ b/homeassistant/components/fronius/const.py @@ -12,7 +12,18 @@ DEFAULT_MODBUS_PORT: Final = 502 type SolarNetId = str -SOLAR_NET_DISCOVERY_NEW: Final = "fronius_discovery_new" +_SOLAR_NET_DISCOVERY_NEW: Final = "fronius_discovery_new" + + +def discovery_signal(entry_id: str) -> str: + """Return the signal carrying coordinators of an entry found after setup. + + One signal per config entry: a device found by one entry's re-scan has + nothing to do with the platforms of another. + """ + return f"{_SOLAR_NET_DISCOVERY_NEW}_{entry_id}" + + SOLAR_NET_ID_POWER_FLOW: SolarNetId = "power_flow" SOLAR_NET_ID_SYSTEM: SolarNetId = "system" SOLAR_NET_RESCAN_TIMER: Final = 60 diff --git a/homeassistant/components/fronius/number.py b/homeassistant/components/fronius/number.py index 303566c263da15..4b7051f0233f73 100644 --- a/homeassistant/components/fronius/number.py +++ b/homeassistant/components/fronius/number.py @@ -5,14 +5,19 @@ from homeassistant.components.number import NumberEntity, NumberEntityDescription from homeassistant.const import PERCENTAGE, EntityCategory, Platform -from homeassistant.core import HomeAssistant +from homeassistant.core import HomeAssistant, callback +from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback +from .const import discovery_signal from .entity import FroniusEntity, FroniusEntityDescription, ModbusComponentFn if TYPE_CHECKING: from . import FroniusConfigEntry - from .coordinator import FroniusModbusSettingsUpdateCoordinator + from .coordinator import ( + FroniusCoordinatorBase, + FroniusModbusSettingsUpdateCoordinator, + ) # writes go to one device at a time PARALLEL_UPDATES: Final = 1 @@ -84,11 +89,27 @@ async def async_setup_entry( async_add_entities: AddConfigEntryEntitiesCallback, ) -> None: """Set up Fronius number entities based on a config entry.""" - for coordinator in config_entry.runtime_data.modbus_settings_coordinators: + solar_net = config_entry.runtime_data + for coordinator in solar_net.modbus_settings_coordinators: coordinator.add_entities_for_seen_keys( async_add_entities, Platform.NUMBER, ModbusSetpointNumber ) + @callback + def async_add_new_entities(coordinator: FroniusCoordinatorBase) -> None: + """Add the entities of a coordinator found after setup.""" + if Platform.NUMBER not in coordinator.valid_descriptions: + return + coordinator.add_entities_for_seen_keys( + async_add_entities, Platform.NUMBER, ModbusSetpointNumber + ) + + config_entry.async_on_unload( + async_dispatcher_connect( + hass, discovery_signal(config_entry.entry_id), async_add_new_entities + ) + ) + class ModbusSetpointNumber(FroniusEntity, NumberEntity): """A writable setpoint of an inverters Modbus interface.""" diff --git a/homeassistant/components/fronius/sensor.py b/homeassistant/components/fronius/sensor.py index ad3831f3691427..9d37a4aba89c49 100644 --- a/homeassistant/components/fronius/sensor.py +++ b/homeassistant/components/fronius/sensor.py @@ -34,10 +34,10 @@ from .const import ( DOMAIN, INVERTER_ERROR_CODES, - SOLAR_NET_DISCOVERY_NEW, InverterStatusCodeOption, MeterLocationCodeOption, OhmPilotStateCodeOption, + discovery_signal, get_inverter_status_message, get_meter_location_description, get_ohmpilot_state_message, @@ -101,6 +101,8 @@ async def async_setup_entry( @callback def async_add_new_entities(coordinator: FroniusCoordinatorBase) -> None: """Add newly found inverter entities.""" + if Platform.SENSOR not in coordinator.valid_descriptions: + return constructor = ( ModbusInverterSensor if coordinator in solar_net.modbus_inverter_coordinators @@ -113,7 +115,7 @@ def async_add_new_entities(coordinator: FroniusCoordinatorBase) -> None: config_entry.async_on_unload( async_dispatcher_connect( hass, - SOLAR_NET_DISCOVERY_NEW, + discovery_signal(config_entry.entry_id), async_add_new_entities, ) ) diff --git a/homeassistant/components/fronius/switch.py b/homeassistant/components/fronius/switch.py index 5a1381621c10a7..42ec94efb496c4 100644 --- a/homeassistant/components/fronius/switch.py +++ b/homeassistant/components/fronius/switch.py @@ -5,14 +5,19 @@ from homeassistant.components.switch import SwitchEntity, SwitchEntityDescription from homeassistant.const import EntityCategory, Platform -from homeassistant.core import HomeAssistant +from homeassistant.core import HomeAssistant, callback +from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback +from .const import discovery_signal from .entity import FroniusEntity, FroniusEntityDescription, ModbusComponentFn if TYPE_CHECKING: from . import FroniusConfigEntry - from .coordinator import FroniusModbusSettingsUpdateCoordinator + from .coordinator import ( + FroniusCoordinatorBase, + FroniusModbusSettingsUpdateCoordinator, + ) # writes go to one device at a time PARALLEL_UPDATES: Final = 1 @@ -63,11 +68,27 @@ async def async_setup_entry( async_add_entities: AddConfigEntryEntitiesCallback, ) -> None: """Set up Fronius switch entities based on a config entry.""" - for coordinator in config_entry.runtime_data.modbus_settings_coordinators: + solar_net = config_entry.runtime_data + for coordinator in solar_net.modbus_settings_coordinators: coordinator.add_entities_for_seen_keys( async_add_entities, Platform.SWITCH, ModbusControlSwitch ) + @callback + def async_add_new_entities(coordinator: FroniusCoordinatorBase) -> None: + """Add the entities of a coordinator found after setup.""" + if Platform.SWITCH not in coordinator.valid_descriptions: + return + coordinator.add_entities_for_seen_keys( + async_add_entities, Platform.SWITCH, ModbusControlSwitch + ) + + config_entry.async_on_unload( + async_dispatcher_connect( + hass, discovery_signal(config_entry.entry_id), async_add_new_entities + ) + ) + class ModbusControlSwitch(FroniusEntity, SwitchEntity): """A control of an inverters Modbus interface that is on or off. diff --git a/tests/components/fronius/test_modbus.py b/tests/components/fronius/test_modbus.py index 9806d210128190..c8c2c6fc4d023c 100644 --- a/tests/components/fronius/test_modbus.py +++ b/tests/components/fronius/test_modbus.py @@ -1,25 +1,28 @@ """Tests for the Fronius Modbus TCP (SunSpec) support.""" from datetime import timedelta +from logging import ERROR from unittest.mock import AsyncMock, MagicMock, patch from freezegun.api import FrozenDateTimeFactory +from fronius_modbus import Mppt from fronius_modbus.testing import MpptModuleSpec, build_sunspec_map +from modbus_connection import ModbusConnectionError from modbus_connection.mock import MockModbusConnection import pytest -from homeassistant.components.fronius.const import SOLAR_NET_RESCAN_TIMER +from homeassistant.components.fronius.const import DOMAIN, SOLAR_NET_RESCAN_TIMER from homeassistant.components.fronius.coordinator import ( FroniusModbusInverterUpdateCoordinator, ) from homeassistant.config_entries import ConfigEntryState -from homeassistant.const import Platform +from homeassistant.const import CONF_HOST, Platform from homeassistant.core import HomeAssistant from homeassistant.helpers import entity_registry as er -from . import mock_responses, setup_fronius_integration +from . import MOCK_HOST, mock_responses, setup_fronius_integration -from tests.common import async_fire_time_changed +from tests.common import MockConfigEntry, async_fire_time_changed from tests.test_util.aiohttp import AiohttpClientMocker # module names as reported by real GEN24 hybrid inverters @@ -250,6 +253,7 @@ async def test_no_mppt_model( aioclient_mock: AiohttpClientMocker, mock_fronius_modbus: MockModbusConnection, entity_registry: er.EntityRegistry, + freezer: FrozenDateTimeFactory, ) -> None: """Test a SunSpec device without MPPT model still gets its controls. @@ -277,6 +281,13 @@ async def test_no_mppt_model( assert not [entry for entry in modbus_entities if "mppt" in entry.unique_id] assert "number" in {entry.domain for entry in modbus_entities} + freezer.tick(timedelta(minutes=SOLAR_NET_RESCAN_TIMER, seconds=1)) + async_fire_time_changed(hass) + await hass.async_block_till_done(wait_background_tasks=True) + + # the re-scan finds the controls already set up + assert len(config_entry.runtime_data.modbus_settings_coordinators) == 1 + @pytest.mark.usefixtures("entity_registry_enabled_by_default") async def test_not_implemented_values( @@ -386,6 +397,7 @@ async def test_modbus_retried_after_setup( aioclient_mock: AiohttpClientMocker, mock_modbus_unavailable: MagicMock, mock_modbus_connection: MockModbusConnection, + entity_registry: er.EntityRegistry, freezer: FrozenDateTimeFactory, ) -> None: """Test an inverter asleep at setup time gets its Modbus entities later. @@ -416,6 +428,10 @@ async def test_modbus_retried_after_setup( assert config_entry.runtime_data.modbus_inverter_coordinators assert_state(hass, "sensor.gen24_storage_mppt_1_dc_power", 3300) + # the Modbus sensors of the re-scan are told apart from the SolarAPI ones + entry = entity_registry.async_get("sensor.gen24_storage_mppt_1_dc_power") + assert entry + assert "-modbus-" in entry.unique_id # the hold on the shared connection is taken once, not once per re-scan assert mock_modbus_unavailable.call_count == 1 @@ -451,3 +467,151 @@ async def test_control_refused_creates_no_control_entities( ) if entry.domain == "number" ] + + +@pytest.mark.parametrize( + ("modules", "include_mppt_model"), + [ + pytest.param(GEN24_HYBRID_MODULES, True, id="mppt_model"), + pytest.param([], False, id="no_mppt_model"), + ], +) +async def test_controls_enabled_later_get_their_entities( + hass: HomeAssistant, + aioclient_mock: AiohttpClientMocker, + mock_fronius_modbus: MockModbusConnection, + freezer: FrozenDateTimeFactory, + caplog: pytest.LogCaptureFixture, + modules: list[MpptModuleSpec], + include_mppt_model: bool, +) -> None: + """Test entities appear for controls a re-scan finds after setup. + + The platforms are set up once, so a coordinator that only comes up on a + later re-scan has to be handed to them through the dispatcher - which + every platform listens to, including those it has nothing for. Whether + the device also has an MPPT model decides whether a readings coordinator + is already there when the controls arrive. + """ + mock_fronius_modbus.for_unit(1).holding.update( + build_sunspec_map( + modules, include_mppt_model=include_mppt_model, storage_wcha_max=12800 + ) + ) + mock_responses(aioclient_mock, fixture_set="gen24_storage") + with patch( + "fronius_modbus.Controls.probe_write_access", AsyncMock(return_value=False) + ): + config_entry = await setup_fronius_integration( + hass, is_logger=False, unique_id="12345678" + ) + assert hass.states.get("number.gen24_storage_ac_power_limit") is None + + # inverter control via Modbus is enabled on the device web interface + freezer.tick(timedelta(minutes=SOLAR_NET_RESCAN_TIMER, seconds=1)) + async_fire_time_changed(hass) + await hass.async_block_till_done(wait_background_tasks=True) + + assert config_entry.runtime_data.modbus_settings_coordinators + assert hass.states.get("number.gen24_storage_ac_power_limit") + assert hass.states.get("switch.gen24_storage_ac_power_limiting") + assert not [record for record in caplog.records if record.levelno >= ERROR] + + +async def test_readings_recover_when_only_the_controls_came_up( + hass: HomeAssistant, + aioclient_mock: AiohttpClientMocker, + mock_fronius_modbus: MockModbusConnection, + freezer: FrozenDateTimeFactory, +) -> None: + """Test a re-scan still adds the MPPT data after it failed once. + + The two coordinators are independent: one of them answering is no reason + to stop retrying the other. + """ + mock_fronius_modbus.for_unit(1).holding.update( + build_sunspec_map(GEN24_HYBRID_MODULES, storage_wcha_max=12800) + ) + mock_responses(aioclient_mock, fixture_set="gen24_storage") + with patch.object( + Mppt, "async_update", side_effect=ModbusConnectionError("no answer") + ): + config_entry = await setup_fronius_integration( + hass, is_logger=False, unique_id="12345678" + ) + assert not config_entry.runtime_data.modbus_inverter_coordinators + assert config_entry.runtime_data.modbus_settings_coordinators + + freezer.tick(timedelta(minutes=SOLAR_NET_RESCAN_TIMER, seconds=1)) + async_fire_time_changed(hass) + await hass.async_block_till_done(wait_background_tasks=True) + + assert config_entry.runtime_data.modbus_inverter_coordinators + # the settings coordinator that was already up is not added a second time + assert len(config_entry.runtime_data.modbus_settings_coordinators) == 1 + + +async def test_wrongly_registered_sensors_are_moved_over( + hass: HomeAssistant, + aioclient_mock: AiohttpClientMocker, + mock_fronius_modbus: MockModbusConnection, + entity_registry: er.EntityRegistry, +) -> None: + """Test 2026.9 Modbus sensors keep their entity ID and history. + + A re-scan registered them with the SolarAPI unique ID format, which the + fixed platform would otherwise leave behind as a stale entity. + """ + config_entry = MockConfigEntry( + domain=DOMAIN, + entry_id="f1e2b9837e8adaed6fa682acaa216fd8", + unique_id="12345678", + data={CONF_HOST: MOCK_HOST, "is_logger": False, "modbus_port": 502}, + minor_version=2, + ) + config_entry.add_to_hass(hass) + stale = entity_registry.async_get_or_create( + "sensor", + DOMAIN, + "12345678-mppt_1_power_dc", + config_entry=config_entry, + suggested_object_id="gen24_storage_mppt_1_dc_power", + ) + untouched = entity_registry.async_get_or_create( + "sensor", + DOMAIN, + "12345678-energy_total", + config_entry=config_entry, + suggested_object_id="gen24_storage_total_energy", + ) + # a restart has already registered a second entity for this one + superseded = entity_registry.async_get_or_create( + "sensor", + DOMAIN, + "12345678-mppt_2_power_dc", + config_entry=config_entry, + suggested_object_id="gen24_storage_mppt_2_dc_power_old", + ) + entity_registry.async_get_or_create( + "sensor", + DOMAIN, + "12345678-modbus-mppt_2_power_dc", + config_entry=config_entry, + suggested_object_id="gen24_storage_mppt_2_dc_power", + ) + mock_fronius_modbus.for_unit(1).holding.update( + build_sunspec_map(GEN24_HYBRID_MODULES, storage_wcha_max=12800) + ) + mock_responses(aioclient_mock, fixture_set="gen24_storage") + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + assert (entry := entity_registry.async_get(stale.entity_id)) + assert entry.unique_id == "12345678-modbus-mppt_1_power_dc" + # a SolarAPI sensor keeps its own format + assert (entry := entity_registry.async_get(untouched.entity_id)) + assert entry.unique_id == "12345678-energy_total" + # and one whose place is taken is left where it is + assert (entry := entity_registry.async_get(superseded.entity_id)) + assert entry.unique_id == "12345678-mppt_2_power_dc"