MycroftAI
/
mycroft-core
mirror of https://github.com/MycroftAI/mycroft-core.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Simplify the converse callings 

Remove the use of the separate error message and use the wait_for_reply method
to get the converse result. The error message is left to guarantee
compatibility.
pull/2479/head
Åke Forslund 2020-02-24 14:05:33 +01:00
parent 02a48f954c
commit 58f0ac8b9e
4 changed files with 52 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
mycroft/skills/intent_service.py
View File

@ -175,8 +175,6 @@ class IntentService:
self.bus.on('remove_context', self.handle_remove_context)
self.bus.on('clear_context', self.handle_clear_context)
# Converse method
self.bus.on('skill.converse.response', self.handle_converse_response)
self.bus.on('skill.converse.error', self.handle_converse_error)
self.bus.on('mycroft.speech.recognition.unknown', self.reset_converse)
self.bus.on('mycroft.skills.loaded', self.update_skill_name_dict)
@ -186,9 +184,6 @@ class IntentService:
self.bus.on('active_skill_request', add_active_skill_handler)
self.active_skills = [] # [skill_id , timestamp]
self.converse_timeout = 5 # minutes to prune active_skills
self.waiting_for_converse = False
self.converse_result = False
self.converse_skill_id = ""
# Intents API
self.registered_intents = []
@ -228,33 +223,20 @@ class IntentService:
self.do_converse(None, skill[0], lang, message)
def do_converse(self, utterances, skill_id, lang, message):
self.waiting_for_converse = True
self.converse_result = False
self.converse_skill_id = skill_id
self.bus.emit(message.reply("skill.converse.request", {
converse_msg = (message.reply("skill.converse.request", {
"skill_id": skill_id, "utterances": utterances, "lang": lang}))
start_time = time.time()
t = 0
while self.waiting_for_converse and t < 5:
t = time.time() - start_time
time.sleep(0.1)
self.waiting_for_converse = False
self.converse_skill_id = ""
return self.converse_result
result = self.bus.wait_for_response(converse_msg,
'skill.converse.response')
if result and 'error' in result.data:
self.handle_converse_error(result)
return False
else:
return result.data.get('result', False)
def handle_converse_error(self, message):
skill_id = message.data["skill_id"]
if message.data["error"] == "skill id does not exist":
self.remove_active_skill(skill_id)
if skill_id == self.converse_skill_id:
self.converse_result = False
self.waiting_for_converse = False
def handle_converse_response(self, message):
skill_id = message.data["skill_id"]
if skill_id == self.converse_skill_id:
self.converse_result = message.data.get("result", False)
self.waiting_for_converse = False
def remove_active_skill(self, skill_id):
for skill in self.active_skills:

22
mycroft/skills/skill_manager.py
View File

@ -406,7 +406,10 @@ class SkillManager(Thread):
self._emit_converse_error(message, skill_id, error_message)
break
try:
self._emit_converse_response(message, skill_loader)
utterances = message.data['utterances']
lang = message.data['lang']
result = skill_loader.instance.converse(utterances, lang)
self._emit_converse_response(result, message, skill_loader)
except Exception:
error_message = 'exception in converse method'
LOG.exception(error_message)
@ -419,16 +422,17 @@ class SkillManager(Thread):
self._emit_converse_error(message, skill_id, error_message)
def _emit_converse_error(self, message, skill_id, error_msg):
reply = message.reply(
'skill.converse.error',
data=dict(skill_id=skill_id, error=error_msg)
)
"""Emit a message reporting the error back to the intent service."""
reply = message.reply('skill.converse.response',
data=dict(skill_id=skill_id, error=error_msg))
self.bus.emit(reply)
# Also emit the old error message to keep compatibility
# TODO Remove in 20.08
reply = message.reply('skill.converse.error',
data=dict(skill_id=skill_id, error=error_msg))
self.bus.emit(reply)
def _emit_converse_response(self, message, skill_loader):
utterances = message.data['utterances']
lang = message.data['lang']
result = skill_loader.instance.converse(utterances, lang)
def _emit_converse_response(self, result, message, skill_loader):
reply = message.reply(
'skill.converse.response',
data=dict(skill_id=skill_loader.skill_id, result=result)

58
test/unittests/skills/test_intent_service.py
View File

@ -12,8 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
from threading import Thread
import time
from unittest import TestCase, mock
from mycroft.messagebus import Message
@ -89,27 +87,22 @@ class ConversationTest(TestCase):
Also check that the skill that handled the query is moved to the
top of the active skill list.
"""
result = None
def response(message, return_msg_type):
c64 = Message(return_msg_type, {'skill_id': 'c64_skill',
'result': False})
atari = Message(return_msg_type, {'skill_id': 'atari_skill',
'result': True})
msgs = {'c64_skill': c64, 'atari_skill': atari}
def runner(utterances, lang, message):
nonlocal result
result = self.intent_service._converse(utterances, lang, message)
return msgs[message.data['skill_id']]
self.intent_service.bus.wait_for_response.side_effect = response
hello = ['hello old friend']
utterance_msg = Message('recognizer_loop:utterance',
data={'lang': 'en-US',
'utterances': hello})
t = Thread(target=runner, args=(hello, 'en-US', utterance_msg))
t.start()
time.sleep(0.5)
self.intent_service.handle_converse_response(
Message('converse.response', {'skill_id': 'c64_skill',
'result': False}))
time.sleep(0.5)
self.intent_service.handle_converse_response(
Message('converse.response', {'skill_id': 'atari_skill',
'result': True}))
t.join()
result = self.intent_service._converse(hello, 'en-US', utterance_msg)
# Check that the active skill list was updated to set the responding
# Skill first.
@ -121,23 +114,26 @@ class ConversationTest(TestCase):
def test_reset_converse(self):
"""Check that a blank stt sends the reset signal to the skills."""
print(self.intent_service.active_skills)
def response(message, return_msg_type):
c64 = Message(return_msg_type,
{'skill_id': 'c64_skill',
'error': 'skill id does not exist'})
atari = Message(return_msg_type, {'skill_id': 'atari_skill',
'result': False})
msgs = {'c64_skill': c64, 'atari_skill': atari}
return msgs[message.data['skill_id']]
reset_msg = Message('mycroft.speech.recognition.unknown',
data={'lang': 'en-US'})
t = Thread(target=self.intent_service.reset_converse,
args=(reset_msg,))
t.start()
time.sleep(0.5)
self.intent_service.handle_converse_error(
Message('converse.error', {'skill_id': 'c64_skill',
'error': 'skill id does not exist'}))
time.sleep(0.5)
self.intent_service.handle_converse_response(
Message('converse.response', {'skill_id': 'atari_skill',
'result': False}))
self.intent_service.bus.wait_for_response.side_effect = response
self.intent_service.reset_converse(reset_msg)
# Check send messages
c64_message = self.intent_service.bus.emit.call_args_list[0][0][0]
wait_for_response_mock = self.intent_service.bus.wait_for_response
c64_message = wait_for_response_mock.call_args_list[0][0][0]
self.assertTrue(check_converse_request(c64_message, 'c64_skill'))
atari_message = self.intent_service.bus.emit.call_args_list[1][0][0]
atari_message = wait_for_response_mock.call_args_list[1][0][0]
self.assertTrue(check_converse_request(atari_message, 'atari_skill'))
first_active_skill = self.intent_service.active_skills[0][0]
self.assertEqual(first_active_skill, 'atari_skill')

5
test/unittests/skills/test_skill_manager.py
View File

@ -83,6 +83,7 @@ class TestSkillManager(MycroftUnitTestBase):
self.skill_loader_mock.instance = Mock()
self.skill_loader_mock.instance.default_shutdown = Mock()
self.skill_loader_mock.instance.converse = Mock()
self.skill_loader_mock.instance.converse.return_value = True
self.skill_loader_mock.skill_id = 'test_skill'
self.skill_manager.skill_loaders = {
str(self.skill_dir): self.skill_loader_mock
@ -217,7 +218,8 @@ class TestSkillManager(MycroftUnitTestBase):
def test_handle_converse_request(self):
message = Mock()
message.data = dict(skill_id='test_skill')
message.data = dict(skill_id='test_skill', utterances=['hey you'],
lang='en-US')
self.skill_loader_mock.loaded = True
converse_response_mock = Mock()
self.skill_manager._emit_converse_response = converse_response_mock
@ -226,6 +228,7 @@ class TestSkillManager(MycroftUnitTestBase):
self.skill_manager.handle_converse_request(message)
converse_response_mock.assert_called_once_with(
True,
message,
self.skill_loader_mock
)