Fix powerview entity unique id migration when the config entry unique id is missing (#129188)

Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>

homeassistant/components/hunterdouglas_powerview/__init__.py

@@ -3,8 +3,6 @@
 import logging
 from typing import TYPE_CHECKING
 
-from aiopvapi.helpers.aiorequest import AioRequest
-from aiopvapi.hub import Hub
 from aiopvapi.resources.model import PowerviewData
 from aiopvapi.rooms import Rooms
 from aiopvapi.scenes import Scenes
@@ -13,13 +11,13 @@ from aiopvapi.shades import Shades
 from homeassistant.const import CONF_API_VERSION, CONF_HOST, Platform
 from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import ConfigEntryNotReady
-from homeassistant.helpers.aiohttp_client import async_get_clientsession
 import homeassistant.helpers.entity_registry as er
 
 from .const import DOMAIN, HUB_EXCEPTIONS
 from .coordinator import PowerviewShadeUpdateCoordinator
-from .model import PowerviewConfigEntry, PowerviewDeviceInfo, PowerviewEntryData
+from .model import PowerviewConfigEntry, PowerviewEntryData
 from .shade_data import PowerviewShadeData
+from .util import async_connect_hub
 
 PARALLEL_UPDATES = 1
 
@@ -37,29 +35,23 @@ _LOGGER = logging.getLogger(__name__)
 
 async def async_setup_entry(hass: HomeAssistant, entry: PowerviewConfigEntry) -> bool:
     """Set up Hunter Douglas PowerView from a config entry."""
-
     config = entry.data
-
+    hub_address: str = config[CONF_HOST]
-    hub_address = config[CONF_HOST]
+    api_version: int | None = config.get(CONF_API_VERSION)
-    api_version = config.get(CONF_API_VERSION, None)
     _LOGGER.debug("Connecting %s at %s with v%s api", DOMAIN, hub_address, api_version)
 
-    websession = async_get_clientsession(hass)
-
-    pv_request = AioRequest(
-        hub_address, loop=hass.loop, websession=websession, api_version=api_version
-    )
-
     # default 15 second timeout for each call in upstream
     try:
-        hub = Hub(pv_request)
+        api = await async_connect_hub(hass, hub_address, api_version)
-        await hub.query_firmware()
-        device_info = await async_get_device_info(hub)
     except HUB_EXCEPTIONS as err:
         raise ConfigEntryNotReady(
             f"Connection error to PowerView hub {hub_address}: {err}"
         ) from err
 
+    hub = api.hub
+    pv_request = api.pv_request
+    device_info = api.device_info
+
     if hub.role != "Primary":
         # this should be caught in config_flow, but account for a hub changing roles
         # this will only happen manually by a user
@@ -94,6 +86,11 @@ async def async_setup_entry(hass: HomeAssistant, entry: PowerviewConfigEntry) ->
         new_data[CONF_API_VERSION] = hub.api_version
         hass.config_entries.async_update_entry(entry, data=new_data)
 
+    if entry.unique_id is None:
+        hass.config_entries.async_update_entry(
+            entry, unique_id=device_info.serial_number
+        )
+
     coordinator = PowerviewShadeUpdateCoordinator(hass, shades, hub)
     coordinator.async_set_updated_data(PowerviewShadeData())
     # populate raw shade data into the coordinator for diagnostics
@@ -113,18 +110,6 @@ async def async_setup_entry(hass: HomeAssistant, entry: PowerviewConfigEntry) ->
     return True
 
 
-async def async_get_device_info(hub: Hub) -> PowerviewDeviceInfo:
-    """Determine device info."""
-    return PowerviewDeviceInfo(
-        name=hub.name,
-        mac_address=hub.mac_address,
-        serial_number=hub.serial_number,
-        firmware=hub.firmware,
-        model=hub.model,
-        hub_address=hub.ip,
-    )
-
-
 async def async_unload_entry(hass: HomeAssistant, entry: PowerviewConfigEntry) -> bool:
     """Unload a config entry."""
     return await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
@@ -138,6 +123,8 @@ async def async_migrate_entry(hass: HomeAssistant, entry: PowerviewConfigEntry)
     if entry.version == 1:
         # 1 -> 2: Unique ID from integer to string
         if entry.minor_version == 1:
+            if entry.unique_id is None:
+                await _async_add_missing_entry_unique_id(hass, entry)
             await _migrate_unique_ids(hass, entry)
             hass.config_entries.async_update_entry(entry, minor_version=2)
 
@@ -146,6 +133,18 @@ async def async_migrate_entry(hass: HomeAssistant, entry: PowerviewConfigEntry)
     return True
 
 
+async def _async_add_missing_entry_unique_id(
+    hass: HomeAssistant, entry: PowerviewConfigEntry
+) -> None:
+    """Add the unique id if its missing."""
+    address: str = entry.data[CONF_HOST]
+    api_version: int | None = entry.data.get(CONF_API_VERSION)
+    api = await async_connect_hub(hass, address, api_version)
+    hass.config_entries.async_update_entry(
+        entry, unique_id=api.device_info.serial_number
+    )
+
+
 async def _migrate_unique_ids(hass: HomeAssistant, entry: PowerviewConfigEntry) -> None:
     """Migrate int based unique ids to str."""
     entity_registry = er.async_get(hass)

homeassistant/components/hunterdouglas_powerview/config_flow.py

@@ -5,8 +5,6 @@ from __future__ import annotations
 import logging
 from typing import TYPE_CHECKING, Any, Self
 
-from aiopvapi.helpers.aiorequest import AioRequest
-from aiopvapi.hub import Hub
 import voluptuous as vol
 
 from homeassistant.components import dhcp, zeroconf
@@ -14,10 +12,9 @@ from homeassistant.config_entries import ConfigFlow, ConfigFlowResult
 from homeassistant.const import CONF_API_VERSION, CONF_HOST, CONF_NAME
 from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import HomeAssistantError
-from homeassistant.helpers.aiohttp_client import async_get_clientsession
 
-from . import async_get_device_info
 from .const import DOMAIN, HUB_EXCEPTIONS
+from .util import async_connect_hub
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -31,18 +28,9 @@ async def validate_input(hass: HomeAssistant, hub_address: str) -> dict[str, str
 
     Data has the keys from DATA_SCHEMA with values provided by the user.
     """
-
+    api = await async_connect_hub(hass, hub_address)
-    websession = async_get_clientsession(hass)
+    hub = api.hub
-
+    device_info = api.device_info
-    pv_request = AioRequest(hub_address, loop=hass.loop, websession=websession)
-
-    try:
-        hub = Hub(pv_request)
-        await hub.query_firmware()
-        device_info = await async_get_device_info(hub)
-    except HUB_EXCEPTIONS as err:
-        raise CannotConnect from err
-
     if hub.role != "Primary":
         raise UnsupportedDevice(
             f"{hub.name} ({hub.hub_address}) is the {hub.role} Hub. "
@@ -111,7 +99,7 @@ class PowerviewConfigFlow(ConfigFlow, domain=DOMAIN):
 
         try:
             info = await validate_input(self.hass, host)
-        except CannotConnect:
+        except HUB_EXCEPTIONS:
             return None, "cannot_connect"
         except UnsupportedDevice:
             return None, "unsupported_device"
@@ -200,9 +188,5 @@ class PowerviewConfigFlow(ConfigFlow, domain=DOMAIN):
         )
 
 
-class CannotConnect(HomeAssistantError):
-    """Error to indicate we cannot connect."""
-
-
 class UnsupportedDevice(HomeAssistantError):
     """Error to indicate the device is not supported."""

homeassistant/components/hunterdouglas_powerview/model.py

@@ -3,20 +3,23 @@
 from __future__ import annotations
 
 from dataclasses import dataclass
+from typing import TYPE_CHECKING
 
 from aiopvapi.helpers.aiorequest import AioRequest
+from aiopvapi.hub import Hub
 from aiopvapi.resources.room import Room
 from aiopvapi.resources.scene import Scene
 from aiopvapi.resources.shade import BaseShade
 
 from homeassistant.config_entries import ConfigEntry
 
-from .coordinator import PowerviewShadeUpdateCoordinator
+if TYPE_CHECKING:
+    from .coordinator import PowerviewShadeUpdateCoordinator
 
 type PowerviewConfigEntry = ConfigEntry[PowerviewEntryData]
 
 
-@dataclass
+@dataclass(slots=True)
 class PowerviewEntryData:
     """Define class for main domain information."""
 
@@ -28,7 +31,7 @@ class PowerviewEntryData:
     device_info: PowerviewDeviceInfo
 
 
-@dataclass
+@dataclass(slots=True)
 class PowerviewDeviceInfo:
     """Define class for device information."""
 
@@ -38,3 +41,12 @@ class PowerviewDeviceInfo:
     firmware: str | None
     model: str
     hub_address: str
+
+
+@dataclass(slots=True)
+class PowerviewAPI:
+    """Define class to hold the Powerview Hub API data."""
+
+    hub: Hub
+    pv_request: AioRequest
+    device_info: PowerviewDeviceInfo

homeassistant/components/hunterdouglas_powerview/util.py

@@ -5,12 +5,38 @@ from __future__ import annotations
 from collections.abc import Iterable
 from typing import Any
 
+from aiopvapi.helpers.aiorequest import AioRequest
 from aiopvapi.helpers.constants import ATTR_ID
+from aiopvapi.hub import Hub
 
-from homeassistant.core import callback
+from homeassistant.core import HomeAssistant, callback
+from homeassistant.helpers.aiohttp_client import async_get_clientsession
+
+from .model import PowerviewAPI, PowerviewDeviceInfo
 
 
 @callback
 def async_map_data_by_id(data: Iterable[dict[str | int, Any]]):
     """Return a dict with the key being the id for a list of entries."""
     return {entry[ATTR_ID]: entry for entry in data}
+
+
+async def async_connect_hub(
+    hass: HomeAssistant, address: str, api_version: int | None = None
+) -> PowerviewAPI:
+    """Create the hub and fetch the device info address."""
+    websession = async_get_clientsession(hass)
+    pv_request = AioRequest(
+        address, loop=hass.loop, websession=websession, api_version=api_version
+    )
+    hub = Hub(pv_request)
+    await hub.query_firmware()
+    info = PowerviewDeviceInfo(
+        name=hub.name,
+        mac_address=hub.mac_address,
+        serial_number=hub.serial_number,
+        firmware=hub.firmware,
+        model=hub.model,
+        hub_address=hub.ip,
+    )
+    return PowerviewAPI(hub, pv_request, info)

tests/components/hunterdouglas_powerview/conftest.py

@@ -33,15 +33,15 @@ def mock_hunterdouglas_hub(
     """Return a mocked Powerview Hub with all data populated."""
     with (
         patch(
-            "homeassistant.components.hunterdouglas_powerview.Hub.request_raw_data",
+            "homeassistant.components.hunterdouglas_powerview.util.Hub.request_raw_data",
             return_value=load_json_object_fixture(device_json, DOMAIN),
         ),
         patch(
-            "homeassistant.components.hunterdouglas_powerview.Hub.request_home_data",
+            "homeassistant.components.hunterdouglas_powerview.util.Hub.request_home_data",
             return_value=load_json_object_fixture(home_json, DOMAIN),
         ),
         patch(
-            "homeassistant.components.hunterdouglas_powerview.Hub.request_raw_firmware",
+            "homeassistant.components.hunterdouglas_powerview.util.Hub.request_raw_firmware",
             return_value=load_json_object_fixture(firmware_json, DOMAIN),
         ),
         patch(

tests/components/hunterdouglas_powerview/test_config_flow.py

@@ -76,7 +76,7 @@ async def test_form_homekit_and_dhcp_cannot_connect(
     ignored_config_entry.add_to_hass(hass)
 
     with patch(
-        "homeassistant.components.hunterdouglas_powerview.Hub.query_firmware",
+        "homeassistant.components.hunterdouglas_powerview.util.Hub.query_firmware",
         side_effect=TimeoutError,
     ):
         result = await hass.config_entries.flow.async_init(
@@ -206,7 +206,7 @@ async def test_form_cannot_connect(
 
     # Simulate a timeout error
     with patch(
-        "homeassistant.components.hunterdouglas_powerview.Hub.query_firmware",
+        "homeassistant.components.hunterdouglas_powerview.util.Hub.query_firmware",
         side_effect=TimeoutError,
     ):
         result2 = await hass.config_entries.flow.async_configure(
@@ -245,11 +245,11 @@ async def test_form_no_data(
 
     with (
         patch(
-            "homeassistant.components.hunterdouglas_powerview.Hub.request_raw_data",
+            "homeassistant.components.hunterdouglas_powerview.util.Hub.request_raw_data",
             return_value={},
         ),
         patch(
-            "homeassistant.components.hunterdouglas_powerview.Hub.request_home_data",
+            "homeassistant.components.hunterdouglas_powerview.util.Hub.request_home_data",
             return_value={},
         ),
     ):
@@ -289,7 +289,7 @@ async def test_form_unknown_exception(
 
     # Simulate a transient error
     with patch(
-        "homeassistant.components.hunterdouglas_powerview.config_flow.Hub.query_firmware",
+        "homeassistant.components.hunterdouglas_powerview.util.Hub.query_firmware",
         side_effect=SyntaxError,
     ):
         result2 = await hass.config_entries.flow.async_configure(
@@ -328,7 +328,7 @@ async def test_form_unsupported_device(
 
     # Simulate a gen 3 secondary hub
     with patch(
-        "homeassistant.components.hunterdouglas_powerview.Hub.request_raw_data",
+        "homeassistant.components.hunterdouglas_powerview.util.Hub.request_raw_data",
         return_value=load_json_object_fixture("gen3/gateway/secondary.json", DOMAIN),
     ):
         result2 = await hass.config_entries.flow.async_configure(