Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use x,y in roborock action call #134133

Merged
merged 2 commits into from
Dec 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions homeassistant/components/roborock/services.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ set_vacuum_goto_position:
integration: roborock
domain: vacuum
fields:
x_coord:
x:
example: 27500
required: true
selector:
text:
type: number
y_coord:
y:
example: 32000
required: true
selector:
Expand Down
4 changes: 2 additions & 2 deletions homeassistant/components/roborock/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -433,11 +433,11 @@
"name": "Go to position",
"description": "Send the vacuum to a specific position.",
"fields": {
"x_coord": {
"x": {
"name": "X-coordinate",
"description": ""
},
"y_coord": {
"y": {
"name": "Y-coordinate",
"description": ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have an empty description? I think it would be valuable to have something here that tells you what this value is

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have a suggestion... but I did not see it, hence leaving it empty to avoid unnecessary translations.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be nice to explain what way it is, as in, higher X -> more to the front, higher Y -> go to the right? Or is that an incorrect assumption

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's all coordinates with the base station as reference... Maybe we should set something there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The x coordinate as seen from the base station of the vaccum"

I think something like this would also explain the user why it wouldn't work anymore if you moved the base station

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has been updated so let me see what you think 👍

}
Expand Down
8 changes: 4 additions & 4 deletions homeassistant/components/roborock/vacuum.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ async def async_setup_entry(
SET_VACUUM_GOTO_POSITION_SERVICE_NAME,
cv.make_entity_service_schema(
{
vol.Required("x_coord"): vol.Coerce(int),
vol.Required("y_coord"): vol.Coerce(int),
vol.Required("x"): vol.Coerce(int),
vol.Required("y"): vol.Coerce(int),
},
),
RoborockVacuum.async_set_vacuum_goto_position.__name__,
Expand Down Expand Up @@ -185,9 +185,9 @@ async def async_set_fan_speed(self, fan_speed: str, **kwargs: Any) -> None:
[self._device_status.get_fan_speed_code(fan_speed)],
)

async def async_set_vacuum_goto_position(self, x_coord: int, y_coord: int) -> None:
async def async_set_vacuum_goto_position(self, x: int, y: int) -> None:
"""Send vacuum to a specific target point."""
await self.send(RoborockCommand.APP_GOTO_TARGET, [x_coord, y_coord])
await self.send(RoborockCommand.APP_GOTO_TARGET, [x, y])

async def async_send_command(
self,
Expand Down
2 changes: 1 addition & 1 deletion tests/components/roborock/test_vacuum.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ async def test_goto(
vacuum = hass.states.get(ENTITY_ID)
assert vacuum

data = {ATTR_ENTITY_ID: ENTITY_ID, "x_coord": 25500, "y_coord": 25500}
data = {ATTR_ENTITY_ID: ENTITY_ID, "x": 25500, "y": 25500}
with patch(
"homeassistant.components.roborock.coordinator.RoborockLocalClientV1.send_command"
) as mock_send_command:
Expand Down
Loading