Fix regression in roborock image entity naming (home-assistant#157432)

Author: allenporter

homeassistant/components/roborock/image.py

@@ -32,7 +32,6 @@ async def async_setup_entry(
         (
             RoborockMap(
                 config_entry,
-                f"{coord.duid_slug}_map_{map_info.name}",
                 coord,
                 coord.properties_api.home,
                 map_info.map_flag,
@@ -55,13 +54,17 @@ class RoborockMap(RoborockCoordinatedEntityV1, ImageEntity):
     def __init__(
         self,
         config_entry: ConfigEntry,
-        unique_id: str,
         coordinator: RoborockDataUpdateCoordinator,
         home_trait: HomeTrait,
         map_flag: int,
         map_name: str,
     ) -> None:
         """Initialize a Roborock map."""
+        map_name = map_name or f"Map {map_flag}"
+        # Note: Map names are not a valid unique id since they can be changed
+        # in the roborock app. This should be migrated to use map flag for
+        # the unique id.
+        unique_id = f"{coordinator.duid_slug}_map_{map_name}"
         RoborockCoordinatedEntityV1.__init__(self, unique_id, coordinator)
         ImageEntity.__init__(self, coordinator.hass)
         self.config_entry = config_entry

tests/components/roborock/conftest.py

@@ -10,7 +10,7 @@
 from unittest.mock import AsyncMock, Mock, PropertyMock, patch
 
 import pytest
-from roborock import RoborockCategory
+from roborock import HomeDataRoom, MultiMapsListMapInfo, RoborockCategory
 from roborock.data import (
     CombinedMapInfo,
     DnDTimer,
@@ -199,6 +199,41 @@ async def set_dnd_timer(timer: DnDTimer) -> None:
     return dnd_trait
 
 
+def make_home_trait(
+    map_info: list[MultiMapsListMapInfo],
+    current_map: int | None,
+    room_mapping: dict[int, int],
+    rooms: list[HomeDataRoom],
+) -> AsyncMock:
+    """Create a mock roborock home trait."""
+    home_trait = make_mock_trait(trait_spec=HomeTrait)
+    home_map_info = {
+        map_data.map_flag: CombinedMapInfo(
+            name=map_data.name,
+            map_flag=map_data.map_flag,
+            rooms=[
+                NamedRoomMapping(
+                    segment_id=room_mapping[room.id],
+                    iot_id=room.id,
+                    name=room.name,
+                )
+                for room in rooms
+            ],
+        )
+        for map_data in map_info
+    }
+    home_map_content = {
+        map_data.map_flag: MapContent(
+            image_content=b"\x89PNG-001", map_data=deepcopy(MAP_DATA)
+        )
+        for map_data in map_info
+    }
+    home_trait.home_map_info = home_map_info
+    home_trait.current_map_data = home_map_info[current_map]
+    home_trait.home_map_content = home_map_content
+    return home_trait
+
+
 def create_v1_properties(network_info: NetworkInfo) -> AsyncMock:
     """Create v1 properties for each fake device."""
     v1_properties = AsyncMock(spec=PropertiesApi)
@@ -239,31 +274,12 @@ def create_v1_properties(network_info: NetworkInfo) -> AsyncMock:
     )
     v1_properties.wash_towel_mode = make_mock_trait(trait_spec=WashTowelModeTrait)
     v1_properties.smart_wash_params = make_mock_trait(trait_spec=SmartWashParamsTrait)
-    v1_properties.home = make_mock_trait(trait_spec=HomeTrait)
-    home_map_info = {
-        map_data.map_flag: CombinedMapInfo(
-            name=map_data.name,
-            map_flag=map_data.map_flag,
-            rooms=[
-                NamedRoomMapping(
-                    segment_id=ROOM_MAPPING[room.id],
-                    iot_id=room.id,
-                    name=room.name,
-                )
-                for room in HOME_DATA.rooms
-            ],
-        )
-        for map_data in MULTI_MAP_LIST.map_info
-    }
-    home_map_content = {
-        map_data.map_flag: MapContent(
-            image_content=b"\x89PNG-001", map_data=deepcopy(MAP_DATA)
-        )
-        for map_data in MULTI_MAP_LIST.map_info
-    }
-    v1_properties.home.home_map_info = home_map_info
-    v1_properties.home.current_map_data = home_map_info[STATUS.current_map]
-    v1_properties.home.home_map_content = home_map_content
+    v1_properties.home = make_home_trait(
+        map_info=MULTI_MAP_LIST.map_info,
+        current_map=STATUS.current_map,
+        room_mapping=ROOM_MAPPING,
+        rooms=HOME_DATA.rooms,
+    )
     v1_properties.network_info = make_mock_trait(
         trait_spec=NetworkInfoTrait,
         dataclass_template=network_info,

tests/components/roborock/mock_data.py

@@ -1153,6 +1153,29 @@
         ],
     }
 )
+MULTI_MAP_LIST_NO_MAP_NAMES = MultiMapsList.from_dict(
+    {
+        "maxMultiMap": 4,
+        "maxBakMap": 1,
+        "multiMapCount": 2,
+        "mapInfo": [
+            {
+                "mapFlag": 0,
+                "addTime": 1686235489,
+                "length": 0,
+                "name": "",
+                "bakMaps": [{"addTime": 1673304288}],
+            },
+            {
+                "mapFlag": 1,
+                "addTime": 1697579901,
+                "length": 0,
+                "name": "",
+                "bakMaps": [{"addTime": 1695521431}],
+            },
+        ],
+    }
+)
 
 MAP_DATA = MapData(0, 0)
 MAP_DATA.image = ImageData(

tests/components/roborock/test_image.py

@@ -7,7 +7,7 @@
 from unittest.mock import patch
 
 import pytest
-from roborock import RoborockException
+from roborock import MultiMapsList, RoborockException
 from roborock.data import RoborockStateCode
 from roborock.devices.traits.v1.map_content import MapContent
 
@@ -16,8 +16,15 @@
 from homeassistant.core import HomeAssistant
 from homeassistant.util import dt as dt_util
 
-from .conftest import FakeDevice
-from .mock_data import MAP_DATA
+from .conftest import FakeDevice, make_home_trait
+from .mock_data import (
+    HOME_DATA,
+    MAP_DATA,
+    MULTI_MAP_LIST,
+    MULTI_MAP_LIST_NO_MAP_NAMES,
+    ROOM_MAPPING,
+    STATUS,
+)
 
 from tests.common import MockConfigEntry, async_fire_time_changed
 from tests.typing import ClientSessionGenerator
@@ -161,3 +168,56 @@ async def test_map_status_change(
         body = await resp.read()
         assert body is not None
         assert body != old_body
+
+
+@pytest.mark.parametrize(
+    ("multi_maps_list", "expected_entity_ids"),
+    [
+        (
+            MULTI_MAP_LIST,
+            {
+                "image.roborock_s7_2_downstairs",
+                "image.roborock_s7_2_upstairs",
+                "image.roborock_s7_maxv_downstairs",
+                "image.roborock_s7_maxv_upstairs",
+            },
+        ),
+        (
+            MULTI_MAP_LIST_NO_MAP_NAMES,
+            {
+                "image.roborock_s7_2_downstairs",
+                "image.roborock_s7_2_upstairs",
+                # Expect default names based on map flags
+                "image.roborock_s7_maxv_map_0",
+                "image.roborock_s7_maxv_map_1",
+            },
+        ),
+    ],
+)
+async def test_image_entity_naming(
+    hass: HomeAssistant,
+    hass_client: ClientSessionGenerator,
+    mock_roborock_entry: MockConfigEntry,
+    fake_vacuum: FakeDevice,
+    multi_maps_list: MultiMapsList,
+    expected_entity_ids: set[str],
+) -> None:
+    """Test entity naming when no map name is set."""
+    # Override one of the vacuums multi map list response based on the
+    # test parameterization
+    assert fake_vacuum.v1_properties
+    fake_vacuum.v1_properties.home = make_home_trait(
+        map_info=multi_maps_list.map_info or [],
+        current_map=STATUS.current_map,
+        room_mapping=ROOM_MAPPING,
+        rooms=HOME_DATA.rooms,
+    )
+
+    # Setup the config entry
+    await hass.config_entries.async_setup(mock_roborock_entry.entry_id)
+    await hass.async_block_till_done()
+
+    # Verify the image entities are created with the expected names
+    assert {
+        state.entity_id for state in hass.states.async_all("image")
+    } == expected_entity_ids