Skip to content
This repository has been archived by the owner on Sep 8, 2024. It is now read-only.

Add wait_for_message method #2628

Merged

Conversation

forslund
Copy link
Collaborator

@forslund forslund commented Jul 1, 2020

Description

Adds wait_for_message() method and refactors the common code shared with wait_for_response() into a separate class.

How to test

Make sure integration tests pass and that for example get_response() in skills work. and the :skills cli command

Contributor license agreement signed?

CLA [ Yes ]

@forslund forslund added the Type: Refactoring and other improvements Improvement of code and documentation that does not alter functionality. label Jul 1, 2020
@devops-mycroft devops-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Jul 1, 2020
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

@MycroftAI MycroftAI deleted a comment from devops-mycroft Jul 2, 2020
@MycroftAI MycroftAI deleted a comment from devops-mycroft Jul 2, 2020
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

@forslund forslund force-pushed the feature/wait-for-response-cleanup branch from c33c151 to 63d8879 Compare July 3, 2020 06:32
@forslund forslund changed the title Clean up the wait_for_response method Add wait_for_message method Jul 3, 2020
@forslund forslund force-pushed the feature/wait-for-response-cleanup branch from 63d8879 to 4707ffc Compare July 3, 2020 06:48
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@forslund forslund force-pushed the feature/wait-for-response-cleanup branch from 4707ffc to 9a71c61 Compare July 3, 2020 09:57
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@forslund forslund force-pushed the feature/wait-for-response-cleanup branch from 9a71c61 to 511b554 Compare July 3, 2020 10:55
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

2 similar comments
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

- Use nonlocal instead of mutable object.
- Set default timeout in method argument
- Refactor message waiting into a MessageWaiter class to be able to use the
  same code in both wait_for_message and wait_for_response.
- Add some basic unittests
@forslund forslund force-pushed the feature/wait-for-response-cleanup branch from 511b554 to 2bb9783 Compare July 5, 2020 14:40
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@krisgesling
Copy link
Contributor

Nice one, love the ability to separate construction from executing the wait.

Merging.

@krisgesling krisgesling merged commit f6dba98 into MycroftAI:dev Jul 6, 2020
@JarbasAl
Copy link
Contributor

JarbasAl commented Jul 6, 2020

when documenting this we should make a note objects can not be reused, since we use self.bus.once we need to create a new object for every call

i dont think refactoring to allow this makes sense either, just ensure its documented to avoid head scratches

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) Type: Refactoring and other improvements Improvement of code and documentation that does not alter functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants