Ecovacs position sensor #117697
Ecovacs position sensor #117697
Conversation
|
Hey there @OverloadUT, @mib1185, @edenhaus, @Augar, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Hi @lnx85
thanks for your contribution, but i've concerns about these sensors and their intention. I think this functionality should be implemented as an integration service with response data.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
@mib1185 With deebot_client I can send the GetPos() command, but the related response is triggered as an event on the event_bus and it is not directly returned by the command call. |
Co-authored-by: Michael <35783820+mib1185@users.noreply.github.com>
|
What is the intention or use case for these sensors? |
|
Before this integration became part of the HA core, I was using the getPos custom command to get the bot coordinates and track its position on a map. You can also trigger automations based on bot position if you know room bounds. I can try implementing the same feature by using an Event entity. I'm also waiting for a PR on deebot_client library which includes the bot direction (0..360deg) that can be exported in addition to bot coordinates (x,y). |
|
i'm not familiar with the former custom component, nor these new ecovacs devices (still own a quiet old and "stupid" deebot slim 🙈) |
|
@mib1185 In my use case position is used to track the bot on a custom home map, not the original one. I can achieve the same goal if the custom_command is not limited to dict (GetPos command requires a list of args) and if custom_command response is available as it was in the previous version of the integration (when in was a custom_component and not in the HA core) |
With some changes in the dependency and HA ecovacs integration, it is possible to return the response for any command. |
|
Sensors removed, event entity added. There's one problem left: getPos command is called only at device init and if you want to update the position you have to call the command yourself. Do I need to write a new service? |
There was a problem hiding this comment.
Will discuss this PR internally if we should allow adding entities for any command or if they will be only available as service responses
I have the feeling that with this PR soon or later other commands will be added as entities (example the trace of the vacuum)
| async def on_event(event: PositionsEvent) -> None: | ||
| """Handle event.""" | ||
| event_data: dict[str, dict] = { | ||
| position.type.name.lower(): { |
There was a problem hiding this comment.
The positions in PositionsEvent are on purpose a list as someone could have multiple charges, which this entity currently does not support.
Please don't make any changes to this PR until I have discussed it internally and come back to you. So we don't waste your time ;)
|
I discussed it internally, and we don't accept the creation of entities for command responses as it will be used only in specific use cases, and it will lead to the entity creation for any command, which is something we don't want. To add support for command responses, changes in the library are required. Feel free to open a PR for it :) I will close this PR as we need to add support for it in the library first. |
@edenhaus I opened a new thread on deebot_client: DeebotUniverse/client.py#507 |
Breaking change
Proposed change
Add position sensors (last_position and charger_position)
Add update_positions command
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
Link to documentation PR: home-assistant/home-assistant.io#32831
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..coveragerc.To help with the load of incoming pull requests: