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 locks in google assistant component #18233

Merged
merged 3 commits into from Nov 6, 2018

Conversation

Projects
None yet
4 participants
@ahayworth
Contributor

ahayworth commented Nov 5, 2018

This is supported by the smarthome API, but there is no documentation
for it. This work is based on an article I found with screenshots of
documentation that was erroneously uploaded:

https://www.androidpolice.com/2018/01/17/google-assistant-home-can-now-natively-control-smart-locks-august-vivint-first-supported/

Google Assistant now supports unlocking certain locks - Nest and August
come to mind - via this API, and this commit allows Home Assistant to
do so as well.

Notably, I've added a config option allow_unlock that controls
whether we actually honor requests to unlock a lock via the google
assistant. It defaults to false.

Additionally, we add the functionNotSupported error, which makes a
little more sense when we're unable to execute the desired state
transition.

https://developers.google.com/actions/reference/smarthome/errors-exceptions#exception_list

Finally, we add a list of devices that will never be exposed to cloud services. That is currently set only to group.all_locks.

Description:

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

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:

Add support for locks in google assistant component
This is supported by the smarthome API, but there is no documentation
for it. This work is based on an article I found with screenshots of
documentation that was erroneously uploaded:

https://www.androidpolice.com/2018/01/17/google-assistant-home-can-now-natively-control-smart-locks-august-vivint-first-supported/

Google Assistant now supports unlocking certain locks - Nest and August
come to mind - via this API, and this commit allows Home Assistant to
do so as well.

Notably, I've added a config option `allow_unlock` that controls
whether we actually honor requests to unlock a lock via the google
assistant. It defaults to false.

Additionally, we add the functionNotSupported error, which makes a
little more sense when we're unable to execute the desired state
transition.

https://developers.google.com/actions/reference/smarthome/errors-exceptions#exception_list
@homeassistant

This comment has been minimized.

homeassistant commented Nov 5, 2018

Hi @ahayworth,

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!

@ahayworth ahayworth requested a review from home-assistant/core as a code owner Nov 5, 2018

@wafflebot wafflebot bot added the in progress label Nov 5, 2018

Show resolved Hide resolved tests/components/google_assistant/test_trait.py Outdated
Show resolved Hide resolved tests/components/google_assistant/test_trait.py Outdated
Show resolved Hide resolved tests/components/google_assistant/test_trait.py
Show resolved Hide resolved tests/components/google_assistant/test_trait.py Outdated
Show resolved Hide resolved tests/components/google_assistant/test_trait.py
Show resolved Hide resolved tests/components/google_assistant/test_trait.py Outdated
Show resolved Hide resolved tests/components/google_assistant/test_trait.py Outdated
Show resolved Hide resolved tests/components/google_assistant/test_trait.py Outdated
Show resolved Hide resolved tests/components/google_assistant/test_trait.py Outdated
Show resolved Hide resolved tests/components/google_assistant/test_trait.py Outdated
@ahayworth

This comment has been minimized.

Contributor

ahayworth commented Nov 5, 2018

Apologies about the style violations, I didn't catch the violations when running tests (wall of text!). I'll fix those later today.

@homeassistant homeassistant added cla-signed and removed cla-needed labels Nov 5, 2018

@ahayworth ahayworth referenced this pull request Nov 5, 2018

Merged

Add documentation for google assistant locks #7377

0 of 2 tasks complete
'type': 'action.devices.types.LOCK',
'willReportState': False
}, {
'id': 'group.all_locks',

This comment has been minimized.

@balloob

balloob Nov 5, 2018

Member

We should not expose this group. That's dangerous. I'm fine with hardcoding group.all_locks to never be exposed.

This comment has been minimized.

@ahayworth

ahayworth Nov 6, 2018

Contributor

Okay! If I understand you correctly, we only want to hide it from cloud services, right? If so, I think 3595d83 takes care of that.

This comment has been minimized.

@balloob

balloob Nov 6, 2018

Member

Perfect.

ahayworth added some commits Nov 6, 2018

Ensure that certain groups are never exposed to cloud entities
For example, the group.all_locks entity - we should probably never
expose this to third party cloud integrations. It's risky.

This is not configurable, but can be extended by adding to the
cloud.const.NEVER_EXPOSED_ENTITIES array.

It's implemented in a modestly hacky fashion, because we determine
whether or not a entity should be excluded/included in several ways.

Notably, we define this array in the top level const.py, to avoid
circular import problems between the cloud/alexa components.

@balloob balloob merged commit 2bf2214 into home-assistant:dev Nov 6, 2018

5 checks passed

Hound No violations found. Woof!
WIP Legacy commit status override — see details
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on add-google-locks at 93.052%
Details

@wafflebot wafflebot bot removed the in progress label Nov 6, 2018

@ahayworth ahayworth deleted the ahayworth:add-google-locks branch Nov 6, 2018

zxdavb added a commit to zxdavb/home-assistant that referenced this pull request Nov 13, 2018

Add support for locks in google assistant component (home-assistant#1…
…8233)

* Add support for locks in google assistant component

This is supported by the smarthome API, but there is no documentation
for it. This work is based on an article I found with screenshots of
documentation that was erroneously uploaded:

https://www.androidpolice.com/2018/01/17/google-assistant-home-can-now-natively-control-smart-locks-august-vivint-first-supported/

Google Assistant now supports unlocking certain locks - Nest and August
come to mind - via this API, and this commit allows Home Assistant to
do so as well.

Notably, I've added a config option `allow_unlock` that controls
whether we actually honor requests to unlock a lock via the google
assistant. It defaults to false.

Additionally, we add the functionNotSupported error, which makes a
little more sense when we're unable to execute the desired state
transition.

https://developers.google.com/actions/reference/smarthome/errors-exceptions#exception_list

* Fix linter warnings

* Ensure that certain groups are never exposed to cloud entities

For example, the group.all_locks entity - we should probably never
expose this to third party cloud integrations. It's risky.

This is not configurable, but can be extended by adding to the
cloud.const.NEVER_EXPOSED_ENTITIES array.

It's implemented in a modestly hacky fashion, because we determine
whether or not a entity should be excluded/included in several ways.

Notably, we define this array in the top level const.py, to avoid
circular import problems between the cloud/alexa components.

@balloob balloob referenced this pull request Nov 29, 2018

Merged

0.83 #18776

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