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

[WIP] Use a dedicated thread for Zigpy ZNP's serial communication #167

Conversation

dumpfheimer
Copy link
Contributor

I have been playing around with letting zigpy-znp run in a dedicated thread.
My experience has been surprisingly pleasing. I feel like lights respond quicker (especially when toggling a bunch of lights simultaneously)

I am quite new to Python and a rudimentary understanding of event loops, but I believe this solution is worth being looked at.

I very much appreciate feedback.

I will take a look at bellows to see if I can learn something from there soon.

@dumpfheimer dumpfheimer changed the title [WIP] Use a dedicated thread for Zigpy ZNP [WIP] Use a dedicated thread for Zigpy ZNP's serial communication Aug 31, 2022
@puddly
Copy link
Collaborator

puddly commented Aug 31, 2022

Have you taken a look at the bellows' ThreadsafeProxy and EventLoopThread implementations?

Managing multiple loops and threads is surprisingly delicate, especially with exceptions, radio probing (which tries/retries connecting multiple times), shutdown, etc. The bellows implementation is not dependent on bellows itself so it should be easy to adapt into ZNP. If it also works, I would like to avoid adding more duplicate code into every single radio library and instead start to move this into zigpy.

@dumpfheimer
Copy link
Contributor Author

How do you handle the "return to the main thread" in bellows?

@puddly
Copy link
Collaborator

puddly commented Sep 13, 2022

How do you handle the "return to the main thread" in bellows?

Everything is done by re-wrapping futures across event loops: https://github.com/zigpy/bellows/blob/3c3ee0296d35eb43d0493eb9b2160bc4484e892c/bellows/thread.py#L106-L107

@dumpfheimer
Copy link
Contributor Author

Discarded in favor of a bellows approach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants