-
-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
Add support for vacuum cleaner and traits #15575
Conversation
I expected docs to be needed for this. |
Don't forget tests! |
Part of: home-assistant/architecture#29 |
|
||
attrs = self.state.attributes | ||
|
||
if attrs[vacuum.ATTR_STATUS] == vacuum.STATE_PAUSED: |
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.
To get it working:
if self.state.state == vacuum.STATE_PAUSED:
if attrs[vacuum.ATTR_STATUS] == vacuum.STATE_PAUSED: | ||
response['isPaused'] = True | ||
response['isRunning'] = False | ||
elif attrs[vacuum.ATTR_STATUS] == vacuum.STATE_CLEANING: |
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.
To get it working:
elif self.state.state == vacuum.STATE_CLEANING:
response = {} | ||
|
||
if domain == vacuum.DOMAIN: | ||
attrs = self.state.attributes |
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.
To get it working, I replaced the contents of the if block with:
if self.state.state == vacuum.STATE_DOCKED:
response['isDocked'] = True
else:
response['isDocked'] = False
"""Test if state is supported.""" | ||
if domain != vacuum.DOMAIN: | ||
return False | ||
|
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.
return features & vacuum.SUPPORT_STATE
Probably a good idea to check for SUPPORT_STATE here. That will exclude any platforms that haven't been ported yet
@arbreng if you want to do the last stretch here and add tests + documentation, feel free to do so, as I'm having a busy week coming up :-) (If you are up for it I can either give you write access to my fork, you can do a PR against my fork and I'll merge it asap or you can take this code and do a new PR). |
Sure, sounds good to me. I'll be able to finish this Friday, can also add Alexa support |
Alexa needs to be in a different PR please. |
service_domain = domain | ||
state = self.state.state | ||
|
||
if command == start_stop_command: |
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.
if command == COMMAND_STARTSTOP:
?
else: | ||
service = vacuum.SERVICE_STOP | ||
|
||
if command == pause_unpause_command: |
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.
COMMAND_PAUSEUNPAUSE
?
Can i be of any help with this PR? E.g. docs, tests... |
Review comments @balloob
Hi @JarnoNijboer, It seems you haven't yet signed a CLA. Please do so here. Once you do that we will be able to review and accept this pull request. Thanks! |
@JarnoNijboer I'm currently very busy, so if you want to move the PR forward by writing tests and docs, feel free to do so. :-) |
Working on writing tests for this unsucessfully. Having a fun time reverse engineering. |
@balloob is there a good document for how the test system works? Right now it's a lot of trial and error. |
As mentioned by @dshokouhi here is a similar PR with tests included: |
#17657 is a more complete implementation of this PR. |
Description:
Add support for vacuum cleaners in Google Assistant.
Added support for StartStopTrait and DockTrait.
Requires #15751 to be merged.
Checklist:
tox
. Your PR cannot be merged unless tests pass