Enable local fulfillment google assistant #63218
Conversation
|
Hi @LoekSangers, 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! |
|
Hey there @home-assistant/cloud, mind taking a look at this pull request as it has been labeled with an integration ( |
|
Currently looking into writing the tests for the added functionality, but unsure of how to approach this correctly. So any advice is appreciated. Also still need to write the documentation. |
|
Note for people wanting to test this you need to restart the home assistant core after the app.js files have been loaded to your Google device for the registration to work and requests to be fulfilled locally. |
|
Is there a way to run tests faster locally? Currently I run: |
Yes, it's in the docs: https://developers.home-assistant.io/docs/development_testing#testing-outside-of-tox 👍 |
| def local_sdk_user_id(self): | ||
| """Return the user ID to be used for actions received via the local SDK.""" | ||
| return list(self._store.agent_user_ids.keys())[0] |
There was a problem hiding this comment.
This function needs to be updated to receive the webhook ID that received the actions and map it to the correct user.
There was a problem hiding this comment.
I think that the best way to approach this is to make the used user id a configuration for the component. This would support the use case as describe by 1. above. As far as I can tell the local sdk does not send this information and has no way of knowing which user agent id should be used otherwise.
Having it be webhook dependent does not really make sense as we only send this webhook information for local fulfillment and we send it in a field that will get overwritten by the next sync with different data, i.e. the way we sync all devices for all user agent ids, to me look like we will only have the last synced value stored by google (as the device id stays the same).
|
Not sure what caused the check to not pass, but I think the pr is done and can be reviewed again. |
|
|
| @@ -161,6 +162,10 @@ def local_sdk_user_id(self): | |||
| """Return the user ID to be used for actions received via the local SDK.""" | |||
| raise NotImplementedError | |||
|
|
|||
| def get_webhook_id(self, agent_user_id): | |||
| """Return the webhook ID to be used for actions for a given agent user id via the local SDK.""" | |||
| return self.local_sdk_webhook_id | |||
There was a problem hiding this comment.
Let's get rid of this property altogether. For cloud/google_config.py you can just rename the property implementation to get_webhook_id.
There was a problem hiding this comment.
Did this and also created a get_local_agent_user_id to simplify the use of the helper classes (i.e. move the methods back to helper from http).
…angers/core into google-assistant-local-fulfillment
|
Nice! looks good. |
|
Awesome! |
Proposed change
Adding support for configuring Google Assistant Local Fulfillment, the actual request handling already exists, just adding the ability to enable it.
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
Enable google assistant local fulfillment home-assistant.io#20987
If the code communicates with devices, web services, or third-party tools:
Not applicable.
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale:
Not sure as I am extending an existing component
To help with the load of incoming pull requests: