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.

Sign up for GitHub

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

Jump to bottom

improve skill loading #245

Merged
merged 22 commits into from
Jan 26, 2023
Merged

improve skill loading #245

merged 22 commits into from
Jan 26, 2023

Conversation

JarbasAl
Copy link
Member

@JarbasAl JarbasAl commented Dec 15, 2022 •
edited
Loading

companion PR to OpenVoiceOS/OVOS-workshop#36

integrates with https://github.com/OpenVoiceOS/ovos-PHAL-plugin-connectivity-events

usage

from ovos_workshop.skills.ovos import OVOSSkill, SkillNetworkRequirements, classproperty

class MySkill(OVOSSkill):

    @classproperty
    def network_requirements(self):
        """ skill developers should override this if they do not require connectivity
         some examples:
         IOT skill that controls skills via LAN could return:
            scans_on_init = True
            SkillNetworkRequirements(internet_before_load=False,
                                     network_before_load=scans_on_init,
                                     requires_internet=False,
                                     requires_network=True,
                                     no_internet_fallback=True,
                                     no_network_fallback=False)
         online search skill with a local cache:
            has_cache = False
            SkillNetworkRequirements(internet_before_load=not has_cache,
                                     network_before_load=not has_cache,
                                     requires_internet=True,
                                     requires_network=True,
                                     no_internet_fallback=True,
                                     no_network_fallback=True)
         a fully offline skill:
            SkillNetworkRequirements(internet_before_load=False,
                                     network_before_load=False,
                                     requires_internet=False,
                                     requires_network=False,
                                     no_internet_fallback=True,
                                     no_network_fallback=True)
        """
        return SkillNetworkRequirements()

@JarbasAl JarbasAl added the enhancement New feature or request label Dec 15, 2022
@JarbasAl JarbasAl requested a review from NeonDaniel December 15, 2022 19:18
@JarbasAl JarbasAl marked this pull request as ready for review December 15, 2022 21:12
@JarbasAl JarbasAl mentioned this pull request Dec 16, 2022
Merged
2 tasks
@JarbasAl JarbasAl mentioned this pull request Jan 10, 2023
Closed
23 tasks
@NeonDaniel
Copy link
Member

This should probably be ported to ovos-workshop

@NeonDaniel NeonDaniel force-pushed the refactor/skill_connectivity branch from 43f74e9 to 1f8412b Compare January 24, 2023 01:00
@NeonDaniel
Copy link
Member

Unit test fixes: #261

@NeonDaniel
Troubleshooting unit test failures (#261)
e229fb1 
* Update logging to prevent Type Errors in testing
Add back `self.loaded` set in skill_loader.py
Add new bus handlers to test_skill_manager.py

* compare set of events to avoid order-related errors

* Update unit test mocking
Debug skill_loader changes

* Replace list event comparison
@codecov
Copy link

codecov bot commented Jan 25, 2023 •
edited
Loading

Codecov Report

Merging #245 (1fcb8b5) into dev (6ceb058) will increase coverage by 2.42%.
The diff coverage is 43.45%.

@@            Coverage Diff             @@
##              dev     #245      +/-   ##
==========================================
+ Coverage   50.35%   52.78%   +2.42%     
==========================================
  Files         119      156      +37     
  Lines       10077     8145    -1932     
==========================================
- Hits         5074     4299     -775     
+ Misses       5003     3846    -1157
Impacted Files Coverage Δ
mycroft/audio/__main__.py 0.00% <0.00%> (ø)
mycroft/client/enclosure/__main__.py 0.00% <0.00%> (ø)
mycroft/client/enclosure/mark1/arduino.py 0.00% <0.00%> (ø)
mycroft/client/enclosure/mark1/eyes.py 0.00% <0.00%> (ø)
mycroft/client/enclosure/mark1/mouth.py 0.00% <0.00%> (ø)
mycroft/client/speech/__main__.py 0.00% <0.00%> (ø)
mycroft/client/speech/hotword_factory.py 0.00% <0.00%> (-88.89%) ⬇️
mycroft/client/speech/service.py 0.00% <0.00%> (ø)
mycroft/client/speech/silence.py 0.00% <0.00%> (-42.86%) ⬇️
mycroft/client/text/__init__.py 0.00% <0.00%> (ø)
... and 154 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

NeonDaniel
NeonDaniel reviewed Jan 25, 2023
View reviewed changes
Copy link
Member

@NeonDaniel NeonDaniel left a comment

Choose a reason for hiding this comment

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

Is the connectivity events plugin a missing dependency here?
I never got a mycroft.ready event emitted

mycroft/skills/skill_manager.py Show resolved Hide resolved
@NeonDaniel NeonDaniel mentioned this pull request Jan 25, 2023
Merged
NeonDaniel
NeonDaniel requested changes Jan 25, 2023
View reviewed changes
requirements/extra-skills.txt Outdated Show resolved Hide resolved
@JarbasAl JarbasAl added this to the 0.0.6 milestone Jan 26, 2023
@NeonDaniel
Copy link
Member

Need to troubleshoot logged errors when loading plugins:

2023-01-25 17:16:30.727 - skills - mycroft.skills.skill_loader:load:576 - INFO - ATTEMPTING TO LOAD PLUGIN SKILL: skill-ovos-homescreen.openvoiceos
2023-01-25 17:16:30.906 - skills - ovos_utils.skills.settings:get_local_settings:96 - WARNING - skill_name is an unused legacy argument, will be removed in 0.0.3 or later
2023-01-25 17:16:30.910 - skills - ovos_utils.skills.settings:get_local_settings:101 - INFO - /home/neon/.config/mycroft/skills/skill-ovos-homescreen.openvoiceos/settings.json
2023-01-25 17:16:31.022 - skills - mycroft.deprecated.skills.settings:__init__:99 - WARNING - skill_name is deprecated! use skill_id instead
2023-01-25 17:16:31.029 - skills - mycroft.deprecated.skills.settings:__init__:110 - INFO - Skill settings sync is disabled, settingsmeta will not be uploaded
2023-01-25 17:16:31.793 - skill-ovos-homescreen.openvoiceos - INFO - Registering resting screen <bound method OVOSHomescreenSkill.handle_idle of <skill_ovos_homescreen.OVOSHomescreenSkill object at 0x7f6c77a190>> for OVOSHomescreen.
2023-01-25 17:16:31.805 - skill-ovos-homescreen.openvoiceos - INFO - Registering resting screen
2023-01-25 17:16:35.037 - skills - mycroft.skills.skill_loader:_communicate_load_status:562 - ERROR - Skill skill-ovos-homescreen.openvoiceos failed to load

@NeonDaniel NeonDaniel self-requested a review January 26, 2023 17:20
@JarbasAl JarbasAl merged commit ebd91fb into dev Jan 26, 2023
@JarbasAl JarbasAl deleted the refactor/skill_connectivity branch January 26, 2023 17:53
@JarbasAl JarbasAl mentioned this pull request Feb 7, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NeonDaniel NeonDaniel NeonDaniel approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
0.0.6
Development

Successfully merging this pull request may close these issues.
2 participants
@JarbasAl @NeonDaniel