Add proper handler for gui release events #57
Conversation
| if get_skill_namespace: | ||
| self.bus.emit(Message("gui.clear.namespace", | ||
| {"__from": get_skill_namespace})) | ||
| self.resting_screen.cancel_override() |
There was a problem hiding this comment.
Will this cancel any override screen regardless of whether it matches the skill_id of the namespace we're clearing?
Eg could the following scenario occur?
- Skill A is triggered, performing some action
- Skill B is triggered and overrides idle
- Skill A finishes and clears it's namespace
- Skill B's idle override gets unintentionally cancelled
There was a problem hiding this comment.
the idle / resting screen implementation to me atleast doesn't seem to account for namespaces or maintain any list of skills that have an override, It seems to only apply the override logic on the current page being displayed so even if Skill A is triggered and is performing some action, Skill B by just being triggered after that will probably cause it to override the previous Skill A idle override, the cancel_override() will then only cancel Skill B override because that is what the resting screen currently knows
There was a problem hiding this comment.
Yeah, definitely outside the scope of this PR, but I wonder if we need to look at that.
I'll create a new issue for that and merge this in.
Thanks :)
Description
Adds a proper handler for self.gui.release() emitted events
This handler is responsible for removing the skill from the namespace using "gui.clear.namespace" and cancelling all idle screen events associated with it.
Type of PR
If your PR fits more than one category, there is a high chance you should submit more than one PR. Please consider this carefully before opening the PR.
Either delete those that do not apply, or add an x between the square brackets like so:
- [x]Testing
Test against: MycroftAI/mycroft-core#2683
and any skill that incorporates "self.gui.release()" method
CLA