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

Conversation

@MatthewScholefield
Copy link
Contributor

@MatthewScholefield MatthewScholefield commented Jul 10, 2017

This is a new system that allows other skills to register as fallbacks to handle general knowledge queries. For now, each skill self-assigns a priority where 0 is the highest and 100 is very low priority. When an intent failure occurs, each fallback gets run until one returns True meaning it found a response to speak to the user. Example usage:

skill-my-fallback/__init__.py:

from mycroft.skills.core import MycroftSkill
class MyFallbackSkilll(MycroftSkill):
    def __init__(self):
        MycroftSkill.__init__(self, name="MyFallbackSkill")

    def initialize(self):
        self.register_fallback(self.handle_fallback, 80)

    def handle_fallback(self, message):
        if 'what is' in message.data['utterance']:
            self.speak_dialog('the answer is always 42')
            return True
        return False

This PR also removes the multi utterance intent fail. It only makes sense to emit an intent_failure regardless of the amount of intents, especially since we only ever receive one utterance anymore.

This PR relies on this PR in skill-wolfram-alpha. Instructions for testing:

git fetch
git checkout feature/fallbacks
mycroft_dir="$(pwd)"
cd /opt/mycroft/skills/skill-wolfram-alpha
git fetch
git checkout feature/fallbacks
cd "$mycroft_dir"
./mycroft.sh start

Then test functionality of Wolfram Alpha for questions like what is a fox (and maybe wolfram skill reloading when /opt/mycroft/skill-wolfram-alpha/__init__.py is edited).

Also removes multi utterence intent fail. Only makes sense to emit an intent_failure regardless of the amount of intents
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 39.337% when pulling 52f2629 on feature/fallbacks into 73b01c9 on dev.

@augustnmonteiro augustnmonteiro added the Status: To be reviewed Concept accepted and PR has sufficient information for full review label Jul 12, 2017
while priority in MycroftSkill.fallback_handlers:
priority += 1

MycroftSkill.fallback_handlers[priority] = handler
Copy link
Contributor

Choose a reason for hiding this comment

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

in this case, if have another skill with the same priority it's gonna override the first one.

Copy link
Contributor Author

@MatthewScholefield MatthewScholefield Jul 12, 2017

Choose a reason for hiding this comment

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

It should keep incrementing the priority in the two lines of code above until there is no handler at that priority.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I didn't saw

@augustnmonteiro augustnmonteiro added Status: Change requested and removed Status: To be reviewed Concept accepted and PR has sufficient information for full review labels Jul 12, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+2.2%) to 41.598% when pulling 1960f69 on feature/fallbacks into 73b01c9 on dev.

@MatthewScholefield MatthewScholefield added Status: To be reviewed Concept accepted and PR has sufficient information for full review and removed Status: Change requested labels Jul 12, 2017
@forslund
Copy link
Collaborator

Would it be an idea to subclass a FallbackSkill from MycroftSkill with these new methods? MycroftSkill keeps growing (Partly my own fault) and this may be a good oppurtunity to split off some parts. Backwards compatibility might be tricky but worst case would be a fork of the original wolfram skill.

@MatthewScholefield
Copy link
Contributor Author

@forslund Yeah, I love the idea! Just implemented.

@coveralls
Copy link

Coverage Status

Coverage increased (+2.2%) to 41.607% when pulling 89d8e72 on feature/fallbacks into 73b01c9 on dev.

@coveralls
Copy link

Coverage Status

Coverage increased (+2.2%) to 41.607% when pulling 89d8e72 on feature/fallbacks into 73b01c9 on dev.

@forslund
Copy link
Collaborator

@MatthewScholefield, looks great!

Copy link
Contributor

@ethanaward ethanaward left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@MatthewScholefield MatthewScholefield merged commit 6ca4161 into dev Jul 14, 2017
@MatthewScholefield MatthewScholefield deleted the feature/fallbacks branch July 14, 2017 22:27
MatthewScholefield added a commit that referenced this pull request Jul 14, 2017
This is because this will break wolfra alpha skill unless they update
skills, but if they update before getting the new version, it will also
break wolfram

This reverts commit 6ca4161.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Status: To be reviewed Concept accepted and PR has sufficient information for full review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants