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

Refactor/converse messages #144

Merged
merged 5 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
49 changes: 21 additions & 28 deletions ovos_workshop/skills/ovos.py
Original file line number Diff line number Diff line change
Expand Up @@ -915,9 +915,9 @@ def _register_system_event_handlers(self):
if self.stop_is_implemented:
self.add_event('mycroft.stop', self.__handle_stop,
speak_errors=False)
self.add_event('skill.converse.ping', self._handle_converse_ack,
self.add_event(f"{self.skill_id}.converse.ping", self._handle_converse_ack,
emphasize marked this conversation as resolved.
Show resolved Hide resolved
speak_errors=False)
self.add_event('skill.converse.request', self._handle_converse_request,
self.add_event(f"{self.skill_id}.converse.request", self._handle_converse_request,
speak_errors=False)
self.add_event(f"{self.skill_id}.activate", self.handle_activate,
speak_errors=False)
Expand All @@ -938,7 +938,7 @@ def _register_system_event_handlers(self):
self.add_event('mycroft.skills.settings.changed',
self.handle_settings_change, speak_errors=False)

self.add_event("skill.converse.get_response", self.__handle_get_response, speak_errors=False)
self.add_event(f"{self.skill_id}.converse.get_response", self.__handle_get_response, speak_errors=False)

def _send_public_api(self, message: Message):
"""
Expand Down Expand Up @@ -1017,7 +1017,7 @@ def _handle_converse_ack(self, message: Message):
may override the property self.converse_is_implemented to enable or
disable converse support. Note that this does not affect a skill's
`active` status.
@param message: `skill.converse.ping` Message
@param message: `{self.skill_id}.converse.ping` Message
"""
self.bus.emit(message.reply(
"skill.converse.pong",
Expand All @@ -1029,26 +1029,24 @@ def _handle_converse_request(self, message: Message):
"""
If this skill is requested and supports converse, handle the user input
with `converse`.
@param message: `skill.converse.request` Message
@param message: `{self.skill_id}.converse.request` Message
"""
skill_id = message.data['skill_id']
if skill_id == self.skill_id:
try:
# converse can have multiple signatures
params = signature(self.converse).parameters
kwargs = {"message": message,
"utterances": message.data['utterances'],
"lang": message.data['lang']}
kwargs = {k: v for k, v in kwargs.items() if k in params}
result = self.converse(**kwargs)
self.bus.emit(message.reply('skill.converse.response',
{"skill_id": self.skill_id,
"result": result}))
except Exception as e:
LOG.error(e)
self.bus.emit(message.reply('skill.converse.response',
{"skill_id": self.skill_id,
"result": False}))
try:
# converse can have multiple signatures
params = signature(self.converse).parameters
kwargs = {"message": message,
"utterances": message.data['utterances'],
"lang": message.data['lang']}
kwargs = {k: v for k, v in kwargs.items() if k in params}
result = self.converse(**kwargs)
self.bus.emit(message.reply('skill.converse.response',
{"skill_id": self.skill_id,
"result": result}))
except Exception as e:
LOG.error(e)
self.bus.emit(message.reply('skill.converse.response',
{"skill_id": self.skill_id,
"result": False}))

def _handle_collect_resting(self, message: Optional[Message] = None):
"""
Expand Down Expand Up @@ -1545,11 +1543,6 @@ def converse(utterances, lang=None):
return ans

def __handle_get_response(self, message):

skill_id = message.data["skill_id"]
if skill_id != self.skill_id:
return # not for us!
emphasize marked this conversation as resolved.
Show resolved Hide resolved

# validate session_id to ensure this isnt another
# user querying the skill at same time
sess2 = SessionManager.get(message)
Expand Down
10 changes: 6 additions & 4 deletions test/end2end/session/test_complete_failure.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def test_complete_failure(self):
SessionManager.sessions = {}
SessionManager.default_session = SessionManager.sessions["default"] = Session("default")
SessionManager.default_session.lang = "en-us"
SessionManager.default_session.active_skills = [(self.skill_id, time.time())]
messages = []

def new_msg(msg):
Expand Down Expand Up @@ -46,7 +47,7 @@ def wait_for_n_messages(n):
expected_messages = [
"recognizer_loop:utterance",
# Converse
"skill.converse.ping",
f"{self.skill_id}.converse.ping",
"skill.converse.pong",
# FallbackV1
"mycroft.skills.fallback",
Expand Down Expand Up @@ -81,7 +82,7 @@ def wait_for_n_messages(n):
self.assertEqual(m.context["session"]["session_id"], "default")

# verify ping/pong answer from hello world skill
self.assertEqual(messages[1].msg_type, "skill.converse.ping")
self.assertEqual(messages[1].msg_type, f"{self.skill_id}.converse.ping")
self.assertEqual(messages[2].msg_type, "skill.converse.pong")
self.assertEqual(messages[2].data["skill_id"], self.skill_id)
self.assertEqual(messages[2].context["skill_id"], self.skill_id)
Expand Down Expand Up @@ -131,6 +132,7 @@ def test_complete_failure_lang_detect(self):
SessionManager.sessions = {}
SessionManager.default_session = SessionManager.sessions["default"] = Session("default")
SessionManager.default_session.lang = "en-us"
SessionManager.default_session.active_skills = [(self.skill_id, time.time())]

stt_lang_detect = "pt-pt"

Expand Down Expand Up @@ -167,7 +169,7 @@ def wait_for_n_messages(n):
expected_messages = [
"recognizer_loop:utterance",
"ovos.session.update_default", # language changed
"skill.converse.ping",
f"{self.skill_id}.converse.ping",
"skill.converse.pong",
"mycroft.skills.fallback",
"mycroft.skill.handler.start",
Expand Down Expand Up @@ -205,7 +207,7 @@ def wait_for_n_messages(n):
self.assertEqual(messages[1].data["session_data"]["lang"], stt_lang_detect)

# verify ping/pong answer from hello world skill
self.assertEqual(messages[2].msg_type, "skill.converse.ping")
self.assertEqual(messages[2].msg_type, f"{self.skill_id}.converse.ping")
self.assertEqual(messages[3].msg_type, "skill.converse.pong")
self.assertEqual(messages[3].data["skill_id"], self.skill_id)
self.assertEqual(messages[3].context["skill_id"], self.skill_id)
Expand Down
Loading
Loading