From 28bee6d1aa46ce4e54e2f4e31aa0ef802d417355 Mon Sep 17 00:00:00 2001 From: Sab44 <64696149+Sab44@users.noreply.github.com> Date: Tue, 28 Oct 2025 10:07:37 +0100 Subject: [PATCH] Fix unique IDs and migrate v1 entries (#155319) --- .../libre_hardware_monitor/__init__.py | 59 ++++++++++++ .../libre_hardware_monitor/config_flow.py | 2 +- .../libre_hardware_monitor/coordinator.py | 2 +- .../libre_hardware_monitor/sensor.py | 11 +-- .../libre_hardware_monitor/conftest.py | 2 + .../snapshots/test_sensor.ambr | 50 +++++----- .../libre_hardware_monitor/test_init.py | 91 +++++++++++++++++++ .../libre_hardware_monitor/test_sensor.py | 13 ++- 8 files changed, 193 insertions(+), 37 deletions(-) create mode 100644 tests/components/libre_hardware_monitor/test_init.py diff --git a/homeassistant/components/libre_hardware_monitor/__init__.py b/homeassistant/components/libre_hardware_monitor/__init__.py index 4f39d51e963..1461b4b449c 100644 --- a/homeassistant/components/libre_hardware_monitor/__init__.py +++ b/homeassistant/components/libre_hardware_monitor/__init__.py @@ -2,9 +2,13 @@ from __future__ import annotations +import logging + from homeassistant.const import Platform from homeassistant.core import HomeAssistant +from homeassistant.helpers import device_registry as dr, entity_registry as er +from .const import DOMAIN from .coordinator import ( LibreHardwareMonitorConfigEntry, LibreHardwareMonitorCoordinator, @@ -12,6 +16,61 @@ from .coordinator import ( PLATFORMS = [Platform.SENSOR] +_LOGGER = logging.getLogger(__name__) + + +async def async_migrate_entry( + hass: HomeAssistant, config_entry: LibreHardwareMonitorConfigEntry +) -> bool: + """Migrate non-unique entity and device ids.""" + _LOGGER.debug("Migrating from version %s", config_entry.version) + + if config_entry.version == 1: + # Migrate entity identifiers + entity_registry = er.async_get(hass) + registry_entries = er.async_entries_for_config_entry( + entity_registry, config_entry.entry_id + ) + for reg_entry in registry_entries: + new_entity_id = f"{config_entry.entry_id}_{reg_entry.unique_id[4:]}" + _LOGGER.debug( + "Migrating entity %s unique id from %s to %s", + reg_entry.entity_id, + reg_entry.unique_id, + new_entity_id, + ) + entity_registry.async_update_entity( + reg_entry.entity_id, new_unique_id=new_entity_id + ) + + # Migrate device identifiers + device_registry = dr.async_get(hass) + device_entries = dr.async_entries_for_config_entry( + registry=device_registry, config_entry_id=config_entry.entry_id + ) + for device in device_entries: + old_device_id = next(iter(device.identifiers))[1] + new_device_id = f"{config_entry.entry_id}_{old_device_id}" + _LOGGER.debug( + "Migrating device %s unique id from %s to %s", + device.name, + old_device_id, + new_device_id, + ) + device_registry.async_update_device( + device_id=device.id, + new_identifiers={(DOMAIN, new_device_id)}, + ) + + hass.config_entries.async_update_entry( + config_entry, data=config_entry.data, version=2 + ) + + _LOGGER.debug("Migration to version 2 successful") + return True + + return True + async def async_setup_entry( hass: HomeAssistant, config_entry: LibreHardwareMonitorConfigEntry diff --git a/homeassistant/components/libre_hardware_monitor/config_flow.py b/homeassistant/components/libre_hardware_monitor/config_flow.py index f24c801254c..7ca51abe359 100644 --- a/homeassistant/components/libre_hardware_monitor/config_flow.py +++ b/homeassistant/components/libre_hardware_monitor/config_flow.py @@ -30,7 +30,7 @@ CONFIG_SCHEMA = vol.Schema( class LibreHardwareMonitorConfigFlow(ConfigFlow, domain=DOMAIN): """Handle a config flow for LibreHardwareMonitor.""" - VERSION = 1 + VERSION = 2 async def async_step_user( self, user_input: dict[str, Any] | None = None diff --git a/homeassistant/components/libre_hardware_monitor/coordinator.py b/homeassistant/components/libre_hardware_monitor/coordinator.py index 6e87fd70301..22f6bbbd47a 100644 --- a/homeassistant/components/libre_hardware_monitor/coordinator.py +++ b/homeassistant/components/libre_hardware_monitor/coordinator.py @@ -114,7 +114,7 @@ class LibreHardwareMonitorCoordinator(DataUpdateCoordinator[LibreHardwareMonitor device_registry = dr.async_get(self.hass) for device_id in orphaned_devices: if device := device_registry.async_get_device( - identifiers={(DOMAIN, device_id)} + identifiers={(DOMAIN, f"{self.config_entry.entry_id}_{device_id}")} ): device_registry.async_update_device( device_id=device.id, diff --git a/homeassistant/components/libre_hardware_monitor/sensor.py b/homeassistant/components/libre_hardware_monitor/sensor.py index cb7d94ae73e..489c66e1b2a 100644 --- a/homeassistant/components/libre_hardware_monitor/sensor.py +++ b/homeassistant/components/libre_hardware_monitor/sensor.py @@ -10,11 +10,9 @@ from homeassistant.helpers.device_registry import DeviceInfo from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback from homeassistant.helpers.update_coordinator import CoordinatorEntity -from . import LibreHardwareMonitorCoordinator +from . import LibreHardwareMonitorConfigEntry, LibreHardwareMonitorCoordinator from .const import DOMAIN -from .coordinator import LibreHardwareMonitorConfigEntry -# Coordinator is used to centralize the data updates PARALLEL_UPDATES = 0 STATE_MIN_VALUE = "min_value" @@ -30,7 +28,7 @@ async def async_setup_entry( lhm_coordinator = config_entry.runtime_data async_add_entities( - LibreHardwareMonitorSensor(lhm_coordinator, sensor_data) + LibreHardwareMonitorSensor(lhm_coordinator, config_entry.entry_id, sensor_data) for sensor_data in lhm_coordinator.data.sensor_data.values() ) @@ -46,6 +44,7 @@ class LibreHardwareMonitorSensor( def __init__( self, coordinator: LibreHardwareMonitorCoordinator, + entry_id: str, sensor_data: LibreHardwareMonitorSensorData, ) -> None: """Initialize an LibreHardwareMonitor sensor.""" @@ -58,13 +57,13 @@ class LibreHardwareMonitorSensor( STATE_MAX_VALUE: self._format_number_value(sensor_data.max), } self._attr_native_unit_of_measurement = sensor_data.unit - self._attr_unique_id: str = f"lhm-{sensor_data.sensor_id}" + self._attr_unique_id: str = f"{entry_id}_{sensor_data.sensor_id}" self._sensor_id: str = sensor_data.sensor_id # Hardware device self._attr_device_info = DeviceInfo( - identifiers={(DOMAIN, sensor_data.device_id)}, + identifiers={(DOMAIN, f"{entry_id}_{sensor_data.device_id}")}, name=sensor_data.device_name, model=sensor_data.device_type, ) diff --git a/tests/components/libre_hardware_monitor/conftest.py b/tests/components/libre_hardware_monitor/conftest.py index cff9e4acf3a..520790d4fde 100644 --- a/tests/components/libre_hardware_monitor/conftest.py +++ b/tests/components/libre_hardware_monitor/conftest.py @@ -31,6 +31,8 @@ def mock_config_entry() -> MockConfigEntry: domain=DOMAIN, title="192.168.0.20:8085", data=VALID_CONFIG, + entry_id="test_entry_id", + version=2, ) diff --git a/tests/components/libre_hardware_monitor/snapshots/test_sensor.ambr b/tests/components/libre_hardware_monitor/snapshots/test_sensor.ambr index 9e26d4d49f7..e2dc96e2757 100644 --- a/tests/components/libre_hardware_monitor/snapshots/test_sensor.ambr +++ b/tests/components/libre_hardware_monitor/snapshots/test_sensor.ambr @@ -32,7 +32,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-amdcpu-0-temperature-2', + 'unique_id': 'test_entry_id_amdcpu-0-temperature-2', 'unit_of_measurement': '°C', }) # --- @@ -86,7 +86,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-amdcpu-0-load-0', + 'unique_id': 'test_entry_id_amdcpu-0-load-0', 'unit_of_measurement': '%', }) # --- @@ -140,7 +140,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-amdcpu-0-power-0', + 'unique_id': 'test_entry_id_amdcpu-0-power-0', 'unit_of_measurement': 'W', }) # --- @@ -194,7 +194,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-amdcpu-0-temperature-3', + 'unique_id': 'test_entry_id_amdcpu-0-temperature-3', 'unit_of_measurement': '°C', }) # --- @@ -248,7 +248,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-amdcpu-0-voltage-3', + 'unique_id': 'test_entry_id_amdcpu-0-voltage-3', 'unit_of_measurement': 'V', }) # --- @@ -302,7 +302,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-amdcpu-0-voltage-2', + 'unique_id': 'test_entry_id_amdcpu-0-voltage-2', 'unit_of_measurement': 'V', }) # --- @@ -356,7 +356,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-lpc-nct6687d-0-voltage-0', + 'unique_id': 'test_entry_id_lpc-nct6687d-0-voltage-0', 'unit_of_measurement': 'V', }) # --- @@ -410,7 +410,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-lpc-nct6687d-0-voltage-1', + 'unique_id': 'test_entry_id_lpc-nct6687d-0-voltage-1', 'unit_of_measurement': 'V', }) # --- @@ -464,7 +464,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-lpc-nct6687d-0-fan-0', + 'unique_id': 'test_entry_id_lpc-nct6687d-0-fan-0', 'unit_of_measurement': 'RPM', }) # --- @@ -518,7 +518,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-lpc-nct6687d-0-temperature-0', + 'unique_id': 'test_entry_id_lpc-nct6687d-0-temperature-0', 'unit_of_measurement': '°C', }) # --- @@ -572,7 +572,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-lpc-nct6687d-0-fan-1', + 'unique_id': 'test_entry_id_lpc-nct6687d-0-fan-1', 'unit_of_measurement': 'RPM', }) # --- @@ -626,7 +626,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-lpc-nct6687d-0-fan-2', + 'unique_id': 'test_entry_id_lpc-nct6687d-0-fan-2', 'unit_of_measurement': None, }) # --- @@ -679,7 +679,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-lpc-nct6687d-0-temperature-1', + 'unique_id': 'test_entry_id_lpc-nct6687d-0-temperature-1', 'unit_of_measurement': '°C', }) # --- @@ -733,7 +733,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-lpc-nct6687d-0-voltage-2', + 'unique_id': 'test_entry_id_lpc-nct6687d-0-voltage-2', 'unit_of_measurement': 'V', }) # --- @@ -787,7 +787,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-gpu-nvidia-0-clock-0', + 'unique_id': 'test_entry_id_gpu-nvidia-0-clock-0', 'unit_of_measurement': 'MHz', }) # --- @@ -841,7 +841,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-gpu-nvidia-0-load-0', + 'unique_id': 'test_entry_id_gpu-nvidia-0-load-0', 'unit_of_measurement': '%', }) # --- @@ -895,7 +895,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-gpu-nvidia-0-temperature-0', + 'unique_id': 'test_entry_id_gpu-nvidia-0-temperature-0', 'unit_of_measurement': '°C', }) # --- @@ -949,7 +949,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-gpu-nvidia-0-fan-1', + 'unique_id': 'test_entry_id_gpu-nvidia-0-fan-1', 'unit_of_measurement': 'RPM', }) # --- @@ -1003,7 +1003,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-gpu-nvidia-0-fan-2', + 'unique_id': 'test_entry_id_gpu-nvidia-0-fan-2', 'unit_of_measurement': 'RPM', }) # --- @@ -1057,7 +1057,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-gpu-nvidia-0-temperature-2', + 'unique_id': 'test_entry_id_gpu-nvidia-0-temperature-2', 'unit_of_measurement': '°C', }) # --- @@ -1111,7 +1111,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-gpu-nvidia-0-clock-4', + 'unique_id': 'test_entry_id_gpu-nvidia-0-clock-4', 'unit_of_measurement': 'MHz', }) # --- @@ -1165,7 +1165,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-gpu-nvidia-0-load-1', + 'unique_id': 'test_entry_id_gpu-nvidia-0-load-1', 'unit_of_measurement': '%', }) # --- @@ -1219,7 +1219,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-gpu-nvidia-0-power-0', + 'unique_id': 'test_entry_id_gpu-nvidia-0-power-0', 'unit_of_measurement': 'W', }) # --- @@ -1273,7 +1273,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-gpu-nvidia-0-throughput-1', + 'unique_id': 'test_entry_id_gpu-nvidia-0-throughput-1', 'unit_of_measurement': 'MB/s', }) # --- @@ -1327,7 +1327,7 @@ 'suggested_object_id': None, 'supported_features': 0, 'translation_key': None, - 'unique_id': 'lhm-gpu-nvidia-0-load-2', + 'unique_id': 'test_entry_id_gpu-nvidia-0-load-2', 'unit_of_measurement': '%', }) # --- diff --git a/tests/components/libre_hardware_monitor/test_init.py b/tests/components/libre_hardware_monitor/test_init.py new file mode 100644 index 00000000000..8780c49cdc3 --- /dev/null +++ b/tests/components/libre_hardware_monitor/test_init.py @@ -0,0 +1,91 @@ +"""Tests for the LibreHardwareMonitor init.""" + +import logging + +from homeassistant.components.libre_hardware_monitor.const import DOMAIN +from homeassistant.core import HomeAssistant +from homeassistant.helpers import device_registry as dr, entity_registry as er + +from . import init_integration +from .conftest import VALID_CONFIG + +from tests.common import MockConfigEntry + +_LOGGER = logging.getLogger(__name__) + + +async def test_migration_to_unique_ids( + hass: HomeAssistant, + entity_registry: er.EntityRegistry, + device_registry: dr.DeviceRegistry, +) -> None: + """Test that non-unique legacy entity and device IDs are updated.""" + legacy_config_entry_v1 = MockConfigEntry( + domain=DOMAIN, + title="192.168.0.20:8085", + data=VALID_CONFIG, + entry_id="test_entry_id", + version=1, + ) + legacy_config_entry_v1.add_to_hass(hass) + + # Set up devices with legacy device ID + legacy_device_ids = ["amdcpu-0", "gpu-nvidia-0", "lpc-nct6687d-0"] + for device_id in legacy_device_ids: + device_registry.async_get_or_create( + config_entry_id=legacy_config_entry_v1.entry_id, + identifiers={(DOMAIN, device_id)}, # Old format without entry_id prefix + name=f"Test Device {device_id}", + ) + + # Set up entity with legacy entity ID + existing_sensor_id = "lpc-nct6687d-0-voltage-0" + legacy_entity_id = f"lhm-{existing_sensor_id}" + + entity_object_id = "sensor.msi_mag_b650m_mortar_wifi_ms_7d76_12v_voltage" + entity_registry.async_get_or_create( + "sensor", + DOMAIN, + legacy_entity_id, + suggested_object_id="msi_mag_b650m_mortar_wifi_ms_7d76_12v_voltage", + config_entry=legacy_config_entry_v1, + ) + + # Verify state before migration + device_entries_before = dr.async_entries_for_config_entry( + registry=device_registry, config_entry_id=legacy_config_entry_v1.entry_id + ) + assert { + next(iter(device.identifiers))[1] for device in device_entries_before + } == set(legacy_device_ids) + + assert ( + entity_registry.async_get_entity_id("sensor", DOMAIN, legacy_entity_id) + == entity_object_id + ) + + await init_integration(hass, legacy_config_entry_v1) + + # Verify state after migration + device_entries_after = dr.async_entries_for_config_entry( + registry=device_registry, config_entry_id=legacy_config_entry_v1.entry_id + ) + expected_unique_device_ids = [ + f"{legacy_config_entry_v1.entry_id}_{device_id}" + for device_id in legacy_device_ids + ] + assert { + next(iter(device.identifiers))[1] for device in device_entries_after + } == set(expected_unique_device_ids) + + entity_entry = entity_registry.async_get(entity_object_id) + assert entity_entry is not None, "Entity should exist after migration" + + new_unique_entity_id = f"{legacy_config_entry_v1.entry_id}_{existing_sensor_id}" + assert entity_entry.unique_id == new_unique_entity_id, ( + f"Unique ID not migrated: {entity_entry.unique_id}" + ) + + assert ( + entity_registry.async_get_entity_id("sensor", DOMAIN, legacy_entity_id) is None + ) diff --git a/tests/components/libre_hardware_monitor/test_sensor.py b/tests/components/libre_hardware_monitor/test_sensor.py index 0ce8f5e1c8f..dc1a5917991 100644 --- a/tests/components/libre_hardware_monitor/test_sensor.py +++ b/tests/components/libre_hardware_monitor/test_sensor.py @@ -88,11 +88,10 @@ async def test_sensors_are_updated( mock_config_entry: MockConfigEntry, freezer: FrozenDateTimeFactory, ) -> None: - """Test sensors are updated.""" + """Test sensors are updated with properly formatted values.""" await init_integration(hass, mock_config_entry) entity_id = "sensor.amd_ryzen_7_7800x3d_package_temperature" - state = hass.states.get(entity_id) assert state @@ -175,7 +174,7 @@ async def test_orphaned_devices_are_removed( device_registry = dr.async_get(hass) orphaned_device = device_registry.async_get_or_create( config_entry_id=mock_config_entry.entry_id, - identifiers={(DOMAIN, "lpc-nct6687d-0")}, + identifiers={(DOMAIN, f"{mock_config_entry.entry_id}_lpc-nct6687d-0")}, ) with patch.object( @@ -209,4 +208,10 @@ async def test_integration_does_not_log_new_devices_on_first_refresh( with caplog.at_level(logging.WARNING): await init_integration(hass, mock_config_entry) - assert len(caplog.records) == 0 + + libre_hardware_monitor_logs = [ + record + for record in caplog.records + if record.name.startswith("homeassistant.components.libre_hardware_monitor") + ] + assert len(libre_hardware_monitor_logs) == 0