self.gui.release()

[JarbasAl]

Signal that this skill is no longer using the GUI, allow different platforms to properly handle this event.

Also calls self.clear() to reset the state variables

Currently self.gui.clear() has been wrongly used for this purpose, for example in my evil-laugh-skill this worked ok if mark2 skill was installed (due to a different bug related to idle screen) which makes it seem like its working, but in bigscreen it causes a black screen to linger around

Platforms can close the window or go back to previous page, clearing the page data is not enough

more recently in MycroftAI/mycroft-timer#82 self.gui.clear() is also being wrongly used for this purpose

[JarbasAl]

alternative ways to accomplish this would be to self.gui.remove_page manually for all pages, but it seemed weird and not intuitive, pages are currently not tracked by SkillGui class and this approach would require a bigger refactor and wouldn't give different platforms as much freedom.

A bus message also provides an event that any other process can react to so i consider it a better approach

@AIIX can provide more context on what the emitted bus message does, since it is already handled by Bigscreen and Android i believe

[devops-mycroft]

Voight Kampff Integration Test Succeeded (Results)

[AIIX]

I like the idea of standardizing a method to tell the GUI to stop showing a screen and let the platform skill control the events that take place when it receives the "mycroft.gui.screen.close" event, for the Mark-2 skill I basically see it replacing the event on the following line https://github.com/MycroftAI/skill-mark-2/blob/master/__init__.py#L256 which restores the idle screen on receiving this standardized message and for other platforms this standardized "mycroft.gui.screen.close" message can be reacted to differently as per their own platform skills.

self.gui.clear method should only be used for clearing session data for a particular skill apparently it should also ignore __override flags when clearing the session data, as this allows for skill displaying the current page to be able to wipe all the session data slate clean and push in new session data if required, without removing the displayed page, maybe this should be renamed to self.gui.clear.data to bring in more clarity (a larger change for another PR if its currently used at to many places)

[forslund]

Really good suggestion and nice and simple implementation!

When clear() was added it's main purpose was to unload the skill data at shutdown to remove any trailing info. On the Mark-2 I imagine the stack would want to be kept in memory mostly to keep the transitions fast.

[krisgesling]

Yeah nice one.

I'm wondering if we should call it self.gui.close() to keep it consistent with the bus message?

Any other arguments for / against either - release vs close?

[JarbasAl]

close sounds like you are closing the actual GUI, but the GUI will still be available

i think release matches better what it actually does. Since most likely you want the previous "active skill" to take over or if there isnt another skill go to desktop / default screen

In the case of an individual voice app, eg, mycroft-gui-app --hideTextInput --skill=peertube-skill.aiix.home this is the same as closing the window and showing the desktop. This would be the usage in bigscreen

But if you are just running the GUI in a mark2 this is not equivalent to closing. You want the idle screen to take over instead

In android you would want to return to the homescreen which shows the icons of all apps (but that homescreen is also running in the GUI)

Maybe we could change the message if that makes sense, but @AIIX would need to handle that in all the platforms linked above, and also in the gui protocol spec

[AIIX]

Maybe we could change the message if that makes sense, but @AIIX would need to handle that in all the platforms linked above, and also in the gui protocol spec

I don't think the emitted message should be changed as its already used internally within mycroft-gui for global back feature and several other places, "mycroft.gui.screen.close" is pretty easy to make sense of that your targeting exactly the mycroft.gui namespace with screen.close action requested from a specific skill_id.

[JarbasAl]

@AIIX is right, the "skill_id" in the message is pretty unambiguous "close this skill screen"

i do prefer self.gui.release for reasons described above, when used in skills it's more ambiguous than in the message

[krisgesling]

If no one else feels strongly, I think release is good.

Consistency can reduce complexity, but not if it makes less sense in that context. So I think we go with whatever makes the most sense in the context of Skill developers.

I have also proposed some extra wording for clear in the PR linked above to redirect dev's to this new method.

[krisgesling]

Tested and working great, thanks again Jarbas!