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

fix issue #1566 #37

Open
wants to merge 2 commits into
base: 20.08
Choose a base branch
from
Open

fix issue #1566 #37

wants to merge 2 commits into from

Conversation

domcross
Copy link
Contributor

@domcross domcross commented Oct 7, 2020

fixes issue #1566

When Mycroft encounters any command with the word "stop," it simply isn't processed.

Make this a FallbackSkill, check in fallback-handler if utterance matches exactly phrase from Stop.voc and emit mycroft.stop accordingly.

@krisgesling
Copy link
Contributor

Hey, I like the concept of using the Fallback, and think we should avoid single keyword Adapt intents where ever possible. However I think we need to consider this one pretty carefully, so I'm just going to lay my thoughts out a few initial thoughts so anyone can add or poke holes.

Behaviours

  • Some individual Skills handle specific requests like "Stop the timer"
  • More generic phrases like "stop the music" are only caught by this Skill (I think). So we probably need to add a new intent to the Common Playback Skill at the very least.

Should we use the Fallback but still match the vocab more broadly rather than requiring the utterance to be an exact match?. This should allow us to still handle utterances like:

  • "stop whatever you're doing"
  • "whatever you're doing shut up"

Will want to make sure this doesn't impact the latency of "stopping". This should be better now that we pause audio output while we wait for the response.

@domcross
Copy link
Contributor Author

domcross commented Oct 8, 2020

The stop-handler matches only if the utterance is the single word "stop" (or one of the other words in Stop.voc), it will ignore all other phrases like "stop the music", "where is the next bus stop" or "play please don't stop the music"

Phrase "silent" is in Stop.voc of mycroft-stop skill and in Mute.voc of mycroft-timer skill. Utterance "silent" is handled by mycroft-timer, in a quick test I found that this behaviour is the same as before.

p.s. Kudos and apologies go to Jarbas - he submitted similar PR two years ago but it never got merged. I found it out only after submitting my PR.

@krisgesling
Copy link
Contributor

Yeah, the challenge is that people often say more than just "stop" eg "please stop". So we need to strike a balance there.

Shifting the Skill to be a Fallback gives everything else the opportunity to respond first, so presuming there is some kind of Transport Skill "where is the next stop" should get handled by that Skill rather than go to the Fallback Skills.

My concern with being too specific on the stop vocab is that people might find that what they used to say suddenly no longer works.

@domcross
Copy link
Contributor Author

Shouldn't be a problem to add phrases like "please stop" or "quiet please" to Stop.voc, there is "shut up" and "be quiet" already in there. The exact phrase must match, though.

@domcross
Copy link
Contributor Author

Another solution could be fuzzy matching:

>>> from mycroft.util.parse import match_one
>>> words = ['stop','silence','shut up', 'be quiet']
>>> match_one('please stop', words)
('stop', 0.5333333333333333)
>>> match_one('quiet', words)
('be quiet', 0.7692307692307693)
>>> match_one('stop', words)
('stop', 1.0)
>>> match_one('quiet please', words)
('be quiet', 0.5)
>>> match_one('shut up now', words)
('shut up', 0.7777777777777778)
>>> match_one('can you shut up please', words)
('shut up', 0.4827586206896552)
>>> match_one('can you shut up', words)
('shut up', 0.6363636363636364)
>>> match_one('just shut up', words)
('shut up', 0.7368421052631579)

But what would be a good threshold value for score then - >=0.7?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants