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

Roborock Vacuum Issues #1

Closed
SLaks opened this issue Mar 17, 2024 · 7 comments
Closed

Roborock Vacuum Issues #1

SLaks opened this issue Mar 17, 2024 · 7 comments

Comments

@SLaks
Copy link
Contributor

SLaks commented Mar 17, 2024

I have an S7 (has auto empty and mop) and a Q Revo (also has water tank in base and dryer).

I tried this integration on my dev copy of HA with rytilahti/python-miio#1914.

However, the set of entities is completely wrong.

  • Each device shows the same set of 8 buttons, even though the S7 cannot wash or dry the mop (clicking those shows an error in the logs)
    image
  • Both vacuums show a status of Unknown
  • There are no sensors at all (for timing, errors, etc), unlike the current integration

How can I help fix these issues? I took a quick look at the code, but I'm not sure how the descriptors work.

I see the following log entries (each entry appears separate for each vacuum):

2024-03-17 16:42:39.970 ERROR (MainThread) [custom_components.xiaomi.device] Unable to find descriptor 'vacuum:status' for <RoborockVacuum: [elided]>
2024-03-17 16:42:39.970 ERROR (MainThread) [custom_components.xiaomi.device] Unable to find descriptor 'vacuum:mode' for <RoborockVacuum: [elided]>
2024-03-17 16:42:39.970 ERROR (MainThread) [custom_components.xiaomi.device] Unable to find descriptor 'battery:level' for <RoborockVacuum: [elided]>

Is there a way to specify which devices support an @action?

@SLaks
Copy link
Contributor Author

SLaks commented Mar 18, 2024

Looking at the code, I don't see anything wrong with the @sensors in the Roborock VacuumStatus.

Interestingly, the log shows Got new state for <RoborockVacuum [elided]>: (lots of state properties, including the three from the errors) immediately below those three errors.

@SLaks
Copy link
Contributor Author

SLaks commented Mar 18, 2024

It may make sense to split conditional features like mop-related commands/sensors into a separate embed() call, which can then be conditional on the model.

@rytilahti
Copy link
Owner

rytilahti commented Mar 18, 2024

Hi @SLaks and thanks for the issue report. Looks like the custom component & the python-miio got out of sync as part of some refactoring, causing several issues especially to the roborock integration which is the most complex one due to legacy reasons, sorry about that...

How can I help fix these issues? I took a quick look at the code, but I'm not sure how the descriptors work.

rytilahti/python-miio#1916 should fix most of the issues you encountered.

The entities were not showing up as the descriptors were not added to the device's descriptorcollection, the roborock status performs several queries and bundles them dynamically inside a single status container for easy API. The missing piece was to collect the descriptors from these individual "status" queries into the main one.

While doing that I realized that there are several other issues here and there, which should be now fixed by the linked PR and my most recent pushes to this repository.

Each device shows the same set of 8 buttons, even though the S7 cannot wash or dry the mop (clicking those shows an error in the logs)
Is there a way to specify which devices support an @action?

There is currently no way to do that, but we definitely should have one.

@rytilahti
Copy link
Owner

I just merged the PR, so if you restart your homeassistant, it should update from git and you should see the entities appearing. Let me know if it's working for you now.

On the conditional features, I'm thinking that it would probably make sense to move that functionality over to their respective modules and add the entities only when the query succeeds. This would allow avoiding hard-coding information about which models support which features with a bit of overhead during the initial update.

@SLaks
Copy link
Contributor Author

SLaks commented Mar 22, 2024

Yes; that fixed it.

@SLaks
Copy link
Contributor Author

SLaks commented Mar 22, 2024

(do you want to file a separate issue to track making entities conditional?)

@rytilahti
Copy link
Owner

Yes, it's better to have separate issues for separate concerns. Makes it easier to track progress and to keep an overview of to-dos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants