Skip to content

Add support for Mode trait in Google Assistant.#18772

Merged
balloob merged 5 commits intohome-assistant:devfrom
marchingphoenix:GoogleAssistant-Expose-Media-Player-Sources
Nov 29, 2018
Merged

Add support for Mode trait in Google Assistant.#18772
balloob merged 5 commits intohome-assistant:devfrom
marchingphoenix:GoogleAssistant-Expose-Media-Player-Sources

Conversation

@marchingphoenix
Copy link
Copy Markdown
Contributor

@marchingphoenix marchingphoenix commented Nov 29, 2018

Description:

Add support for Modes Trait for Google Assistant.
Requires a pre-defined set of definitions for "settings"
https://developers.google.com/actions/reference/smarthome/traits/modes
Code will try to match media_player sources to available settings and present them to Google Assistant.
Modes are currently only supported by media_players but the structure is there to support other device TYPES as needed.

Related issue (if applicable):
Relates to feature request here:
https://community.home-assistant.io/t/control-denon-receivers-via-google-assistant/80126

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

Example entry for configuration.yaml (if applicable):

No changes.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

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
    Not applicable..

If the code does not interact with devices:

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

Copy link
Copy Markdown
Contributor

@michaelarnauts michaelarnauts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are still a few duplicates or comma's.

@ghost ghost removed the in progress label Nov 29, 2018
@ghost ghost added the in progress label Nov 29, 2018
@balloob balloob merged commit e50a6ef into home-assistant:dev Nov 29, 2018
@ghost ghost removed the in progress label Nov 29, 2018
@balloob
Copy link
Copy Markdown
Member

balloob commented Nov 29, 2018

Nice! 🎉

@marchingphoenix marchingphoenix deleted the GoogleAssistant-Expose-Media-Player-Sources branch November 30, 2018 05:08
@hf1985
Copy link
Copy Markdown

hf1985 commented Dec 2, 2018

Does this mean that Home Assistant will soon support controlling my Bluesound media player with Google Assistant, which does not work right now?

@marchingphoenix
Copy link
Copy Markdown
Contributor Author

I never had issues turning on and off my receiver during testing.
Looking at the media_player.bluesound code, that component does not support the on/off feature in HA.

@hf1985
Copy link
Copy Markdown

hf1985 commented Dec 2, 2018

Looking at the media_player.bluesound code, that component does not support the on/off feature in HA.

That makes sense to me, since you would not want to turn your Bluesound media player on/off, you would want to make it play/pause. Might there be a way to turn the Google Assistant command “turn media player on/off” into the HA service call “media_player.media_play/media_player.media_pause”?

@marchingphoenix
Copy link
Copy Markdown
Contributor Author

This is a bit off topic for this pr. Any change would affect all media_players, I'm not sure if that's something we'd want to do. Might be able to reverse engineer what GA does for chromecasts.

@hf1985
Copy link
Copy Markdown

hf1985 commented Dec 2, 2018

Wouldn't it be both possible and relevant to make the module ask (sorry for my layman language, I have no idea about the technical side of this): "Does this media player support on/off?" "No." "Okay, I'll sent play/pause commands instead"?

@hf1985
Copy link
Copy Markdown

hf1985 commented Dec 2, 2018

Or would it be possible with the mode trait supported to enable a command like "set media player to play" and "set media player to pause"?

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Dec 2, 2018
@balloob
Copy link
Copy Markdown
Member

balloob commented Dec 2, 2018

No discussion in merged PRs.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants