diff --git a/homeassistant/components/harmony/__init__.py b/homeassistant/components/harmony/__init__.py index 6ba63ee0f81..8445c7be937 100644 --- a/homeassistant/components/harmony/__init__.py +++ b/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) diff --git a/homeassistant/components/harmony/data.py b/homeassistant/components/harmony/data.py index 6c3ad874fa9..8c1d137bc85 100644 --- a/homeassistant/components/harmony/data.py +++ b/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 diff --git a/homeassistant/components/harmony/switch.py b/homeassistant/components/harmony/switch.py index 2832872c2ef..5aac145e749 100644 --- a/homeassistant/components/harmony/switch.py +++ b/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.""" diff --git a/tests/components/harmony/conftest.py b/tests/components/harmony/conftest.py index cde8c43fe89..6ca483b2588 100644 --- a/tests/components/harmony/conftest.py +++ b/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 -PLAY_MUSIC_ACTIVITY_ID = 456 +_LOGGER = logging.getLogger(__name__) 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"}, ] } ) diff --git a/tests/components/harmony/const.py b/tests/components/harmony/const.py index 1911ea949af..488fe30dec3 100644 --- a/tests/components/harmony/const.py +++ b/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 diff --git a/tests/components/harmony/test_init.py b/tests/components/harmony/test_init.py new file mode 100644 index 00000000000..c63727f8738 --- /dev/null +++ b/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}"