Port accessories to asyncio#76
Open
thomaspurchas wants to merge 4 commits intoikalchev:asyncio_runfrom
Open
Conversation
Not a complete port, unfortunately some of the dependent libraries don’t support asyncio at the moment. However as Accessories are still running in separate threads at the moment we can ignore that. Additionally subprocess can’t use asyncio until the main thread is converted.
Owner
|
Hey! I will go over the PR later when I get to a computer. As for the loop in the driver - this is the core of the PR I opened, would you mind if I finish it? |
Author
|
Ahh, I thought the PR you opened was only for the |
This makes sure that each accessory thread get a complete event loop to run the accessory with. Also ensure that the event loop is shutdown cleanly.
thomaspurchas
changed the title
Closed
Owner
|
Hey! Too busy the last week, but I submitted the asyncio today (in dev). Could you revise this? |
Author
|
Sure thing. Might take me few days tho, I have also had a very busy week.
Best
Thomas
…On 13 Apr 2018, 07:11 +0100, Ivan Kalchev ***@***.***>, wrote:
Hey! Too busy the last week, but I submitted the asyncio today (in dev). Could you revise this?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
cdce8p
reviewed
Apr 14, 2018
| pyqrcode | ||
| base36 No newline at end of file | ||
| base36 | ||
| aiohttp==3.1.1 |
Contributor
There was a problem hiding this comment.
I don't like that we add a dependency that is only used for an optional accessory. If that's the way to go, we should move all optional accessories elsewhere #43.
Owner
|
Note that one of the merges broke the accessories (the Category is now in costs.py and is not a class). Will try to fix this soon |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This should merge into #74. Gonna start working on moving the HTTP bridge.
Unfortunately there are some limits to running the event loop in threads. Notably subprocess don't work in threads, unless the main thread is running an event loop.
I think I might start work migrating the driver over to asyncio, so an event loop is running in the main thread.
With regards to this port, feedback welcome, also testing as I don't have access to all the hardware.