fix alarm skill and make stateful

[ken-mycroft]

Description

Fix alarm skill news skill interaction issues and make alarm skill stateful

If needed follow up with as much detail as required.

Type of PR

• [X] Bugfix

Testing

Set an alarm that goes off while news is playing. Stop should only stop the alarm, not the news

Documentation

Will need new VK tests

[ken-mycroft]

OK, latest commit should address review input

[MetaGamer]

This is simply a string constant which describes the skill category. What was once a simple

self.category = 'system'

has now become

self.skill_control.category = 'system'

with a whole separate class (skill_control) to just hold 3 new attributes. It will next become

self.skill_control.category = 'who cares'

or could easily become

self.state_manager.something_besides_category_because_somebody_didnt_like_the_word_category_here = 'something_other_than_system_because_somebody_didnt_like_the_word_system'

I'll change it but every change like this runs the risk of breaking something and keeping these changes from getting pushed out and tested. In this particular case the new attribute value will undoubtedly not convey what the true intent of this value for this attribute is; namely this is a system skill to be treated differently than other non system level skills. This level of control does not exist in our current system as I know it.

This particular constant is used in nearly all the PRs I just submitted so I'll have to revisit every one and make the changes and then test to make sure nothing is broken and then update the PR. I suspect those changes will be ready sometime tomorrow so maybe we can get this code pushed out by the end of the week but I have already spent more time changing the style of this code and variable names then I did designing and implementing it.

---

Ken: I think the current "system" designation is used for things like volume, wifi, and if i recall correctly, something like a half dozen "invisible" Skills we never talk about. (Those need to be added to the list of things to look at). [Obvs I might be remembering all this incorrectly. ]

Perhaps the category could be descriptive, rather than "categorical". E.g. "interrupter" or "non-modal-notifier". But I doubt any scheme we come up with off the cuff here will stand the test of time. My 2c: Just pick a name that isn't "system" and we'll give the whole state system a thorough arch review before Pass 2 on the essential Skills, with the benefit of knowing better what we need to make things work, and what behaviors we want to define.

On May 18, 2021 1:25:22 PM ken-mycroft @.***> wrote:

@ken-mycroft commented on this pull request. In init.py:

@@ -87,6 +67,7 @@ def init(self): self.flash_state = 0 self.recurrence_dict = None self.sound_name = None

• self.skill_control.category = 'system' What is a system skill? How are they currently delineated? Categories are used to maintain backward compatibility with existing skills and to allow skills defined as category=system to have priority over non system skills during converse. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[ken-mycroft]

I really hate this tool. I can not find previous comments for some reason. Anyway, there is a roman numeral routine in here and Chris commented he did not have to have that in his weather skill, however, Lingua Franca returns roman numerals for things like the fifth, the twenty sixth and if you don't handle them correctly you will get the behavior you have in the weather skill if you ask it what is the weather for the 21st or the seventh.

[JarbasAl]

Anyway, there is a roman numeral routine in here and Chris commented he did not have to have that in his weather skill, however, Lingua Franca returns roman numerals for things like the fifth, the twenty sixth

clarification, lingua_franca does no such thing, the STT engine might return that, but LF contains no code to handle that at all, roman numerals are neither valid input nor output for LF