Notice a SolarEdge Modbus control block that only answers later (#183365)

homeassistant/components/solaredge_modbus/__init__.py

@@ -12,7 +12,12 @@ from functools import partial
 from typing import TYPE_CHECKING
 
 from modbus_connection import ModbusUnit
-from solaredged import SolarEdge, SolarEdgeConnectionError, SolarEdgeError
+from solaredged import (
+    InverterExtended,
+    SolarEdge,
+    SolarEdgeConnectionError,
+    SolarEdgeError,
+)
 
 from homeassistant.components.modbus import async_get_unit
 from homeassistant.const import Platform
@@ -34,8 +39,12 @@ from .const import (
     SETTINGS_SCAN_INTERVAL,
     SUBSYSTEM_BATTERIES,
     SUBSYSTEM_COMMON,
+    SUBSYSTEM_EXPORT_CONTROL,
+    SUBSYSTEM_GRID_STATUS,
     SUBSYSTEM_INVERTER,
     SUBSYSTEM_METERS,
+    SUBSYSTEM_POWER_CONTROL,
+    SUBSYSTEM_STORAGE_CONTROL,
 )
 from .coordinator import (
     SolarEdgeModbusConfigEntry,
@@ -148,15 +157,13 @@ async def async_setup_entry(
         attachments=_attachment_identities(solaredge),
     )
 
-    if silent := solaredge.unresponsive_blocks & {
-        SUBSYSTEM_BATTERIES,
-        SUBSYSTEM_METERS,
-    }:
+    if silent := solaredge.unresponsive_blocks & _probed_blocks(solaredge).keys():
         LOGGER.warning(
-            "%s did not answer for its %s while probing, so their entities are"
-            " missing until it does; reloading probes again",
+            "%s did not answer for %s while probing, so the entities those"
+            " would carry are missing; this is looked at again every %s minutes",
             entry.title,
             " and ".join(sorted(silent)),
+            int(ATTACHMENT_SCAN_INTERVAL.total_seconds() // 60),
         )
 
     _async_remove_stale_devices(hass, entry, solaredge, serial_number, silent=silent)
@@ -192,6 +199,24 @@ def _attachment_identities(solaredge: SolarEdge) -> frozenset[str]:
     )
 
 
+def _probed_blocks(solaredge: SolarEdge) -> dict[str, int]:
+    """Return what probing found, as far as this entry is built on it.
+
+    A block that answers decides which entities exist, and probing happens once
+    while setting up, so a block that was silent then leaves its platform empty
+    until the entry loads again. Counted rather than flagged, so what is there
+    once reads the same as what can be there three times.
+    """
+    return {
+        SUBSYSTEM_METERS: len(solaredge.meters),
+        SUBSYSTEM_BATTERIES: len(solaredge.batteries),
+        SUBSYSTEM_GRID_STATUS: isinstance(solaredge.inverter, InverterExtended),
+        SUBSYSTEM_STORAGE_CONTROL: solaredge.storage_control is not None,
+        SUBSYSTEM_EXPORT_CONTROL: solaredge.export_control is not None,
+        SUBSYSTEM_POWER_CONTROL: solaredge.power_control is not None,
+    }
+
+
 async def _async_reload_when_attachments_change(
     hass: HomeAssistant,
     entry: SolarEdgeModbusConfigEntry,
@@ -219,23 +244,21 @@ async def _async_reload_when_attachments_change(
         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:
+    known = _probed_blocks(solaredge)
+    for name, found in _probed_blocks(probed).items():
+        if found == known[name]:
             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:
+        # device, or a whole platform, over one timeout.
+        if found < known[name] 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,
+            known[name],
             found,
         )
         hass.config_entries.async_schedule_reload(entry.entry_id)

homeassistant/components/solaredge_modbus/const.py

@@ -23,10 +23,14 @@ DEFAULT_UNIT_ID: Final = 1
 SUBSYSTEM_COMMON: Final = "common"
 SUBSYSTEM_INVERTER: Final = "inverter"
 
-# How the library names the blocks it probes for.
+# How the library names the blocks it probes for. Storage and export control
+# are probed apart, where a poll reports the two together as site control.
 SUBSYSTEM_BATTERIES: Final = "batteries"
+SUBSYSTEM_EXPORT_CONTROL: Final = "export_control"
+SUBSYSTEM_GRID_STATUS: Final = "grid_status"
 SUBSYSTEM_METERS: Final = "meters"
 SUBSYSTEM_STORAGE_CAPACITY: Final = "storage_capacity"
+SUBSYSTEM_STORAGE_CONTROL: Final = "storage_control"
 
 # The writable control blocks, as an UpdateReport names them. Export control's
 # read spans storage control, so the library reads and reports the two as one.

tests/components/solaredge_modbus/test_init.py

@@ -60,11 +60,18 @@ METER_MODEL_REGISTER = 40188
 # An address inside the pooled storage and export control read.
 SITE_CONTROL_REGISTER = 57348
 
+# The first register of the power control block, which the probe asks for.
+POWER_CONTROL_REGISTER = 61440
+
 # 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"
+ACTIVE_POWER_LIMIT_ENTITY = "number.solaredge_se10000h_active_power_limit"
+BACKUP_RESERVE_ENTITY = "number.solaredge_se10000h_backup_reserve"
+SITE_EXPORT_LIMIT_ENTITY = "number.solaredge_se10000h_site_export_limit"
+ON_GRID_ENTITY = "binary_sensor.solaredge_se10000h_on_grid"
 
 
 async def _setup(hass: HomeAssistant, entry: MockConfigEntry) -> None:
@@ -758,6 +765,63 @@ async def test_replaced_meter_is_picked_up(
     )
 
 
+@pytest.mark.parametrize(
+    ("register", "entity_id"),
+    [
+        pytest.param(40113, ON_GRID_ENTITY, id="grid status"),
+        pytest.param(57348, BACKUP_RESERVE_ENTITY, id="storage control"),
+        pytest.param(57344, SITE_EXPORT_LIMIT_ENTITY, id="export control"),
+        pytest.param(61440, ACTIVE_POWER_LIMIT_ENTITY, id="power control"),
+    ],
+)
+async def test_block_that_answers_later_is_picked_up(
+    hass: HomeAssistant,
+    freezer: FrozenDateTimeFactory,
+    mock_config_entry: MockConfigEntry,
+    mock_modbus_unit: MockModbusUnit,
+    register: int,
+    entity_id: str,
+) -> None:
+    """A block that answers only after setup still gets its entities."""
+    mock_modbus_unit.fail_read(register, ModbusTimeoutError("timed out"))
+    await _setup(hass, mock_config_entry)
+
+    assert hass.states.get(entity_id) is None
+
+    mock_modbus_unit.fail_read(register, None)
+
+    await _tick_attachment_check(hass, freezer)
+
+    assert hass.states.get(entity_id) is not None
+
+
+@pytest.mark.parametrize(
+    ("register", "entity_id"),
+    [
+        pytest.param(40113, ON_GRID_ENTITY, id="grid status"),
+        pytest.param(57348, BACKUP_RESERVE_ENTITY, id="storage control"),
+        pytest.param(57344, SITE_EXPORT_LIMIT_ENTITY, id="export control"),
+        pytest.param(61440, ACTIVE_POWER_LIMIT_ENTITY, id="power control"),
+    ],
+)
+async def test_block_going_quiet_does_not_trigger_a_reload(
+    hass: HomeAssistant,
+    freezer: FrozenDateTimeFactory,
+    mock_config_entry: MockConfigEntry,
+    mock_modbus_unit: MockModbusUnit,
+    register: int,
+    entity_id: str,
+) -> None:
+    """A block going quiet is not the inverter saying it does not have one."""
+    await _setup(hass, mock_config_entry)
+
+    mock_modbus_unit.fail_read(register, ModbusTimeoutError("timed out"))
+
+    assert await _tick_attachment_check(hass, freezer) == 1
+
+    assert hass.states.get(entity_id) is not None
+
+
 async def test_silent_attachment_does_not_trigger_a_reload(
     hass: HomeAssistant,
     freezer: FrozenDateTimeFactory,