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/ww_cfg #207

Merged
merged 2 commits into from Oct 7, 2022
Merged

fix/ww_cfg #207

merged 2 commits into from Oct 7, 2022

Conversation

JarbasAl
Copy link
Member

@JarbasAl JarbasAl commented Oct 7, 2022

handle naming collisions due to white space and underscore differences

handle "active" flag based on the concept of "main wake word", compatible with the mycroft way and expectations, multiple wake words will require explicitly setting "active": True in config from now on

fix possible iteration bug on stop

handle naming collisions due to white space and underscore differences

handle "active" flag based on the concept of "main wake word", compatible with the mycroft way and expectations, multiple wake words will require explicitly setting "active": True in config from now on

fix possible iteration bug on stop
@JarbasAl JarbasAl added bug Something isn't working bug-ovos bug introdues in ovos-core, not present in mycroft-core labels Oct 7, 2022
@codecov
Copy link

codecov bot commented Oct 7, 2022

Codecov Report

Merging #207 (9a44b44) into dev (6ceb058) will increase coverage by 3.82%.
The diff coverage is 36.87%.

@@            Coverage Diff             @@
##              dev     #207      +/-   ##
==========================================
+ Coverage   50.35%   54.17%   +3.82%     
==========================================
  Files         119      156      +37     
  Lines       10077     9733     -344     
==========================================
+ Hits         5074     5273     +199     
+ Misses       5003     4460     -543     
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 141 more

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

mycroft/configuration/mycroft.conf Show resolved Hide resolved
# normalization step to avoid naming collisions
# TODO - move this to ovos_config package, on changes to the hotwords section this should be enforced directly
# this approach does not fully solve the issue, config merging may be messed up
word = word.replace(" ", "_")
Copy link
Member Author

Choose a reason for hiding this comment

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

@NeonDaniel a bit out of scope but related, should we enforce this in every key for mycroft.conf, not only for hotwords?

Copy link
Member

Choose a reason for hiding this comment

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

I agree its out of scope.. My main concern would be breaking compatibility with plugin config; maybe we could log warnings about it in ovos_config and then remove it when we release 1.0.0 there?

@JarbasAl JarbasAl merged commit 35b6469 into dev Oct 7, 2022
4 checks passed
@JarbasAl JarbasAl deleted the fix/ww_cfg branch October 7, 2022 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working bug-ovos bug introdues in ovos-core, not present in mycroft-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants