Skip to content

Raise an issue when the Roborock local api is unavailable.#154576

Merged
allenporter merged 9 commits intohome-assistant:devfrom
Lash-L:roborock_cloud_api_issue
Oct 26, 2025
Merged

Raise an issue when the Roborock local api is unavailable.#154576
allenporter merged 9 commits intohome-assistant:devfrom
Lash-L:roborock_cloud_api_issue

Conversation

@Lash-L
Copy link
Copy Markdown
Contributor

@Lash-L Lash-L commented Oct 16, 2025

Breaking change

Proposed change

Right now we simply put a warning in the logs. I think a lot of users miss this and leave their integration in a less than ideal state. This hopefully will make it clear to them that they should make changes to their networking setup.

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:
  • Link to developer documentation pull request:
  • Link to frontend pull request:

Checklist

  • I understand the code I am submitting and can explain how it works.
  • 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.
  • Any generated code has been carefully reviewed for correctness and compliance with project standards.

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

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:

@home-assistant
Copy link
Copy Markdown

Hey there @allenporter, 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.

Comment thread homeassistant/components/roborock/strings.json Outdated
@frenck frenck requested a review from Copilot October 16, 2025 07:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR raises a Repairs issue when the Roborock integration falls back to the cloud API, guiding users to adjust their network setup to enable local access.

  • Add issue creation/deletion in the coordinator when local API fails/succeeds.
  • Add localized strings for the new issue.
  • Add tests verifying issue creation and cleanup when switching between cloud and local API.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
homeassistant/components/roborock/coordinator.py Creates a Repairs issue when the integration uses the cloud API and deletes it when local API is reachable again.
homeassistant/components/roborock/strings.json Adds title and description for the new Repairs issue.
tests/components/roborock/test_coordinator.py Adds a test to assert issue creation for cloud fallback and removal after returning to local API.

Comment on lines +176 to +179
with patch(
"homeassistant.components.roborock.coordinator.RoborockLocalClientV1.ping",
side_effect=RoborockException(),
):
Copy link

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

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

RoborockLocalClientV1.ping is awaited in the coordinator; patching it without an AsyncMock can cause await errors. Use an AsyncMock for the side effect and import AsyncMock: with patch('homeassistant.components.roborock.coordinator.RoborockLocalClientV1.ping', new=AsyncMock(side_effect=RoborockException()))

Copilot uses AI. Check for mistakes.
Comment on lines +193 to +195
with patch(
"homeassistant.components.roborock.coordinator.RoborockLocalClientV1.ping"
):
Copy link

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

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

This patch replaces an awaited coroutine with a regular MagicMock, which will raise 'object MagicMock can't be used in 'await' expression'. Patch with an AsyncMock that returns None: with patch('homeassistant.components.roborock.coordinator.RoborockLocalClientV1.ping', new=AsyncMock(return_value=None))

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think these are necessary, but if a core dev wants me to I will make these changes to AsyncMock

Comment thread tests/components/roborock/test_coordinator.py Outdated
Comment thread homeassistant/components/roborock/strings.json Outdated
Lash-L and others added 4 commits October 16, 2025 09:55
Co-authored-by: Josef Zweck <josef@zweck.dev>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@allenporter allenporter left a comment

Choose a reason for hiding this comment

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

This is a great change, thank you for doing this.

I think it would be useful to also have something in the integration documentation with additional detail about what this error means and how a user can fix it, since many users may not know practically what to do. Want to send a PR for that and add a more info link to the integration docs in the repair issue text?

Comment thread homeassistant/components/roborock/strings.json Outdated
@home-assistant home-assistant Bot marked this pull request as draft October 18, 2025 14:15
@home-assistant
Copy link
Copy Markdown

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.

@Lash-L Lash-L marked this pull request as ready for review October 26, 2025 00:52
@home-assistant home-assistant Bot requested a review from allenporter October 26, 2025 00:52
@Lash-L
Copy link
Copy Markdown
Contributor Author

Lash-L commented Oct 26, 2025

Almost certainly will conflict with #154837

@allenporter allenporter merged commit 3c46b40 into home-assistant:dev Oct 26, 2025
36 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants