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

Bug Fix - Don't clear namespace on every system page call #2814

Merged
merged 1 commit into from Jan 22, 2021

Conversation

AIIX
Copy link
Collaborator

@AIIX AIIX commented Jan 21, 2021

Description

Calling self.clear() on every show some system page event causes mycroft to remove the namespace from active index and reinsert it constantly with a flood of messages, skills showing a page of text self.gui.show_text() for example should only emit the data changes and not try reinserting and removing skill namespace constantly. If an skill author wants to clear the data they can manually call the self.gui.clear() method before calling self.gui.show_text()

How to test

Use self.gui.show_text("some text..") in a while loop and make sure it does not keep removing and re inserting the skill namespace from active skill index only sends the set data message with the changed data values

Contributor license agreement signed?

CLA [x] (Whether you have signed a CLA - Contributor Licensing Agreement

@devops-mycroft devops-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Jan 21, 2021
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@krisgesling
Copy link
Contributor

Hey thanks, this is working as described :)

My only possible concern would be whether Skill B might inadvertently show content from a previous Skill A. However they use a unique namespace for each Skill so this couldn't happen. Is that right?

@krisgesling krisgesling added the Type: Bug - quick Bug fixes that are quick to review and the implications of the change are clear and contained. label Jan 22, 2021
@AIIX
Copy link
Collaborator Author

AIIX commented Jan 22, 2021

Hey thanks, this is working as described :)

My only possible concern would be whether Skill B might inadvertently show content from a previous Skill A. However they use a unique namespace for each Skill so this couldn't happen. Is that right?

Yes each skill uses its own namespace so there should be no collision even if a skill B wants to tell skill A to display some content, if the skill A is imported in skill B, and skill B calls a method in skill A to display something, skill A will still use the Skill A namespace and not the skill B namespace. In another scenario if skill B imports skill A and fetches values from skill A, and decides to a show a page from skill B from values derived from skill A it will still use the skill B namespace because skill B asked to display a page, so getting content from any skill does not matter each skill will use its own namespace to display content.

@krisgesling krisgesling merged commit 8020e54 into MycroftAI:dev Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) Type: Bug - quick Bug fixes that are quick to review and the implications of the change are clear and contained.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants