home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
71.11k stars 29.79k forks source link

Xiaomi Miio: Roborock s7: mop attached attribute incorrect #51361

Closed OGKevin closed 2 years ago

OGKevin commented 3 years ago

The problem

The Mop attached attribute for Roborock s7 is set to true even though the mop is not attached. This is because the s7's mob is no longer attached to the water tank. Hence the following code is incorrect for s7:

https://github.com/home-assistant/core/blob/1d9d9021deed31337910a4a8d80feb665d33d0b0/homeassistant/components/xiaomi_miio/vacuum.py#L343

This code needs to be modified for the s7 and introduce another attribute "Water box attached". Would such change be welcoming?

What is version of Home Assistant Core has the issue?

core-2021.5.5

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Core

Integration causing the issue

Xiaomi Miio

Link to integration documentation on our website

https://www.home-assistant.io/integrations/xiaomi_miio

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

OGKevin commented 3 years ago

I'm happy to work on this issue :)

probot-home-assistant[bot] commented 3 years ago

Hey there @rytilahti, @syssi, @starkillerog, mind taking a look at this issue as its been labeled with an integration (xiaomi_miio) you are listed as a codeowner for? Thanks! (message by CodeOwnersMention)

OGKevin commented 3 years ago

@rytilahti will this be fixed with https://github.com/home-assistant/core/pull/52366 then?

jbouwh commented 3 years ago

@rytilahti will this be fixed with #52366 then? PR #52366 is only addressing Humidifiers, no other devices.

OGKevin commented 3 years ago

@jbouwh alright. Will the Humidifiers rework cause conflicts if I do a vacuum rework? If so, then ill wait until you are finished or work on top of your work.

jbouwh commented 3 years ago

@jbouwh alright. Will the Humidifiers rework cause conflicts if I do a vacuum rework? If so, then ill wait until you are finished or work on top of your work.

No it should not. But if you are changing the switch, sensor platforms you could beter wait before you start. This update will not give a problem.

rytilahti commented 3 years ago

I'm not sure about if refactoring vacuum will require any changes to switch platform (potential candidates for that would be things like "reset dirty status for X" that are not currently implemented?), but there will be plenty of sensors as most if not all state attributes (https://github.com/home-assistant/core/blob/dev/homeassistant/components/xiaomi_miio/vacuum.py#L313) should be sensors.

OGKevin commented 3 years ago

https://github.com/rytilahti/python-miio/releases/tag/0.5.7 just got released which contains the needed patch. So this can be picked up, shall I take this up?

So basically what i did in the closed PR + refactoring to sensors?

rytilahti commented 3 years ago

@OGKevin feel free to start converting vacuum attributes to sensors following the example of other sensors :-) I have a rockrobo gen 1 so I can assist with testing.

Blakout007 commented 2 years ago

Do you know aanything about Who can add mopping features for e.g. Roborock S7?