Roborock Add vacuum_clean_rooms service #134148
Conversation
|
Hey there @Lash-L, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
|
doc pr follows soon |
| vacuum_clean_rooms: | ||
| target: | ||
| entity: | ||
| integration: roborock | ||
| domain: vacuum | ||
| fields: | ||
| room_ids: | ||
| example: 1,3,4 | ||
| required: true | ||
| selector: | ||
| text: | ||
| repeat: | ||
| example: 1 | ||
| required: false | ||
| selector: | ||
| number: | ||
| min: 1 | ||
| max: 3 |
There was a problem hiding this comment.
@allenporter There isn't a way that this could be dynamic is there? We have individual room names and that would be a lot better for the user, but if I'm not mistaken it isn't really possible to do dynamic fields in Actions.
There was a problem hiding this comment.
I've already asked myself the question, due to the usability, the potential usability in Homekit/Matter (I think vaccums are supported from upcomming iOS 18.3) and the usability in HA Voice Assistant (which I haven't looked into yet) In a perfect world, this would also correspond to the rooms in HomeAssistant
There was a problem hiding this comment.
I believe I tried this in the past and I came up short and decided I would wait for Multiselect entities but they are indefinitely on hold now.
| vol.Required("room_ids"): vol.Any( | ||
| vol.Coerce(int), [vol.Coerce(int)], vol.Coerce(str) | ||
| ), |
There was a problem hiding this comment.
I am a bit unsure if it works but could we maybe use the SelectSelector here and construct it with the proper values and room names?
There was a problem hiding this comment.
Similar to the discussion about coordinates, how are users expected to know how to use rooms? These room number ids won't really be usable right?
This feels like we're bypassing some of the discussion in home-assistant/architecture#913 and home-assistant/architecture#950 missing an opportunity to figure out a user friendly way to surface rooms.
That's why the proposal above to use a |
Even though the IDs are essentially random, I still think the way to obtain them is reasonably clean. "description": "Ids of the Rooms to clean, separated by comma. (retrieve the ids with the service get_maps)" The numbers shouldn’t change very often anyway. I haven’t found anything in the core code about using SelectSelector in actions. Overall, I agree with the usability argument and think this approach makes sense. Correct me if I’m wrong, but doesn’t this action appear under a section labeled “Developer Tools”? If so, it seems perfectly fine to fetch an ID via one service and then use it in another. However, if this is meant for a scenario where an everyday user would run the service, it should be more encapsulated—so they can simply say “clean room XY,” and that could be handled behind the scenes, perhaps via a button on a dashboard. |
|
I was thinking a bit more and the schema can not really be dynamic and the selections are robot specific so it would not work with my proposal. |
|
I was thinking along those lines as well. My idea was to check if the value is a string (after splitting by the comma) and then do a translation from the room name to an integer. However, the plan was to wait until the maps are really cached, and then add this feature. That wouldn't be a breaking change either. |
|
I don't see the logic about that. We might as well just make a call to get the maps and then use that information. Any caching for the map can come later (as I don't think these will be used extensively and therefore not burden the system really). It should also not be a text field (with comma separation) but a multi text or a select with custom input. |
Both will need to be available so I think extend is the key word. As unnamed rooms can still be cleaned via an id.
The coordinator should hold all this info in self.maps or self.maps info I believe. I have all of the map ids and the corresponding rooms. Should be trivial to get a room name from an id with that, I believe I even do it in the image component. I'm not able to check right now as I'm on mobile, but I believe logic exists |
with latest commit it accepts ids names and mixes of both |
Breaking change
Proposed change
adds an action/service to clean rooms
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: