Roborock Add vacuum_goto service

[RaHehl]

Breaking change

Proposed change

Unlike the Xiaomi integration and the HACS Roborock integration, the Roborock integration lacks a goto service. I personally need it because my Roborock is under the stairs, and I have to drive it out to change the mop.

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request: roborock add vacuum_goto action home-assistant.io#36554

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• I have followed the perfect PR recommendations
• The code has been formatted using Ruff (ruff format homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io roborock add vacuum_goto action home-assistant.io#36554

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

Hey there @Lash-L, mind taking a look at this pull request as it has been labeled with an integration (roborock) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of roborock can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign roborock Removes the current integration label and assignees on the pull request, add the integration domain after the command.
• @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
• @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

[Lash-L]

Merry Christmas/ happy holidays! Looks good to me - but I'll defer to a core dev for a review

[allenporter]

Makes sense to have a command to send the robot vacuum to a specific place.

However, I could use some help understanding how users are expected to know the coordinate system. How do I as a user figure this out? The documentation is pretty minimal, and even then just wondering in practice how this will work. (e.g. do we also need to provide things like coordinates of the base, or current coordinates, to make this actually useful in automations)

[Lash-L]

Makes sense to have a command to send the robot vacuum to a specific place.

However, I could use some help understanding how users are expected to know the coordinate system. How do I as a user figure this out? The documentation is pretty minimal, and even then just wondering in practice how this will work. (e.g. do we also need to provide things like coordinates of the base, or current coordinates, to make this actually useful in automations)

@allenporter
I think this thread may answer your question. TLDR; not really easy right now but could be in the future. Dock is a set coordinates and some people guess and check
#133994 (comment)

[allenporter]

Makes sense to have a command to send the robot vacuum to a specific place.
However, I could use some help understanding how users are expected to know the coordinate system. How do I as a user figure this out? The documentation is pretty minimal, and even then just wondering in practice how this will work. (e.g. do we also need to provide things like coordinates of the base, or current coordinates, to make this actually useful in automations)

@allenporter I think this thread may answer your question. TLDR; not really easy right now but could be in the future. Dock is a set coordinates and some people guess and check #133994 (comment)

Of course :) sorry I missed this. Can get back to working with you on this now...

[Lash-L]

No rush! Appreciate your help on that one Allen. But I think to get the coordinates in a sane way requires some more refactoring so it doesn't make sense to refactor while refactoring. But it also doesn't make sense to make that pr even bigger!

Should this pr hold off until we get a means of displaying coordinates?

[allenporter]

Should this pr hold off until we get a means of displaying coordinates?

I would say yes. I don't think we should add services to consume coordinates when we haven't specified how coordinators work yet.

[RaHehl]

Please understand this as feedback and not as criticism: If anyone wants to understand why people prefer maintaining HACS duplicates of integrations rather than improving the proper core integrations (and I see this multiple times in my HA installation), they only need to look at the process here. I absolutely agree that we need something for coordinates, and the current method of guessing, triggering actions, and checking in the app where the pin landed is not ideal. However, please don’t forget that this is simply an exact replica of the behavior of the Xiaomi integration. This doesn’t motivate me as a contributor to submit further commits when such small additions are endlessly debated.

[Lash-L]

Please understand this as feedback and not as criticism: If anyone wants to understand why people prefer maintaining HACS duplicates of integrations rather than improving the proper core integrations (and I see this multiple times in my HA installation), they only need to look at the process here. I absolutely agree that we need something for coordinates, and the current method of guessing, triggering actions, and checking in the app where the pin landed is not ideal. However, please don’t forget that this is simply an exact replica of the behavior of the Xiaomi integration. This doesn’t motivate me as a contributor to submit further commits when such small additions are endlessly debated.

I understand the frustration- but 2 points

1. Just because it exists in the Xiaomi integration doesn't mean it is the best way to do it. HA has changed a lot since the Xiaomi integration was added. I've often talked a lot with the core devs about adding something to Roborock when Xiaomi does something similar, but the reality is best practices change, new functionality gets added to core, and old code doesn't always mean the best way to do something. That's why you got some feedback on the service to ensure it was the best for all users and took advantage of
2. I understand the frustration about waiting for something else, but the true reality is that if this got merged before users had a clear way to easily get coordinates, I would be bombarded by GitHub issues, forum posts, etc. all of users asking how to get the coordinates or struggling to guess and check. I'm the sole maintainer for the Roborock integration - with Allen helping me out when he can(but he has plenty of his own integrations that he maintains).

Those are just my thoughts!

[allenporter]

Please understand this as feedback and not as criticism: If anyone wants to understand why people prefer maintaining HACS duplicates of integrations rather than improving the proper core integrations (and I see this multiple times in my HA installation), they only need to look at the process here. I absolutely agree that we need something for coordinates, and the current method of guessing, triggering actions, and checking in the app where the pin landed is not ideal. However, please don’t forget that this is simply an exact replica of the behavior of the Xiaomi integration. This doesn’t motivate me as a contributor to submit further commits when such small additions are endlessly debated.

I understand the frustration- but 2 points

1. Just because it exists in the Xiaomi integration doesn't mean it is the best way to do it. HA has changed a lot since the Xiaomi integration was added. I've often talked a lot with the core devs about adding something to Roborock when Xiaomi does something similar, but the reality is best practices change, new functionality gets added to core, and old code doesn't always mean the best way to do something. That's why you got some feedback on the service to ensure it was the best for all users and took advantage of
2. I understand the frustration about waiting for something else, but the true reality is that if this got merged before users had a clear way to easily get coordinates, I would be bombarded by GitHub issues, forum posts, etc. all of users asking how to get the coordinates or struggling to guess and check. I'm the sole maintainer for the Roborock integration - with Allen helping me out when he can(but he has plenty of his own integrations that he maintains).

Those are just my thoughts!

Yes we have a higher bar here. This is working as intended. The goal isn't to just copy features, but actually solve the harder problem of making them usable for a broader audience which requires more design work.

We are all robock users here on this PR and motivated to add new features, and help you with contributing.

[RaHehl]

@Lash-L @allenporter I’ve now added a “Roborock: Get current position” service, which works perfectly for me without any delay. We can switch it over later once there’s a cached map in place; however, for this particular use case, the direct call might still be the optimal approach. You’ll have to decide that, though, since I’m not too familiar with the Roborock API.

[home-assistant]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

[RaHehl]

@gjohansson-ST The actions have now been renamed to get_vacuum_current_position and set_vacuum_goto_position. The descriptions have been adjusted, and the error behavior has been corrected. Thank you very much for your review!

[gjohansson-ST]

Thanks @RaHehl 👍