feat: use get_rooms to limit issues with missing room names

Author: Lash-L

roborock/devices/traits/v1/__init__.py

@@ -196,7 +196,7 @@ def __init__(
         self.rooms = RoomsTrait(home_data)
         self.maps = MapsTrait(self.status)
         self.map_content = MapContentTrait(map_parser_config)
-        self.home = HomeTrait(self.status, self.maps, self.map_content, self.rooms, self._device_cache)
+        self.home = HomeTrait(self.status, self.maps, self.map_content, self.rooms, self._device_cache, web_api)
         self.network_info = NetworkInfoTrait(device_uid, self._device_cache)
         self.routines = RoutinesTrait(device_uid, web_api)
 

roborock/devices/traits/v1/home.py

@@ -26,6 +26,7 @@
 from roborock.devices.traits.v1 import common
 from roborock.exceptions import RoborockDeviceBusy, RoborockException, RoborockInvalidStatus
 from roborock.roborock_typing import RoborockCommand
+from roborock.web_api import UserWebApiClient
 
 from .map_content import MapContent, MapContentTrait
 from .maps import MapsTrait
@@ -49,6 +50,7 @@ def __init__(
         map_content: MapContentTrait,
         rooms_trait: RoomsTrait,
         device_cache: DeviceCache,
+        web_api: UserWebApiClient | None = None,
     ) -> None:
         """Initialize the HomeTrait.
 
@@ -71,6 +73,8 @@ def __init__(
         self._map_content = map_content
         self._rooms_trait = rooms_trait
         self._device_cache = device_cache
+        self._web_api = web_api
+        self._seen_room_iot_ids_by_map: dict[int, set[str]] = {}
         self._discovery_completed = False
         self._home_map_info: dict[int, CombinedMapInfo] | None = None
         self._home_map_content: dict[int, MapContent] | None = None
@@ -90,6 +94,7 @@ async def discover_home(self) -> None:
         if device_cache_data and device_cache_data.home_map_info:
             _LOGGER.debug("Home cache already populated, skipping discovery")
             self._home_map_info = device_cache_data.home_map_info
+            self._seed_seen_room_iot_ids(device_cache_data.home_map_info)
             self._discovery_completed = True
             try:
                 self._home_map_content = {
@@ -138,6 +143,40 @@ async def _refresh_map_info(self, map_info) -> CombinedMapInfo:
                         # or if the room name isn't unknown.
                         rooms[room.segment_id] = room
 
+        # If we discovered at least one new unknown room iot_id for this map, fetch names from web API.
+        seen_room_iot_ids = self._seen_room_iot_ids_by_map.setdefault(map_info.map_flag, set())
+        has_new_unknown_room_iot_id = any(
+            room.name == "Unknown" and room.iot_id not in seen_room_iot_ids for room in rooms.values()
+        )
+        if self._web_api and has_new_unknown_room_iot_id:
+            try:
+                web_rooms = await self._web_api.get_rooms()
+                web_room_names = {str(r.id): r.name for r in web_rooms}
+                for segment_id, room in rooms.items():
+                    if room.name == "Unknown" and room.iot_id in web_room_names:
+                        rooms[segment_id] = NamedRoomMapping(
+                            segment_id=room.segment_id,
+                            iot_id=room.iot_id,
+                            name=web_room_names[room.iot_id],
+                        )
+                # Merge new rooms into home_data so future refreshes benefit
+                if web_rooms:
+                    self._rooms_trait.merge_home_data_rooms(web_rooms)
+            except Exception:
+                # Broad exception as we don't want anything here to make us fail upwards, we are okay with 'unknowns'
+                _LOGGER.debug("Failed to fetch rooms from web API for map %s", map_info.map_flag)
+
+        # Replace remaining "Unknown" names with "Room {room_id}" fallback
+        for segment_id, room in rooms.items():
+            if room.name == "Unknown":
+                rooms[segment_id] = NamedRoomMapping(
+                    segment_id=room.segment_id,
+                    iot_id=room.iot_id,
+                    name=f"Room {room.segment_id}",
+                )
+
+        self._seen_room_iot_ids_by_map[map_info.map_flag].update(room.iot_id for room in rooms.values())
+
         return CombinedMapInfo(
             map_flag=map_info.map_flag,
             name=map_info.name,
@@ -254,6 +293,7 @@ async def _update_home_cache(
         await self._device_cache.set(device_cache_data)
         self._home_map_info = home_map_info
         self._home_map_content = home_map_content
+        self._seed_seen_room_iot_ids(home_map_info)
 
     async def _update_current_map(
         self,
@@ -282,7 +322,13 @@ async def _update_current_map(
         if self._home_map_info is None:
             self._home_map_info = {}
         self._home_map_info[map_flag] = map_info
+        self._seed_seen_room_iot_ids({map_flag: map_info})
 
         if self._home_map_content is None:
             self._home_map_content = {}
         self._home_map_content[map_flag] = map_content
+
+    def _seed_seen_room_iot_ids(self, home_map_info: dict[int, CombinedMapInfo]) -> None:
+        """Seed known room iot_ids from cached/current map information."""
+        for map_flag, map_info in home_map_info.items():
+            self._seen_room_iot_ids_by_map.setdefault(map_flag, set()).update(room.iot_id for room in map_info.rooms)

roborock/devices/traits/v1/rooms.py

@@ -3,7 +3,7 @@
 import logging
 from dataclasses import dataclass
 
-from roborock.data import HomeData, NamedRoomMapping, RoborockBase
+from roborock.data import HomeData, HomeDataRoom, NamedRoomMapping, RoborockBase
 from roborock.devices.traits.v1 import common
 from roborock.roborock_typing import RoborockCommand
 
@@ -55,6 +55,16 @@ def _parse_response(self, response: common.V1ResponseData) -> Rooms:
             ]
         )
 
+    def merge_home_data_rooms(self, rooms: list[HomeDataRoom]) -> None:
+        """Merge newly discovered rooms into home data by room id."""
+        existing_ids = {room.id for room in self._home_data.rooms or ()}
+        updated_rooms = list(self._home_data.rooms or ())
+        for room in rooms:
+            if room.id not in existing_ids:
+                updated_rooms.append(room)
+                existing_ids.add(room.id)
+        self._home_data.rooms = updated_rooms
+
 
 def _extract_segment_pairs(response: list) -> list[tuple[int, str]]:
     """Extract segment_id and iot_id pairs from the response.

roborock/web_api.py

@@ -544,10 +544,10 @@ async def get_rooms(self, user_data: UserData, home_id: int | None = None) -> li
             rriot.r.a,
             self.session,
             {
-                "Authorization": _get_hawk_authentication(rriot, "/v2/user/homes/" + str(home_id)),
+                "Authorization": _get_hawk_authentication(rriot, f"/user/homes/{home_id}/rooms"),
             },
         )
-        room_response = await room_request.request("get", f"/user/homes/{str(home_id)}/rooms" + str(home_id))
+        room_response = await room_request.request("get", f"/user/homes/{home_id}/rooms")
         if not room_response.get("success"):
             raise RoborockException(room_response)
         rooms = room_response.get("result")
@@ -752,6 +752,10 @@ async def get_routines(self, device_id: str) -> list[HomeDataScene]:
         """Fetch routines (scenes) for a specific device."""
         return await self._web_api.get_scenes(self._user_data, device_id)
 
+    async def get_rooms(self) -> list[HomeDataRoom]:
+        """Fetch rooms using the API client."""
+        return await self._web_api.get_rooms(self._user_data)
+
     async def execute_routine(self, scene_id: int) -> None:
         """Execute a specific routine (scene) by its ID."""
         await self._web_api.execute_scene(self._user_data, scene_id)

tests/devices/traits/v1/test_home.py

@@ -6,7 +6,7 @@
 
 import pytest
 
-from roborock.data.containers import CombinedMapInfo, NamedRoomMapping
+from roborock.data.containers import CombinedMapInfo, HomeDataRoom, NamedRoomMapping
 from roborock.data.v1.v1_code_mappings import RoborockStateCode
 from roborock.data.v1.v1_containers import MultiMapsListMapInfo, MultiMapsListRoom
 from roborock.devices.cache import DeviceCache, DeviceCacheData, InMemoryCache
@@ -139,16 +139,25 @@ def rooms_trait(device: RoborockDevice) -> RoomsTrait:
     return device.v1_properties.rooms
 
 
+@pytest.fixture
+def mock_web_api() -> AsyncMock:
+    """Create a mock web API client."""
+    mock = AsyncMock()
+    mock.get_rooms.return_value = []
+    return mock
+
+
 @pytest.fixture
 def home_trait(
     status_trait: StatusTrait,
     maps_trait: MapsTrait,
     map_content_trait: MapContentTrait,
     rooms_trait: RoomsTrait,
     device_cache: DeviceCache,
+    mock_web_api: AsyncMock,
 ) -> HomeTrait:
     """Create a HomeTrait instance with mocked dependencies."""
-    return HomeTrait(status_trait, maps_trait, map_content_trait, rooms_trait, device_cache)
+    return HomeTrait(status_trait, maps_trait, map_content_trait, rooms_trait, device_cache, mock_web_api)
 
 
 @pytest.fixture(autouse=True)
@@ -227,7 +236,7 @@ async def test_discover_home_empty_cache(
     assert map_123_data.rooms[0].segment_id == 18
     assert map_123_data.rooms[0].name == "Example room 3"
     assert map_123_data.rooms[1].segment_id == 19
-    assert map_123_data.rooms[1].name == "Unknown"  # Not in mock home data
+    assert map_123_data.rooms[1].name == "Map 123"  # Not in mock home data
 
     map_123_content = home_trait.home_map_content[123]
     assert map_123_content is not None
@@ -620,6 +629,7 @@ async def test_single_map_no_switching(
 async def test_refresh_map_info_room_override_and_addition_logic(
     home_trait: HomeTrait,
     rooms_trait: RoomsTrait,
+    mock_web_api: AsyncMock,
 ) -> None:
     """Test the room override and addition logic in _refresh_map_info.
 
@@ -660,6 +670,9 @@ async def test_refresh_map_info_room_override_and_addition_logic(
         NamedRoomMapping(segment_id=18, iot_id="2362041", name="Example room 3"),  # Not in map_info, should be added
     ]
 
+    # Mock web API to return empty rooms (no resolution)
+    mock_web_api.get_rooms.return_value = []
+
     # Mock rooms_trait.refresh to prevent actual device calls
     with patch.object(rooms_trait, "refresh", new_callable=AsyncMock):
         result = await home_trait._refresh_map_info(map_info)
@@ -676,9 +689,9 @@ async def test_refresh_map_info_room_override_and_addition_logic(
     assert sorted_rooms[0].name == "Kitchen from map_info"
     assert sorted_rooms[0].iot_id == "2362048"
 
-    # Room 17: from rooms_trait with "Unknown", added because not in map_info
+    # Room 17: from rooms_trait with "Unknown", falls back to "Map 0"
     assert sorted_rooms[1].segment_id == 17
-    assert sorted_rooms[1].name == "Unknown"
+    assert sorted_rooms[1].name == "Map 0"
     assert sorted_rooms[1].iot_id == "2362044"
 
     # Room 18: from rooms_trait with valid name, added because not in map_info
@@ -690,3 +703,163 @@ async def test_refresh_map_info_room_override_and_addition_logic(
     assert sorted_rooms[3].segment_id == 19
     assert sorted_rooms[3].name == "Updated Bedroom Name"
     assert sorted_rooms[3].iot_id == "2362042"
+
+
+async def test_get_rooms_resolves_unknown_rooms(
+    home_trait: HomeTrait,
+    rooms_trait: RoomsTrait,
+    mock_web_api: AsyncMock,
+) -> None:
+    """Test that get_rooms from web API resolves unknown room names."""
+    map_info = MultiMapsListMapInfo(
+        map_flag=0,
+        name="Test Map",
+        rooms=[
+            MultiMapsListRoom(id=16, iot_name_id="2362048", iot_name=None),
+            MultiMapsListRoom(id=17, iot_name_id="2362044", iot_name=None),
+        ],
+    )
+
+    rooms_trait.rooms = [
+        NamedRoomMapping(segment_id=16, iot_id="2362048", name="Unknown"),
+        NamedRoomMapping(segment_id=17, iot_id="2362044", name="Unknown"),
+    ]
+
+    # Web API returns fresh room names
+    mock_web_api.get_rooms.return_value = [
+        HomeDataRoom(id=2362048, name="Living Room"),
+        HomeDataRoom(id=2362044, name="Kitchen"),
+    ]
+
+    with patch.object(rooms_trait, "refresh", new_callable=AsyncMock):
+        result = await home_trait._refresh_map_info(map_info)
+
+    sorted_rooms = sorted(result.rooms, key=lambda r: r.segment_id)
+    assert sorted_rooms[0].name == "Living Room"
+    assert sorted_rooms[1].name == "Kitchen"
+    mock_web_api.get_rooms.assert_called_once()
+
+
+async def test_get_rooms_called_once_for_same_unknown_room(
+    home_trait: HomeTrait,
+    rooms_trait: RoomsTrait,
+    mock_web_api: AsyncMock,
+) -> None:
+    """Test that get_rooms is not re-called for an already-seen unknown room."""
+    map_info = MultiMapsListMapInfo(
+        map_flag=42,
+        name="Test Map",
+        rooms=[
+            MultiMapsListRoom(id=16, iot_name_id="2362048", iot_name=None),
+        ],
+    )
+
+    rooms_trait.rooms = [
+        NamedRoomMapping(segment_id=16, iot_id="2362048", name="Unknown"),
+    ]
+
+    # Web API returns empty (no resolution)
+    mock_web_api.get_rooms.return_value = []
+
+    with patch.object(rooms_trait, "refresh", new_callable=AsyncMock):
+        result1 = await home_trait._refresh_map_info(map_info)
+        result2 = await home_trait._refresh_map_info(map_info)
+
+    # Both calls should fall back to "Map 42"
+    assert result1.rooms[0].name == "Map 42"
+    assert result2.rooms[0].name == "Map 42"
+    # get_rooms should only be called once for the same unknown room
+    mock_web_api.get_rooms.assert_called_once()
+
+
+async def test_get_rooms_called_again_for_new_unknown_room(
+    home_trait: HomeTrait,
+    rooms_trait: RoomsTrait,
+    mock_web_api: AsyncMock,
+) -> None:
+    """Test that get_rooms is called again when a new unknown room appears."""
+    map_info = MultiMapsListMapInfo(
+        map_flag=42,
+        name="Test Map",
+        rooms=[
+            MultiMapsListRoom(id=16, iot_name_id="2362048", iot_name=None),
+        ],
+    )
+
+    rooms_trait.rooms = [
+        NamedRoomMapping(segment_id=16, iot_id="2362048", name="Unknown"),
+    ]
+
+    # Web API returns empty (no resolution)
+    mock_web_api.get_rooms.return_value = []
+
+    with patch.object(rooms_trait, "refresh", new_callable=AsyncMock):
+        result1 = await home_trait._refresh_map_info(map_info)
+
+        # Add a brand-new unknown room for the same map
+        rooms_trait.rooms = [
+            NamedRoomMapping(segment_id=16, iot_id="2362048", name="Unknown"),
+            NamedRoomMapping(segment_id=17, iot_id="2362044", name="Unknown"),
+        ]
+        result2 = await home_trait._refresh_map_info(map_info)
+
+    assert sorted(room.name for room in result1.rooms) == ["Map 42"]
+    assert sorted(room.name for room in result2.rooms) == ["Map 42", "Map 42"]
+    assert mock_web_api.get_rooms.call_count == 2
+
+
+async def test_get_rooms_called_again_for_new_unknown_iot_id_same_segment(
+    home_trait: HomeTrait,
+    rooms_trait: RoomsTrait,
+    mock_web_api: AsyncMock,
+) -> None:
+    """Test that get_rooms is called again for a new unknown iot_id on the same segment."""
+    map_info = MultiMapsListMapInfo(
+        map_flag=42,
+        name="Test Map",
+    )
+
+    rooms_trait.rooms = [
+        NamedRoomMapping(segment_id=16, iot_id="2362048", name="Unknown"),
+    ]
+
+    mock_web_api.get_rooms.return_value = []
+
+    with patch.object(rooms_trait, "refresh", new_callable=AsyncMock):
+        await home_trait._refresh_map_info(map_info)
+
+        # Same segment, but now with a different iot_id.
+        rooms_trait.rooms = [
+            NamedRoomMapping(segment_id=16, iot_id="2362999", name="Unknown"),
+        ]
+        await home_trait._refresh_map_info(map_info)
+
+    assert mock_web_api.get_rooms.call_count == 2
+
+
+async def test_get_rooms_failure_falls_back_to_map_flag(
+    home_trait: HomeTrait,
+    rooms_trait: RoomsTrait,
+    mock_web_api: AsyncMock,
+) -> None:
+    """Test that get_rooms failure gracefully falls back to 'Map {flag}'."""
+    map_info = MultiMapsListMapInfo(
+        map_flag=7,
+        name="Test Map",
+        rooms=[
+            MultiMapsListRoom(id=16, iot_name_id="2362048", iot_name=None),
+        ],
+    )
+
+    rooms_trait.rooms = [
+        NamedRoomMapping(segment_id=16, iot_id="2362048", name="Unknown"),
+    ]
+
+    # Web API raises an exception
+    mock_web_api.get_rooms.side_effect = Exception("API error")
+
+    with patch.object(rooms_trait, "refresh", new_callable=AsyncMock):
+        result = await home_trait._refresh_map_info(map_info)
+
+    assert result.rooms[0].name == "Map 7"
+    mock_web_api.get_rooms.assert_called_once()