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

Bugfix/audioservice invalid utterance #1857

Merged

Conversation

forslund
Copy link
Collaborator

Description

The CommonPlaybackSkill may send None as an utterance when invoking the audioservice.play() method. This fix makes sure that None is handled in a sane way.

Additionally the CPS_play method has been altered to not add an utterance if the skill tries to add one.

How to test

Try invoking the NPR_news skill with "what's the news" and make sure it works.

Contributor license agreement signed?

CLA [Yes]

play would happily send on None to the audioservice even though it's not a proper sentence. This will handle None and default it to an empty string.
If an utterance is provided already the method shall not try to override it with a stored utterance.
@pep8speaks
Copy link

Hello @forslund! Thanks for submitting the PR.

Copy link
Contributor

@LearnedVector LearnedVector left a comment

Choose a reason for hiding this comment

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

LGTM

@forslund
Copy link
Collaborator Author

Codacy seems to have hung. I'm going ahead with the merge

@forslund forslund merged commit 84694be into MycroftAI:dev Oct 23, 2018
@penrods penrods added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Dec 27, 2018
@forslund forslund deleted the bugfix/audioservice-invalid-utterance branch February 18, 2019 09:55
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants