From e3cdee84f59f4da340aab7199a4e446809591f1d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 8 Jul 2026 01:55:07 -0500 Subject: [PATCH] Fix ESPHome UTF-8 unique id collisions (#174814) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> --- homeassistant/components/esphome/entity.py | 8 +- .../components/esphome/entry_data.py | 38 ++++---- homeassistant/components/esphome/manager.py | 15 +-- tests/components/esphome/test_entity.py | 92 ++++++++++++++----- tests/components/esphome/test_entry_data.py | 65 +++++++++++-- tests/components/esphome/test_repairs.py | 12 ++- tests/components/esphome/test_sensor.py | 6 +- 7 files changed, 170 insertions(+), 66 deletions(-) diff --git a/homeassistant/components/esphome/entity.py b/homeassistant/components/esphome/entity.py index eb6f2772d2cb..2d5ae2161d13 100644 --- a/homeassistant/components/esphome/entity.py +++ b/homeassistant/components/esphome/entity.py @@ -12,6 +12,7 @@ from aioesphomeapi import ( EntityCategory as EsphomeEntityCategory, EntityInfo, EntityState, + build_device_unique_id, ) import voluptuous as vol @@ -31,12 +32,7 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback from .const import DOMAIN # Import config flow so that it's added to the registry -from .entry_data import ( - DeviceEntityKey, - ESPHomeConfigEntry, - RuntimeEntryData, - build_device_unique_id, -) +from .entry_data import DeviceEntityKey, ESPHomeConfigEntry, RuntimeEntryData from .enum_mapper import EsphomeEnumMapper _LOGGER = logging.getLogger(__name__) diff --git a/homeassistant/components/esphome/entry_data.py b/homeassistant/components/esphome/entry_data.py index 7dd004c2af83..3003728406dc 100644 --- a/homeassistant/components/esphome/entry_data.py +++ b/homeassistant/components/esphome/entry_data.py @@ -45,7 +45,7 @@ from aioesphomeapi import ( UserService, ValveInfo, WaterHeaterInfo, - build_unique_id, + build_device_unique_id, ) from aioesphomeapi.model import ButtonInfo from bleak_esphome.backend.device import ESPHomeBluetoothDevice @@ -103,22 +103,6 @@ INFO_TYPE_TO_PLATFORM: dict[type[EntityInfo], Platform] = { } -def build_device_unique_id(mac: str, entity_info: EntityInfo) -> str: - """Build unique ID for entity, appending @device_id if it belongs to a sub-device. - - This wrapper around build_unique_id ensures that entities belonging to sub-devices - have their device_id appended to the unique_id to handle proper migration when - entities move between devices. - """ - base_unique_id = build_unique_id(mac, entity_info) - - # If entity belongs to a sub-device, append @device_id - if entity_info.device_id: - return f"{base_unique_id}@{entity_info.device_id}" - - return base_unique_id - - class StoreData(TypedDict, total=False): """ESPHome storage data.""" @@ -310,11 +294,31 @@ class RuntimeEntryData: infos_by_type: defaultdict[type[EntityInfo], list[EntityInfo]] = defaultdict( list ) + ent_reg = er.async_get(hass) + registry_get_entity = ent_reg.async_get_entity_id for info in infos: info_type = type(info) if platform := info_types_to_platform.get(info_type): needed_platforms.add(platform) infos_by_type[info_type].append(info) + # Migrate legacy unique ids to the version 3 format that fixes + # UTF-8 collisions. Skip when a version 3 id already exists so a + # downgrade then upgrade keeps the original entity. When two + # legacy ids collided (the bug this fixes) only one registry + # entry exists for it, so the first iterated info claims it and + # the rest get fresh version 3 ids. + old_unique_id = build_device_unique_id(mac, info, version=1) + new_unique_id = build_device_unique_id(mac, info, version=3) + if ( + old_unique_id != new_unique_id + and ( + old_entry := registry_get_entity( + platform, DOMAIN, old_unique_id + ) + ) + and not registry_get_entity(platform, DOMAIN, new_unique_id) + ): + ent_reg.async_update_entity(old_entry, new_unique_id=new_unique_id) else: _LOGGER.warning( "Entity type %s is not supported in this version of Home Assistant", diff --git a/homeassistant/components/esphome/manager.py b/homeassistant/components/esphome/manager.py index 2a3c44357d6d..a1428ddc702e 100644 --- a/homeassistant/components/esphome/manager.py +++ b/homeassistant/components/esphome/manager.py @@ -1417,13 +1417,14 @@ async def async_replace_device( upper_mac = new_mac.upper() old_upper_mac = old_mac.upper() for entity in er.async_entries_for_config_entry(ent_reg, entry.entry_id): - # -- - old_unique_id = entity.unique_id.split("-") - new_unique_id = "-".join([upper_mac, *old_unique_id[1:]]) - if entity.unique_id != new_unique_id and entity.unique_id.startswith( - old_upper_mac - ): - ent_reg.async_update_entity(entity.entity_id, new_unique_id=new_unique_id) + # The mac is the leading segment of the unique id in every format, + # so swap the prefix without parsing the rest. + if entity.unique_id.startswith(old_upper_mac): + new_unique_id = upper_mac + entity.unique_id[len(old_upper_mac) :] + if new_unique_id != entity.unique_id: + ent_reg.async_update_entity( + entity.entity_id, new_unique_id=new_unique_id + ) domain_data = DomainData.get(hass) store = domain_data.get_or_create_store(hass, entry) diff --git a/tests/components/esphome/test_entity.py b/tests/components/esphome/test_entity.py index 36bf86366220..c64d8955631f 100644 --- a/tests/components/esphome/test_entity.py +++ b/tests/components/esphome/test_entity.py @@ -607,7 +607,7 @@ async def test_entity_id_preserved_on_upgrade_when_in_storage( ent_reg_entry = entity_registry.async_get_or_create( Platform.BINARY_SENSOR, DOMAIN, - "11:22:33:44:55:AA-binary_sensor-my", + "11:22:33:44:55:AA/0/binary_sensor/my", ) entity_registry.async_update_entity( ent_reg_entry.entity_id, @@ -1161,6 +1161,58 @@ async def test_entity_id_with_empty_sub_device_name( assert hass.states.get("binary_sensor.main_device_sensor") is not None +async def test_legacy_unique_id_migrated_to_v3_sub_device( + hass: HomeAssistant, + entity_registry: er.EntityRegistry, + mock_client: APIClient, + mock_esphome_device: MockESPHomeDeviceType, +) -> None: + """Test a legacy sub-device unique id is migrated to the version 3 format.""" + sub_devices = [ + SubDeviceInfo(device_id=22222222, name="kitchen_controller", area_id=0), + ] + device_info = {"name": "test", "devices": sub_devices} + entity_info = [ + BinarySensorInfo( + object_id="temperature", + key=1, + name="Temperature", + device_id=22222222, + ), + ] + states = [BinarySensorState(key=1, state=True, missing_state=False)] + + # Seed a registry entry in the legacy format with the @device_id suffix + legacy_entry = entity_registry.async_get_or_create( + Platform.BINARY_SENSOR, + DOMAIN, + "11:22:33:44:55:AA-binary_sensor-temperature@22222222", + suggested_object_id="kitchen_controller_temperature", + ) + + await mock_esphome_device( + mock_client=mock_client, + device_info=device_info, + entity_info=entity_info, + states=states, + ) + + entity_entry = entity_registry.async_get(legacy_entry.entity_id) + assert entity_entry is not None + # The legacy id is renamed to version 3, keeping the same entity + assert ( + entity_entry.unique_id == "11:22:33:44:55:AA/22222222/binary_sensor/Temperature" + ) + assert ( + entity_registry.async_get_entity_id( + Platform.BINARY_SENSOR, + DOMAIN, + "11:22:33:44:55:AA-binary_sensor-temperature@22222222", + ) + is None + ) + + async def test_unique_id_migration_when_entity_moves_between_devices( hass: HomeAssistant, entity_registry: er.EntityRegistry, @@ -1204,8 +1256,8 @@ async def test_unique_id_migration_when_entity_moves_between_devices( entity_entry = entity_registry.async_get("binary_sensor.test_temperature") assert entity_entry is not None initial_unique_id = entity_entry.unique_id - # Initial unique_id should not have @device_id suffix since it's on main device - assert "@" not in initial_unique_id + # Main device entities use device_id 0 in the unique id + assert "/0/" in initial_unique_id # Add sub-device to device info sub_devices = [ @@ -1260,8 +1312,8 @@ async def test_unique_id_migration_when_entity_moves_between_devices( # Wait for entity to be updated await hass.async_block_till_done() - # The entity_id doesn't change when moving between devices - # Only the unique_id gets updated with @device_id suffix + # The entity_id doesn't change when moving between devices, + # only the device segment of the unique_id changes state = hass.states.get("binary_sensor.test_temperature") assert state is not None @@ -1269,9 +1321,8 @@ async def test_unique_id_migration_when_entity_moves_between_devices( entity_entry = entity_registry.async_get("binary_sensor.test_temperature") assert entity_entry is not None - # Unique ID should have been migrated to include @device_id - # This is done by our build_device_unique_id wrapper - expected_unique_id = f"{initial_unique_id}@22222222" + # Unique ID device segment should now be the sub-device id + expected_unique_id = initial_unique_id.replace("/0/", "/22222222/") assert entity_entry.unique_id == expected_unique_id # Entity should now be associated with the sub-device @@ -1331,8 +1382,8 @@ async def test_unique_id_migration_sub_device_to_main_device( ) assert entity_entry is not None initial_unique_id = entity_entry.unique_id - # Initial unique_id should have @device_id suffix since it's on sub-device - assert "@22222222" in initial_unique_id + # Sub-device entities carry the sub-device id in the unique id + assert "/22222222/" in initial_unique_id # Update entity info - move to main device new_entity_info = [ @@ -1365,8 +1416,8 @@ async def test_unique_id_migration_sub_device_to_main_device( ) assert entity_entry is not None - # Unique ID should have been migrated to remove @device_id suffix - expected_unique_id = initial_unique_id.replace("@22222222", "") + # Unique ID device segment should now be the main device id 0 + expected_unique_id = initial_unique_id.replace("/22222222/", "/0/") assert entity_entry.unique_id == expected_unique_id # Entity should now be associated with the main device @@ -1427,8 +1478,8 @@ async def test_unique_id_migration_between_sub_devices( ) assert entity_entry is not None initial_unique_id = entity_entry.unique_id - # Initial unique_id should have @22222222 suffix - assert "@22222222" in initial_unique_id + # Sub-device entities carry the sub-device id in the unique id + assert "/22222222/" in initial_unique_id # Update entity info - move to second sub-device new_entity_info = [ @@ -1461,8 +1512,8 @@ async def test_unique_id_migration_between_sub_devices( ) assert entity_entry is not None - # Unique ID should have been migrated from @22222222 to @33333333 - expected_unique_id = initial_unique_id.replace("@22222222", "@33333333") + # Unique ID device segment should have moved from 22222222 to 33333333 + expected_unique_id = initial_unique_id.replace("/22222222/", "/33333333/") assert entity_entry.unique_id == expected_unique_id # Entity should now be associated with the second sub-device @@ -1524,8 +1575,8 @@ async def test_entity_device_id_rename_in_yaml( entity_entry = entity_registry.async_get("binary_sensor.old_device_sensor") assert entity_entry is not None initial_unique_id = entity_entry.unique_id - # Should have @11111111 suffix - assert "@11111111" in initial_unique_id + # Sub-device entities carry the sub-device id in the unique id + assert "/11111111/" in initial_unique_id # Simulate user renaming device_id in YAML config # The device_id hash changes from 11111111 to 99999999 @@ -1587,9 +1638,8 @@ async def test_entity_device_id_rename_in_yaml( entity_entry = entity_registry.async_get("binary_sensor.renamed_device_sensor") assert entity_entry is not None - # Unique ID should have the new device_id - base_unique_id = initial_unique_id.replace("@11111111", "") - expected_unique_id = f"{base_unique_id}@99999999" + # Unique ID device segment should have the new device_id + expected_unique_id = initial_unique_id.replace("/11111111/", "/99999999/") assert entity_entry.unique_id == expected_unique_id # Entity should be associated with the new device diff --git a/tests/components/esphome/test_entry_data.py b/tests/components/esphome/test_entry_data.py index e06c77a2824d..4d63c764ffb3 100644 --- a/tests/components/esphome/test_entry_data.py +++ b/tests/components/esphome/test_entry_data.py @@ -21,6 +21,53 @@ from homeassistant.helpers.service_info.esphome import ESPHomeServiceInfo from .conftest import MockGenericDeviceEntryType +async def test_migrate_entity_unique_id( + hass: HomeAssistant, + entity_registry: er.EntityRegistry, + mock_client: APIClient, + mock_generic_device_entry: MockGenericDeviceEntryType, +) -> None: + """Test a legacy unique id is migrated to the version 3 format.""" + entity_registry.async_get_or_create( + SENSOR_DOMAIN, + DOMAIN, + "11:22:33:44:55:AA-sensor-mysensor", + suggested_object_id="my_sensor", + disabled_by=None, + ) + entity_info = [ + SensorInfo( + object_id="mysensor", + key=1, + name="my sensor", + entity_category=ESPHomeEntityCategory.DIAGNOSTIC, + icon="mdi:leaf", + ) + ] + states = [SensorState(key=1, state=50)] + user_service = [] + await mock_generic_device_entry( + mock_client=mock_client, + entity_info=entity_info, + user_service=user_service, + states=states, + ) + state = hass.states.get("sensor.my_sensor") + assert state is not None + assert state.state == "50" + entry = entity_registry.async_get("sensor.my_sensor") + assert entry is not None + # The legacy unique id should have been renamed to the version 3 format, + # keeping the entity (and its entity_id) instead of creating a new one + assert entry.unique_id == "11:22:33:44:55:AA/0/sensor/my sensor" + assert ( + entity_registry.async_get_entity_id( + SENSOR_DOMAIN, DOMAIN, "11:22:33:44:55:AA-sensor-mysensor" + ) + is None + ) + + async def test_migrate_entity_unique_id_downgrade_upgrade( hass: HomeAssistant, entity_registry: er.EntityRegistry, @@ -28,18 +75,20 @@ async def test_migrate_entity_unique_id_downgrade_upgrade( mock_generic_device_entry: MockGenericDeviceEntryType, ) -> None: """Test unique id migration prefers the original entity on downgrade upgrade.""" + # The original entity, already in the version 3 format entity_registry.async_get_or_create( SENSOR_DOMAIN, DOMAIN, - "my_sensor", - suggested_object_id="old_sensor", + "11:22:33:44:55:AA/0/sensor/my sensor", + suggested_object_id="new_sensor", disabled_by=None, ) + # A duplicate left behind in the legacy format by a downgrade entity_registry.async_get_or_create( SENSOR_DOMAIN, DOMAIN, "11:22:33:44:55:AA-sensor-mysensor", - suggested_object_id="new_sensor", + suggested_object_id="old_sensor", disabled_by=None, ) entity_info = [ @@ -64,17 +113,17 @@ async def test_migrate_entity_unique_id_downgrade_upgrade( assert state.state == "50" entry = entity_registry.async_get("sensor.new_sensor") assert entry is not None - # Confirm we did not touch the entity that was created + # Confirm we did not touch the legacy entity that was created # on downgrade so when they upgrade again they can delete the # entity that was only created on downgrade and they keep # the original one. assert ( - entity_registry.async_get_entity_id(SENSOR_DOMAIN, DOMAIN, "my_sensor") + entity_registry.async_get_entity_id( + SENSOR_DOMAIN, DOMAIN, "11:22:33:44:55:AA-sensor-mysensor" + ) is not None ) - # Note that ESPHome includes the EntityInfo type in the unique id - # as this is not a 1:1 mapping to the entity platform (ie. text_sensor) - assert entry.unique_id == "11:22:33:44:55:AA-sensor-mysensor" + assert entry.unique_id == "11:22:33:44:55:AA/0/sensor/my sensor" async def test_discover_zwave() -> None: diff --git a/tests/components/esphome/test_repairs.py b/tests/components/esphome/test_repairs.py index 5a6090234a48..975f0fb31eff 100644 --- a/tests/components/esphome/test_repairs.py +++ b/tests/components/esphome/test_repairs.py @@ -139,13 +139,15 @@ async def test_device_conflict_migration( ent_reg_entry = entity_registry.async_get("binary_sensor.test_my_binary_sensor") assert ent_reg_entry - assert ent_reg_entry.unique_id == "11:22:33:44:55:AA-binary_sensor-mybinary_sensor" + assert ( + ent_reg_entry.unique_id == "11:22:33:44:55:AA/0/binary_sensor/my binary_sensor" + ) entries = er.async_entries_for_config_entry( entity_registry, mock_config_entry.entry_id ) assert entries is not None for entry in entries: - assert entry.unique_id.startswith("11:22:33:44:55:AA-") + assert entry.unique_id.startswith("11:22:33:44:55:AA/") disconnect_done = hass.loop.create_future() async def async_disconnect(*args, **kwargs) -> None: @@ -201,14 +203,16 @@ async def test_device_conflict_migration( assert mock_config_entry.unique_id == "11:22:33:44:55:ab" ent_reg_entry = entity_registry.async_get("binary_sensor.test_my_binary_sensor") assert ent_reg_entry - assert ent_reg_entry.unique_id == "11:22:33:44:55:AB-binary_sensor-mybinary_sensor" + assert ( + ent_reg_entry.unique_id == "11:22:33:44:55:AB/0/binary_sensor/my binary_sensor" + ) entries = er.async_entries_for_config_entry( entity_registry, mock_config_entry.entry_id ) assert entries is not None for entry in entries: - assert entry.unique_id.startswith("11:22:33:44:55:AB-") + assert entry.unique_id.startswith("11:22:33:44:55:AB/") dev_entry = device_registry.async_get_device( identifiers={}, connections={(dr.CONNECTION_NETWORK_MAC, "11:22:33:44:55:ab")} diff --git a/tests/components/esphome/test_sensor.py b/tests/components/esphome/test_sensor.py index 926d24a7869b..adea7007c2c8 100644 --- a/tests/components/esphome/test_sensor.py +++ b/tests/components/esphome/test_sensor.py @@ -129,7 +129,7 @@ async def test_generic_numeric_sensor_with_entity_category_and_icon( assert entry is not None # Note that ESPHome includes the EntityInfo type in the unique id # as this is not a 1:1 mapping to the entity platform (ie. text_sensor) - assert entry.unique_id == "11:22:33:44:55:AA-sensor-mysensor" + assert entry.unique_id == "11:22:33:44:55:AA/0/sensor/my sensor" assert entry.entity_category is EntityCategory.DIAGNOSTIC @@ -168,7 +168,7 @@ async def test_generic_numeric_sensor_state_class_measurement( assert entry is not None # Note that ESPHome includes the EntityInfo type in the unique id # as this is not a 1:1 mapping to the entity platform (ie. text_sensor) - assert entry.unique_id == "11:22:33:44:55:AA-sensor-mysensor" + assert entry.unique_id == "11:22:33:44:55:AA/0/sensor/my sensor" assert entry.entity_category is None @@ -205,7 +205,7 @@ async def test_generic_numeric_sensor_state_class_measurement_angle( assert entry is not None # Note that ESPHome includes the EntityInfo type in the unique id # as this is not a 1:1 mapping to the entity platform (ie. text_sensor) - assert entry.unique_id == "11:22:33:44:55:AA-sensor-mysensor" + assert entry.unique_id == "11:22:33:44:55:AA/0/sensor/my sensor" assert entry.entity_category is None