-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
Restore status attribute for xiaomi_vacuum #16366
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
Conversation
This break my automation differentiate between spot cleaning and full cleaning https://community.home-assistant.io/t/xiaomi-vacuum-spot-cleaning-state/66998 |
@@ -68,6 +68,11 @@ | |||
STATE_PAUSED = STATE_PAUSED | |||
STATE_RETURNING = 'returning' | |||
STATE_ERROR = 'error' | |||
STATE_REMOTE = 'remote controlled' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes to the entity model need to be discussed and cleared in an issue in our architecture repo.
https://developers.home-assistant.io/docs/en/entity_index.html#changing-the-entity-model
This is the current one about vaccum:
home-assistant/architecture#29
But that one is more or less ready already.
@MartinHjelmare the original PR #15643 doesn't even mention that "status" attr. will be removed. If you look at the commit messages or the PR description itself, you have like 0 chance to figure out what will happen. |
Start by commenting in the existing arch issue. If someone thinks a new issue should be opened, they can say so. We don't discuss in closed PRs. |
I'm not sure whether it makes sense to add more "generic states" (maybe it does), but I'll just say that IMO the 'raw status' from the device should be somehow accessible to the user. |
@rytilahti as there is a call for spot cleaning in the generic vacuum class, I think having a STATE_SPOT_CLEANING is a must. Also, having a "STATE_UPDATING" could come in handy because you really don't want to start a cleaning during an update process. However, I agree that these changes should go through the arch. ticket, so I'll revert every STATE_* modifications and only restore the original "status" call for the xiaomi_vacuum. This will ensure, that we won't break any real-world automation and gives us enough time to do a proper migration to STATE_*. |
@MartinHjelmare removed all the changes new STATE_* stuff and restored the "status" attribute for xiaomi_vacuum. This gives us some time to discuss STATE_* stuff in home-assistant/architecture#29 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
I just want to say thanks, I was planning to add the status back when I had time, as I can see that a lot of people were having problems with the changes. :-) |
…cuum
Description:
Pre #15643 xiaomi_vacuum component provided specific states ( like "Going to target", "Zoned cleanup" ).
As a side effect of the mentioned PR, device state got simplified, having the same general string "Cleaning" for many specific actions ( like zoned cleanup, spot cleanup and going to target).
This breaks existing automation, which relies on a specific state of the device and limits the toolset for doing a complex script.
I think trying to standardize the vacuum states is a good idea, but I strongly advise against making such a change without depr. warning.
This PR contains two things related to the vacuum component:
Checklist:
tox
. Your PR cannot be merged unless tests passIf the code communicates with devices, web services, or third-party tools:
REQUIREMENTS
variable (example).