Fix google assistant request sync service call

[awarecan]

Description:

context.user_id will be None if request_sync service call was triggered by platform event in Automation.

The fix add an optional agent_user_id field to request_sync service, so that automation script can provide the user id

Related issue (if applicable): fixes #17380

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>

Example entry for configuration.yaml (if applicable):

# automations.yaml
- id: refresh_google_assistant
  alias: "Refresh Google Assistant devices"
  hide_entity: true
  trigger:
    platform: homeassistant
    event: start
  action:
    - service: google_assistant.request_sync
      data:
        # you can find the owner user id from configuration -> users
        agent_user_id: OWNER_USER_ID

Checklist:

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

[CedrickFlocon]

I am new on this project. I have trouble configuring SSL on a dev env.
But as a test I print the agent_user_id from the homeassistant startup automation
On master

2018-10-14 12:24:11 INFO (MainThread) [homeassistant.components.google_assistant] agent_user_id None

On awarecan-google-assistant-patch

2018-10-14 13:12:10 INFO (MainThread) [homeassistant.components.google_assistant] agent_user_id home-assistant

If you have some advise on how I could test this feature easily fell free to help me.

[awarecan]

@CedrickFlocon can you direct apply my change to your production instance? Since the changes only 2 lines, you can directly edit your local homeassistant/components/google_assistant/__init__.py file

[CedrickFlocon]

@awarecan I try but I wasn't able to locate the source code directory.
My production instance is base on HASS.IO and I can only access with SSH.

If I well understand the project, the source code should be in a docker container ?

[balloob]

We should always send the same agent_user_id as was set in https://github.com/home-assistant/home-assistant/blob/bd450ee9ffdb7e7aa765070a1c86118cde6f9e1b/homeassistant/components/google_assistant/smart_home.py#L244-L247

If it's not the same, it won't trigger a resync.

So we should just hardcode this to home-assistant since this is a single tenant skill anyway?

[dshokouhi]

This is also noted in the original documentation as well. Failing to set a consistent Agent User ID causes the user to relink assistant so they can issue a sync command.

https://www.home-assistant.io/components/google_assistant/#troubleshooting-the-request_sync-service

I think hard-coding it is a much better approach so the user doesn't need to worry about it at all.

[awarecan]

It is fine to supply a const user id for now, since we don't have permission system in place yet. All users will get same access anyway. However, after we introduce permission, thing will become interesting.

Just a reminder, we need a way to supply context.user to system triggered event, not only for this particular issue.

[awarecan]

Rethink it again. I should not revert user id to a fixed value only because it breaks automation script. Instead, I am going to add an optional field to the service call, so if user need it, can be provided in the automation.

[balloob]

What is the use case of supporting multiple agent_user_id ? It's linked to a test skill and this is the only instance?

[awarecan]

If you have multiple Google users, they can link to different HA user. If those HA users have different permission, they will get different devices linked in Google Assistant.

[balloob]

good point.

In that case we should have this service fail if not called from the context of a user?

[edif30]

Is this ready to be tested?

[awarecan]

Yes, ready for test.

[edif30]

Ok, Will run this through in a bit and report back.

[edif30]

Brackets will fail yaml and produce a traceback. In testing I removed the brackets and this will load.

I think the way this is worded is slightly confusing. Perhaps try the below?

description: Optional.  Only needed for automations.  Specific Home Assistant user id to sync with Google Assistant. Do not need when you call this service through Home Assistant front end or API. Used in automation script or other place where context.user_id is missing

[edif30]

Sorry for the delay. Had some things come up.

I tested two different ways.

#
  - id: ha_startup
    alias: HA Startup
    initial_state: 'on'
    trigger:
      platform: homeassistant
      event: start
    action:
      - service: google_assistant.request_sync

This above produces the same 400 error as noted in the related issue.

#
  - id: ha_startup
    alias: HA Startup
    initial_state: 'on'
    trigger:
      platform: homeassistant
      event: start
    action:
      - service: google_assistant.request_sync
        data:
          agent_user_id: <my_id>

2018-11-03 08:59:51 ERROR (MainThread) [homeassistant.components.google_assistant] request_sync request failed: 404 b'{\n  "error": {\n    "code": 404,\n    "message": "Requested entity was not found.",\n    "status": "NOT_FOUND"\n  }\n}\n'

This above produces a 404 not found.

Also, not sure exactly why but this change caused severe slowdown during HA startup even without any automations using the sync service. Both my Google Calendar and Nest components get stuck and never load.

[balloob]

This change looks fine

[edif30]

@balloob This change causes a 404 on the request. How does this change look fine?

[balloob]

You get a 404 if the ID is not the same as the one that you used to sync with. So you might have gotten the ID wrong.

[edif30]

I validated the ID was correct. I even created a 2nd account and tried this on my dev environment. Still gets a 404. I think this needs more testing.

[edif30]

@balloob @awarecan I was able to find some time to test this further. I was confused by the documentation where previously we could specify a user_agent_id. I cannot recall the reasons why but most had used the user_id of their respective google accounts. There was no particular reason.
You could use anything. This also matched in the Google Actions console. Therefore when this PR was submitted, I mistakenly thought "agent_user_id" was the same as previous documentation indicated. In fact the new documentation states that this user_id is in the config > users section. However some also might interpret this as the "username" of the HA user. Which I did. The correct user_id is the random generated id in that same section.

Since the submission of this PR, the init.py has been updated to allow GA to "unlock". This PR was never merged to master (At least that I can see). I can provide a new PR and can confirm these changes do in fact resolve the issue.

Sorry for the long delay and thank you @awarecan for the help.

[edif30]

I opened #19113 but I guess the changes in this PR for init.py are already merged to dev so they just need to be merged to master. I did update the services.yaml and the description.