Skip to content

fix: support home discovery when there are no maps #809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
allenporter merged 2 commits into from
Apr 6, 2026
+23 −5
Merged

fix: support home discovery when there are no maps #809

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9188b2c
fix: support home discovery when there are no maps
allenporter Apr 5, 2026
ad89a0e
test: update home discovery test to verify refresh behavior and state…
allenporter Apr 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions roborock/devices/traits/v1/home.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,10 @@ async def discover_home(self) -> None:

await self._maps_trait.refresh()
if self._maps_trait.current_map_info is None:
raise RoborockException("Cannot perform home discovery without current map info")
_LOGGER.debug("Cannot perform home discovery without current map info")
self._discovery_completed = True
await self._update_home_cache({}, {})
return
Comment on lines +110 to +113
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

current_map_info is None can occur for reasons other than “no maps” (e.g., StatusTrait.current_map not refreshed / is None, or the current map flag not present in map_info). In those cases this path marks discovery as completed and persists an empty cache, which can mask a real precondition error and prevent refresh() from attempting full discovery later. Consider narrowing this branch to the explicit “no maps” case (e.g., self._maps_trait.multi_map_count == 0 and not self._maps_trait.map_info) and otherwise keep the previous failure behavior (raise) or leave _discovery_completed false and return without overwriting the cache.

Suggested change
_LOGGER.debug("Cannot perform home discovery without current map info")
self._discovery_completed = True
await self._update_home_cache({}, {})
return
if self._maps_trait.multi_map_count == 0 and not self._maps_trait.map_info:
_LOGGER.debug("No maps available for home discovery")
self._discovery_completed = True
await self._update_home_cache({}, {})
return
raise RoborockInvalidStatus("Cannot perform home discovery without current map info")

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@allenporter allenporter Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is worth changing.

Copy link
Copy Markdown
Collaborator

@Lash-L Lash-L Apr 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I agree?

Our cache should be able to handle going from None to new data I believe on any errors or anything


home_map_info, home_map_content = await self._build_home_map_info()
_LOGGER.debug("Home discovery complete, caching data for %d maps", len(home_map_info))
Expand Down Expand Up @@ -198,7 +201,8 @@ async def refresh(self) -> None:
if (current_map_info := self._maps_trait.current_map_info) is None or (
map_flag := self._maps_trait.current_map
) is None:
raise RoborockException("Cannot refresh home data without current map info")
_LOGGER.debug("Cannot refresh home data without current map info")
return

# Refresh the map content to ensure we have the latest image and object positions
new_map_content = await self._refresh_map_content()
Expand Down
20 changes: 17 additions & 3 deletions tests/devices/traits/v1/test_home.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,11 +367,25 @@ async def test_discover_home_no_maps(
"""Test discovery when no maps are available."""
# Setup mock to return empty maps list
mock_mqtt_rpc_channel.send_command.side_effect = [
[{"max_multi_map": 0, "max_bak_map": 0, "multi_map_count": 0, "map_info": []}]
# Discover home
[{"max_multi_map": 0, "max_bak_map": 0, "multi_map_count": 0, "map_info": []}],
# Refresh
[{"max_multi_map": 0, "max_bak_map": 0, "multi_map_count": 0, "map_info": []}],
]

with pytest.raises(Exception, match="Cannot perform home discovery without current map info"):
await home_trait.discover_home()
# Discover home should not change anything
await home_trait.discover_home()
assert home_trait.current_map_data is None
assert home_trait.home_map_info == {}
assert home_trait.home_map_content == {}
assert home_trait.current_rooms == []

# Refresh should not change anything
await home_trait.refresh()
assert home_trait.current_map_data is None
assert home_trait.home_map_info == {}
assert home_trait.home_map_content == {}
assert home_trait.current_rooms == []


async def test_refresh_updates_current_map_cache(
Expand Down
Loading