From 439f22f5844e49ce1f89dbce44596074d7b2a712 Mon Sep 17 00:00:00 2001 From: Andrew Sayre <6730289+andrewsayre@users.noreply.github.com> Date: Sun, 19 Jan 2025 08:07:00 -0600 Subject: [PATCH] Fix HEOS device information (#135940) --- homeassistant/components/heos/__init__.py | 18 +++++++-- homeassistant/components/heos/media_player.py | 10 +++-- .../components/heos/quality_scale.yaml | 6 +-- tests/components/heos/conftest.py | 4 +- tests/components/heos/test_init.py | 40 +++++++++++++++++++ tests/components/heos/test_media_player.py | 4 +- 6 files changed, 67 insertions(+), 15 deletions(-) diff --git a/homeassistant/components/heos/__init__.py b/homeassistant/components/heos/__init__.py index 3b38e5c935a..1004ffd2738 100644 --- a/homeassistant/components/heos/__init__.py +++ b/homeassistant/components/heos/__init__.py @@ -81,6 +81,18 @@ async def async_setup_entry(hass: HomeAssistant, entry: HeosConfigEntry) -> bool if entry.unique_id is None: hass.config_entries.async_update_entry(entry, unique_id=DOMAIN) + # Migrate non-string device identifiers. + device_registry = dr.async_get(hass) + for device in device_registry.devices.get_devices_for_config_entry_id( + entry.entry_id + ): + for domain, player_id in device.identifiers: + if domain == DOMAIN and not isinstance(player_id, str): + device_registry.async_update_device( + device.id, new_identifiers={(DOMAIN, str(player_id))} + ) + break + host = entry.data[CONF_HOST] credentials: Credentials | None = None if entry.options: @@ -221,13 +233,13 @@ class ControllerManager: # update device registry assert self._device_registry is not None entry = self._device_registry.async_get_device( - identifiers={(DOMAIN, old_id)} # type: ignore[arg-type] # Fix in the future + identifiers={(DOMAIN, str(old_id))} ) - new_identifiers = {(DOMAIN, new_id)} + new_identifiers = {(DOMAIN, str(new_id))} if entry: self._device_registry.async_update_device( entry.id, - new_identifiers=new_identifiers, # type: ignore[arg-type] # Fix in the future + new_identifiers=new_identifiers, ) _LOGGER.debug( "Updated device %s identifiers to %s", entry.id, new_identifiers diff --git a/homeassistant/components/heos/media_player.py b/homeassistant/components/heos/media_player.py index 981a39f53dc..69aedaa4648 100644 --- a/homeassistant/components/heos/media_player.py +++ b/homeassistant/components/heos/media_player.py @@ -136,11 +136,15 @@ class HeosMediaPlayer(MediaPlayerEntity): self._source_manager = source_manager self._group_manager = group_manager self._attr_unique_id = str(player.player_id) + model_parts = player.model.split(maxsplit=1) + manufacturer = model_parts[0] if len(model_parts) == 2 else "HEOS" + model = model_parts[1] if len(model_parts) == 2 else player.model self._attr_device_info = DeviceInfo( - identifiers={(HEOS_DOMAIN, player.player_id)}, - manufacturer="HEOS", - model=player.model, + identifiers={(HEOS_DOMAIN, str(player.player_id))}, + manufacturer=manufacturer, + model=model, name=player.name, + serial_number=player.serial, # Only available for some models sw_version=player.version, ) diff --git a/homeassistant/components/heos/quality_scale.yaml b/homeassistant/components/heos/quality_scale.yaml index 4cd39434521..3135cca3f9d 100644 --- a/homeassistant/components/heos/quality_scale.yaml +++ b/homeassistant/components/heos/quality_scale.yaml @@ -50,11 +50,7 @@ rules: 4. Recommend using snapshot in test_state_attributes. 5. Find a way to avoid using internal dispatcher in test_updates_from_connection_event. # Gold - devices: - status: todo - comment: | - The integraiton creates devices, but needs to stringify the id for the device identifier and - also migrate the device. + devices: done diagnostics: todo discovery-update-info: status: todo diff --git a/tests/components/heos/conftest.py b/tests/components/heos/conftest.py index 38d2f237907..4a11a3511d5 100644 --- a/tests/components/heos/conftest.py +++ b/tests/components/heos/conftest.py @@ -125,8 +125,8 @@ def player_fixture(quick_selects): player = HeosPlayer( player_id=i, name="Test Player" if i == 1 else f"Test Player {i}", - model="Test Model", - serial="", + model="HEOS Drive HS2" if i == 1 else "Speaker", + serial="123456", version="1.0.0", line_out=LineOutLevelType.VARIABLE, is_muted=False, diff --git a/tests/components/heos/test_init.py b/tests/components/heos/test_init.py index a8cd4bea1d2..f802529ac82 100644 --- a/tests/components/heos/test_init.py +++ b/tests/components/heos/test_init.py @@ -19,6 +19,7 @@ from homeassistant.config_entries import SOURCE_REAUTH, ConfigEntryState from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_USERNAME from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady +from homeassistant.helpers import device_registry as dr from homeassistant.setup import async_setup_component from tests.common import MockConfigEntry @@ -217,3 +218,42 @@ async def test_update_sources_retry( while "Unable to update sources" not in caplog.text: await asyncio.sleep(0.1) assert controller.get_favorites.call_count == 2 + + +async def test_device_info( + hass: HomeAssistant, + device_registry: dr.DeviceRegistry, + config_entry: MockConfigEntry, +) -> None: + """Test device information populates correctly.""" + config_entry.add_to_hass(hass) + assert await hass.config_entries.async_setup(config_entry.entry_id) + device = device_registry.async_get_device({(DOMAIN, "1")}) + assert device.manufacturer == "HEOS" + assert device.model == "Drive HS2" + assert device.name == "Test Player" + assert device.serial_number == "123456" + assert device.sw_version == "1.0.0" + device = device_registry.async_get_device({(DOMAIN, "2")}) + assert device.manufacturer == "HEOS" + assert device.model == "Speaker" + + +async def test_device_id_migration( + hass: HomeAssistant, + device_registry: dr.DeviceRegistry, + config_entry: MockConfigEntry, +) -> None: + """Test that legacy non-string device identifiers are migrated to strings.""" + config_entry.add_to_hass(hass) + # Create a device with a legacy identifier + device_registry.async_get_or_create( + config_entry_id=config_entry.entry_id, identifiers={(DOMAIN, 1)} + ) + device_registry.async_get_or_create( + config_entry_id=config_entry.entry_id, identifiers={("Other", 1)} + ) + assert await hass.config_entries.async_setup(config_entry.entry_id) + assert device_registry.async_get_device({("Other", 1)}) is not None + assert device_registry.async_get_device({(DOMAIN, 1)}) is None + assert device_registry.async_get_device({(DOMAIN, "1")}) is not None diff --git a/tests/components/heos/test_media_player.py b/tests/components/heos/test_media_player.py index f2b54ecec81..e71614564f2 100644 --- a/tests/components/heos/test_media_player.py +++ b/tests/components/heos/test_media_player.py @@ -271,7 +271,7 @@ async def test_updates_from_players_changed_new_ids( event = asyncio.Event() # Assert device registry matches current id - assert device_registry.async_get_device(identifiers={(DOMAIN, 1)}) + assert device_registry.async_get_device(identifiers={(DOMAIN, "1")}) # Assert entity registry matches current id assert ( entity_registry.async_get_entity_id(MEDIA_PLAYER_DOMAIN, DOMAIN, "1") @@ -292,7 +292,7 @@ async def test_updates_from_players_changed_new_ids( # Assert device registry identifiers were updated assert len(device_registry.devices) == 2 - assert device_registry.async_get_device(identifiers={(DOMAIN, 101)}) + assert device_registry.async_get_device(identifiers={(DOMAIN, "101")}) # Assert entity registry unique id was updated assert len(entity_registry.entities) == 2 assert (