diff --git a/homeassistant/components/solaredge_modbus/__init__.py b/homeassistant/components/solaredge_modbus/__init__.py index 28f426f35a02..004c9a35e19e 100644 --- a/homeassistant/components/solaredge_modbus/__init__.py +++ b/homeassistant/components/solaredge_modbus/__init__.py @@ -33,6 +33,7 @@ from homeassistant.helpers.event import async_track_time_interval from .const import ( ATTACHMENT_SCAN_INTERVAL, CONF_UNIT_ID, + DISCOVERY_SUBSYSTEMS, DOMAIN, LOGGER, SCAN_INTERVAL, @@ -237,13 +238,24 @@ async def _async_reload_when_attachments_change( return try: - probed = await SolarEdge.async_probe(unit) + probed = await SolarEdge.async_probe( + unit, assume_absent=entry.runtime_data.settled_silent_blocks + ) 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 + # Silent while setting up and silent again here. A block that answered at + # setup and merely blipped now is not settled, or one timeout would hide a + # later removal until the entry loads again. Blocks that finding hardware + # depends on are never settled: picking up what was wired in later is what + # asking is for. + entry.runtime_data.settled_silent_blocks = ( + probed.unresponsive_blocks & solaredge.unresponsive_blocks + ) - DISCOVERY_SUBSYSTEMS + known = _probed_blocks(solaredge) for name, found in _probed_blocks(probed).items(): if found == known[name]: diff --git a/homeassistant/components/solaredge_modbus/const.py b/homeassistant/components/solaredge_modbus/const.py index 023026f45645..0a1d45670766 100644 --- a/homeassistant/components/solaredge_modbus/const.py +++ b/homeassistant/components/solaredge_modbus/const.py @@ -29,9 +29,18 @@ SUBSYSTEM_BATTERIES: Final = "batteries" SUBSYSTEM_EXPORT_CONTROL: Final = "export_control" SUBSYSTEM_GRID_STATUS: Final = "grid_status" SUBSYSTEM_METERS: Final = "meters" +SUBSYSTEM_MMPPT: Final = "mmppt" SUBSYSTEM_STORAGE_CAPACITY: Final = "storage_capacity" SUBSYSTEM_STORAGE_CONTROL: Final = "storage_control" +# The blocks that finding hardware depends on, so they are worth a timeout +# however long they stay quiet. Meters and batteries each bring a device, and +# the multiple-MPPT block decides the offset the meters are looked for at: +# taking it for absent would look for them at the wrong addresses for good. +DISCOVERY_SUBSYSTEMS: Final = frozenset( + {SUBSYSTEM_BATTERIES, SUBSYSTEM_METERS, SUBSYSTEM_MMPPT} +) + # 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. SUBSYSTEM_ADVANCED_POWER_CONTROL: Final = "advanced_power_control" diff --git a/homeassistant/components/solaredge_modbus/coordinator.py b/homeassistant/components/solaredge_modbus/coordinator.py index aa6964151857..20ac9dcaa87d 100644 --- a/homeassistant/components/solaredge_modbus/coordinator.py +++ b/homeassistant/components/solaredge_modbus/coordinator.py @@ -168,10 +168,18 @@ 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] + # Blocks that answered nothing while setting up and nothing again on the + # first check after it. Asking costs a full timeout each, which on a shared + # link is time every other inverter spends waiting, so they are taken for + # absent until the entry loads again. Empty until that first check, so a + # block that was merely busy at setup still gets picked up. + settled_silent_blocks: frozenset[str] = frozenset() + @property def solaredge(self) -> SolarEdge: """Return the polled device, which both coordinators share.""" diff --git a/homeassistant/components/solaredge_modbus/manifest.json b/homeassistant/components/solaredge_modbus/manifest.json index 0bfb15d167b2..670803f6d5ca 100644 --- a/homeassistant/components/solaredge_modbus/manifest.json +++ b/homeassistant/components/solaredge_modbus/manifest.json @@ -9,6 +9,6 @@ "iot_class": "local_polling", "loggers": ["modbus_connection", "solaredged", "tmodbus"], "quality_scale": "platinum", - "requirements": ["solaredged==0.4.0"], + "requirements": ["solaredged==0.5.0"], "zeroconf": ["_solaredge-modbus._tcp.local."] } diff --git a/requirements_all.txt b/requirements_all.txt index 95c9a1ae96d9..274856190906 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -3124,7 +3124,7 @@ sofar-modbus==0.17.0 solaredge-web==0.4.0 # homeassistant.components.solaredge_modbus -solaredged==0.4.0 +solaredged==0.5.0 # homeassistant.components.solarlog solarlog_cli==0.7.1 diff --git a/tests/components/solaredge_modbus/test_init.py b/tests/components/solaredge_modbus/test_init.py index ebe518f5d96c..7ad80076d77d 100644 --- a/tests/components/solaredge_modbus/test_init.py +++ b/tests/components/solaredge_modbus/test_init.py @@ -57,6 +57,9 @@ INVERTER_REGISTER = 40069 # The register the probe counts meters by. METER_MODEL_REGISTER = 40188 +# The identifier the multiple-MPPT probe reads, which sets the meter offset. +MMPPT_REGISTER = 40121 + # An address inside the pooled storage and export control read. SITE_CONTROL_REGISTER = 57348 @@ -643,10 +646,12 @@ async def _tick_attachment_check( probes = 0 probe = SolarEdge.async_probe - async def counting_probe(unit: ModbusUnit) -> SolarEdge: + async def counting_probe( + unit: ModbusUnit, *, assume_absent: frozenset[str] = frozenset() + ) -> SolarEdge: nonlocal probes probes += 1 - return await probe(unit) + return await probe(unit, assume_absent=assume_absent) with patch.object(SolarEdge, "async_probe", counting_probe): freezer.tick(ATTACHMENT_SCAN_INTERVAL) @@ -1042,3 +1047,117 @@ async def test_setup_error_when_link_settings_are_in_use( await _setup(hass, mock_config_entry) assert mock_config_entry.state is ConfigEntryState.SETUP_ERROR + + +async def test_a_block_that_stays_silent_stops_being_asked( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + mock_config_entry: MockConfigEntry, + mock_modbus_unit: MockModbusUnit, +) -> None: + """A block silent at setup is looked for once more, then left alone. + + Asking costs a full timeout each time, and the link is shared, so that is + time every other inverter on it spends queued behind the question. + """ + mock_modbus_unit.fail_read(POWER_CONTROL_REGISTER, ModbusTimeoutError("timed out")) + await _setup(hass, mock_config_entry) + + asked: list[frozenset[str]] = [] + probe = SolarEdge.async_probe + + async def recording_probe( + unit: ModbusUnit, *, assume_absent: frozenset[str] = frozenset() + ) -> SolarEdge: + asked.append(assume_absent) + return await probe(unit, assume_absent=assume_absent) + + with patch.object(SolarEdge, "async_probe", recording_probe): + for _ in range(2): + freezer.tick(ATTACHMENT_SCAN_INTERVAL) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert asked[0] == frozenset(), "the first check still looks for it" + assert "power_control" in asked[1], "a block still silent is not asked again" + + +async def test_a_block_that_blips_once_keeps_being_asked( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + mock_config_entry: MockConfigEntry, + mock_modbus_unit: MockModbusUnit, +) -> None: + """A block that answered at setup is not settled by one timeout later. + + Settling it would hide the block going away for good, since a block taken + for absent is reported back as though it had been silent again. + """ + await _setup(hass, mock_config_entry) + + asked: list[frozenset[str]] = [] + probe = SolarEdge.async_probe + + async def recording_probe( + unit: ModbusUnit, *, assume_absent: frozenset[str] = frozenset() + ) -> SolarEdge: + asked.append(assume_absent) + return await probe(unit, assume_absent=assume_absent) + + with patch.object(SolarEdge, "async_probe", recording_probe): + mock_modbus_unit.fail_read( + POWER_CONTROL_REGISTER, ModbusTimeoutError("timed out") + ) + freezer.tick(ATTACHMENT_SCAN_INTERVAL) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + mock_modbus_unit.fail_read(POWER_CONTROL_REGISTER, None) + freezer.tick(ATTACHMENT_SCAN_INTERVAL) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert "power_control" not in asked[1], "a blip settled a block that answered" + + +@pytest.mark.parametrize( + ("register", "subsystem"), + [ + pytest.param(BATTERY_RATED_ENERGY, "batteries", id="batteries"), + pytest.param(METER_MODEL_REGISTER, "meters", id="meters"), + pytest.param(MMPPT_REGISTER, "mmppt", id="mmppt"), + ], +) +async def test_a_block_discovery_depends_on_keeps_being_asked( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + mock_config_entry: MockConfigEntry, + mock_modbus_unit: MockModbusUnit, + register: int, + subsystem: str, +) -> None: + """A block that finding hardware depends on is asked however quiet it stays. + + Meters and batteries each bring a device. The multiple-MPPT block decides + the offset the meters are looked for at, so taking it for absent would look + for them at the wrong addresses for good. + """ + mock_modbus_unit.fail_read(register, ModbusTimeoutError("timed out")) + await _setup(hass, mock_config_entry) + + asked: list[frozenset[str]] = [] + probe = SolarEdge.async_probe + + async def recording_probe( + unit: ModbusUnit, *, assume_absent: frozenset[str] = frozenset() + ) -> SolarEdge: + asked.append(assume_absent) + return await probe(unit, assume_absent=assume_absent) + + with patch.object(SolarEdge, "async_probe", recording_probe): + for _ in range(2): + freezer.tick(ATTACHMENT_SCAN_INTERVAL) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert subsystem not in asked[1], "a block discovery depends on was settled"