Skip to content
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

RFC: Adding Default Hass Intents to Components #12087

Closed
tschmidty69 opened this issue Jan 31, 2018 · 22 comments · Fixed by #12181
Closed

RFC: Adding Default Hass Intents to Components #12087

tschmidty69 opened this issue Jan 31, 2018 · 22 comments · Fixed by #12181

Comments

@tschmidty69
Copy link
Contributor

tschmidty69 commented Jan 31, 2018

This is to discuss how we could add default intents to components that could be used with voice assistants. As it is users need to create intents for every function resulting in a lot of duplication. Providing some sensible defaults would result in a lot less configuration needed. But would like input on how best to implement it for the three main platforms (Alexa/GA/Snips).

In conversation, we have the start of that with HassTurnOn and HassTurnOff which calls SERVICE_TURN_OFF. @balloob proposed moving those registrations to component/init.py so they wouldn't be dependent on conversation being loaded.

A more general proposal would involve not doing this per component but actually per service . Since services are already handled at a component level with a defined schema, we can define intent handlers for fairly easily. Individual components could extend that if they wanted/needed to.

Notes from discord discussion:

I think all entity components should expose their services as intents
I almost wonder if we should make "adapters" between intents and services. It's pretty much the same
After we match entity we hand it off to service
The service intent adapter would be a class that adds name to entity id resolution and wraps a service. Delegation of all validation to schema. Might sometimes need more mappings (ie red to RGB color)

So where would this exist? Called when we register the service? So we add this class to intents and in register_service we add this to the intent_adapter class on intents with entity_id, service, schema. Intent handler can validate hassIntent based on the defined schema. We would need to pass schema and save it there in case it was extended by the component.

A nice feature to have would be a front end piece that would allow you to view all the available registered intents/services/schemas so you could define skills without digging in.

@balloob
Copy link
Member

balloob commented Jan 31, 2018

Registering intents for a component should happen inside that component setup method. Registration should happen at the same time as the service is being registered. That way we only register intents for things that are loaded and are keeping up with the approach.

We could define the IntentServiceAdapter in the helpers.intent package.

For the frontend, it is indeed a great idea to add an intent dev tool. Would work similar to the service dev tool: show them, show the values they expect and allow calling them.

@uchagani
Copy link
Contributor

Please keep default intents optional. I like the idea of default intents or pre-built intents that people can add but some of us already have intents that we regularly use that might be considered duplicate. I'd rather not have two intents doing similar things.

@balloob
Copy link
Member

balloob commented Feb 1, 2018

They are optional in that light intents will only load when for example the light component loads. If they wrap services they will just be some name in a registry.

@balloob
Copy link
Member

balloob commented Feb 1, 2018

All Home Assistant intents are prefixed with Hass so shouldn't conflict either

@tschmidty69
Copy link
Contributor Author

Exactly, unless you decide to name your intent HassMyIntent you'll be fine!

@uchagani
Copy link
Contributor

uchagani commented Feb 1, 2018

ok that makes sense. And my snips assistant will only match what is in my bundle vs what is in HA, correct?

@balloob
Copy link
Member

balloob commented Feb 2, 2018

Correct

@tschmidty69
Copy link
Contributor Author

So take a look here: #12181

I actually stepped back a little and decided to just extend the IntentHandler to be able to handle builtin service calls rather than create a new class. This is a first step since I need to add service schema validation and restrict intents to platforms. But this removes some code duplication without changing too much. I decided I don't like the idea of tracking entity_ids alongside the intents, since then we need to be called as entities are added, removed, renamed, etc. I think the idea of a service that can enumerate intents along with the entities that they apply to should be enough to allow an outside service to query capability. Thoughts?

@tschmidty69
Copy link
Contributor Author

So I wondering where to put a web rest api view to list intents, etc? I am thinking the snips component since that is the only voice assistant that requires it now since google and alexa have their own way to get services, etc. Make sense?

@balloob
Copy link
Member

balloob commented Feb 7, 2018

Intents are a core part of HASS. An API to return current registered intents should be added to the API component.

@tschmidty69
Copy link
Contributor Author

So I still want to add some other intents for a few other components, thinking covers and media players. Those are the only ones I can see making sense for us to add, but I think that would give a really nice impression of what you can do. Then I need to take a run at the docs for both snips and the snips-addon to bring them up to date.

@balloob
Copy link
Member

balloob commented Feb 15, 2018

Shouldn't each component that provides entities have intents added? I would expect some intents for Light like SetColor, SetBrightness etc.

@tschmidty69
Copy link
Contributor Author

The basic turn/on off is handled of course but we could add some thing thing like HassLightSet and HassLightColor under the lights component to handle on/off and set brightness/color for sure. Are you thinking we also add this into conversation?

Maybe something to handle scenes and fans makes scenes too. I have a limited set of devices at this point so was just scrolling through services and seeing which ones it made sense to add default intents for.

So lights (brightness/color), fans, scenes, covers, and media players. The basic intents for those are pretty straight forward. I hesitate on things like climate since there is a lot of individual setups.

@balloob
Copy link
Member

balloob commented Feb 16, 2018

I think that we should add intents for all component services if it makes sense (so not group visibility).

If you want to try out a component, the demo platform is your friend. ie:

light:
  platform: demo

climate:
  platform: demo

For the conversation component, I think that I want to make it auto detect loaded component sentences:

  • Components should add a intents.yaml in their directory (just like services.yaml) and it should contain the sentences. Once we support non-english in the frontend, we can have things like intents.nl.yaml etc.
  • On setup, the conversation component will iterate over all the loaded components and load their sentences
  • On setup, the conversation component will listen for component_loaded event and setup the sentences when a new component is loaded.

@balloob
Copy link
Member

balloob commented Feb 16, 2018

Guess makes more sense to call it conversation.yaml since it contains about sentences. Format could be something like this:

<intent name>:
 - <sentence>
 - <sentence>

HassLightColor:
 - Change {name} to {color}
 - Make {name} the color {color}

@tschmidty69
Copy link
Contributor Author

That works, and I definitely like splitting out the text from the components. I saw in another PR someone was talking about wanting to use json for internationalization, is that right? Would obviously prefer yaml for readability and ease of course.

@balloob
Copy link
Member

balloob commented Feb 18, 2018

The translations are managed through lokalise.co, they are not handwritten via PRs. Since it's machine generated by lokalise.co, it should be JSON.

@armills, do you have an insight how we can incorporate conversation sentences per component properly with lokalise.co?

@emlove
Copy link
Contributor

emlove commented Feb 18, 2018

I'd probably put it on hold for a moment and reevaluate once we get #12453 merged. The approach might be more obvious once we have something started for backend translations.

@tschmidty69
Copy link
Contributor Author

Makes sense. I'll just poke at the codey bits for now.

@balloob
Copy link
Member

balloob commented Feb 18, 2018

Ok. We can still go ahead and add intents for all components. We'll add conversation sentences later.

@balloob balloob mentioned this issue Feb 24, 2018
3 tasks
@tschmidty69
Copy link
Contributor Author

@balloob So I'd mentioned this on discord but always tough to find info there. So I created this wiki here to allow people to offer edits for intents https://github.com/tschmidty69/hass-snips-bundle-intents/wiki This is exported from the snips console and uploaded via git to github. Snips console will be the definitive place, but I am also working on importing the git into the snips console.

They just released a new version breaking console->snips compatibility so we will need to update the hassio addon in the next couple days (@pvizeli) when they update the docker image. There seem to be a couple issues so waiting to see if they get them straightened out.

One last note is they open sourced the snips nlu so I may look at trying to incorporate some version of that into the conversation component since I think it would be an interesting use (use your bowser anywhere to talk to HA with the standard HA intents).

@balloob
Copy link
Member

balloob commented Mar 22, 2018

I think that we should add your wiki to our Snips docs too

@home-assistant home-assistant locked and limited conversation to collaborators Jul 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants