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

Remove UART thread #598

Merged
merged 1 commit into from
Dec 6, 2023
Merged

Remove UART thread #598

merged 1 commit into from
Dec 6, 2023

Conversation

puddly
Copy link
Contributor

@puddly puddly commented Dec 4, 2023

Zigpy uses pyserial-asyncio-fast if it is installed. This fork of pyserial-asyncio fixes a bug where writes are buffered unnecessarily, essentially introducing unnecessary delays to serial writes (@pipiche38 you may want to add this as a dependency to benefit from it).

The purpose of the UART thread was to allow bellows to instantly respond to packets with an ACK and avoid timing problems caused by a busy main asyncio event loop. This problem is fixed by pyserial-asyncio-fast, as protocol.data_received synchronously parses any incoming data and immediately calls transport.write, without invoking the event loop at all.

In my local tests, 4608/4608 transport.write calls immediately resulted in a successful write syscall, meaning the protocol's write buffer was never filled at all.

Copy link

codecov bot commented Dec 4, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e4c624a) 99.81% compared to head (a390415) 99.78%.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #598      +/-   ##
==========================================
- Coverage   99.81%   99.78%   -0.03%     
==========================================
  Files          68       67       -1     
  Lines        4808     4719      -89     
==========================================
- Hits         4799     4709      -90     
- Misses          9       10       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pipiche38
Copy link
Contributor

@puddly thanks for the ping. I had a look to the PR and you never refer to the pyserial-asyncio-fast. At what level is pyserial-asyncio-fast replacing pyserial-asyncio ?

@puddly
Copy link
Contributor Author

puddly commented Dec 4, 2023

@pipiche38 Zigpy does it automatically if the package is available: https://github.com/zigpy/zigpy/blob/fb19ae5904d79bee43a0188636088499cc674a7b/zigpy/serial.py#L15-L20

pipiche38 added a commit to zigbeefordomoticz/Domoticz-Zigbee that referenced this pull request Dec 4, 2023
@puddly puddly merged commit 1ac01ed into zigpy:dev Dec 6, 2023
13 of 14 checks passed
puddly added a commit to puddly/bellows that referenced this pull request Dec 28, 2023
puddly added a commit that referenced this pull request Dec 29, 2023
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