Google Actions for Assistant #9632
Conversation
|
Hello @philk, When attempting to inspect the commits of your pull request for CLA signature status among all authors we encountered commit(s) which were not linked to a GitHub account, thus not allowing us to determine their status(es). The commits that are missing a linked GitHub account are the following:
Unfortunately, we are unable to accept this pull request until this situation is corrected. Here are your options:
We apologize for this inconvenience, especially since it usually bites new contributors to Home Assistant. We hope you understand the need for us to protect ourselves and the great community we all have built legally. The best thing to come out of this is that you only need to fix this once and it benefits the entire Home Assistant and GitHub community. Thanks, I look forward to checking this PR again soon! ❤️ |
There was a problem hiding this comment.
This is awesome and off to a great start! Yes, OAuth2 is currently pretty cumbersome but that's something we can improve later. I think for now the current flow is fine.
Added some comments, mainly around let's follow the pattern in alexa/smart_home.py and decouple message handling from I/O.
.coveragerc
Outdated
| @@ -74,6 +74,9 @@ omit = | |||
| homeassistant/components/google.py | |||
| homeassistant/components/*/google.py | |||
|
|
|||
| homeassistant/components/googleactions.py | |||
There was a problem hiding this comment.
This is not needed, you wrote tests 👍
| '{}/r/{}#access_token={}&token_type=bearer&state={}' | ||
|
|
||
|
|
||
| def setup(hass: HomeAssistant, yaml_config: Dict[str, Any]): |
There was a problem hiding this comment.
Please make this async
@asyncio.coroutine
def async_setup(hass, config):| def __init__(self, hass: HomeAssistant, cfg: Dict[str, Any]) -> None: | ||
| """Initialize instance of the view.""" | ||
| super().__init__() | ||
| self.hass = hass |
There was a problem hiding this comment.
You can get the hass instance from the request, you don't have to store it in the instance:
# inside get(…)
hass = request.app['hass']
| """Initialize Google Actions view.""" | ||
| super().__init__() | ||
|
|
||
| self.hass = hass |
|
|
||
| return is_default_exposed or explicit_expose | ||
|
|
||
| def handle_sync(self, hass: HomeAssistant, request_id: str): |
There was a problem hiding this comment.
Please decouple the I/O from the handling of the messages. Have a look at the Alexa smart home skill as an example: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/alexa/smart_home.py#L32-L44
There was a problem hiding this comment.
I would even consider breaking it up in multiple files: google_assistant/smart_home.py and google_assistant/http.py
| def post(self, request: Request): | ||
| """Handle Google Action requests.""" | ||
| auth = request.headers.get('Authorization', '') | ||
| if self.access_token not in auth: |
There was a problem hiding this comment.
Let's do actual comparison. not in only checks if string A exists inside string B.
|
Thanks for the feedback, I hadn't looked at the rewritten Alexa component but I like that layout and I'm stealing some ideas from there that should tidy up my code quite a lot 😁 |
|
Hi! Great idea! I found a project that could be use as a reference : Phlex. The setup is somewhat easy. It is actually a server that connects to a Plex server instance to enable voice commands. It connects to Google Assistant and Alexa using a simple login interface. The Google Action is officially accessible from Android phones and Google Home and is named Flex TV. This way, users don't have to create their own private Google Action but still can use a private server. I tried this project and it seems to work properly, but I haven't look at the source since it is written in php, but I think it could be a good help for you. Good work! |
|
We should not depend on Phlex as we should not promote the use of external gateway services. |
|
I am not talking about dependency, but more as a design reference. |
|
Yeah, I'm not sure exactly how they do it but I don't want people's credentials passing through a system I run which is the only way I can think of to host one "Home-Assistant" action that shows up for everyone in their Google Home app. |
|
Can't wait to get this merged!! |
|
Is there setup documentation for this, yet? I'd love to give it a go.... |
|
Not yet, should get some time to work on it this weekend. |
|
Hi,
thanks for your work! very helpful feature ;)
I am not sure to understand you last email.
If my understanding is correct the Google smart home mechanism is to define
a mapping (more or less a database with a service connection) to list and
expose commands to the Google assistant cloud. so I would expect that the
"home assistant plug-in" will deal with Google assistant account
authentication and connections directly from client side. The user will
have to expose/define actions and fill his own credentials using a proper
config file...
Am I wrong?
thanks and BR,
David.
Le 9 oct. 2017 23:26, "Phil Kates" <notifications@github.com> a écrit :
… Yeah, I'm not sure exactly how they do it but I don't want people's
credentials passing through a system I run which is the only way I can
think of to host one "Home-Assistant" action that shows up for everyone in
their Google Home app.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#9632 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AeTwicvIxmitMI-r5XNTfBxrN9IFxynGks5sqo96gaJpZM4Ppswc>
.
|
|
The way the Google Actions works is either through a service like API.ai (now DialogFlow) or through the Google Compute Platform (via the Actions API). The short version (since I'm currently working on the step by step docs) is:
Like I said in my initial comment it's a lot more complicated than I'd like it to be but at least somewhat less so than the current downgrade the Google Home app process to get emulated_hue working. |
I know it probably isn't of much use, but it looks like Google (or Philips) blocked this from working, as even the downgraded app now redirect to the Hue auth page. (I haven't tried downgrading the Google app or anything like that yet, to see if that could fix it). |
|
Emulated Hue will not work at all now. |
|
Philips just change a little bit it's protocol to break the compatibility
with the new Ikea connected lights as both are using zigbee... that company
that volontuary are not compliant we the standard....
I suppose this is why th Haas implementation is broken..
Le 15 oct. 2017 07:23, "Conrad Juhl Andersen" <notifications@github.com> a
écrit :
… than the current downgrade the Google Home app process to get emulated_hue
working.
I know it probably isn't of much use, but it looks like Google (or
Philips) blocked this from working, as even the downgraded app now redirect
to the Hue auth page. (I haven't tried downgrading the Google app or
anything like that yet, to see if that could fix it).
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#9632 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AeTwiTHqqCT3s9so-BZdCLhvRMIgmdGPks5ssTRTgaJpZM4Ppswc>
.
|
|
@philk Sorry to be impatient.... But, any update on the setup guide so some of us can test it? |
|
attempted to set this up and got a good way there. During the setup process within the Home app i'm getting:
|
|
Looks like it didn't like the init.py in the custom_components/http folder so I overwrote it. Now I'm getting:
Within the Home App after it redirects. |
|
I get something close to that, it redirects in the Google Home app, goes to a spinner, then says: "Something went wrong, please try again". |
- The way I originally wrote the MAPPING_COMPONENT and the way it's actual used changed so the comment was updated to match that. - Use const's in more places - Handle the ActivateScene command correctly
Was failing on 3.4.2 and 3.5, this is more correct anyway.
|
@ALL Guys, let us use this thread in the forums to discuss support/configuration issues and not pollute this PR. Others will face similar issues and would benefit from this discussion. |
|
@kylerw Please don't use the PR for support/feature requests. |
|
Deleting all comments not related to the code. |
|
🎉 🐬 Thanks. |
Description:
I'm looking for some feedback on this both as a component and as a specific implementation.
The current
emulated_hueimplementation can only work with either the Echo or Google Home, but not both at the same time so I took some of the ideas from the Alexa component.Google provides a Smart Home API for interacting with Google Assistant. It's a relatively straightforward API that just requires some mapping of Home-Assistant objects to a couple of schemas. I think that part all makes sense and is a reasonable component for hass. I will acknowledge there was a lot of copy paste from the hue and Alexa components to figure out how aiohttp works. Happy to take feedback on the /right/ way to do things for sure 😅
Where I'm most looking for feedback though is really UX, basically the Google Actions deploy and setup process. You have to set up OAuth for your action to authenticate against and the way I did it is...hacky to say the least. I'm really curious what the path forward for the Cloud Auth stuff is but right now it looks pretty AWS specific and more for handling login to hass rather than being an OAuth provider.
I'm also not super happy with having to ask people to go through the setup steps for creating an app in GCP. We could probably break it down to:
gactions testcommand with the json fileWhich isn't /too/ bad but doesn't feel as user friendly as hass has gotten lately (OMG, not having to deal with Zwave compilation and install, life saving).
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#3657
Example entry for
configuration.yaml(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests pass.coveragerc.