Hand Fronius Modbus coordinators found by a re-scan to all platforms (#181402)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

homeassistant/components/fronius/__init__.py

@@ -18,9 +18,9 @@ from pyfronius import Fronius, FroniusError
 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 @@ from .const import (
     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 @@ PLATFORMS: Final = [
 
 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: `<inverter>-<key>` instead of `<inverter>-modbus-<key>`.
+    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 @@ class FroniusSolarNet:
             # 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 @@ class FroniusSolarNet:
 
     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 @@ class FroniusSolarNet:
                 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 @@ class FroniusSolarNet:
                 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 @@ class FroniusSolarNet:
                 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 @@ class FroniusSolarNet:
             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 @@ class FroniusSolarNet:
         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(

homeassistant/components/fronius/const.py

@@ -12,7 +12,18 @@ CONF_MODBUS_PORT: Final = "modbus_port"
 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

homeassistant/components/fronius/number.py

@@ -5,14 +5,19 @@ from typing import TYPE_CHECKING, Final, override
 
 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."""

homeassistant/components/fronius/sensor.py

@@ -34,10 +34,10 @@ from homeassistant.helpers.typing import StateType
 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 @@ async def async_setup_entry(
     config_entry.async_on_unload(
         async_dispatcher_connect(
             hass,
-            SOLAR_NET_DISCOVERY_NEW,
+            discovery_signal(config_entry.entry_id),
             async_add_new_entities,
         )
     )

homeassistant/components/fronius/switch.py

@@ -5,14 +5,19 @@ from typing import TYPE_CHECKING, Any, Final, override
 
 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.

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
@@ -298,6 +301,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.
 
@@ -325,6 +329,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(
@@ -434,6 +445,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.
@@ -464,6 +476,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
 
@@ -499,3 +515,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"