[endoftheline818]

Some vacuums show "NO MAP" even though a saved map is already selected. Selecting that same map again restores the image. This change performs that recovery automatically when the vacuum is idle or charging, without interrupting cleaning or a return to the dock.

The fix belongs in HomeTrait, as requested in home-assistant/core#186678:

• Load the known selected map before fetching its image during single-map discovery and normal refresh. Reuse the existing map-loading delay, and avoid loading a map twice during multi-map discovery.
• Read status again after the map-loading delay. The immediate status read inside set_current_map() can still report map_present == 0 while the map is loading, causing another load command on the next refresh.
• Defer discovery while the device is busy, or while a cached home needs rediscovery and the device reports no current map. Translate a refused reload into RoborockDeviceBusy and let refresh read the current map without repeating the rejected command.
• Keep "NO MAP" placeholders out of the saved map cache and preserve the last good map during a temporary empty response. Placeholders cached by older versions are excluded from memory and replaced after discovery successfully runs again. While discovery is blocked (device busy or no current map), it stays pending and the saved cache is left unchanged, so the missing map is rediscovered on the first later refresh that can run it.
• During discovery, copy cached map metadata and filter parsed content into separate dictionaries, leaving the saved cache unchanged while rediscovery is pending.

Fixes #985. Related: home-assistant/core#185133 and home-assistant/core#186678.

Validation:

• The added recovery, rediscovery and status tests reproduce the failures before their fixes and pass afterward.
• Full Linux test suite on Python 3.11: 1,650 passed, 53 expected failures, 11 real-broker tests excluded by the default configuration; 99 snapshots passed.
• Full Linux test suite on Python 3.14: 1,650 passed, 53 expected failures, 11 real-broker tests excluded by the default configuration; 99 snapshots passed.
• On Python 3.11, all pre-commit checks passed, including Ruff, mypy and codespell, and the wheel and source distribution builds passed.

Tests use simulated device responses. A physical Q5 Pro has not been tested with this change.

AI assistance: OpenAI Codex and Anthropic Claude Code helped prepare, review, and test this change under the contributor's direction.

[swastiktawate]

This is a thorough fix. The recovery path is properly guarded — idle/charging states only, never while cleaning or returning — and the fallback to recover_missing=False on a busy device means a locked action degrades gracefully instead of failing the whole refresh. Filtering placeholder 'NO MAP' images out of both the in-memory and persisted cache is the right call, and the guard matrix plus the exactly-once reload assertion in the tests show the edge cases were thought through.

[Lash-L]

Automated code review by Gemini 4 Argon in Antigravity using our /review (/code-review) skill (.agents/skills/code-review/SKILL.md / AGENTS.md).

Summary

Well-structured fix for #985 that keeps missing-map recovery and placeholder filtering encapsulated within HomeTrait, avoids redundant LOAD_MULTI_MAP calls during multi-map discovery, and degrades gracefully when map switching is locked (RoborockInvalidStatus -> RoborockDeviceBusy).

• Layer 1 — Public Trait APIs & Consumer Contracts: Preserves the public HomeTrait API (discover_home(), refresh(), home_map_info, home_map_content) while broadening RoborockDeviceBusy handling when the vacuum cannot switch maps.
• Layer 2 — Data Lifecycle & State Management: Filters empty placeholder maps (_has_map_image) before updating both in-memory _home_map_content and persistent DeviceCache, preserving the last known good map across transient empty responses.
• Layer 5 — Tests & Fixtures: Comprehensive parameterized coverage in tests/devices/traits/v1/test_home.py covering state/flag guards, action-locked fallback, placeholder eviction, and single- vs. multi-map discovery.

A few lifecycle and state-handling items noted inline:

1. Include RoborockStateCode.charger_disconnected (and check in_fresh_state / is_locating) in _MAP_LOADING_STATES (roborock/devices/traits/v1/home.py#L39): State 2 (charger_disconnected) is the V1 sleeping / idle-off-dock state when the robot is not on the charger and no task is active (in_fresh_state == 1). Excluding it causes discover_home() and missing-map recovery to treat a sleeping/idle off-dock robot as busy.
2. set_current_map() Refreshes StatusTrait Before MAP_SLEEP (roborock/devices/traits/v1/home.py#L178-L181): MapsTrait.set_current_map() sends LOAD_MULTI_MAP and immediately calls self._status_trait.refresh() before await asyncio.sleep(MAP_SLEEP). If the vacuum does not flip map_present to 1 until the MAP_SLEEP delay completes, self._status_trait.map_present remains 0 in memory and can trigger another reload on the next HomeTrait.refresh() if StatusTrait is not refreshed again.
3. Evicting Cached Placeholders in discover_home() Without Re-Discovering (roborock/devices/traits/v1/home.py#L112-L122): Stripping placeholder entries from home_map_content_base64 and returning early with self._discovery_completed = True leaves non-active maps on multi-map devices permanently unpopulated (since refresh() only refreshes the active map once _discovery_completed is True). Also, setting self._discovery_completed = True before the cache-parsing try block leaks _discovery_completed = True if cache parsing fails and the subsequent busy check raises RoborockDeviceBusy.

[Lash-L]

Include Off-Dock Idle / Sleeping State (RoborockStateCode.charger_disconnected) and Guard Against is_locating

On V1 devices, state 2 (RoborockStateCode.charger_disconnected) represents the sleeping / idle-off-dock state when the vacuum is stationary off the dock with no task running (in_fresh_state == 1, in_cleaning == 0, in_returning == 0). Excluding RoborockStateCode.charger_disconnected from _MAP_LOADING_STATES means a vacuum sitting idle/sleeping off the charger will raise RoborockDeviceBusy during discover_home() and never recover a missing map.

Additionally, in _refresh_map_content() / discover_home(), this should also avoid loading a map when self._status_trait.is_locating == 1 (or when self._status_trait.in_fresh_state is not None and self._status_trait.in_fresh_state != 1), as the robot invalidates the current map ID while locating.

Suggested change

	_MAP_LOADING_STATES = (RoborockStateCode.idle, RoborockStateCode.charging, RoborockStateCode.charging_complete)
	_MAP_LOADING_STATES = (
	RoborockStateCode.charger_disconnected,
	RoborockStateCode.idle,
	RoborockStateCode.charging,
	RoborockStateCode.charging_complete,
	)

[endoftheline818]

Thanks for reviewing. I’ve left state 2 and the additional status guards unchanged for now. Could you share device diagnostics or protocol references supporting state 2 as safe for map loading and clarify the expected in_fresh_state and is_locating values

[Lash-L]

Cached Placeholder Cleanup Skips Re-Discovery for Missing Maps & Sets _discovery_completed Too Early

Two subtle lifecycle issues here:

1. self._discovery_completed = True is set on line 102 before the try block. If async_parse_map_content raises (ValueError, RoborockException) and the fallback discovery then raises RoborockDeviceBusy at line 129, self._discovery_completed remains True even though discovery failed and self._home_map_content is {}.
2. When an older version cached a "NO MAP" placeholder for a non-active floor on a multi-map device, filtering it out of self._home_map_content and device_cache_data.home_map_content_base64 and returning early with self._discovery_completed = True means that floor's map will never be fetched (since subsequent refresh() calls only update current_map_info). Similarly, for a single-map device, discover_home() returns with self._home_map_content == {} until a separate refresh() is called.

Consider only returning early when all maps in self._home_map_info still have valid map content (or re-running discovery when cached entries had to be evicted), and moving self._discovery_completed = True into the else: branch after cache validation succeeds.

[endoftheline818]

Addressed in fdc71a8 and ed7b28e. Cached placeholders now trigger rediscovery while good cached images remain available. Discovery stays pending if the robot is busy or no current map is available, and the saved cache remains unchanged until discovery completes. Regression tests cover busy-to-idle recovery and recovery after a current map becomes available.

[Lash-L]

StatusTrait.refresh() Inside set_current_map() Runs Before MAP_SLEEP

self._maps_trait.set_current_map(...) sends LOAD_MULTI_MAP and immediately calls await self._status_trait.refresh() before await asyncio.sleep(MAP_SLEEP) here. As modeled in test_multi_map_discovery_does_not_reload_twice, the immediate GET_STATUS response right after LOAD_MULTI_MAP can still report map_present == 0 while the vacuum is loading the map over MAP_SLEEP.

If a caller runs home_trait.refresh() again without an intervening status_trait.refresh(), self._status_trait.map_present will still be 0 and _refresh_map_content() will issue LOAD_MULTI_MAP + sleep(MAP_SLEEP) again even though the map is now loaded. Refreshing self._status_trait after await asyncio.sleep(MAP_SLEEP) (or skipping recovery when self._home_map_content already holds a valid image for current_map_info.map_flag unless map_present == 0 was freshly observed) avoids redundant reloads.

[endoftheline818]

Addressed in fdc71a8. Both single-map recovery and multi-map discovery now refresh status again after the map-loading delay. Tests model the immediate status reporting map_present == 0, the later read reporting 1, and a subsequent refresh sending no additional load command.

[endoftheline818]

Updated the PR description with the follow-up fixes and final Linux validation results. I've also replied to all three review threads. Hosted CI for ed7b28e is awaiting maintainer approval. Could you approve the workflow run when convenient?

[allenporter]

Automated code review by Gemini 4 Argon in Antigravity using our code-review skill and AGENTS.md.

---

Thanks for digging into #985 / home-assistant/core#185133 and iterating on the earlier feedback!

Problem Acknowledgment

The underlying issue you are targeting is definitely real and belongs in HomeTrait: V1 vacuums can wake from a reboot or sleep with a valid selected map (current_map / GET_MULTI_MAPS_LIST) while reporting map_present == 0 in GET_STATUS, causing GET_MAP_V1 to return an empty 0x0 "NO MAP" image (map_data.image.is_empty). Today on main, HomeTrait skips LOAD_MULTI_MAP during single-map discovery and overwrites previously valid maps in both memory and DeviceCache with "NO MAP" placeholders.

Issues with the Current Implementation (ed7b28e)

Stepping back to look at the full lifecycle across polls, restarts, and downstream callers, the current state machine introduces several regressions and edge-case traps:

1. Cache-completeness check (len(self._home_map_content) == len(cached_content)) locks out rediscovery across restarts (home.py#L118-L123, #L340-L348):

   • If any floor returns a placeholder during discover_home(), _update_home_cache() saves all floors in home_map_info while omitting the placeholder floor from home_map_content_base64.
   • On the next restart, cached_content only contains the non-empty floor(s) (or {}), so len(self._home_map_content) == len(cached_content) evaluates True (1 == 1 or 0 == 0), sets _discovery_completed = True, and permanently skips rediscovery of the missing floor.
   • Note that several cached=True test cases in test_home.py (#L700-L705, #L791-L795, #L849-L853) seed home_map_info with home_map_content_base64 = None and only pass because 0 == 0 treats a mapless cache as complete.

2. Repeated discover_home() calls clobber live in-memory maps and re-parse cached maps on every poll (home.py#L103-L129, #L257-L266):

   • While _discovery_completed is False (e.g., a cached placeholder while the vacuum is cleaning), every refresh() poll calls discover_home(), which unconditionally overwrites self._home_map_info and self._home_map_content back to the stale disk cache—wiping out live maps fetched on earlier busy polls and spawning asyncio.to_thread PIL renders for every cached floor on every poll. When the robot later goes idle, line 118 again overwrites live in-memory maps with older cached copies right before line 151's fallback merge.
   • Even after _discovery_completed is True, discover_home() has no if self._discovery_completed: return guard. Callers that run await discover_home(); await refresh() (such as Home Assistant's coordinator.update_map()) re-decode and re-parse all cached maps on every update—and re-run full physical map switching when NoCache (the default in DeviceManager) is used.

3. Deleted-map wedge, asymmetric current_map_info is None handling, and corrupt-cache data loss (home.py#L109-L146):

   • Lines 138–142 (if self._maps_trait.current_map_info is None: if keep_cached_home: raise RoborockDeviceBusy(...)) conflate "no current map selected yet (current_map is None)" with "all maps were deleted on the vacuum (map_info == [])". If a user deletes all maps on an idle vacuum while DeviceCache holds a legacy placeholder, discover_home() raises RoborockDeviceBusy forever instead of clearing the stale cache. Conversely, when keep_cached_home is False and saved maps exist (map_info non-empty) while current_map is temporarily None (map_status == 252), lines 144–145 mark _discovery_completed = True and write {} to cache.
   • Also note that callers like Home Assistant's coordinator.update_map() call await discover_home() before await refresh() without catching RoborockDeviceBusy separately; raising RoborockDeviceBusy on cached-placeholder homes across all non-idle states aborts update_map() before refresh() can update the active map during cleaning/returning/paused states.
   • In except (ValueError, RoborockException): (#L114, which should also catch TypeError to match MapContentTrait.refresh()), keep_cached_home remains False (so a subsequent current_map_info is None wipes valid persisted home_map_info with {}), and parsing inside a single dict comprehension discards all valid cached floors when only one floor fails to parse.

4. Unbounded LOAD_MULTI_MAP loop when map_present stays 0, plus double reloads on manual map switches (home.py#L178-L207, maps.py#L80-L84):

   • If a device continues reporting map_present == 0 after LOAD_MULTI_MAP + MAP_SLEEP (modeled in test_multi_map_discovery_does_not_reload_twice, or on an unmapped floor slot), every subsequent refresh() will issue LOAD_MULTI_MAP (over cloud-only MQTT) + sleep(3) + GET_STATUS indefinitely, with no per-map retry guard and even when _home_map_content already holds a valid image.
   • Patching around MapsTrait.set_current_map()'s immediate status.refresh() inside HomeTrait._wait_for_map_load() leaves external callers of MapsTrait.set_current_map() (e.g., Home Assistant's map select entity, which runs set_current_map() → sleep(MAP_SLEEP) → home.refresh()) with a stale map_present == 0, triggering a second LOAD_MULTI_MAP + 3s sleep on every manual map switch, and doubling GET_STATUS RPCs during multi-map discovery.
   • When discover_home() raises RoborockDeviceBusy after refreshing _maps_trait (e.g., -10007 action locked), refresh() immediately calls _maps_trait.refresh() (GET_MULTI_MAPS_LIST over cloud MQTT) a second time (#L269).

5. Omitting placeholders when no prior image exists desynchronizes home_map_info and home_map_content (home.py#L340, #L383-L384):

   • Keeping placeholders out of persistent DeviceCache and never letting a placeholder overwrite an existing valid map in _home_map_content makes sense. However, dropping map_content from _home_map_content even when map_flag has no prior image leaves map_flag in home_map_info while absent from home_map_content, causing downstream consumers (such as Home Assistant's RoborockMap.async_image()) to raise map_not_found errors instead of serving the rendered "NO MAP" PNG until a real map is available.

6. Concurrency, mid-discovery state changes, and V1 state guards (home.py#L39, #L215-L247):

   • Both discover_home() and refresh() can now switch maps on the device and yield across 3s+ MAP_SLEEP windows without an asyncio.Lock (risking cross-floor cache contamination if calls overlap) and without re-checking idle state between loop iterations in _build_home_map_info().
   • Regarding the earlier thread question on V1 status flags: RoborockStateCode.charger_disconnected (2) is the standard V1 stationary/idle-off-dock state when the robot is off the charging contacts with no active task (in_cleaning == 0, in_returning == 0; mapped to VacuumActivity.IDLE in Home Assistant and present in V1 protocol/sensor snapshots), so excluding it blocks recovery whenever a vacuum rests off-dock or its dock loses power. Conversely, is_locating == 1 (active positioning) and is_exploring == 1 should block map loading.

7. Test mocks & sizing (test_home.py):

   • test_missing_selected_map_recovery (#L726-L731) returns MAP_BYTES_RESPONSE_2 (a valid map) rather than EMPTY_MAP_BYTES prior to LOAD_MULTI_MAP.
   • test_placeholder_rediscovery_waits_for_current_map (#L1088-L1090) simulates "no current map" using multi_map_count: 0, map_info: [] (all maps deleted) rather than map_status = 252 (NO_MAP = 63 → current_map is None) with saved maps present.
   • V1VacuumSimulator in roborock.testing on main now supports serving V1 maps directly (see tests/devices/traits/v1/test_map_content.py), which could replace much of the custom TwoFloorDevice harness (+590 lines).

---

Suggested Simpler Alternative

Rather than managing a multi-branch keep_cached_home rediscovery state machine, this can likely be solved in ~35–45 lines with much simpler invariants:

1. One-time cache hydration & fast-path in discover_home():

   • Serialize discover_home() / refresh() with an asyncio.Lock and return immediately at the top of discover_home() if self._discovery_completed is True.
   • Hydrate DeviceCache at most once (self._cache_loaded), parsing home_map_content_base64 per floor (catching (TypeError, ValueError, RoborockException) per entry) and merging beneath any newer in-memory entries.
   • Mark self._discovery_completed = True from cache iff self._home_map_info is non-empty and every map_flag in self._home_map_info has a _has_map_image entry in self._home_map_content.
   • If not _can_load_map(): when self._home_map_info is already populated from cache, return early (leaving self._discovery_completed = False so rediscovery runs once idle, without breaking await discover_home(); await refresh() callers while busy); only raise RoborockDeviceBusy when no cached home exists yet.

2. Preserve in-memory fallback while keeping DeviceCache placeholder-free:

   • Never persist not _has_map_image(content) entries to device_cache_data.home_map_content_base64.
   • In memory (self._home_map_content), never overwrite an existing _has_map_image entry with a placeholder, but still store the placeholder if map_flag not in self._home_map_content so every floor in home_map_info always has renderable PNG bytes.

3. Bounded, single-shot missing-map recovery inside _refresh_map_content():

   • Track self._attempted_map_reload: set[int] (cleared when map_present == 1 is observed) so a device or unmapped slot whose map_present stays 0 only triggers LOAD_MULTI_MAP once per missing state rather than on every poll.
   • Define _can_load_map() as state in (charger_disconnected, idle, charging, charging_complete) and not (in_cleaning, in_returning, is_locating, is_exploring).
   • In _refresh_map_content(), if recover_missing and map_present == 0 and _can_load_map() and current_map_info hasn't been attempted yet: record its map_flag, call set_current_map(), wait MAP_SLEEP + refresh status, and if RoborockInvalidStatus is raised, log debug and fall through directly to await self._map_content.refresh() (avoiding the exception bounce and duplicate calls in refresh()).

4. Clean map_info vs. current_map_info check in discover_home():

   • After await self._maps_trait.refresh(), if not self._maps_trait.map_info (genuinely zero maps on the vacuum), clear the cache (_update_home_cache({}, {})) and set _discovery_completed = True.
   • Otherwise, if self._maps_trait.current_map_info is None (saved maps exist, none currently active), defer discovery without wiping the cache.