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

Move HassIntent handler code into helpers/intent #12181

Merged
merged 27 commits into from
Feb 11, 2018
Merged
Show file tree
Hide file tree
Changes from 25 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
7 changes: 7 additions & 0 deletions homeassistant/components/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import homeassistant.config as conf_util
from homeassistant.exceptions import HomeAssistantError
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,
SERVICE_HOMEASSISTANT_STOP, SERVICE_HOMEASSISTANT_RESTART,
Expand Down Expand Up @@ -154,6 +155,12 @@ 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(intent.ServiceIntentHandler(
intent.INTENT_TURN_ON, ha.DOMAIN, SERVICE_TURN_ON, "Turned on {}"))
hass.helpers.intent.async_register(intent.ServiceIntentHandler(
intent.INTENT_TURN_OFF, ha.DOMAIN, SERVICE_TURN_OFF, "Turned off {}"))
hass.helpers.intent.async_register(intent.ServiceIntentHandler(
intent.INTENT_TOGGLE, ha.DOMAIN, SERVICE_TOGGLE, "Toggled {}"))

@asyncio.coroutine
def async_handle_core_service(call):
Expand Down
96 changes: 7 additions & 89 deletions homeassistant/components/conversation.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,15 @@
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.helpers import config_validation as cv
from homeassistant.helpers import intent
from homeassistant.loader import bind_hass

REQUIREMENTS = ['fuzzywuzzy==0.16.0']
from homeassistant.loader import bind_hass

_LOGGER = logging.getLogger(__name__)

Expand All @@ -28,9 +24,6 @@
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 +43,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 +68,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 +93,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,
async_register(hass, intent.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.INTENT_TURN_OFF,
['Turn {name} off', 'Turn off {name}'])
async_register(hass, intent.INTENT_TOGGLE,
['Toggle {name}', '{name} toggle'])

return True

Expand Down Expand Up @@ -151,79 +142,6 @@ 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."""

Expand Down
74 changes: 71 additions & 3 deletions homeassistant/helpers/intent.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,27 @@
"""Module to coordinate user intentions."""
import asyncio
import logging
import re

import voluptuous as vol

from homeassistant.core import callback
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers import config_validation as cv
from homeassistant.loader import bind_hass
from homeassistant.const import ATTR_ENTITY_ID


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

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

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

DATA_KEY = 'intent'

SPEECH_TYPE_PLAIN = 'plain'
SPEECH_TYPE_SSML = 'ssml'

Expand Down Expand Up @@ -87,7 +94,7 @@ class IntentHandler:
intent_type = None
slot_schema = None
_slot_schema = None
platforms = None
platforms = []

@callback
def async_can_handle(self, intent_obj):
Expand Down Expand Up @@ -117,6 +124,67 @@ def __repr__(self):
return '<{} - {}>'.format(self.__class__.__name__, self.intent_type)


def fuzzymatch(name, entities):
"""Fuzzy matching function."""
matches = []
pattern = '.*?'.join(name)
regex = re.compile(pattern, re.IGNORECASE)
for entity_id, entity_name in entities.items():
match = regex.search(entity_name)
if match:
matches.append((len(match.group()), match.start(), entity_id))
return [x for _, _, x in sorted(matches)]


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.

Can you specify in both the name and doc of this class that it is only working for services that call entities and that it needs a name slot that will be mapped to entity_id in the service

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. I think having the slot schema in the class also makes it clearer.

"""Service Intent handler registration.

Service specific intent handler that calls a service by name/entity_id.
"""

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

def __init__(self, intent_type, domain, service, speech):
"""Create Service Intent Handler."""
self.intent_type = intent_type
self.domain = domain
self.service = service
self.speech = speech

@asyncio.coroutine
def async_handle(self, intent_obj):
"""Handle the hass intent."""
hass = intent_obj.hass
slots = self.async_validate_slots(intent_obj.slots)
response = intent_obj.create_response()

name = slots['name']['value']
entities = {state.entity_id: state.name for state
in hass.states.async_all()}

matches = fuzzymatch(name, entities)
entity_id = matches[0] if matches else None
_LOGGER.debug("%s matched entity: %s", name, entity_id)

response = intent_obj.create_response()
if not entity_id:
response.async_set_speech(
"Could not find entity id matching {}.".format(name))
_LOGGER.error("Could not find entity id matching %s.", name)
Copy link
Member

Choose a reason for hiding this comment

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

Please remove period at the end.

return response

yield from hass.services.async_call(
self.domain, self.service, {
ATTR_ENTITY_ID: entity_id
})

response.async_set_speech(
self.speech.format(name))
return response


class Intent:
"""Hold the intent."""

Expand Down
3 changes: 0 additions & 3 deletions requirements_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,6 @@ fritzhome==1.0.4
# homeassistant.components.media_player.frontier_silicon
fsapi==0.0.7

# homeassistant.components.conversation
fuzzywuzzy==0.16.0

# homeassistant.components.tts.google
gTTS-token==1.1.1

Expand Down
3 changes: 0 additions & 3 deletions requirements_test_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@ evohomeclient==0.2.5
# homeassistant.components.sensor.geo_rss_events
feedparser==5.2.1

# homeassistant.components.conversation
fuzzywuzzy==0.16.0

# homeassistant.components.tts.google
gTTS-token==1.1.1

Expand Down
1 change: 0 additions & 1 deletion script/gen_requirements_all.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
'ephem',
'evohomeclient',
'feedparser',
'fuzzywuzzy',
'gTTS-token',
'ha-ffmpeg',
'haversine',
Expand Down
45 changes: 45 additions & 0 deletions tests/components/test_conversation.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from homeassistant.setup import async_setup_component
from homeassistant.components import conversation
import homeassistant.components as component
from homeassistant.helpers import intent

from tests.common import async_mock_intent, async_mock_service
Expand All @@ -16,6 +17,9 @@ def test_calling_intent(hass):
"""Test calling an intent from a conversation."""
intents = async_mock_intent(hass, 'OrderBeer')

result = yield from component.async_setup(hass, {})
assert result

result = yield from async_setup_component(hass, 'conversation', {
'conversation': {
'intents': {
Expand Down Expand Up @@ -46,6 +50,9 @@ def test_register_before_setup(hass):
"""Test calling an intent from a conversation."""
intents = async_mock_intent(hass, 'OrderBeer')

result = yield from component.async_setup(hass, {})
Copy link
Member

Choose a reason for hiding this comment

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

Why does this need to be setup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

init for components doesn't explicitly get called so the intents aren't registered without it.

Copy link
Member

Choose a reason for hiding this comment

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

but this is a test for OrderBeer intent, not one of the control devices intents?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Arg, you are right of course, went a little nuts with the cut/paste. Sorry you are having to review this so much, I have been multitasking and not giving it as much undivided attention as I should.

Copy link
Member

Choose a reason for hiding this comment

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

So can we remove this line?

assert result

hass.components.conversation.async_register('OrderBeer', [
'A {type} beer, please'
])
Expand Down Expand Up @@ -145,6 +152,9 @@ def async_handle(self, intent):
@pytest.mark.parametrize('sentence', ('turn on kitchen', 'turn kitchen on'))
def test_turn_on_intent(hass, sentence):
"""Test calling the turn on intent."""
result = yield from component.async_setup(hass, {})
assert result

result = yield from async_setup_component(hass, 'conversation', {})
assert result

Expand All @@ -168,6 +178,9 @@ def test_turn_on_intent(hass, sentence):
@pytest.mark.parametrize('sentence', ('turn off kitchen', 'turn kitchen off'))
def test_turn_off_intent(hass, sentence):
"""Test calling the turn on intent."""
result = yield from component.async_setup(hass, {})
assert result

result = yield from async_setup_component(hass, 'conversation', {})
assert result

Expand All @@ -187,9 +200,38 @@ def test_turn_off_intent(hass, sentence):
assert call.data == {'entity_id': 'light.kitchen'}


@asyncio.coroutine
@pytest.mark.parametrize('sentence', ('toggle kitchen', 'kitchen toggle'))
def test_toggle_intent(hass, sentence):
Copy link
Member

Choose a reason for hiding this comment

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

The tests for the generic intents need to be moved to tests.components.test_init.

Copy link
Member

Choose a reason for hiding this comment

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

And then they shouldn't depend on conversation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can add test_init but I think these tests should remain here since they are also testing the utterance matching and such.

Copy link
Member

Choose a reason for hiding this comment

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

components/test_init.py already exists.

Testing the utterances is good, but we also need to add tests to the other file as that's where they are defined. We can't have core logic be only tested inside component tests.

"""Test calling the turn on intent."""
result = yield from component.async_setup(hass, {})
assert result

result = yield from async_setup_component(hass, 'conversation', {})
assert result

hass.states.async_set('light.kitchen', 'on')
calls = async_mock_service(hass, 'homeassistant', 'toggle')

yield from hass.services.async_call(
'conversation', 'process', {
conversation.ATTR_TEXT: sentence
})
yield from hass.async_block_till_done()

assert len(calls) == 1
call = calls[0]
assert call.domain == 'homeassistant'
assert call.service == 'toggle'
assert call.data == {'entity_id': 'light.kitchen'}


@asyncio.coroutine
def test_http_api(hass, test_client):
"""Test the HTTP conversation API."""
result = yield from component.async_setup(hass, {})
assert result

result = yield from async_setup_component(hass, 'conversation', {})
assert result

Expand All @@ -212,6 +254,9 @@ def test_http_api(hass, test_client):
@asyncio.coroutine
def test_http_api_wrong_data(hass, test_client):
"""Test the HTTP conversation API."""
result = yield from component.async_setup(hass, {})
assert result

result = yield from async_setup_component(hass, 'conversation', {})
assert result

Expand Down
Loading