OpenVoiceOS / ovos-core

OpenVoiceOS Core, the FOSS Artificial Intelligence platform.
https://openvoiceos.org
Apache License 2.0
128 stars 18 forks source link

msm disabled by default #24

Closed JarbasAl closed 2 years ago

JarbasAl commented 2 years ago

we should make all default skills pip installable after #9

msm should then be disabled by default + default skills added to requirements.txt

this allows us to pin skill version appropriate for the ovos-core version and avoids conflicts between ovos/mycroft skills, it is currently confusing for users when we tell them to blacklist official skills and install our versions manually

we can also look into replacing msm with osm (let the user choose), but the integration in core is only used to download default skills, so we either include a deeper integration (appstore sync etc) or there is no point in adding osm either.

There is also some decoupling needed between msm and selene skills manifest upload before we can fully remove msm

AIIX commented 2 years ago

I don't think I would be comfortable with adding default skills to requirements.txt specifically when targeting a different platform than a smart speaker. I would rather suggest looking into having osm install the default skills for a specific platform if specified through a configuration key or not have it install any default skills at all

ChanceNCounter commented 2 years ago

Doesn't the assistant depend on the presence of certain API that comes from default skills?

NeonDaniel commented 2 years ago

I don't think I would be comfortable with adding default skills to requirements.txt specifically when targeting a different platform than a smart speaker. I would rather suggest looking into having osm install the default skills for a specific platform if specified through a configuration key or not have it install any default skills at all

That about an optional requirements list(s)? i.e. you could pip install ovos-core[skills_default] or ovos_core[skills_bigscreen]?

NeonDaniel commented 2 years ago

Doesn't the assistant depend on the presence of certain API that comes from default skills?

I don't think core has any skill dependencies? Unless there's something in pairing..

JarbasAl commented 2 years ago

pairing process is tied to pairing skill, but it can be disabled in .conf so its not really core

i was thinking about stop and unknown skills being essential, but they really arent...

I'm inclined to just add a default skills list option, we can create a separate meta package ovos-default-skills that simply lists and pins versions of default skills in it's requirements, and we add that package to ovos-core[all] and ovos-core[default_skills]

NeonDaniel commented 2 years ago

Not sure I see the benefit of ovos-default-skills vs direct dependencies lists in ovos-core requirements; seems like it's just obfuscating what you get in ovos-core.

I agree that there aren't any really "essential" skills, all are optional (though stop and unknown are expected, along with homescreen for anything running the gui)

JarbasAl commented 2 years ago

The idea is that diff platforms will disagree with our default skills so they should not be installed in first place. So i agree they should not be part of the minimal install. But i would add them to a full install

Think about someone wanting vanilla mycroft skills or distros like bigscreen that dont even want our pairing skill

ChanceNCounter commented 2 years ago

Bumpity. @JarbasAl?

NeonDaniel commented 2 years ago

I believe msm is removed in the 0.0.2a3 build. It caused some backwards-compat problem in Neon, but it was easily resolved (just a reference to msm.all_skills). I'm happy with any solution here; I'll likely maintain a separate dependency set for Neon, so the location of OVOS defaults could be anywhere..

JarbasAl commented 2 years ago

i am rebasing this and will push soon, this PR removed too much stuff, also tweaking the deprecation warning logs