Skip to content

Commit 62024db

Browse files
authored
refactor!: drop old pipeline plugins and deprecated methods (#690)
* feat:pipeline plugin factory loads pipeline plugins from config 🎉 no longer tied to adapt/padatious * migrate pipeline names to new style * logs * add model2vec test * add model2vec test * ensure working fallback skill version * reuse minicroft instance to speed up tests * rework pipeline matchers * fix tearDown of tests * log level to help debug failing tests * explicitly check that intent service is ready + remove deprecated skill loading from folder * update tests * fallback skill tests * feat: standalone intent service support * pipeline plugin shutdown * coderabbit suggestions * coderabbit suggestions * coderabbit suggestions * test converse * test converse * fix: sess injection
1 parent a916266 commit 62024db

34 files changed

+1295
-1774
lines changed

.github/workflows/unit_tests.yml

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,6 @@ on:
2929

3030
jobs:
3131
unit_tests:
32-
strategy:
33-
max-parallel: 3
34-
matrix:
35-
python-version: ["3.11", "3.12"]
3632
runs-on: ubuntu-latest
3733
permissions:
3834
# Gives the action the necessary permissions for publishing new
@@ -45,10 +41,10 @@ jobs:
4541
timeout-minutes: 35
4642
steps:
4743
- uses: actions/checkout@v4
48-
- name: Set up python ${{ matrix.python-version }}
44+
- name: Set up python
4945
uses: actions/setup-python@v5
5046
with:
51-
python-version: ${{ matrix.python-version }}
47+
python-version: "3.11"
5248
- name: Install System Dependencies
5349
run: |
5450
sudo apt-get update

ovos_core/intent_services/__init__.py

Lines changed: 1 addition & 973 deletions
Large diffs are not rendered by default.

ovos_core/intent_services/adapt_service.py

Lines changed: 0 additions & 12 deletions
This file was deleted.

ovos_core/intent_services/commonqa_service.py

Lines changed: 0 additions & 11 deletions
This file was deleted.

ovos_core/intent_services/converse_service.py

Lines changed: 51 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,33 @@
11
import time
22
from threading import Event
3-
from typing import Optional, List
3+
from typing import Optional, Dict, List, Union
44

5+
from ovos_bus_client.client import MessageBusClient
56
from ovos_bus_client.message import Message
67
from ovos_bus_client.session import SessionManager, UtteranceState, Session
7-
from ovos_bus_client.util import get_message_lang
88
from ovos_config.config import Configuration
9-
from ovos_config.locale import setup_locale
10-
from ovos_plugin_manager.templates.pipeline import PipelineMatch, PipelinePlugin
119
from ovos_utils import flatten_list
10+
from ovos_utils.fakebus import FakeBus
1211
from ovos_utils.lang import standardize_lang_tag
1312
from ovos_utils.log import LOG
13+
14+
from ovos_plugin_manager.templates.pipeline import PipelinePlugin, IntentHandlerMatch
1415
from ovos_workshop.permissions import ConverseMode, ConverseActivationMode
1516

1617

1718
class ConverseService(PipelinePlugin):
1819
"""Intent Service handling conversational skills."""
1920

20-
def __init__(self, bus):
21-
self.bus = bus
21+
def __init__(self, bus: Optional[Union[MessageBusClient, FakeBus]] = None,
22+
config: Optional[Dict] = None):
23+
config = config or Configuration().get("skills", {}).get("converse", {})
24+
super().__init__(bus, config)
2225
self._consecutive_activations = {}
23-
self.bus.on('mycroft.speech.recognition.unknown', self.reset_converse)
2426
self.bus.on('intent.service.skills.deactivate', self.handle_deactivate_skill_request)
2527
self.bus.on('intent.service.skills.activate', self.handle_activate_skill_request)
26-
self.bus.on('active_skill_request', self.handle_activate_skill_request) # TODO backwards compat, deprecate
2728
self.bus.on('intent.service.active_skills.get', self.handle_get_active_skills)
2829
self.bus.on("skill.converse.get_response.enable", self.handle_get_response_enable)
2930
self.bus.on("skill.converse.get_response.disable", self.handle_get_response_disable)
30-
super().__init__(config=Configuration().get("skills", {}).get("converse") or {})
3131

3232
@property
3333
def active_skills(self):
@@ -209,17 +209,15 @@ def _converse_allowed(self, skill_id: str) -> bool:
209209

210210
def _collect_converse_skills(self, message: Message) -> List[str]:
211211
"""use the messagebus api to determine which skills want to converse
212-
This includes all skills and external applications"""
213-
session = SessionManager.get(message)
214212
213+
Individual skills respond to this request via the `can_converse` method"""
215214
skill_ids = []
216-
# include all skills in get_response state
217-
want_converse = [skill_id for skill_id, state in session.utterance_states.items()
218-
if state == UtteranceState.RESPONSE]
219-
skill_ids += want_converse # dont wait for these pong answers (optimization)
220-
221-
active_skills = self.get_active_skills()
215+
want_converse = []
216+
session = SessionManager.get(message)
222217

218+
# note: this is sorted by priority already
219+
active_skills = [skill_id for skill_id in self.get_active_skills(message)
220+
if session.utterance_states.get(skill_id, UtteranceState.INTENT) == UtteranceState.INTENT]
223221
if not active_skills:
224222
return want_converse
225223

@@ -246,8 +244,7 @@ def handle_ack(msg):
246244

247245
# ask skills if they want to converse
248246
for skill_id in active_skills:
249-
self.bus.emit(message.forward(f"{skill_id}.converse.ping",
250-
{"skill_id": skill_id}))
247+
self.bus.emit(message.forward(f"{skill_id}.converse.ping", {**message.data, "skill_id": skill_id}))
251248

252249
# wait for all skills to acknowledge they want to converse
253250
event.wait(timeout=0.5)
@@ -264,73 +261,25 @@ def _check_converse_timeout(self, message: Message):
264261
skill for skill in session.active_skills
265262
if time.time() - skill[1] <= timeouts.get(skill[0], def_timeout)]
266263

267-
def converse(self, utterances: List[str], skill_id: str, lang: str, message: Message) -> bool:
268-
"""Call skill and ask if they want to process the utterance.
269-
270-
Args:
271-
utterances (list of tuples): utterances paired with normalized
272-
versions.
273-
skill_id: skill to query.
274-
lang (str): current language
275-
message (Message): message containing interaction info.
276-
277-
Returns:
278-
handled (bool): True if handled otherwise False.
279-
"""
280-
lang = standardize_lang_tag(lang)
281-
session = SessionManager.get(message)
282-
session.lang = lang
283-
284-
state = session.utterance_states.get(skill_id, UtteranceState.INTENT)
285-
if state == UtteranceState.RESPONSE:
286-
converse_msg = message.reply(f"{skill_id}.converse.get_response",
287-
{"utterances": utterances,
288-
"lang": lang})
289-
self.bus.emit(converse_msg)
290-
return True
291-
292-
if self._converse_allowed(skill_id):
293-
converse_msg = message.reply(f"{skill_id}.converse.request",
294-
{"utterances": utterances,
295-
"lang": lang})
296-
result = self.bus.wait_for_response(converse_msg,
297-
'skill.converse.response',
298-
timeout=self.config.get("max_skill_runtime", 10))
299-
if result and 'error' in result.data:
300-
error_msg = result.data['error']
301-
LOG.error(f"{skill_id}: {error_msg}")
302-
return False
303-
elif result is not None:
304-
return result.data.get('result', False)
305-
else:
306-
# abort any ongoing converse
307-
# if skill crashed or returns False, all good
308-
# if it is just taking a long time, more than 1 skill would end up answering
309-
self.bus.emit(message.forward("ovos.skills.converse.force_timeout",
310-
{"skill_id": skill_id}))
311-
LOG.warning(f"{skill_id} took too long to answer, "
312-
f'increasing "max_skill_runtime" in mycroft.conf might help alleviate this issue')
313-
return False
314-
315-
def converse_with_skills(self, utterances: List[str], lang: str, message: Message) -> Optional[PipelineMatch]:
264+
def match(self, utterances: List[str], lang: str, message: Message) -> Optional[IntentHandlerMatch]:
316265
"""
317266
Attempt to converse with active skills for a given set of utterances.
318-
267+
319268
Iterates through active skills to find one that can handle the utterance. Filters skills based on timeout and blacklist status.
320-
269+
321270
Args:
322271
utterances (List[str]): List of utterance strings to process
323272
lang (str): 4-letter ISO language code for the utterances
324273
message (Message): Message context for generating a reply
325-
274+
326275
Returns:
327276
PipelineMatch: Match details if a skill successfully handles the utterance, otherwise None
328277
- handled (bool): Whether the utterance was fully handled
329278
- match_data (dict): Additional match metadata
330279
- skill_id (str): ID of the skill that handled the utterance
331280
- updated_session (Session): Current session state after skill interaction
332281
- utterance (str): The original utterance processed
333-
282+
334283
Notes:
335284
- Standardizes language tag
336285
- Filters out blacklisted skills
@@ -342,22 +291,43 @@ def converse_with_skills(self, utterances: List[str], lang: str, message: Messag
342291

343292
# we call flatten in case someone is sending the old style list of tuples
344293
utterances = flatten_list(utterances)
294+
295+
# note: this is sorted by priority already
296+
gr_skills = [skill_id for skill_id in self.get_active_skills(message)
297+
if session.utterance_states.get(skill_id, UtteranceState.INTENT) == UtteranceState.RESPONSE]
298+
299+
# check if any skill wants to capture utterance for self.get_response method
300+
for skill_id in gr_skills:
301+
if skill_id in session.blacklisted_skills:
302+
LOG.debug(f"ignoring match, skill_id '{skill_id}' blacklisted by Session '{session.session_id}'")
303+
continue
304+
LOG.debug(f"utterance captured by skill.get_response method: {skill_id}")
305+
return IntentHandlerMatch(
306+
match_type=f"{skill_id}.converse.get_response",
307+
match_data={"utterances": utterances, "lang": lang},
308+
skill_id=skill_id,
309+
utterance=utterances[0],
310+
updated_session=session
311+
)
312+
345313
# filter allowed skills
346314
self._check_converse_timeout(message)
347-
# check if any skill wants to handle utterance
315+
316+
# check if any skill wants to converse
348317
for skill_id in self._collect_converse_skills(message):
349318
if skill_id in session.blacklisted_skills:
350319
LOG.debug(f"ignoring match, skill_id '{skill_id}' blacklisted by Session '{session.session_id}'")
351320
continue
352321
LOG.debug(f"Attempting to converse with skill: {skill_id}")
353-
if self.converse(utterances, skill_id, lang, message):
354-
state = session.utterance_states.get(skill_id, UtteranceState.INTENT)
355-
return PipelineMatch(handled=state != UtteranceState.RESPONSE,
356-
# handled == True -> emit "ovos.utterance.handled"
357-
match_data={},
358-
skill_id=skill_id,
359-
updated_session=session,
360-
utterance=utterances[0])
322+
if self._converse_allowed(skill_id):
323+
return IntentHandlerMatch(
324+
match_type=f"{skill_id}.converse.request",
325+
match_data={"utterances": utterances, "lang": lang},
326+
skill_id=skill_id,
327+
utterance=utterances[0],
328+
updated_session=session
329+
)
330+
361331
return None
362332

363333
@staticmethod
@@ -400,11 +370,6 @@ def handle_deactivate_skill_request(self, message: Message):
400370
if sess.session_id == "default":
401371
SessionManager.sync(message)
402372

403-
def reset_converse(self, message: Message):
404-
"""Let skills know there was a problem with speech recognition"""
405-
lang = get_message_lang()
406-
self.converse_with_skills([], lang, message)
407-
408373
def handle_get_active_skills(self, message: Message):
409374
"""Send active skills to caller.
410375
@@ -415,10 +380,8 @@ def handle_get_active_skills(self, message: Message):
415380
{"skills": self.get_active_skills(message)}))
416381

417382
def shutdown(self):
418-
self.bus.remove('mycroft.speech.recognition.unknown', self.reset_converse)
419383
self.bus.remove('intent.service.skills.deactivate', self.handle_deactivate_skill_request)
420384
self.bus.remove('intent.service.skills.activate', self.handle_activate_skill_request)
421-
self.bus.remove('active_skill_request', self.handle_activate_skill_request) # TODO backwards compat, deprecate
422385
self.bus.remove('intent.service.active_skills.get', self.handle_get_active_skills)
423386
self.bus.remove("skill.converse.get_response.enable", self.handle_get_response_enable)
424387
self.bus.remove("skill.converse.get_response.disable", self.handle_get_response_disable)

0 commit comments

Comments
 (0)