Feature/skill api #1822

forslund · 2018-09-28T12:43:25Z

Description

This is a suggestion for an easy way for skills to publish and use other skills shared methods. The idea is that for example the alarm skill wants to

The skill api object is generated from the skill's public_api property. It's a dict where each key is turned into a method on the api object. The method is defined as

  "api_method": {
    "type": message type string
    "func": handler method
    "help": help string for cli
  }

This dict is generated from methods in the skill tagged with the skill_api_method decorator.

Example skill:

from mycroft.skills import MycroftSkill, skill_api_method

class Test2(MycroftSkill):
    @skill_api_method
    def speak_meth(self, message):
        """Speak the test sentence."""
        self.speak('This is a test')
 
    @skill_api_method
    def speak_meth2(self, message):
        """Speak another test sentence."""
        self.speak('This is another test')

To use this skill from another skill

import the SkillApi class

from mycroft.skills.api import SkillApi

and then get an api object and use that to trigger the skill:

my_skill = SkillApi.get('myskill.forslund')
my_skill.speak_meth()

This also adds a help interface to the api in the CLI, :api SKILL will list available api methods.

Contributor license agreement signed?

CLA [ Yes ]

pep8speaks · 2018-09-28T12:43:30Z

Hello @forslund! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-02-08 07:02:54 UTC

JarbasAl · 2018-10-09T18:10:40Z

what about some methods to handle binary data? base64 encode/decode basically

penrods

This looks solid to me. I'm good with merging it.

forslund · 2019-12-08T07:16:57Z

Rebase Needed

devops-mycroft · 2020-07-06T07:53:11Z

Voight Kampff Integration Test Succeeded (Results)

devops-mycroft · 2020-07-06T08:43:58Z

Voight Kampff Integration Test Succeeded (Results)

devops-mycroft · 2020-07-06T08:46:59Z

Voight Kampff Integration Test Failed (Results)

devops-mycroft · 2020-07-06T09:52:44Z

Voight Kampff Integration Test Succeeded (Results)

devops-mycroft · 2020-07-06T11:14:51Z

Voight Kampff Integration Test Succeeded (Results)

krisgesling · 2020-07-30T10:19:13Z

mycroft/skills/mycroft_skill/mycroft_skill.py

+ return wrapper
+
+ methods = [a for a in get_non_properties(self)
+ if hasattr(a, '__name__')]


None of my decorated methods seem to have a __name__ attribute. However they do get assigned the api_method.

Example Skill with an API method: https://github.com/krisgesling/skill-api-source-skill

I get True If I do self.log.info(hasattr(self.speak_from_other_skill, '__name__')) on your skill

The check does look a bit wonky though. Let's see if I can improve it...

Yeah, same if I log from the Skill itself, but logging from here returns False:

if self.skill_id == 'skill-api-source-skill': for a in get_non_properties(self): LOG.info("{} has __name__: {}".format(a, hasattr(a, '__name__')))

Ahh, the get_non_properties() returns a string not a reference to the attribute, so looks like we need this to be:

methods = [a for a in get_non_properties(self) if hasattr(getattr(self, a), '__name__')]

Ah great catch...how has this ever worked?

Pushed your suggested fix and updated the CLI code to handle :api without a skill argument. Gonna rebase this and fix the conflict, then I'll see if I can re-verify the functionality

forslund · 2020-09-23T09:18:30Z

Rebased and fixed the conflict

devops-mycroft · 2020-09-23T09:53:35Z

Voight Kampff Integration Test Succeeded (Results)

krisgesling · 2021-02-02T07:52:49Z

Hey, this is working well now.

Have you thought much about what some unit tests for this might look like?

Also, I just did tiny edit of the top comment to fix a typo in the example code.

forslund · 2021-02-03T14:18:19Z

Haven't thought too much about it but I think I can write up a small suite of tests for it.

The skill api allows skills to define a public api which can easily be accessed by other skills over the message bus just as easy as working with a normal object. The skill api object is generated from the skill's public_api property. It's a dict where each key is turned into a method on the api object. The method is defined as "api_method": { "type": message type string "func": handler method "help": help string for cli } Example skill: class Test2(MycroftSkill): def __init__(self): MycroftSkill.__init__(self) self.public_api = { 'speak': { 'type': 't2.speak', 'func': self.handle_speak, 'help': 'speak the test sentence\nand another line\n\nlast' }, 'speak2': { 'type': 't2.speak2', 'func': self.handle_speak2, 'help': 'speak the other sentence' } } def handle_speak(self, message): self.speak('This is a test') self.bus.emit(message.response(data=None)) def handle_speak2(self, message): self.speak('This is another test') self.bus.emit(message.response(data=None))

The api methods are now much easier to use, almost transparent. The current caveat is that only "standarad" python types are acceptable (int, float, str, list, bool, None) due to the json serialization. - api methods are now created with the skill_api_method decorator - both arguments and keyword arguments are sent to the api method instead of the message object - api methods now uses a normal return statement instead of having to handle creating response messages on the bus. For example if the datetime skill wants to make the datetime string fetchable simply add the skill_api_method decorator to the get_display_date method. @skill_api_method def get_display_date(self, day=None, location=None): """Returns the date and time as a string.""" [...] The methods return value will be sent back to the caller and can be used from a skill through datetime = SkillApi.get('mycroft-date-time.mycroftai') self.log.info(datetime.get_display_date())

forslund · 2021-02-06T16:59:56Z

Rebased and fixed conflict and added unittests for the key components.

devops-mycroft · 2021-02-06T17:46:59Z

Voight Kampff Integration Test Succeeded (Results)

- Tests for the MycroftSkill side implementation - Tests for the generated SkillApi objects

devops-mycroft · 2021-02-08T07:51:57Z

Voight Kampff Integration Test Succeeded (Results)

Touhey · 2021-02-22T11:54:29Z

hello will the skills api be added to pycroft soon?

krisgesling · 2021-02-23T00:34:35Z

Hey @Touhey - if you switch to the dev channel and pull the latest changes, this will now be available :D

We'll also be releasing the next major version of mycroft-core in a few weeks. Then it will be available on the stable / master branch

Touhey · 2021-02-23T10:50:58Z

Hey @Touhey - if you switch to the dev channel and pull the latest changes, this will now be available :D

We'll also be releasing the next major version of mycroft-core in a few weeks. Then it will be available on the stable / master branch

hi thanks for speedy reply where can i find the dev channel for picroft?

krisgesling · 2021-02-25T06:10:07Z

@Touhey - this is the dev channel of mycroft-core so if you SSH into the device, or hook up a keyboard and monitor - the following should work:

cd ~/mycroft-core
mycroft-stop
git checkout dev
git pull
./dev_setup.sh
mycroft-start all

penrods added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Oct 1, 2018

JarbasAI mentioned this pull request Oct 2, 2018

Messagebus interface #1013

Closed

JarbasAI mentioned this pull request Nov 22, 2018

Installing Mycroft-Social-Network-Manager skill should automatically install browser_service ldevalbray/mycroft-social-network-manager#1

Open

forslund force-pushed the feature/skill-api branch from 9badff4 to 9d0411d Compare May 10, 2019 11:20

forslund added the Status: Work in progress PR being actively worked on, not yet ready for review. label May 14, 2019

penrods added the tagged for review and potential merge label Jun 3, 2019

penrods previously approved these changes Jun 5, 2019

View reviewed changes

forslund removed the Status: Work in progress PR being actively worked on, not yet ready for review. label Jul 12, 2019

forslund mentioned this pull request Jul 1, 2020

Add wait_for_message method #2628

Merged

forslund force-pushed the feature/skill-api branch from 07f18ac to 998c00f Compare July 6, 2020 07:02

forslund force-pushed the feature/skill-api branch 2 times, most recently from 38de5f6 to b3a4bf9 Compare July 6, 2020 08:02

forslund force-pushed the feature/skill-api branch from b3a4bf9 to aa40a3f Compare July 6, 2020 09:01

forslund force-pushed the feature/skill-api branch from aa40a3f to dd202ff Compare July 6, 2020 10:24

krisgesling reviewed Jul 30, 2020

View reviewed changes

forslund force-pushed the feature/skill-api branch from dd202ff to 6f5f2ff Compare July 30, 2020 13:27

krisgesling added the Status: Work in progress PR being actively worked on, not yet ready for review. label Aug 11, 2020

forslund dismissed penrods’s stale review via b3d4202 September 23, 2020 09:18

forslund force-pushed the feature/skill-api branch from 6f5f2ff to b3d4202 Compare September 23, 2020 09:18

forslund removed the Status: Work in progress PR being actively worked on, not yet ready for review. label Sep 23, 2020

krisgesling added 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. and removed tagged for review and potential merge labels Sep 24, 2020

krisgesling linked an issue Sep 24, 2020 that may be closed by this pull request

Is it possible for skills to share information? #799

Closed

krisgesling mentioned this pull request Dec 9, 2020

Use a static custom settings file to save user informationn #2712

Closed

JarbasAl mentioned this pull request Dec 29, 2020

waiting_for_mycroft OpenVoiceOS/zzz-old-ovos-utils#15

Merged

forslund added 3 commits February 6, 2021 17:55

Add skill api help to CLI

1397659

forslund force-pushed the feature/skill-api branch from b3d4202 to 0ccc0f2 Compare February 6, 2021 16:58

Add test cases for the SkillApi

5685259

- Tests for the MycroftSkill side implementation - Tests for the generated SkillApi objects

forslund force-pushed the feature/skill-api branch from 0ccc0f2 to 5685259 Compare February 8, 2021 07:02

krisgesling added Status: Accepted PR has been reviewed and accepted. There must be some reason why it isn't being merged. and removed Status: To be reviewed Concept accepted and PR has sufficient information for full review labels Feb 16, 2021

krisgesling approved these changes Feb 23, 2021

View reviewed changes

krisgesling merged commit 38839a1 into MycroftAI:dev Feb 23, 2021

JarbasAl mentioned this pull request Mar 6, 2021

patch/remove skill api OpenVoiceOS/zzz-old-ovos-utils#48

Merged

JarbasAl mentioned this pull request Aug 25, 2021

Event handling for active alarms MycroftAI/skill-alarm#121

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/skill api #1822

Feature/skill api #1822

forslund commented Sep 28, 2018 •

edited by krisgesling

Loading

pep8speaks commented Sep 28, 2018 •

edited

Loading

JarbasAl commented Oct 9, 2018

penrods left a comment

forslund commented Dec 8, 2019

devops-mycroft commented Jul 6, 2020

devops-mycroft commented Jul 6, 2020

devops-mycroft commented Jul 6, 2020

devops-mycroft commented Jul 6, 2020

devops-mycroft commented Jul 6, 2020

krisgesling Jul 30, 2020

forslund Jul 30, 2020

forslund Jul 30, 2020

krisgesling Jul 30, 2020 •

edited

Loading

krisgesling Jul 30, 2020

forslund Jul 30, 2020

forslund Jul 30, 2020 •

edited

Loading

forslund commented Sep 23, 2020

devops-mycroft commented Sep 23, 2020

krisgesling commented Feb 2, 2021

forslund commented Feb 3, 2021

forslund commented Feb 6, 2021

devops-mycroft commented Feb 6, 2021

devops-mycroft commented Feb 8, 2021

Touhey commented Feb 22, 2021

krisgesling commented Feb 23, 2021

Touhey commented Feb 23, 2021

krisgesling commented Feb 25, 2021 •

edited

Loading

Feature/skill api #1822

Feature/skill api #1822

Conversation

forslund commented Sep 28, 2018 • edited by krisgesling Loading

Description

Contributor license agreement signed?

pep8speaks commented Sep 28, 2018 • edited Loading

Comment last updated at 2021-02-08 07:02:54 UTC

JarbasAl commented Oct 9, 2018

penrods left a comment

Choose a reason for hiding this comment

forslund commented Dec 8, 2019

devops-mycroft commented Jul 6, 2020

devops-mycroft commented Jul 6, 2020

devops-mycroft commented Jul 6, 2020

devops-mycroft commented Jul 6, 2020

devops-mycroft commented Jul 6, 2020

krisgesling Jul 30, 2020

Choose a reason for hiding this comment

forslund Jul 30, 2020

Choose a reason for hiding this comment

forslund Jul 30, 2020

Choose a reason for hiding this comment

krisgesling Jul 30, 2020 • edited Loading

Choose a reason for hiding this comment

krisgesling Jul 30, 2020

Choose a reason for hiding this comment

forslund Jul 30, 2020

Choose a reason for hiding this comment

forslund Jul 30, 2020 • edited Loading

Choose a reason for hiding this comment

forslund commented Sep 23, 2020

devops-mycroft commented Sep 23, 2020

krisgesling commented Feb 2, 2021

forslund commented Feb 3, 2021

forslund commented Feb 6, 2021

devops-mycroft commented Feb 6, 2021

devops-mycroft commented Feb 8, 2021

Touhey commented Feb 22, 2021

krisgesling commented Feb 23, 2021

Touhey commented Feb 23, 2021

krisgesling commented Feb 25, 2021 • edited Loading

forslund commented Sep 28, 2018 •

edited by krisgesling

Loading

pep8speaks commented Sep 28, 2018 •

edited

Loading

krisgesling Jul 30, 2020 •

edited

Loading

forslund Jul 30, 2020 •

edited

Loading

krisgesling commented Feb 25, 2021 •

edited

Loading