zigpy / zigpy-znp

TI CC2531, CC13x2, CC26x2 radio support for Zigpy and ZHA
GNU General Public License v3.0
144 stars 40 forks source link

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

Closed dumpfheimer closed 1 year ago

dumpfheimer commented 1 year ago

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.

puddly commented 1 year ago

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 commented 1 year ago

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

puddly commented 1 year ago

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 commented 1 year ago

Discarded in favor of a bellows approach