fix: add rooms from map_info which is occassionally available #750
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds support for extracting room information from the map_info response, which some devices occasionally provide. The implementation merges room data from both map_info and the existing rooms_trait, with intelligent prioritization to preserve meaningful room names.
Changes:
- Added
MultiMapsListRoomdata container androomsfield toMultiMapsListMapInfoto capture room data from API responses - Modified
_refresh_map_infomethod to merge rooms frommap_infoandrooms_trait, preferring non-"Unknown" names - Added comprehensive test coverage for the new room merging logic
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| roborock/data/v1/v1_containers.py | Adds MultiMapsListRoom dataclass and rooms field to MultiMapsListMapInfo to support room data in map responses |
| roborock/devices/traits/v1/home.py | Implements room merging logic in _refresh_map_info to combine rooms from map_info and rooms_trait, preserving meaningful names |
| tests/devices/traits/v1/test_home.py | Adds comprehensive test for room override and addition logic covering all four scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Add the room to rooms if the room segment is not already in it | ||
| # or if the room name isn't unknown. |
There was a problem hiding this comment.
The comment is slightly confusing. It says "Add the room to rooms if the room segment is not already in it or if the room name isn't unknown" but this doesn't clarify that if both conditions are true (room exists AND name is "Unknown"), the room won't be added/overridden. Consider rewording to: "Add or override the room unless it already exists in rooms and the current name is 'Unknown'".
| # Add the room to rooms if the room segment is not already in it | |
| # or if the room name isn't unknown. | |
| # Add or override the room unless it already exists in rooms | |
| # and the current name is "Unknown". |
|
|
||
| rooms: dict[int, NamedRoomMapping] = {} | ||
| if map_info.rooms: | ||
| # Not all vacuums resopnd with rooms inside map_info. |
There was a problem hiding this comment.
Spelling error: "resopnd" should be "respond".
| # Not all vacuums resopnd with rooms inside map_info. | |
| # Not all vacuums respond with rooms inside map_info. |
|
Fixed tests - re-approve whenever you have a moment - no rush @allenporter |
Codecov Report❌ Patch coverage is
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
relates to: home-assistant/core#160812
Some devices seem to sometimes contain rooms in the map info response.
Use it unless the get_room_mapping is giving good data back. I think there might be a bigger issue at play, but without seeing the user's home data, I'm not sure I can figure it out. This is likely a good change regardless though and should add some more stability to room names (for the devices that support it)