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.

Sign up for GitHub

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

Move HassIntent handler code into helpers/intent #12181

Merged
merged 27 commits into from
Feb 11, 2018
Merged

Move HassIntent handler code into helpers/intent #12181

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
eedb3d0
Moved TurnOn/Off Intents to component
tschmidty69 Feb 3, 2018
bf015c9
Removed unused import
tschmidty69 Feb 3, 2018
b3d7bf6
Lint fix which my local runs dont catch apparently...
tschmidty69 Feb 3, 2018
8651566
Moved hass intent code into intent
tschmidty69 Feb 4, 2018
c842be2
Added test for toggle to conversation.
tschmidty69 Feb 4, 2018
04b407a
Fixed toggle tests
tschmidty69 Feb 4, 2018
c479075
Update intent.py
tschmidty69 Feb 4, 2018
13d0d9a
Added homeassistant.helpers to gen_requirements script.
tschmidty69 Feb 5, 2018
ac5d2e5
Merge branch 'intent-registry-2' of github.com:tschmidty69/home-assis…
tschmidty69 Feb 5, 2018
2dcfe18
Update intent.py
tschmidty69 Feb 5, 2018
861cdc5
Update intent.py
tschmidty69 Feb 5, 2018
2214096
Changed return value for _match_entity
tschmidty69 Feb 5, 2018
90280d7
Moved consts and requirements
tschmidty69 Feb 6, 2018
f287689
Removed unused import
tschmidty69 Feb 6, 2018
7992a73
Removed http view
tschmidty69 Feb 6, 2018
5e987e8
Removed http import
tschmidty69 Feb 6, 2018
5496efd
Removed fuzzywuzzy dependency
tschmidty69 Feb 7, 2018
cb8ab73
woof
tschmidty69 Feb 7, 2018
2d9becd
A few cleanups
tschmidty69 Feb 8, 2018
0d955d3
Added domain filtering to entities
tschmidty69 Feb 8, 2018
1262ba3
Clarified class doc string
tschmidty69 Feb 8, 2018
ed1bcf2
Added doc string
tschmidty69 Feb 8, 2018
6cfce83
Added test in test_init
tschmidty69 Feb 9, 2018
95d30c6
woof
tschmidty69 Feb 9, 2018
42273fc
Cleanup entity matching
tschmidty69 Feb 10, 2018
a43bc92
Update intent.py
tschmidty69 Feb 10, 2018
b2ad2f9
removed uneeded setup from tests
tschmidty69 Feb 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion homeassistant/components/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@
import homeassistant.core as ha
import homeassistant.config as conf_util
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers import config_validation as cv
from homeassistant.helpers.service import extract_entity_ids
from homeassistant.helpers import intent
from homeassistant.const import (
ATTR_ENTITY_ID, SERVICE_TURN_ON, SERVICE_TURN_OFF, SERVICE_TOGGLE,
ATTR_ENTITY_ID, INTENT_TURN_ON, INTENT_TURN_OFF, INTENT_TOGGLE,
SERVICE_TURN_ON, SERVICE_TURN_OFF, SERVICE_TOGGLE,
SERVICE_HOMEASSISTANT_STOP, SERVICE_HOMEASSISTANT_RESTART,
RESTART_EXIT_CODE)

Expand Down Expand Up @@ -154,6 +157,9 @@ def async_handle_turn_service(service):
ha.DOMAIN, SERVICE_TURN_ON, async_handle_turn_service)
hass.services.async_register(
ha.DOMAIN, SERVICE_TOGGLE, async_handle_turn_service)
hass.helpers.intent.async_register(TurnOnIntent())
hass.helpers.intent.async_register(TurnOffIntent())
hass.helpers.intent.async_register(ToggleIntent())

@asyncio.coroutine
def async_handle_core_service(call):
Expand Down Expand Up @@ -200,3 +206,39 @@ def async_handle_reload_config(call):
ha.DOMAIN, SERVICE_RELOAD_CORE_CONFIG, async_handle_reload_config)

return True


class TurnOnIntent(intent.IntentHandler):
"""Handle component turn intents."""

intent_type = INTENT_TURN_ON
slot_schema = {
'name': cv.string,
}
domain = ha.DOMAIN
service = SERVICE_TURN_ON
response = "Turned on {}"


class TurnOffIntent(intent.IntentHandler):
"""Handle component turn intents."""

intent_type = INTENT_TURN_OFF
slot_schema = {
'name': cv.string,
}
domain = ha.DOMAIN
service = SERVICE_TURN_OFF
response = "Turned off {}"


class ToggleIntent(intent.IntentHandler):
"""Handle component turn intents."""

intent_type = INTENT_TOGGLE
slot_schema = {
'name': cv.string,
}
domain = ha.DOMAIN
service = SERVICE_TOGGLE
response = "Toggled {}"
96 changes: 7 additions & 89 deletions homeassistant/components/conversation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,24 @@
import asyncio
import logging
import re
import warnings

import voluptuous as vol

from homeassistant import core
from homeassistant.components import http
from homeassistant.const import (
ATTR_ENTITY_ID, SERVICE_TURN_OFF, SERVICE_TURN_ON)
from homeassistant.const import (INTENT_TURN_ON,
INTENT_TURN_OFF, INTENT_TOGGLE)
from homeassistant.helpers import config_validation as cv
from homeassistant.helpers import intent
from homeassistant.loader import bind_hass

REQUIREMENTS = ['fuzzywuzzy==0.16.0']

_LOGGER = logging.getLogger(__name__)

ATTR_TEXT = 'text'

DEPENDENCIES = ['http']
DOMAIN = 'conversation'

INTENT_TURN_OFF = 'HassTurnOff'
INTENT_TURN_ON = 'HassTurnOn'

REGEX_TURN_COMMAND = re.compile(r'turn (?P<name>(?: |\w)+) (?P<command>\w+)')
REGEX_TYPE = type(re.compile(''))

Expand All @@ -50,7 +44,7 @@
@core.callback
@bind_hass
def async_register(hass, intent_type, utterances):
"""Register an intent.
"""Register utterances and any custom intents.

Registrations don't require conversations to be loaded. They will become
active once the conversation component is loaded.
Expand All @@ -75,8 +69,6 @@ def async_register(hass, intent_type, utterances):
@asyncio.coroutine
def async_setup(hass, config):
"""Register the process service."""
warnings.filterwarnings('ignore', module='fuzzywuzzy')
Copy link
Member

Choose a reason for hiding this comment

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

We should add this back where we import fuzzywuzzy. I remember it spamming a lot.

Copy link
Member

Choose a reason for hiding this comment

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

I see you've removed fuzzywuzzy, ok too 👍


config = config.get(DOMAIN, {})
intents = hass.data.get(DOMAIN)

Expand All @@ -102,12 +94,12 @@ def process(service):

hass.http.register_view(ConversationProcessView)

hass.helpers.intent.async_register(TurnOnIntent())
hass.helpers.intent.async_register(TurnOffIntent())
async_register(hass, INTENT_TURN_ON,
['Turn {name} on', 'Turn on {name}'])
async_register(hass, INTENT_TURN_OFF, [
'Turn {name} off', 'Turn off {name}'])
async_register(hass, INTENT_TURN_OFF,
['Turn {name} off', 'Turn off {name}'])
async_register(hass, INTENT_TOGGLE,
['Toggle {name}', '{name} toggle'])

return True

Expand Down Expand Up @@ -151,82 +143,8 @@ def _process(hass, text):
return response


@core.callback
def _match_entity(hass, name):
"""Match a name to an entity."""
from fuzzywuzzy import process as fuzzyExtract
entities = {state.entity_id: state.name for state
in hass.states.async_all()}
entity_id = fuzzyExtract.extractOne(
name, entities, score_cutoff=65)[2]
return hass.states.get(entity_id) if entity_id else None


class TurnOnIntent(intent.IntentHandler):
"""Handle turning item on intents."""

intent_type = INTENT_TURN_ON
slot_schema = {
'name': cv.string,
}

@asyncio.coroutine
def async_handle(self, intent_obj):
"""Handle turn on intent."""
hass = intent_obj.hass
slots = self.async_validate_slots(intent_obj.slots)
name = slots['name']['value']
entity = _match_entity(hass, name)

if not entity:
_LOGGER.error("Could not find entity id for %s", name)
return None

yield from hass.services.async_call(
core.DOMAIN, SERVICE_TURN_ON, {
ATTR_ENTITY_ID: entity.entity_id,
}, blocking=True)

response = intent_obj.create_response()
response.async_set_speech(
'Turned on {}'.format(entity.name))
return response


class TurnOffIntent(intent.IntentHandler):
"""Handle turning item off intents."""

intent_type = INTENT_TURN_OFF
slot_schema = {
'name': cv.string,
}

@asyncio.coroutine
def async_handle(self, intent_obj):
"""Handle turn off intent."""
hass = intent_obj.hass
slots = self.async_validate_slots(intent_obj.slots)
name = slots['name']['value']
entity = _match_entity(hass, name)

if not entity:
_LOGGER.error("Could not find entity id for %s", name)
return None

yield from hass.services.async_call(
core.DOMAIN, SERVICE_TURN_OFF, {
ATTR_ENTITY_ID: entity.entity_id,
}, blocking=True)

response = intent_obj.create_response()
response.async_set_speech(
'Turned off {}'.format(entity.name))
return response


class ConversationProcessView(http.HomeAssistantView):
"""View to retrieve shopping list content."""

url = '/api/conversation/process'
name = "api:conversation:process"

Expand Down
5 changes: 5 additions & 0 deletions homeassistant/const.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@
EVENT_LOGBOOK_ENTRY = 'logbook_entry'
EVENT_THEMES_UPDATED = 'themes_updated'

# #### INTENTS ####
Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine to keep these defined inside helpers.intent .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That works, components can just import them from intents, just felt a little cleaner to put the in consts but all good.

INTENT_TURN_OFF = 'HassTurnOff'
INTENT_TURN_ON = 'HassTurnOn'
INTENT_TOGGLE = 'HassToggle'

# #### STATES ####
STATE_ON = 'on'
STATE_OFF = 'off'
Expand Down
43 changes: 41 additions & 2 deletions homeassistant/helpers/intent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,18 @@
from homeassistant.core import callback
from homeassistant.exceptions import HomeAssistantError
from homeassistant.loader import bind_hass
from homeassistant.const import ATTR_ENTITY_ID

REQUIREMENTS = ['fuzzywuzzy==0.16.0', 'python-Levenshtein==0.12.0']
Copy link
Member

Choose a reason for hiding this comment

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

Helpers can't have requirements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, i saw that, but the problem in this particular case is that component/init.py isn't considered a module so doesn't allow requirements. I could put it anywhere under components but that raises a dependency issue.



DATA_KEY = 'intent'
_LOGGER = logging.getLogger(__name__)

SLOT_SCHEMA = vol.Schema({
}, extra=vol.ALLOW_EXTRA)

DATA_KEY = 'intent'

SPEECH_TYPE_PLAIN = 'plain'
SPEECH_TYPE_SSML = 'ssml'

Expand Down Expand Up @@ -80,6 +84,17 @@ class IntentHandleError(IntentError):

pass

@callback
Copy link

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

@bind_hass
def _match_entity(hass, name):
"""Match a name to an entity."""
from fuzzywuzzy import process as fuzzyExtract
Copy link
Member

Choose a reason for hiding this comment

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

This is installed as part of the conversation component. You can't use it here.

The solution is that now we're going to need to add fuzzywuzzy as a core dependency. I just don't know if I like that.

Is the added value of fuzzywuzzy that much that we need it? Could we just map against name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I put it in conversation so it would get added somewhere since I can't add a requirement to a helper.

We could. Would be a little more convoluted with some regexs but I would guess we can cover most cases fairly easily.

Copy link
Member

Choose a reason for hiding this comment

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

If the user doesn't have the conversation component enabled, it will not install the dependency.

entities = {state.entity_id: state.name for state
in hass.states.async_all()}
entity_id = fuzzyExtract.extractOne(
name, entities, score_cutoff=65)[2]
return hass.states.get(entity_id) if entity_id else None


class IntentHandler:
"""Intent handler registration."""
Expand All @@ -88,12 +103,16 @@ class IntentHandler:
slot_schema = None
_slot_schema = None
platforms = None
domain = None
service = None
response = ''

@callback
def async_can_handle(self, intent_obj):
"""Test if an intent can be handled."""
return self.platforms is None or intent_obj.platform in self.platforms


@callback
def async_validate_slots(self, slots):
"""Validate slot information."""
Expand All @@ -110,7 +129,27 @@ def async_validate_slots(self, slots):
@asyncio.coroutine
def async_handle(self, intent_obj):
"""Handle the intent."""
raise NotImplementedError()
if not self.service:
Copy link
Member

Choose a reason for hiding this comment

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

I don't like this, we're conflating concerns. Please extract this into a new class that extends from IntentHandler:

class ServiceIntentHandler(IntentHandler):

Copy link
Member

Choose a reason for hiding this comment

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

Alternative, instead of a class, you can make it a method.

@bind_hass
@callback
def expose_entity_service_intent(intent_name, domain, service):

The automatic wrapping of a service in an intent has a very narrow use case. It only applies to services that only take in an entity_id. If it takes in anything more (like color in light.turn_on), you want to write your own.

raise NotImplementedError()
hass = intent_obj.hass
slots = self.async_validate_slots(intent_obj.slots)
name = slots['name']['value']
entity = _match_entity(hass, name)
Copy link
Member

Choose a reason for hiding this comment

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

This is retrieving a state not an entity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It;s a roundabout way of retrieving all entites, by getting all states and using the entities that belong to them. Need to add filtering by domain.

Copy link
Member

Choose a reason for hiding this comment

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

The state machine doesn't store entities, it stores states. You should change the variable name to reflect what it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The function returns the entity_id though and not the state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

return hass.states.get(entity_id) if entity_id else None

Copy link
Member

Choose a reason for hiding this comment

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

No, it returns a state:
https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/core.py#L662-L668

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see what you mean. I didn't write that bit. It would be more confusing to return a state, so rewrote the return

Copy link
Member

Choose a reason for hiding this comment

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

Let's rename it to entity_id, as that is what is returned from _match_entity.


if not entity:
_LOGGER.error("Could not find entity id for %s", name)
return None

yield from hass.services.async_call(
self.domain, self.service, {
ATTR_ENTITY_ID: entity.entity_id,
}, blocking=True)
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove blocking, it has been causing trouble for Alexa/Google Assistant as turning on can sometimes take a long time and we don't actually know if a service is successful.


response = intent_obj.create_response()
response.async_set_speech(
self.response.format(entity.name))
return response


def __repr__(self):
"""Represent a string of an intent handler."""
Expand Down
Loading