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

Fixes #1547 Addition of say as tts engine #1548

Closed
wants to merge 5 commits into from

Conversation

bhaveshAn
Copy link

Description

Fixes #1547 Addition of say as tts engine

How to test

(Description of how to validate or test this PR)

Contributor license agreement signed?

CLA [ ] (Whether you have signed a CLA - Contributor Licensing Agreement
@forslund @penrods @aatchison Please review. thanks.

@pep8speaks
Copy link

pep8speaks commented Apr 19, 2018

Hello @bhaveshAn! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on April 19, 2018 at 21:45 Hours UTC

from mycroft.tts import TTS, TTSValidator


class Say(TTS):
Copy link
Contributor

Choose a reason for hiding this comment

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

Since say is pretty vague in the context of text to speech (even though that's what it's called), would you mind renaming it to something like RSynth? This also gives people a better search term to use to download it.


def validate_connection(self):
try:
subprocess.call(['say', 'hello'])
Copy link
Contributor

Choose a reason for hiding this comment

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

This actually produces the audio hello every time the device boots up or speaks. This should be changed to something that doesn't output audio. Perhaps a check of distutils.spawn.find_executable('say') would work.

@bhaveshAn
Copy link
Author

Hi @MatthewScholefield please review the code.

@MatthewScholefield
Copy link
Contributor

MatthewScholefield commented Apr 19, 2018

So, I just realized, you could also be referring to the say command on Mac OS. If that's the case, I think we should wait to add this feature until we properly support Macs as a whole.

@bhaveshAn
Copy link
Author

Initially say used in mac.

But I am able to use say on linux as well.
You can check here : https://www.youtube.com/watch?v=-aLWuUkYqJs

@penrods penrods added the CLA: Needed Need signed CLA from https://mycroft.ai/cla label May 3, 2018
@penrods
Copy link
Contributor

penrods commented Sep 24, 2018

With no CLA I cannot accept these changes. I'm closing unmerged, but am happy to revisit if you would like to request a CLA via https://mycroft.ai/cla

@penrods penrods closed this Sep 24, 2018
@bhaveshAn
Copy link
Author

@penrods I can't get email for cla.

@penrods penrods reopened this Sep 24, 2018
@penrods
Copy link
Contributor

penrods commented Sep 24, 2018

I checked and the CLA request should be heading your way. Search your email for something from the Hellosign service.

@bhaveshAn
Copy link
Author

@penrods I have signed it. Please check

@penrods penrods added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Oct 1, 2018
@penrods penrods removed the CLA: Needed Need signed CLA from https://mycroft.ai/cla label Oct 19, 2018
@krisgesling krisgesling added Type: Enhancement - proposed New proposal for a feature that is not currently a priority on the roadmap. and removed type: feature labels Sep 24, 2020
@krisgesling
Copy link
Contributor

krisgesling commented Sep 24, 2020

Hi there, not sure why this didn't get merged previously, but we're planning to add a plugin system for audio backends, STT and TTS services.

This system will allow users (or developers configuring Mycroft for use in other projects) to pull in these services as they're needed rather than having them all included by default on every Mycroft installation. The aim is to keep mycroft-core lighter but still provide the same level of features and customization through the provision of these plugins.

So I'm going to close this PR but only because it won't need to be included in mycroft-core. We'll be transitioning all of these out to be independent plugins as per #2701 and can include this service in that work.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) Type: Enhancement - proposed New proposal for a feature that is not currently a priority on the roadmap.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Addition of say as tts engine
5 participants