Add Roborock dock control buttons and mop wash mode select#163373
Add Roborock dock control buttons and mop wash mode select#163373iainswarts wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
Hi @iainswarts
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
There was a problem hiding this comment.
Pull request overview
Adds dock control entities to the Roborock integration (3 dock action buttons + a mop wash mode select), gated by supported device traits.
Changes:
- Add dock command button entities (empty, wash mop, stop drying) under the dock sub-device.
- Add a wash towel / mop wash mode select entity under the dock sub-device.
- Extend tests and test fixtures to cover new entities and default wash mode.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/components/roborock/test_button.py | Adds tests for new dock command buttons (success/failure + trait gating). |
| tests/components/roborock/conftest.py | Updates fake v1 properties with a default wash towel mode. |
| homeassistant/components/roborock/strings.json | Adds translations for new dock buttons and mop wash mode select states. |
| homeassistant/components/roborock/select.py | Introduces select description for wash towel (mop wash) mode. |
| homeassistant/components/roborock/button.py | Adds dock command button descriptions/entities and wires them into setup. |
| async def async_press(self) -> None: | ||
| """Send the dock command.""" | ||
| await self.send(self.entity_description.api_command) |
There was a problem hiding this comment.
ButtonEntity.async_press is commonly implemented with an **kwargs: Any signature (as you already do for RoborockRoutineButtonEntity). Keeping the same signature here avoids potential TypeError if the button service ever passes keyword args through to async_press. Suggest changing this to async def async_press(self, **kwargs: Any) -> None:.
| "light": "[%key:component::roborock::entity::select::dust_collection_mode::state::light%]", | ||
| "balanced": "[%key:component::roborock::entity::vacuum::roborock::state_attributes::fan_speed::state::balanced%]", | ||
| "deep": "[%key:component::roborock::entity::select::mop_mode::state::deep%]", | ||
| "smart": "[%key:component::roborock::entity::select::dust_collection_mode::state::smart%]" |
There was a problem hiding this comment.
These state labels are pulled from several unrelated translation paths (dust collection mode, vacuum fan speed, mop mode). While it deduplicates strings, it also tightly couples this select’s UX text to other entities’ translation structures and semantics. Consider defining the four labels directly under mop_wash_mode.state (or referencing a more clearly “shared/common” key if available) to avoid future breakage when other entity translation keys change.
| "light": "[%key:component::roborock::entity::select::dust_collection_mode::state::light%]", | |
| "balanced": "[%key:component::roborock::entity::vacuum::roborock::state_attributes::fan_speed::state::balanced%]", | |
| "deep": "[%key:component::roborock::entity::select::mop_mode::state::deep%]", | |
| "smart": "[%key:component::roborock::entity::select::dust_collection_mode::state::smart%]" | |
| "light": "Light", | |
| "balanced": "Balanced", | |
| "deep": "Deep", | |
| "smart": "Smart" |
|
No |
|
|
|
The PR description mentions tests for buttons but doesn't explicitly call out select tests. The conftest update adds a default |
joostlek
left a comment
There was a problem hiding this comment.
The pre commit checks are failing, can you take a look?
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
| async def test_press_dock_command_button_success( | ||
| hass: HomeAssistant, | ||
| setup_entry: MockConfigEntry, | ||
| entity_id: str, | ||
| expected_command: RoborockCommand, | ||
| fake_vacuum: FakeDevice, | ||
| ) -> None: |
There was a problem hiding this comment.
For consistency with other tests in this file that use the setup_entry fixture, these tests should include the bypass_api_client_fixture parameter. This fixture patches the API client during integration setup to avoid making real API calls. While the tests may work without it due to mocking, including it ensures consistency with the existing test patterns in test_button.py (see test_update_success, test_update_failure, and test_press_routine_button_success).
| if api.wash_towel_mode is not None | ||
| else None | ||
| ), | ||
| parameter_lambda=lambda key, _: [RoborockDockWashTowelModeCode.as_dict()[key]], |
There was a problem hiding this comment.
The parameter_lambda should use .get(key) instead of [key] for consistency with the dust_collection_mode select entity on line 133 and for defensive programming. While Home Assistant's select entity framework should prevent invalid keys from being passed, using dictionary .get() provides better error handling and consistency across the codebase.
| parameter_lambda=lambda key, _: [RoborockDockWashTowelModeCode.as_dict()[key]], | |
| parameter_lambda=lambda key, _: [ | |
| RoborockDockWashTowelModeCode.as_dict().get(key) | |
| ], |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
89c67eb to
69b570b
Compare
| RoborockSelectDescription( | ||
| key="wash_towel_mode", | ||
| translation_key="mop_wash_mode", | ||
| api_command=RoborockCommand.SET_WASH_TOWEL_MODE, | ||
| value_fn=lambda api: ( | ||
| mode.name | ||
| if api.wash_towel_mode is not None | ||
| and (mode := api.wash_towel_mode.wash_mode) is not None | ||
| else None | ||
| ), | ||
| entity_category=EntityCategory.CONFIG, | ||
| options_lambda=lambda api: ( | ||
| RoborockDockWashTowelModeCode.keys() | ||
| if api.wash_towel_mode is not None | ||
| else None | ||
| ), | ||
| parameter_lambda=lambda key, _: [ | ||
| RoborockDockWashTowelModeCode.as_dict().get(key) | ||
| ], | ||
| is_dock_entity=True, |
There was a problem hiding this comment.
parameter_lambda uses RoborockDockWashTowelModeCode.as_dict().get(key), which can return None if a service call provides an unsupported option. Since RoborockSelectEntity.async_select_option currently forwards whatever it receives without validating against self.options, this can result in sending [None] to the API. Consider validating option against the available options (and raising ServiceValidationError, similar to the A01 select entity) and/or making the lookup strict so unsupported options cannot be sent.
There was a problem hiding this comment.
This follows the same pattern as dust_collection_mode directly above (line 167), which also uses .get(key). Changing one without the other would introduce inconsistency. Happy to update both if a maintainer prefers strict lookup, but that feels like a separate cleanup.
allenporter
left a comment
There was a problem hiding this comment.
I saw another test problem?
Delete 6 unused snapshot entries (button.zeo_one, _2, _3) that were generated during an earlier snapshot update when entity naming was broken. The correctly-named snapshots (_pause, _shutdown, _start) remain. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Hopefully its all working now, I tried running all the tests I could locally but couldn't perfectly replicate the CI system set up in GitHub (this is also my first OSS contribution, so excuse my clumsiness) |
|
Hey @iainswarts - congrats on your first contribution and thank you! I think if possible, let's hold off on the mop wash select entity. That doesn't actually cover all of the modes as they are dynamic per device. I am working on this on the library side here: Python-roborock/python-roborock#765 but i don't have it done yet. Would you mind removing the select entity and I can ping you when I get this finished on the library side? Alternatively, you can just wait until I get it done on the library side and keep it in this PR. What are your thoughts? Edit: will likely get it done early this week - a few extra things to still figure out |
Lash-L noted the wash towel modes are dynamic per device, not a fixed enum. Removing select entity until python-roborock#765 lands with proper per-device mode support. Dock buttons retained. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Happy to follow up with re-adding it after your changes have landed :) |
| }, | ||
| "button": { | ||
| "dock_empty": { | ||
| "name": "Empty" |
There was a problem hiding this comment.
What does empty mean? Dock Empty but what does it empty? Maybe this is obvious as I don't have the device, but now it sounds very ambiguous
There was a problem hiding this comment.
I think it means to start emptying the robot vaccum's dust bin into the dock's dust bin. So "Empty" as a verb meaning empty the robots dust bin. The other name is to start dust connection.
Looking at some videos online, the app has a button that says "Empty" and the device speaks "Empty Dust Bin" so maybe that would be a better name?
There was a problem hiding this comment.
Yeah, I have a roborock with dock and when it is docked, I can push the button in the roborock app on my phone and it empties the dustbin of the robot into the dock (which is much much bigger). So it would Empty the robot into the dock
| }, | ||
| "button": { | ||
| "dock_empty": { | ||
| "name": "Empty" |
There was a problem hiding this comment.
I think it means to start emptying the robot vaccum's dust bin into the dock's dust bin. So "Empty" as a verb meaning empty the robots dust bin. The other name is to start dust connection.
Looking at some videos online, the app has a button that says "Empty" and the device speaks "Empty Dust Bin" so maybe that would be a better name?
|
Hey @iainswarts - The cutoff to be included in 2026.4/the beta is this Wednesday the 25th. Looks like there are a few outstanding comments as well as merge conflicts. I know you said this was your first contribution and I'm sure you're looking for the feature in your HA instance, so I figured I would let you know as if it is not done by (typically early) Wednesday, the earliest you will see it in HA is 2026.5 |
Review: Add Roborock dock control buttons and mop wash mode selectNice feature addition — the dock control buttons are a welcome complement to the existing Roborock integration. A few things I noticed: 🔴 Blocker: Unrelated Zeo snapshot regressionsThe PR includes significant snapshot changes for Zeo entities that are unrelated to the dock control feature:
These are breaking changes for existing Zeo users — entity IDs change, friendly names become generic, automations break. This looks like a translation/naming regression, likely from a rebase or dependency bump. Please revert the unrelated snapshot changes and address them in a separate PR if needed. 🟡 Missing error handling in
|
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
Proposed change
Exposes Roborock dock controls as Home Assistant entities, matching the three actions visible in the Roborock app's "Dock Control" section, plus a mop wash mode select.
New button entities (under the dock sub-device):
button.*_dock_empty— triggers auto-empty of the dustbin (APP_START_COLLECT_DUST)button.*_dock_wash_mop— triggers mop washing (APP_START_WASH)button.*_dock_stop_drying— stops the air-drying cycle (APP_STOP_WASH)New select entity (under the dock sub-device):
select.*_mop_wash_mode— sets the wash towel mode (light/balanced/deep/smart)All entities are only created when the device hardware supports the relevant trait (
dust_collection_modeorwash_towel_mode).Type of change
Additional information
APP_START_COLLECT_DUST,APP_START_WASH,APP_STOP_WASH,SET_WASH_TOWEL_MODE) are already present in python-roborock v4.14.0.smart_wash_params(wash interval + smart wash toggle) is intentionally left out — setting it requires sending both fields together, which does not fit the existing number/switch patterns cleanly. Can be a follow-up PR.Checklist
pytest tests/components/roborock/)ruff format homeassistant tests)