fix: report base-station maintenance as docked, not cleaning - #615
philjackson wants to merge 1 commit into
Conversation
RoboVacEntity.activity ends its status chain with a bare else that returns CLEANING, so any status string the integration does not recognise is reported as actively cleaning. damacus#598 hit this from the other side on T2258. On an X8 Pro SES (T2276) the station reports RollAutoCleaning while running its roller self-clean. The robot is parked in the dock throughout, but the entity sat at "cleaning" for around six hours. Captured from one poll while docked: 2 = False # not running a clean 15 = 'RollAutoCleaning' # STATUS 104 = 100 # battery full 109 = 0 # cleaning time zero 126 = {"dustCollect": {"state": "C"}, "rollAutoClean": {"switch": "ON", "state": "R"}} rollAutoClean.state "R" and dustCollect.state "C" are the station working; 2 == False and 109 == 0 are the robot not. Station housekeeping statuses now map to DOCKED. Matching is casefolded, in keeping with the case-insensitive device-response convention in DEVELOPMENT.md. Only RollAutoCleaning is confirmed against hardware. The other four names are plausible siblings for mop and dry docks and want confirmation from someone who has one. Unmatched strings keep the existing behaviour, so a wrong guess costs nothing. The catch-all default is deliberately left alone. Returning CLEANING for an unenumerated status is the worst available guess, since it silently breaks "is the vacuum running" automations, but changing it to None or IDLE would shift behaviour for every model at once and seems worth its own discussion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The mapping must be reachable for mapped entities, and an entity-level regression test is needed.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates vacuum activity reporting so base-station maintenance is reported as DOCKED rather than CLEANING.
Changes:
- Adds case-insensitive station maintenance status mapping.
- Adds unit tests for recognized and unknown statuses.
File summaries
| File | Summary | Review notes |
|---|---|---|
tests/test_vacuum/test_station_maintenance_activity.py |
Tests station maintenance mappings and case handling. | Nit: add an entity-level regression test. |
custom_components/robovac/vacuum.py |
Maps station housekeeping states to DOCKED. |
Moderate issue (2 votes): ensure the mapping is checked for entities with activity_mapping. |
Review details
Suppressed comments (1)
tests/test_vacuum/test_station_maintenance_activity.py:21
- These assertions exercise the helper in isolation, but not the new
RoboVacEntity.activitybranch at lines 629-637. An entity-level test withtuya_state = "RollAutoCleaning"would catch wiring or branch-order regressions; the helper tests would still pass if the call were unreachable (for example, behind an activity mapping).
assert _activity_from_station_maintenance(status) == VacuumActivity.DOCKED
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| station_activity = _activity_from_station_maintenance( | ||
| self._attr_tuya_state | ||
| ) | ||
| if station_activity is not None: | ||
| _LOGGER.debug( | ||
| "Station maintenance status %s - reporting docked", | ||
| self._attr_tuya_state, | ||
| ) | ||
| return station_activity |
There was a problem hiding this comment.
The flow claim is right, the helper doesn't get reached when a model has an activity map. But those models don't have the bug: the mapping branch returns None on a miss, not CLEANING, so there's nothing to rescue there. T2276 (the model in the capture) has no activity map and lands in the else, which is where I put the check.
RoboVacEntity.activity ends its status chain with a bare else that returns CLEANING, so any status string the integration does not recognise is reported as actively cleaning. #598 hit this from the other side on T2258.
On an X8 Pro SES (T2276) the station reports RollAutoCleaning while running its roller self-clean. The robot is parked in the dock throughout, but the entity sat at "cleaning" for around six hours. Captured from one poll while docked:
rollAutoClean.state "R" and dustCollect.state "C" are the station working; 2 == False and 109 == 0 are the robot not.
Station housekeeping statuses now map to DOCKED. Matching is casefolded, in keeping with the case-insensitive device-response convention in DEVELOPMENT.md.
Only RollAutoCleaning is confirmed against hardware. The other four names are plausible siblings for mop and dry docks and want confirmation from someone who has one. Unmatched strings keep the existing behaviour, so a wrong guess costs nothing.
The catch-all default is deliberately left alone. Returning CLEANING for an unenumerated status is the worst available guess, since it silently breaks "is the vacuum running" automations, but changing it to None or IDLE would shift behaviour for every model at once and seems worth its own discussion.