Aurora abb defer unique_id assignment during yaml import (#58887)

* Defer unique_id assignment during yaml import if dark

* Back out variable name change to simplify.

* Allow config flow yaml setup deferral.

* Fix deferred yaml import

* Code review: only wrap necessary lines in try blk

* Code review: catch possible duplicate unique_id

* Simplify assignment.

* Code review: use timedelta to retry yaml import

* Code review: if a different error occurs, raise it

* Remove current config entry if duplicate unique_id

* Code review: remove unnecessary line.

* Code review: revert change, leave to other PR.

* Code review: remove unnecessary patch & min->sec

* Remove unnecessary else after raise.

* Increase test coverage.

* Check the number of config entries at each stage

* Raise ConfigEntryNotReady when connection fails.

* Log & return false for error on yaml import

homeassistant/components/aurora_abb_powerone/__init__.py

@@ -10,13 +10,15 @@
 
 import logging
 
-from aurorapy.client import AuroraSerialClient
+from aurorapy.client import AuroraError, AuroraSerialClient
 
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import CONF_ADDRESS, CONF_PORT
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import ConfigEntryNotReady
 
-from .const import DOMAIN
+from .config_flow import validate_and_connect
+from .const import ATTR_SERIAL_NUMBER, DOMAIN
 
 PLATFORMS = ["sensor"]
 
@@ -29,9 +31,43 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry):
     comport = entry.data[CONF_PORT]
     address = entry.data[CONF_ADDRESS]
     serclient = AuroraSerialClient(address, comport, parity="N", timeout=1)
+    # To handle yaml import attempts in darkeness, (re)try connecting only if
+    # unique_id not yet assigned.
+    if entry.unique_id is None:
+        try:
+            res = await hass.async_add_executor_job(
+                validate_and_connect, hass, entry.data
+            )
+        except AuroraError as error:
+            if "No response after" in str(error):
+                raise ConfigEntryNotReady("No response (could be dark)") from error
+            _LOGGER.error("Failed to connect to inverter: %s", error)
+            return False
+        except OSError as error:
+            if error.errno == 19:  # No such device.
+                _LOGGER.error("Failed to connect to inverter: no such COM port")
+                return False
+            _LOGGER.error("Failed to connect to inverter: %s", error)
+            return False
+        else:
+            # If we got here, the device is now communicating (maybe after
+            # being in darkness). But there's a small risk that the user has
+            # configured via the UI since we last attempted the yaml setup,
+            # which means we'd get a duplicate unique ID.
+            new_id = res[ATTR_SERIAL_NUMBER]
+            # Check if this unique_id has already been used
+            for existing_entry in hass.config_entries.async_entries(DOMAIN):
+                if existing_entry.unique_id == new_id:
+                    _LOGGER.debug(
+                        "Remove already configured config entry for id %s", new_id
+                    )
+                    hass.async_create_task(
+                        hass.config_entries.async_remove(entry.entry_id)
+                    )
+                    return False
+            hass.config_entries.async_update_entry(entry, unique_id=new_id)
 
     hass.data.setdefault(DOMAIN, {})[entry.unique_id] = serclient
-
     hass.config_entries.async_setup_platforms(entry, PLATFORMS)
 
     return True

homeassistant/components/aurora_abb_powerone/aurora_device.py

@@ -1,4 +1,6 @@
 """Top level class for AuroraABBPowerOneSolarPV inverters and sensors."""
+from __future__ import annotations
+
 import logging
 
 from aurorapy.client import AuroraSerialClient
@@ -29,9 +31,11 @@ class AuroraDevice(Entity):
         self._available = True
 
     @property
-    def unique_id(self) -> str:
+    def unique_id(self) -> str | None:
         """Return the unique id for this device."""
-        serial = self._data[ATTR_SERIAL_NUMBER]
+        serial = self._data.get(ATTR_SERIAL_NUMBER)
+        if serial is None:
+            return None
         return f"{serial}_{self.entity_description.key}"
 
     @property

homeassistant/components/aurora_abb_powerone/config_flow.py

@@ -81,16 +81,10 @@ class AuroraABBConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
             return self.async_abort(reason="already_setup")
 
         conf = {}
-        conf[ATTR_SERIAL_NUMBER] = "sn_unknown_yaml"
-        conf[ATTR_MODEL] = "model_unknown_yaml"
-        conf[ATTR_FIRMWARE] = "fw_unknown_yaml"
         conf[CONF_PORT] = config["device"]
         conf[CONF_ADDRESS] = config["address"]
         # config["name"] from yaml is ignored.
 
-        await self.async_set_unique_id(self.flow_id)
-        self._abort_if_unique_id_configured()
-
         return self.async_create_entry(title=DEFAULT_INTEGRATION_TITLE, data=conf)
 
     async def async_step_user(self, user_input=None):

tests/components/aurora_abb_powerone/test_config_flow.py

@@ -1,4 +1,5 @@
 """Test the Aurora ABB PowerOne Solar PV config flow."""
+from datetime import timedelta
 from logging import INFO
 from unittest.mock import patch
 
@@ -12,7 +13,20 @@ from homeassistant.components.aurora_abb_powerone.const import (
     ATTR_SERIAL_NUMBER,
     DOMAIN,
 )
+from homeassistant.config_entries import ConfigEntryState
 from homeassistant.const import CONF_ADDRESS, CONF_PORT
+from homeassistant.util.dt import utcnow
+
+from tests.common import async_fire_time_changed
+
+
+def _simulated_returns(index, global_measure=None):
+    returns = {
+        3: 45.678,  # power
+        21: 9.876,  # temperature
+        5: 12345,  # energy
+    }
+    return returns[index]
 
 
 async def test_form(hass):
@@ -150,16 +164,161 @@ async def test_form_invalid_com_ports(hass):
 
 
 # Tests below can be deleted after deprecation period is finished.
-async def test_import(hass):
+async def test_import_day(hass):
-    """Test configuration.yaml import used during migration."""
+    """Test .yaml import when the inverter is able to communicate."""
-    TESTDATA = {"device": "/dev/ttyUSB7", "address": 3, "name": "MyAuroraPV"}
+    TEST_DATA = {"device": "/dev/ttyUSB7", "address": 3, "name": "MyAuroraPV"}
-    with patch(
+
-        "homeassistant.components.generic.camera.GenericCamera.async_camera_image",
+    with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None,), patch(
-        return_value=None,
+        "aurorapy.client.AuroraSerialClient.serial_number",
-    ):
+        return_value="9876543",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.version",
+        return_value="9.8.7.6",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.pn",
+        return_value="A.B.C",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.firmware",
+        return_value="1.234",
+    ) as mock_setup_entry:
         result = await hass.config_entries.flow.async_init(
-            DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, data=TESTDATA
+            DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, data=TEST_DATA
         )
+
     assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
     assert result["data"][CONF_PORT] == "/dev/ttyUSB7"
     assert result["data"][CONF_ADDRESS] == 3
+    assert len(hass.config_entries.async_entries(DOMAIN)) == 1
+
+    assert len(mock_setup_entry.mock_calls) == 1
+
+
+async def test_import_night(hass):
+    """Test .yaml import when the inverter is inaccessible (e.g. darkness)."""
+    TEST_DATA = {"device": "/dev/ttyUSB7", "address": 3, "name": "MyAuroraPV"}
+
+    # First time round, no response.
+    with patch(
+        "aurorapy.client.AuroraSerialClient.connect",
+        side_effect=AuroraError("No response after"),
+    ) as mock_connect:
+        result = await hass.config_entries.flow.async_init(
+            DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, data=TEST_DATA
+        )
+
+    configs = hass.config_entries.async_entries(DOMAIN)
+    assert len(configs) == 1
+    entry = configs[0]
+    assert not entry.unique_id
+    assert entry.state == ConfigEntryState.SETUP_RETRY
+
+    assert len(mock_connect.mock_calls) == 1
+    assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
+    assert result["data"][CONF_PORT] == "/dev/ttyUSB7"
+    assert result["data"][CONF_ADDRESS] == 3
+
+    # Second time round, talking this time.
+    with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None,), patch(
+        "aurorapy.client.AuroraSerialClient.serial_number",
+        return_value="9876543",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.version",
+        return_value="9.8.7.6",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.pn",
+        return_value="A.B.C",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.firmware",
+        return_value="1.234",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.measure",
+        side_effect=_simulated_returns,
+    ):
+        # Wait >5seconds for the config to auto retry.
+        async_fire_time_changed(hass, utcnow() + timedelta(seconds=6))
+        await hass.async_block_till_done()
+        assert entry.state == ConfigEntryState.LOADED
+        assert entry.unique_id
+
+        assert len(mock_connect.mock_calls) == 1
+        assert hass.states.get("sensor.power_output").state == "45.7"
+    assert len(hass.config_entries.async_entries(DOMAIN)) == 1
+
+
+async def test_import_night_then_user(hass):
+    """Attempt yaml import and fail (dark), but user sets up manually before auto retry."""
+    TEST_DATA = {"device": "/dev/ttyUSB7", "address": 3, "name": "MyAuroraPV"}
+
+    # First time round, no response.
+    with patch(
+        "aurorapy.client.AuroraSerialClient.connect",
+        side_effect=AuroraError("No response after"),
+    ) as mock_connect:
+        result = await hass.config_entries.flow.async_init(
+            DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, data=TEST_DATA
+        )
+
+    configs = hass.config_entries.async_entries(DOMAIN)
+    assert len(configs) == 1
+    entry = configs[0]
+    assert not entry.unique_id
+    assert entry.state == ConfigEntryState.SETUP_RETRY
+
+    assert len(mock_connect.mock_calls) == 1
+    assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
+    assert result["data"][CONF_PORT] == "/dev/ttyUSB7"
+    assert result["data"][CONF_ADDRESS] == 3
+
+    # Failed once, now simulate the user initiating config flow with valid settings.
+    fakecomports = []
+    fakecomports.append(list_ports_common.ListPortInfo("/dev/ttyUSB7"))
+    with patch(
+        "serial.tools.list_ports.comports",
+        return_value=fakecomports,
+    ):
+        result = await hass.config_entries.flow.async_init(
+            DOMAIN, context={"source": config_entries.SOURCE_USER}
+        )
+    assert result["type"] == "form"
+    assert result["errors"] == {}
+
+    with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None,), patch(
+        "aurorapy.client.AuroraSerialClient.serial_number",
+        return_value="9876543",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.version",
+        return_value="9.8.7.6",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.pn",
+        return_value="A.B.C",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.firmware",
+        return_value="1.234",
+    ):
+        result2 = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            {CONF_PORT: "/dev/ttyUSB7", CONF_ADDRESS: 7},
+        )
+
+    assert result2["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
+    assert len(hass.config_entries.async_entries(DOMAIN)) == 2
+
+    # Now retry yaml - it should fail with duplicate
+    with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None,), patch(
+        "aurorapy.client.AuroraSerialClient.serial_number",
+        return_value="9876543",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.version",
+        return_value="9.8.7.6",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.pn",
+        return_value="A.B.C",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.firmware",
+        return_value="1.234",
+    ):
+        # Wait >5seconds for the config to auto retry.
+        async_fire_time_changed(hass, utcnow() + timedelta(seconds=6))
+        await hass.async_block_till_done()
+        assert entry.state == ConfigEntryState.NOT_LOADED
+    assert len(hass.config_entries.async_entries(DOMAIN)) == 1

tests/components/aurora_abb_powerone/test_init.py

@@ -19,6 +19,18 @@ async def test_unload_entry(hass):
     with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None), patch(
         "homeassistant.components.aurora_abb_powerone.sensor.AuroraSensor.update",
         return_value=None,
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.serial_number",
+        return_value="9876543",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.version",
+        return_value="9.8.7.6",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.pn",
+        return_value="A.B.C",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.firmware",
+        return_value="1.234",
     ):
         mock_entry = MockConfigEntry(
             domain=DOMAIN,

tests/components/aurora_abb_powerone/test_sensor.py

@@ -64,6 +64,18 @@ async def test_setup_platform_valid_config(hass):
     with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None), patch(
         "aurorapy.client.AuroraSerialClient.measure",
         side_effect=_simulated_returns,
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.serial_number",
+        return_value="9876543",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.version",
+        return_value="9.8.7.6",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.pn",
+        return_value="A.B.C",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.firmware",
+        return_value="1.234",
     ), patch(
         "aurorapy.client.AuroraSerialClient.cumulated_energy",
         side_effect=_simulated_returns,
@@ -94,6 +106,18 @@ async def test_sensors(hass):
     with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None), patch(
         "aurorapy.client.AuroraSerialClient.measure",
         side_effect=_simulated_returns,
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.serial_number",
+        return_value="9876543",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.version",
+        return_value="9.8.7.6",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.pn",
+        return_value="A.B.C",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.firmware",
+        return_value="1.234",
     ), patch(
         "aurorapy.client.AuroraSerialClient.cumulated_energy",
         side_effect=_simulated_returns,
@@ -123,6 +147,18 @@ async def test_sensor_dark(hass):
     # sun is up
     with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None), patch(
         "aurorapy.client.AuroraSerialClient.measure", side_effect=_simulated_returns
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.serial_number",
+        return_value="9876543",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.version",
+        return_value="9.8.7.6",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.pn",
+        return_value="A.B.C",
+    ), patch(
+        "aurorapy.client.AuroraSerialClient.firmware",
+        return_value="1.234",
     ):
         mock_entry.add_to_hass(hass)
         await hass.config_entries.async_setup(mock_entry.entry_id)