Use activity ids for unique_id for Harmony switches (#46139)

homeassistant/components/harmony/__init__.py

@@ -1,16 +1,20 @@
 """The Logitech Harmony Hub integration."""
 import asyncio
+import logging
 
 from homeassistant.components.remote import ATTR_ACTIVITY, ATTR_DELAY_SECS
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import CONF_HOST, CONF_NAME
 from homeassistant.core import HomeAssistant, callback
 from homeassistant.exceptions import ConfigEntryNotReady
+from homeassistant.helpers import entity_registry
 from homeassistant.helpers.dispatcher import async_dispatcher_send
 
 from .const import DOMAIN, HARMONY_OPTIONS_UPDATE, PLATFORMS
 from .data import HarmonyData
 
+_LOGGER = logging.getLogger(__name__)
+
 
 async def async_setup(hass: HomeAssistant, config: dict):
     """Set up the Logitech Harmony Hub component."""
@@ -40,6 +44,8 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry):
 
     hass.data[DOMAIN][entry.entry_id] = data
 
+    await _migrate_old_unique_ids(hass, entry.entry_id, data)
+
     entry.add_update_listener(_update_listener)
 
     for component in PLATFORMS:
@@ -50,6 +56,33 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry):
     return True
 
 
+async def _migrate_old_unique_ids(
+    hass: HomeAssistant, entry_id: str, data: HarmonyData
+):
+    names_to_ids = {activity["label"]: activity["id"] for activity in data.activities}
+
+    @callback
+    def _async_migrator(entity_entry: entity_registry.RegistryEntry):
+        # Old format for switches was {remote_unique_id}-{activity_name}
+        # New format is activity_{activity_id}
+        parts = entity_entry.unique_id.split("-", 1)
+        if len(parts) > 1:  # old format
+            activity_name = parts[1]
+            activity_id = names_to_ids.get(activity_name)
+
+            if activity_id is not None:
+                _LOGGER.info(
+                    "Migrating unique_id from [%s] to [%s]",
+                    entity_entry.unique_id,
+                    activity_id,
+                )
+                return {"new_unique_id": f"activity_{activity_id}"}
+
+        return None
+
+    await entity_registry.async_migrate_entries(hass, entry_id, _async_migrator)
+
+
 @callback
 def _async_import_options_from_data_if_missing(hass: HomeAssistant, entry: ConfigEntry):
     options = dict(entry.options)

homeassistant/components/harmony/data.py

@@ -34,18 +34,22 @@ class HarmonyData(HarmonySubscriberMixin):
             ip_address=address, callbacks=ClientCallbackType(**callbacks)
         )
 
+    @property
+    def activities(self):
+        """List of all non-poweroff activity objects."""
+        activity_infos = self._client.config.get("activity", [])
+        return [
+            info
+            for info in activity_infos
+            if info["label"] is not None and info["label"] != ACTIVITY_POWER_OFF
+        ]
+
     @property
     def activity_names(self):
         """Names of all the remotes activities."""
-        activity_infos = self._client.config.get("activity", [])
+        activity_infos = self.activities
         activities = [activity["label"] for activity in activity_infos]
 
-        # Remove both ways of representing PowerOff
-        if None in activities:
-            activities.remove(None)
-        if ACTIVITY_POWER_OFF in activities:
-            activities.remove(ACTIVITY_POWER_OFF)
-
         return activities
 
     @property

homeassistant/components/harmony/switch.py

@@ -15,12 +15,12 @@ _LOGGER = logging.getLogger(__name__)
 async def async_setup_entry(hass, entry, async_add_entities):
     """Set up harmony activity switches."""
     data = hass.data[DOMAIN][entry.entry_id]
-    activities = data.activity_names
+    activities = data.activities
 
     switches = []
     for activity in activities:
         _LOGGER.debug("creating switch for activity: %s", activity)
-        name = f"{entry.data[CONF_NAME]} {activity}"
+        name = f"{entry.data[CONF_NAME]} {activity['label']}"
         switches.append(HarmonyActivitySwitch(name, activity, data))
 
     async_add_entities(switches, True)
@@ -29,11 +29,12 @@ async def async_setup_entry(hass, entry, async_add_entities):
 class HarmonyActivitySwitch(ConnectionStateMixin, SwitchEntity):
     """Switch representation of a Harmony activity."""
 
-    def __init__(self, name: str, activity: str, data: HarmonyData):
+    def __init__(self, name: str, activity: dict, data: HarmonyData):
         """Initialize HarmonyActivitySwitch class."""
         super().__init__()
         self._name = name
-        self._activity = activity
+        self._activity_name = activity["label"]
+        self._activity_id = activity["id"]
         self._data = data
 
     @property
@@ -44,7 +45,7 @@ class HarmonyActivitySwitch(ConnectionStateMixin, SwitchEntity):
     @property
     def unique_id(self):
         """Return the unique id."""
-        return f"{self._data.unique_id}-{self._activity}"
+        return f"activity_{self._activity_id}"
 
     @property
     def device_info(self):
@@ -55,7 +56,7 @@ class HarmonyActivitySwitch(ConnectionStateMixin, SwitchEntity):
     def is_on(self):
         """Return if the current activity is the one for this switch."""
         _, activity_name = self._data.current_activity
-        return activity_name == self._activity
+        return activity_name == self._activity_name
 
     @property
     def should_poll(self):
@@ -69,7 +70,7 @@ class HarmonyActivitySwitch(ConnectionStateMixin, SwitchEntity):
 
     async def async_turn_on(self, **kwargs):
         """Start this activity."""
-        await self._data.async_start_activity(self._activity)
+        await self._data.async_start_activity(self._activity_name)
 
     async def async_turn_off(self, **kwargs):
         """Stop this activity."""

tests/components/harmony/conftest.py

@@ -7,21 +7,22 @@ import pytest
 
 from homeassistant.components.harmony.const import ACTIVITY_POWER_OFF
 
-_LOGGER = logging.getLogger(__name__)
+from .const import NILE_TV_ACTIVITY_ID, PLAY_MUSIC_ACTIVITY_ID, WATCH_TV_ACTIVITY_ID
 
-WATCH_TV_ACTIVITY_ID = 123
+_LOGGER = logging.getLogger(__name__)
-PLAY_MUSIC_ACTIVITY_ID = 456
 
 ACTIVITIES_TO_IDS = {
     ACTIVITY_POWER_OFF: -1,
     "Watch TV": WATCH_TV_ACTIVITY_ID,
     "Play Music": PLAY_MUSIC_ACTIVITY_ID,
+    "Nile-TV": NILE_TV_ACTIVITY_ID,
 }
 
 IDS_TO_ACTIVITIES = {
     -1: ACTIVITY_POWER_OFF,
     WATCH_TV_ACTIVITY_ID: "Watch TV",
     PLAY_MUSIC_ACTIVITY_ID: "Play Music",
+    NILE_TV_ACTIVITY_ID: "Nile-TV",
 }
 
 TV_DEVICE_ID = 1234
@@ -111,6 +112,7 @@ class FakeHarmonyClient:
             return_value=[
                 {"name": "Watch TV", "id": WATCH_TV_ACTIVITY_ID},
                 {"name": "Play Music", "id": PLAY_MUSIC_ACTIVITY_ID},
+                {"name": "Nile-TV", "id": NILE_TV_ACTIVITY_ID},
             ]
         )
         type(config).devices = PropertyMock(
@@ -121,8 +123,11 @@ class FakeHarmonyClient:
         type(config).config = PropertyMock(
             return_value={
                 "activity": [
+                    {"id": 10000, "label": None},
+                    {"id": -1, "label": "PowerOff"},
                     {"id": WATCH_TV_ACTIVITY_ID, "label": "Watch TV"},
                     {"id": PLAY_MUSIC_ACTIVITY_ID, "label": "Play Music"},
+                    {"id": NILE_TV_ACTIVITY_ID, "label": "Nile-TV"},
                 ]
             }
         )

tests/components/harmony/const.py

@@ -4,3 +4,8 @@ HUB_NAME = "Guest Room"
 ENTITY_REMOTE = "remote.guest_room"
 ENTITY_WATCH_TV = "switch.guest_room_watch_tv"
 ENTITY_PLAY_MUSIC = "switch.guest_room_play_music"
+ENTITY_NILE_TV = "switch.guest_room_nile_tv"
+
+WATCH_TV_ACTIVITY_ID = 123
+PLAY_MUSIC_ACTIVITY_ID = 456
+NILE_TV_ACTIVITY_ID = 789

tests/components/harmony/test_init.py

@@ -0,0 +1,72 @@
+"""Test init of Logitch Harmony Hub integration."""
+from homeassistant.components.harmony.const import DOMAIN
+from homeassistant.const import CONF_HOST, CONF_NAME
+from homeassistant.helpers import entity_registry
+from homeassistant.setup import async_setup_component
+
+from .const import (
+    ENTITY_NILE_TV,
+    ENTITY_PLAY_MUSIC,
+    ENTITY_WATCH_TV,
+    HUB_NAME,
+    NILE_TV_ACTIVITY_ID,
+    PLAY_MUSIC_ACTIVITY_ID,
+    WATCH_TV_ACTIVITY_ID,
+)
+
+from tests.common import MockConfigEntry, mock_registry
+
+
+async def test_unique_id_migration(mock_hc, hass, mock_write_config):
+    """Test migration of switch unique ids to stable ones."""
+    entry = MockConfigEntry(
+        domain=DOMAIN, data={CONF_HOST: "192.0.2.0", CONF_NAME: HUB_NAME}
+    )
+
+    entry.add_to_hass(hass)
+    mock_registry(
+        hass,
+        {
+            # old format
+            ENTITY_WATCH_TV: entity_registry.RegistryEntry(
+                entity_id=ENTITY_WATCH_TV,
+                unique_id="123443-Watch TV",
+                platform="harmony",
+                config_entry_id=entry.entry_id,
+            ),
+            # old format, activity name with -
+            ENTITY_NILE_TV: entity_registry.RegistryEntry(
+                entity_id=ENTITY_NILE_TV,
+                unique_id="123443-Nile-TV",
+                platform="harmony",
+                config_entry_id=entry.entry_id,
+            ),
+            # new format
+            ENTITY_PLAY_MUSIC: entity_registry.RegistryEntry(
+                entity_id=ENTITY_PLAY_MUSIC,
+                unique_id=f"activity_{PLAY_MUSIC_ACTIVITY_ID}",
+                platform="harmony",
+                config_entry_id=entry.entry_id,
+            ),
+            # old entity which no longer has a matching activity on the hub. skipped.
+            "switch.some_other_activity": entity_registry.RegistryEntry(
+                entity_id="switch.some_other_activity",
+                unique_id="123443-Some Other Activity",
+                platform="harmony",
+                config_entry_id=entry.entry_id,
+            ),
+        },
+    )
+    assert await async_setup_component(hass, DOMAIN, {})
+    await hass.async_block_till_done()
+
+    ent_reg = await entity_registry.async_get_registry(hass)
+
+    switch_tv = ent_reg.async_get(ENTITY_WATCH_TV)
+    assert switch_tv.unique_id == f"activity_{WATCH_TV_ACTIVITY_ID}"
+
+    switch_nile = ent_reg.async_get(ENTITY_NILE_TV)
+    assert switch_nile.unique_id == f"activity_{NILE_TV_ACTIVITY_ID}"
+
+    switch_music = ent_reg.async_get(ENTITY_PLAY_MUSIC)
+    assert switch_music.unique_id == f"activity_{PLAY_MUSIC_ACTIVITY_ID}"