Fix serial handling in ViCare integration (#125495)

* hand down device serial into common entity

* fix platforms

* Revert "fix platforms"

This reverts commit 067af2b567538989f97c5a764be64f8744663daf.

* handle event loop issue

* hand in serial

* Revert "Revert "fix platforms""

This reverts commit 9bbb55ee6da96ea31b98896e82c4b45ab001707b.

* fix get serial call

* handle other exceptions

* also check device model for migration

* merge entity and device migration

* add test fixture without serial

* adjust test cases

* add dummy fixture

* remove commented code

* modify migration

* use continue

* break comment

homeassistant/components/vicare/__init__.py

@@ -18,7 +18,7 @@ from PyViCare.PyViCareUtils import (
 from homeassistant.components.climate import DOMAIN as DOMAIN_CLIMATE
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import CONF_CLIENT_ID, CONF_PASSWORD, CONF_USERNAME
-from homeassistant.core import HomeAssistant, callback
+from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import ConfigEntryAuthFailed
 from homeassistant.helpers import device_registry as dr, entity_registry as er
 from homeassistant.helpers.storage import STORAGE_DIR
@@ -31,7 +31,7 @@ from .const import (
     UNSUPPORTED_DEVICES,
 )
 from .types import ViCareDevice
-from .utils import get_device
+from .utils import get_device, get_device_serial
 
 _LOGGER = logging.getLogger(__name__)
 _TOKEN_FILENAME = "vicare_token.save"
@@ -51,9 +51,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
 
     for device in hass.data[DOMAIN][entry.entry_id][DEVICE_LIST]:
         # Migration can be removed in 2025.4.0
-        await async_migrate_devices(hass, entry, device)
+        await async_migrate_devices_and_entities(hass, entry, device)
-        # Migration can be removed in 2025.4.0
-        await async_migrate_entities(hass, entry, device)
 
     await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
 
@@ -117,70 +115,72 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     return unload_ok
 
 
-async def async_migrate_devices(
+async def async_migrate_devices_and_entities(
     hass: HomeAssistant, entry: ConfigEntry, device: ViCareDevice
 ) -> None:
     """Migrate old entry."""
-    registry = dr.async_get(hass)
+    device_registry = dr.async_get(hass)
+    entity_registry = er.async_get(hass)
 
     gateway_serial: str = device.config.getConfig().serial
-    device_serial: str = device.api.getSerial()
+    device_id = device.config.getId()
+    device_serial: str | None = await hass.async_add_executor_job(
+        get_device_serial, device.api
+    )
+    device_model = device.config.getModel()
 
     old_identifier = gateway_serial
-    new_identifier = f"{gateway_serial}_{device_serial}"
+    new_identifier = (
+        f"{gateway_serial}_{device_serial if device_serial is not None else device_id}"
+    )
 
     # Migrate devices
-    for device_entry in dr.async_entries_for_config_entry(registry, entry.entry_id):
+    for device_entry in dr.async_entries_for_config_entry(
-        if device_entry.identifiers == {(DOMAIN, old_identifier)}:
+        device_registry, entry.entry_id
-            _LOGGER.debug("Migrating device %s", device_entry.name)
+    ):
-            registry.async_update_device(
+        if (
+            device_entry.identifiers == {(DOMAIN, old_identifier)}
+            and device_entry.model == device_model
+        ):
+            _LOGGER.debug(
+                "Migrating device %s to new identifier %s",
+                device_entry.name,
+                new_identifier,
+            )
+            device_registry.async_update_device(
                 device_entry.id,
                 serial_number=device_serial,
                 new_identifiers={(DOMAIN, new_identifier)},
             )
 
+            # Migrate entities
+            for entity_entry in er.async_entries_for_device(
+                entity_registry, device_entry.id, True
+            ):
+                if entity_entry.unique_id.startswith(new_identifier):
+                    # already correct, nothing to do
+                    continue
+                unique_id_parts = entity_entry.unique_id.split("-")
+                # replace old prefix `<gateway-serial>`
+                # with `<gateways-serial>_<device-serial>`
+                unique_id_parts[0] = new_identifier
+                # convert climate entity unique id
+                # from `<device_identifier>-<circuit_no>`
+                # to `<device_identifier>-heating-<circuit_no>`
+                if entity_entry.domain == DOMAIN_CLIMATE:
+                    unique_id_parts[len(unique_id_parts) - 1] = (
+                        f"{entity_entry.translation_key}-{unique_id_parts[len(unique_id_parts)-1]}"
+                    )
+                entity_new_unique_id = "-".join(unique_id_parts)
 
-async def async_migrate_entities(
+                _LOGGER.debug(
-    hass: HomeAssistant, entry: ConfigEntry, device: ViCareDevice
+                    "Migrating entity %s to new unique id %s",
-) -> None:
+                    entity_entry.name,
-    """Migrate old entry."""
+                    entity_new_unique_id,
-    gateway_serial: str = device.config.getConfig().serial
+                )
-    device_serial: str = device.api.getSerial()
+                entity_registry.async_update_entity(
-    new_identifier = f"{gateway_serial}_{device_serial}"
+                    entity_id=entity_entry.entity_id, new_unique_id=entity_new_unique_id
-
+                )
-    @callback
-    def _update_unique_id(
-        entity_entry: er.RegistryEntry,
-    ) -> dict[str, str] | None:
-        """Update unique ID of entity entry."""
-        if not entity_entry.unique_id.startswith(gateway_serial):
-            # belongs to other device/gateway
-            return None
-        if entity_entry.unique_id.startswith(f"{gateway_serial}_"):
-            # Already correct, nothing to do
-            return None
-
-        unique_id_parts = entity_entry.unique_id.split("-")
-        unique_id_parts[0] = new_identifier
-
-        # convert climate entity unique id from `<device_identifier>-<circuit_no>` to `<device_identifier>-heating-<circuit_no>`
-        if entity_entry.domain == DOMAIN_CLIMATE:
-            unique_id_parts[len(unique_id_parts) - 1] = (
-                f"{entity_entry.translation_key}-{unique_id_parts[len(unique_id_parts)-1]}"
-            )
-
-        entity_new_unique_id = "-".join(unique_id_parts)
-
-        _LOGGER.debug(
-            "Migrating entity %s from %s to new id %s",
-            entity_entry.entity_id,
-            entity_entry.unique_id,
-            entity_new_unique_id,
-        )
-        return {"new_unique_id": entity_new_unique_id}
-
-    # Migrate entities
-    await er.async_migrate_entries(hass, entry.entry_id, _update_unique_id)
 
 
 def get_supported_devices(

homeassistant/components/vicare/binary_sensor.py

@@ -31,7 +31,13 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from .const import DEVICE_LIST, DOMAIN
 from .entity import ViCareEntity
 from .types import ViCareDevice, ViCareRequiredKeysMixin
-from .utils import get_burners, get_circuits, get_compressors, is_supported
+from .utils import (
+    get_burners,
+    get_circuits,
+    get_compressors,
+    get_device_serial,
+    is_supported,
+)
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -116,6 +122,7 @@ def _build_entities(
         entities.extend(
             ViCareBinarySensor(
                 description,
+                get_device_serial(device.api),
                 device.config,
                 device.api,
             )
@@ -131,6 +138,7 @@ def _build_entities(
             entities.extend(
                 ViCareBinarySensor(
                     description,
+                    get_device_serial(device.api),
                     device.config,
                     device.api,
                     component,
@@ -166,12 +174,15 @@ class ViCareBinarySensor(ViCareEntity, BinarySensorEntity):
     def __init__(
         self,
         description: ViCareBinarySensorEntityDescription,
+        device_serial: str | None,
         device_config: PyViCareDeviceConfig,
         device: PyViCareDevice,
         component: PyViCareHeatingDeviceComponent | None = None,
     ) -> None:
         """Initialize the sensor."""
-        super().__init__(description.key, device_config, device, component)
+        super().__init__(
+            description.key, device_serial, device_config, device, component
+        )
         self.entity_description = description
 
     @property

homeassistant/components/vicare/button.py

@@ -24,7 +24,7 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from .const import DEVICE_LIST, DOMAIN
 from .entity import ViCareEntity
 from .types import ViCareDevice, ViCareRequiredKeysMixinWithSet
-from .utils import is_supported
+from .utils import get_device_serial, is_supported
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -55,6 +55,7 @@ def _build_entities(
     return [
         ViCareButton(
             description,
+            get_device_serial(device.api),
             device.config,
             device.api,
         )
@@ -88,11 +89,12 @@ class ViCareButton(ViCareEntity, ButtonEntity):
     def __init__(
         self,
         description: ViCareButtonEntityDescription,
+        device_serial: str | None,
         device_config: PyViCareDeviceConfig,
         device: PyViCareDevice,
     ) -> None:
         """Initialize the button."""
-        super().__init__(description.key, device_config, device)
+        super().__init__(description.key, device_serial, device_config, device)
         self.entity_description = description
 
     def press(self) -> None:

homeassistant/components/vicare/climate.py

@@ -40,7 +40,7 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from .const import DEVICE_LIST, DOMAIN
 from .entity import ViCareEntity
 from .types import HeatingProgram, ViCareDevice
-from .utils import get_burners, get_circuits, get_compressors
+from .utils import get_burners, get_circuits, get_compressors, get_device_serial
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -87,6 +87,7 @@ def _build_entities(
     """Create ViCare climate entities for a device."""
     return [
         ViCareClimate(
+            get_device_serial(device.api),
             device.config,
             device.api,
             circuit,
@@ -143,12 +144,15 @@ class ViCareClimate(ViCareEntity, ClimateEntity):
 
     def __init__(
         self,
+        device_serial: str | None,
         device_config: PyViCareDeviceConfig,
         device: PyViCareDevice,
         circuit: PyViCareHeatingCircuit,
     ) -> None:
         """Initialize the climate device."""
-        super().__init__(self._attr_translation_key, device_config, device, circuit)
+        super().__init__(
+            self._attr_translation_key, device_serial, device_config, device, circuit
+        )
         self._device = device
         self._attributes: dict[str, Any] = {}
         self._attributes["vicare_programs"] = self._api.getPrograms()

homeassistant/components/vicare/entity.py

@@ -20,14 +20,16 @@ class ViCareEntity(Entity):
     def __init__(
         self,
         unique_id_suffix: str,
+        device_serial: str | None,
         device_config: PyViCareDeviceConfig,
         device: PyViCareDevice,
         component: PyViCareHeatingDeviceComponent | None = None,
     ) -> None:
         """Initialize the entity."""
         gateway_serial = device_config.getConfig().serial
-        device_serial = device.getSerial()
+        device_id = device_config.getId()
-        identifier = f"{gateway_serial}_{device_serial}"
+
+        identifier = f"{gateway_serial}_{device_serial if device_serial is not None else device_id}"
 
         self._api: PyViCareDevice | PyViCareHeatingDeviceComponent = (
             component if component else device

homeassistant/components/vicare/fan.py

@@ -29,6 +29,7 @@ from homeassistant.util.percentage import (
 
 from .const import DEVICE_LIST, DOMAIN
 from .entity import ViCareEntity
+from .utils import get_device_serial
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -100,7 +101,7 @@ async def async_setup_entry(
 
     async_add_entities(
         [
-            ViCareFan(device.config, device.api)
+            ViCareFan(get_device_serial(device.api), device.config, device.api)
             for device in device_list
             if isinstance(device.api, PyViCareVentilationDevice)
         ]
@@ -125,11 +126,14 @@ class ViCareFan(ViCareEntity, FanEntity):
 
     def __init__(
         self,
+        device_serial: str | None,
         device_config: PyViCareDeviceConfig,
         device: PyViCareDevice,
     ) -> None:
         """Initialize the fan entity."""
-        super().__init__(self._attr_translation_key, device_config, device)
+        super().__init__(
+            self._attr_translation_key, device_serial, device_config, device
+        )
 
     def update(self) -> None:
         """Update state of fan."""

homeassistant/components/vicare/number.py

@@ -33,7 +33,7 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from .const import DEVICE_LIST, DOMAIN
 from .entity import ViCareEntity
 from .types import HeatingProgram, ViCareDevice, ViCareRequiredKeysMixin
-from .utils import get_circuits, is_supported
+from .utils import get_circuits, get_device_serial, is_supported
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -279,6 +279,7 @@ def _build_entities(
         entities.extend(
             ViCareNumber(
                 description,
+                get_device_serial(device.api),
                 device.config,
                 device.api,
             )
@@ -289,6 +290,7 @@ def _build_entities(
         entities.extend(
             ViCareNumber(
                 description,
+                get_device_serial(device.api),
                 device.config,
                 device.api,
                 circuit,
@@ -324,12 +326,15 @@ class ViCareNumber(ViCareEntity, NumberEntity):
     def __init__(
         self,
         description: ViCareNumberEntityDescription,
+        device_serial: str | None,
         device_config: PyViCareDeviceConfig,
         device: PyViCareDevice,
         component: PyViCareHeatingDeviceComponent | None = None,
     ) -> None:
         """Initialize the number."""
-        super().__init__(description.key, device_config, device, component)
+        super().__init__(
+            description.key, device_serial, device_config, device, component
+        )
         self.entity_description = description
 
     @property

homeassistant/components/vicare/sensor.py

@@ -51,7 +51,13 @@ from .const import (
 )
 from .entity import ViCareEntity
 from .types import ViCareDevice, ViCareRequiredKeysMixin
-from .utils import get_burners, get_circuits, get_compressors, is_supported
+from .utils import (
+    get_burners,
+    get_circuits,
+    get_compressors,
+    get_device_serial,
+    is_supported,
+)
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -868,6 +874,7 @@ def _build_entities(
         entities.extend(
             ViCareSensor(
                 description,
+                get_device_serial(device.api),
                 device.config,
                 device.api,
             )
@@ -883,6 +890,7 @@ def _build_entities(
             entities.extend(
                 ViCareSensor(
                     description,
+                    get_device_serial(device.api),
                     device.config,
                     device.api,
                     component,
@@ -920,12 +928,15 @@ class ViCareSensor(ViCareEntity, SensorEntity):
     def __init__(
         self,
         description: ViCareSensorEntityDescription,
+        device_serial: str | None,
         device_config: PyViCareDeviceConfig,
         device: PyViCareDevice,
         component: PyViCareHeatingDeviceComponent | None = None,
     ) -> None:
         """Initialize the sensor."""
-        super().__init__(description.key, device_config, device, component)
+        super().__init__(
+            description.key, device_serial, device_config, device, component
+        )
         self.entity_description = description
 
     @property

homeassistant/components/vicare/utils.py

@@ -7,7 +7,12 @@ from PyViCare.PyViCareDeviceConfig import PyViCareDeviceConfig
 from PyViCare.PyViCareHeatingDevice import (
     HeatingDeviceWithComponent as PyViCareHeatingDeviceComponent,
 )
-from PyViCare.PyViCareUtils import PyViCareNotSupportedFeatureError
+from PyViCare.PyViCareUtils import (
+    PyViCareInvalidDataError,
+    PyViCareNotSupportedFeatureError,
+    PyViCareRateLimitError,
+)
+import requests
 
 from homeassistant.config_entries import ConfigEntry
 
@@ -27,6 +32,23 @@ def get_device(
     )()
 
 
+def get_device_serial(device: PyViCareDevice) -> str | None:
+    """Get device serial for device if supported."""
+    try:
+        return device.getSerial()
+    except PyViCareNotSupportedFeatureError:
+        _LOGGER.debug("Device does not offer a 'device.serial' data point")
+    except PyViCareRateLimitError as limit_exception:
+        _LOGGER.debug("Vicare API rate limit exceeded: %s", limit_exception)
+    except PyViCareInvalidDataError as invalid_data_exception:
+        _LOGGER.debug("Invalid data from Vicare server: %s", invalid_data_exception)
+    except requests.exceptions.ConnectionError:
+        _LOGGER.debug("Unable to retrieve data from ViCare server")
+    except ValueError:
+        _LOGGER.debug("Unable to decode data from ViCare server")
+    return None
+
+
 def is_supported(
     name: str,
     entity_description: ViCareRequiredKeysMixin,

homeassistant/components/vicare/water_heater.py

@@ -28,7 +28,7 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from .const import DEVICE_LIST, DOMAIN
 from .entity import ViCareEntity
 from .types import ViCareDevice
-from .utils import get_circuits
+from .utils import get_circuits, get_device_serial
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -69,6 +69,7 @@ def _build_entities(
 
     return [
         ViCareWater(
+            get_device_serial(device.api),
             device.config,
             device.api,
             circuit,
@@ -108,12 +109,13 @@ class ViCareWater(ViCareEntity, WaterHeaterEntity):
 
     def __init__(
         self,
+        device_serial: str | None,
         device_config: PyViCareDeviceConfig,
         device: PyViCareDevice,
         circuit: PyViCareHeatingCircuit,
     ) -> None:
         """Initialize the DHW water_heater device."""
-        super().__init__(circuit.id, device_config, device)
+        super().__init__(circuit.id, device_serial, device_config, device)
         self._circuit = circuit
         self._attributes: dict[str, Any] = {}
 

tests/components/vicare/fixtures/dummy-device-no-serial.json

@@ -0,0 +1,3 @@
+{
+  "data": []
+}

tests/components/vicare/test_init.py

@@ -14,74 +14,78 @@ from tests.common import MockConfigEntry
 
 
 # Device migration test can be removed in 2025.4.0
-async def test_device_migration(
+async def test_device_and_entity_migration(
     hass: HomeAssistant,
     device_registry: dr.DeviceRegistry,
+    entity_registry: er.EntityRegistry,
     mock_config_entry: MockConfigEntry,
 ) -> None:
     """Test that the device registry is updated correctly."""
-    fixtures: list[Fixture] = [Fixture({"type:boiler"}, "vicare/Vitodens300W.json")]
+    fixtures: list[Fixture] = [
+        Fixture({"type:boiler"}, "vicare/Vitodens300W.json"),
+        Fixture({"type:boiler"}, "vicare/dummy-device-no-serial.json"),
+    ]
     with (
         patch(f"{MODULE}.vicare_login", return_value=MockPyViCare(fixtures)),
         patch(f"{MODULE}.PLATFORMS", [Platform.CLIMATE]),
     ):
         mock_config_entry.add_to_hass(hass)
 
-        device_registry.async_get_or_create(
+        # device with serial data point
+        device0 = device_registry.async_get_or_create(
             config_entry_id=mock_config_entry.entry_id,
             identifiers={
                 (DOMAIN, "gateway0"),
             },
+            model="model0",
         )
-
+        entry0 = entity_registry.async_get_or_create(
-        await hass.config_entries.async_setup(mock_config_entry.entry_id)
-
-        await hass.async_block_till_done()
-
-    assert device_registry.async_get_device(identifiers={(DOMAIN, "gateway0")}) is None
-
-    assert (
-        device_registry.async_get_device(
-            identifiers={(DOMAIN, "gateway0_deviceSerialVitodens300W")}
-        )
-        is not None
-    )
-
-
-# Entity migration test can be removed in 2025.4.0
-async def test_climate_entity_migration(
-    hass: HomeAssistant,
-    entity_registry: er.EntityRegistry,
-    mock_config_entry: MockConfigEntry,
-) -> None:
-    """Test that the climate entity unique_id gets migrated correctly."""
-    fixtures: list[Fixture] = [Fixture({"type:boiler"}, "vicare/Vitodens300W.json")]
-    with (
-        patch(f"{MODULE}.vicare_login", return_value=MockPyViCare(fixtures)),
-        patch(f"{MODULE}.PLATFORMS", [Platform.CLIMATE]),
-    ):
-        mock_config_entry.add_to_hass(hass)
-
-        entry1 = entity_registry.async_get_or_create(
             domain=Platform.CLIMATE,
             platform=DOMAIN,
             config_entry=mock_config_entry,
             unique_id="gateway0-0",
             translation_key="heating",
+            device_id=device0.id,
         )
-        entry2 = entity_registry.async_get_or_create(
+        entry1 = entity_registry.async_get_or_create(
             domain=Platform.CLIMATE,
             platform=DOMAIN,
             config_entry=mock_config_entry,
             unique_id="gateway0_deviceSerialVitodens300W-heating-1",
             translation_key="heating",
+            device_id=device0.id,
         )
-        entry3 = entity_registry.async_get_or_create(
+        # device without serial data point
+        device1 = device_registry.async_get_or_create(
+            config_entry_id=mock_config_entry.entry_id,
+            identifiers={
+                (DOMAIN, "gateway1"),
+            },
+            model="model1",
+        )
+        entry2 = entity_registry.async_get_or_create(
             domain=Platform.CLIMATE,
             platform=DOMAIN,
             config_entry=mock_config_entry,
             unique_id="gateway1-0",
             translation_key="heating",
+            device_id=device1.id,
+        )
+        # device is not provided by api
+        device2 = device_registry.async_get_or_create(
+            config_entry_id=mock_config_entry.entry_id,
+            identifiers={
+                (DOMAIN, "gateway2"),
+            },
+            model="model2",
+        )
+        entry3 = entity_registry.async_get_or_create(
+            domain=Platform.CLIMATE,
+            platform=DOMAIN,
+            config_entry=mock_config_entry,
+            unique_id="gateway2-0",
+            translation_key="heating",
+            device_id=device2.id,
         )
 
         await hass.config_entries.async_setup(mock_config_entry.entry_id)
@@ -89,11 +93,15 @@ async def test_climate_entity_migration(
         await hass.async_block_till_done()
 
     assert (
-        entity_registry.async_get(entry1.entity_id).unique_id
+        entity_registry.async_get(entry0.entity_id).unique_id
         == "gateway0_deviceSerialVitodens300W-heating-0"
     )
     assert (
-        entity_registry.async_get(entry2.entity_id).unique_id
+        entity_registry.async_get(entry1.entity_id).unique_id
         == "gateway0_deviceSerialVitodens300W-heating-1"
     )
-    assert entity_registry.async_get(entry3.entity_id).unique_id == "gateway1-0"
+    assert (
+        entity_registry.async_get(entry2.entity_id).unique_id
+        == "gateway1_deviceId1-heating-0"
+    )
+    assert entity_registry.async_get(entry3.entity_id).unique_id == "gateway2-0"