-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
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
Changes from 25 commits
eedb3d0
bf015c9
b3d7bf6
8651566
c842be2
04b407a
c479075
13d0d9a
ac5d2e5
2dcfe18
861cdc5
2214096
90280d7
f287689
7992a73
5e987e8
5496efd
cb8ab73
2d9becd
0d955d3
1262ba3
ed1bcf2
6cfce83
95d30c6
42273fc
a43bc92
b2ad2f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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' | ||
|
||
|
@@ -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): | ||
|
@@ -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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.""" | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,7 +46,6 @@ | |
'ephem', | ||
'evohomeclient', | ||
'feedparser', | ||
'fuzzywuzzy', | ||
'gTTS-token', | ||
'ha-ffmpeg', | ||
'haversine', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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': { | ||
|
@@ -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, {}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this need to be setup? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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' | ||
]) | ||
|
@@ -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 | ||
|
||
|
@@ -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 | ||
|
||
|
@@ -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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The tests for the generic intents need to be moved to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And then they shouldn't depend on conversation. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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 | ||
|
||
|
@@ -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 | ||
|
||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 👍