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

Added Dock and Pause/Unpause support for vacuum through google assistant #17657

Merged
merged 1 commit into from Oct 26, 2018

Conversation

Projects
None yet
9 participants
@mariuszluciow
Contributor

mariuszluciow commented Oct 21, 2018

Description:

Not a python developer, also my first contribution to the backend - please be thorough reviewing the changes. I've added to the google assistant support:

  • ability to dock the vacuum,
  • ability to start/stop/pause the vacuum

Code was tested locally trough ngrok setup, I can use my voice to manipulate the vacuum. Also customisation of types allows to choose precise icons and better intent recognition.

Related issue (if applicable): #17659, #17660, #17662, #17663, #17664

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#6989

Example entry for configuration.yaml (if applicable):

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

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

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@mariuszluciow mariuszluciow force-pushed the mariuszluciow:google-assistant/vacuum-dock branch from 8dbbf8c to af21001 Oct 21, 2018

@mariuszluciow mariuszluciow force-pushed the mariuszluciow:google-assistant/vacuum-dock branch from af21001 to d5f6dec Oct 21, 2018

@mariuszluciow mariuszluciow changed the title from [WIP] Added dock support for vacuum through google assistant to WIP Added dock support for vacuum through google assistant Oct 21, 2018

@mariuszluciow mariuszluciow force-pushed the mariuszluciow:google-assistant/vacuum-dock branch from d5f6dec to e62b6a8 Oct 21, 2018

@mariuszluciow mariuszluciow force-pushed the mariuszluciow:google-assistant/vacuum-dock branch from e62b6a8 to bb17688 Oct 21, 2018

@mariuszluciow mariuszluciow force-pushed the mariuszluciow:google-assistant/vacuum-dock branch from bb17688 to 2742cd0 Oct 21, 2018

@mariuszluciow mariuszluciow changed the title from WIP Added dock support for vacuum through google assistant to WIP Added dock support for vacuum through google assistant and adjusted the integration Oct 21, 2018

@mariuszluciow mariuszluciow force-pushed the mariuszluciow:google-assistant/vacuum-dock branch from 2742cd0 to 930a419 Oct 21, 2018

@mariuszluciow mariuszluciow force-pushed the mariuszluciow:google-assistant/vacuum-dock branch from 930a419 to 4dc97a1 Oct 21, 2018

@mariuszluciow mariuszluciow referenced this pull request Oct 21, 2018

Merged

Added docs for vacuums #6989

2 of 2 tasks complete

@mariuszluciow mariuszluciow force-pushed the mariuszluciow:google-assistant/vacuum-dock branch 2 times, most recently from f33f46e to cd261f3 Oct 21, 2018

@mariuszluciow mariuszluciow force-pushed the mariuszluciow:google-assistant/vacuum-dock branch from cd261f3 to 99d8539 Oct 21, 2018

@home-assistant home-assistant deleted a comment from homeassistant Oct 21, 2018

@mariuszluciow mariuszluciow force-pushed the mariuszluciow:google-assistant/vacuum-dock branch 2 times, most recently from 45d4e67 to d20b21e Oct 21, 2018

@MartinHjelmare

This comment has been minimized.

Member

MartinHjelmare commented Oct 24, 2018

Sorry, I'm not into the google assistant integration. Someone that knows about that should review.

@bachya

This comment has been minimized.

Contributor

bachya commented Oct 24, 2018

I'm not a Google Assistant user, either. Sorry!

@dshokouhi

This comment has been minimized.

Contributor

dshokouhi commented Oct 24, 2018

It seems this PR is trying to do the same thing that an earlier PR did that is still in progress. Should we consider combining this? The other PR was waiting on tests but this one seems to have it all completed.

#15575 is the PR I am referring to

@mariuszluciow

This comment has been minimized.

Contributor

mariuszluciow commented Oct 25, 2018

@dshokouhi That's true, although there is an extra type for fans and ability to specify custom type for google (mostly for fan vs airfilter and outlet vs swich, which are different types in google while HASS treats them as one)

@mariuszluciow mariuszluciow referenced this pull request Oct 25, 2018

Closed

Add support for vacuum cleaner and traits #15575

2 of 2 tasks complete
@cnrd

This comment has been minimized.

Contributor

cnrd commented Oct 25, 2018

You can just close my PR and merge this instead, I don't have the time to finish my PR.

@mariuszluciow you should probably split this into two different PRs, as this seems to change stuff for multiple types of devices at once, which makes it harder to follow the changes.

@@ -107,6 +110,9 @@ def sync_serialize(self):
if not traits:
return None
device_type = (entity_config.get(CONF_TYPE) or

This comment has been minimized.

@balloob

balloob Oct 25, 2018

Member

We should not offer this functionality. We should just do the right thing. Please remove this.

This comment has been minimized.

@balloob

balloob Oct 25, 2018

Member

We should never put the burden on the user. Instead, a new air purifier device class should be added to the fan component. (here is an example how binary sensor does it)

This comment has been minimized.

@mariuszluciow

mariuszluciow Oct 25, 2018

Contributor

Well, I'm fine with using device_class for it, although it seems that its implemented only for binary sensor, where we need it on switch (outlet vs generic switch) and for the fan and airfilter. Anyway, I will change the implementation to choose the type for fan and switch using the device_class if it's defined, although in different MR if you fine with it.

BTW, as in the HomeKit the outlet is also a different entity, should it be added to HASS as well, rather than relying on device_type?

This comment has been minimized.

@mariuszluciow

mariuszluciow Oct 27, 2018

Contributor

Followup PR from this discussion: #17866

@balloob

This can be merged once the user overridable types support has been removed.

@cnrd

This comment has been minimized.

Contributor

cnrd commented Oct 25, 2018

@balloob It seems like this is way more of a problem when stuff needs to interact with Google Assistant, is this something that was dictated by Google? Or is it just a coincidence that it was decided, that we should no longer create hacks for better support, at the same time as the Google Assistant implementation?

@dshokouhi

This comment has been minimized.

Contributor

dshokouhi commented Oct 25, 2018

@cnrd is it really a hack if the first sentence in google documentation say "pick one that best aligns with your product". The verbiage on this page does not show any type of dictation from Google. Even when you look at Device Traits it says "you can add whichever ones you want" at the top description. Maybe it was something during the initial hass.io certification process but the docs have changed this month according to the footnotes.

https://developers.google.com/actions/smarthome/guides/
https://developers.google.com/actions/smarthome/traits/

@mariuszluciow mariuszluciow force-pushed the mariuszluciow:google-assistant/vacuum-dock branch 2 times, most recently from 21ad95c to 8cd92e3 Oct 25, 2018

async def handle_devices_execute(hass, config, payload):
async

This comment has been minimized.

@houndci-bot

houndci-bot Oct 25, 2018

blank lines found after function decorator

async def async_devices_query(hass, config, payload):
async

This comment has been minimized.

@houndci-bot

houndci-bot Oct 25, 2018

blank lines found after function decorator

async def async_devices_sync(hass, config, payload):
async

This comment has been minimized.

@houndci-bot

houndci-bot Oct 25, 2018

SyntaxError: invalid syntax
blank lines found after function decorator

@mariuszluciow mariuszluciow force-pushed the mariuszluciow:google-assistant/vacuum-dock branch 3 times, most recently from 9e8aa92 to d99c871 Oct 25, 2018

@mariuszluciow mariuszluciow changed the title from Added dock support for vacuum through google assistant and adjusted the integration to Added Dock and Pause/Unpause support for vacuum through google assistant Oct 25, 2018

@mariuszluciow

This comment has been minimized.

Contributor

mariuszluciow commented Oct 25, 2018

@balloob done.

@balloob balloob merged commit cfbd84f into home-assistant:dev Oct 26, 2018

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.001%) to 93.274%
Details

@wafflebot wafflebot bot removed the in progress label Oct 26, 2018

@balloob balloob referenced this pull request Nov 9, 2018

Merged

0.82 #18335

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment