Skip to content
This repository has been archived by the owner on Sep 8, 2024. It is now read-only.

Plugin system for stt, tts and audioservices #2594

Merged
merged 7 commits into from
Oct 19, 2020

Conversation

forslund
Copy link
Collaborator

@forslund forslund commented May 24, 2020

Description

This adds a simple plugin system for loading external stt, tts and audioservices. The "plugins" are pip install-able modules presenting one or more entrypoints with a entrypoint group defined in setup.py (Example)

TTS group: "mycroft.plugin.tts"
STT group: "mycroft.plugin.stt"
Audioservice group: "mycroft.plugin.audioservice"
Wake-word group: "mycroft.plugin.wake_word"

STT and TTS loads a single object for the specified module (as specified in the config), while audioservice loads all found audioservices and append to the list of builtins services.

This system makes it easier to quickly add third-party services without changing core, it also makes it easier for users to select things like Polly where the requirement will automatically be installed together with the TTS module.

The detection system is based on the documentation for python packaging. Link

-Future work: hotword engines. Can the same system be leveraged for skills? Update and reloading needs to be considered...-
@JarbasAl contributed an implementation of hot/wake-word engine plugins. There is a test plugin available here

How to test

mycroft-pip install git+https://github.com/forslund/mycroft-tts-plugin-gtts.git

Then set the tts module to google_tts_plug assert that the google tts voice is heard

Contributor license agreement signed?

CLA [ Yes ]

@devs-mycroft devs-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label May 24, 2020
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@krisgesling
Copy link
Contributor

Hey sorry it's taken some time to get back to this. I really like it though.

I think it will be great if we can keep mycroft-core to what is actually "core" to running any instance of Mycroft, and then provide the tooling to build up what's needed on top of that for specific implementations.

The example plugin doesn't seem to be working for me. Looks like gtts.lang.tts_langs was added in gTTS v2.0.0 but the plugin is installing v1.1.7? I tried a quick requirement update in line with core (v2.0.4) but no luck and haven't dug into that further.

@chrisveilleux if you agree with the addition of the plugin system, it would be good to merge it in pretty soon so it can be tested in the wild. Even though this isn't a breaking change itself, it will mean for 20.08 we can confidently remove non-core services and provide them as plugins instead.

@forslund
Copy link
Collaborator Author

You're right that the requirement was a bit old. I've now updated it to track core's and for me it works [TM].

Do you get any error in the audio.log?

If the plug is loaded it should show

2020-07-29 07:56:47.651 | INFO     |  8201 | mycroft.tts.tts:create:524 | Loaded plugin google_tts_plug

@krisgesling
Copy link
Contributor

Hooray, working now. I think it was using a cached wheel when I tried to update it.

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

Copy link
Member

@chrisveilleux chrisveilleux left a comment

Choose a reason for hiding this comment

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

I like the idea of a plugin system to keep the core code cleaner.

@krisgesling Our documentation should be updated to explain how to use this. I would prefer not to deploy it until the documentation is ready.

Some sort of automated testing should be added for this code.

mycroft/audio/audioservice.py Outdated Show resolved Hide resolved
mycroft/audio/audioservice.py Outdated Show resolved Hide resolved
mycroft/audio/audioservice.py Show resolved Hide resolved
mycroft/audio/audioservice.py Outdated Show resolved Hide resolved
mycroft/audio/audioservice.py Outdated Show resolved Hide resolved
mycroft/audio/audioservice.py Show resolved Hide resolved
mycroft/stt/__init__.py Show resolved Hide resolved
mycroft/tts/tts.py Show resolved Hide resolved
@forslund
Copy link
Collaborator Author

I've updated the code styling changes requested and tried to answer the comments. I still think the small wrapper functions makes sense to have from a readability point of view but if you really like them gone I can inline them.

I'll write some unit tests and an entry for the documentation this weekend to go with this update.

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

1 similar comment
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@krisgesling
Copy link
Contributor

Documentation for the plugin system is now available at:
https://mycroft-ai.gitbook.io/docs/mycroft-technologies/mycroft-core/plugins

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

Entrypoints registered under "mycroft.plugin.tts" will be detected and
able to run.
pip packages registering entrypoints under 'mycroft.plugin.stt' will be
detected.
All modules registered under the entrypoint group "mycroft.plugin.audioservice"

These shall refer to a module with either an autodetect function or a
load_service function.
@forslund
Copy link
Collaborator Author

I've rebased this and fixed the conflict. I've also added some unittests for the plugins util. Documentation is available here: https://mycroft-ai.gitbook.io/docs/mycroft-technologies/mycroft-core/plugins

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

@forslund
Copy link
Collaborator Author

forslund commented Sep 23, 2020

Hmm, must have messed up the rebase in audioservice...will invesigate

Edit: No...locally working...

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@JarbasAl
Copy link
Contributor

JarbasAl commented Oct 3, 2020

I'm thinking we need this for wake words and enclosures

@forslund
Copy link
Collaborator Author

forslund commented Oct 3, 2020

Wakewords is something I've planned for as well but wanted to make as a follow up PR since this is enough to review.

Enclosures is a nice idea I didn't think of. Previously the idea was to extract the enclosure system from core entirely and just use the messagebus connection for it (mycroft-messagebus-client + a base enclosure module) but including it as a pluggable system should work quite well also.

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

Copy link
Contributor

@krisgesling krisgesling left a comment

Choose a reason for hiding this comment

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

This all looks great, working as expected, and I've updated the docs for the addition of Wake Word Plugins:
https://mycroft-ai.gitbook.io/docs/mycroft-technologies/mycroft-core/plugins

@krisgesling krisgesling dismissed chrisveilleux’s stale review October 19, 2020 06:10

Requested changes have been made and comments resolved.

@krisgesling krisgesling merged commit 8a1b989 into MycroftAI:dev Oct 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) Status: To be reviewed Concept accepted and PR has sufficient information for full review Type: Enhancement - roadmapped Implementation of a feature that is a priority on the roadmap.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants