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

Bump up dependencies on pyserial and pyserial-asyncio #50

Closed
wants to merge 2 commits into from

Conversation

Hedda
Copy link
Contributor

@Hedda Hedda commented Dec 11, 2020

Bump up dependencies on pyserial and pyserial-asyncio for projects other projects that use this library but are not Home Assistant.

Adminiuga did the same dependency version bump for Home Assistant in home-assistant/core#44089

pyserial-asyncio depends on pyserial and this bumps pyserial-asyncio to 0.5 version release and pyserial to 3.5 version release.

https://github.com/pyserial/pyserial-asyncio/releases/tag/v0.5

https://github.com/pyserial/pyserial/releases/tag/v3.5

Bump up dependencies on pyserial and pyserial-asyncio for projects other projects that use this library but are not Home Assistant.

Adminiuga did the same dependency version bump for Home Assistant in home-assistant/core#44089

pyserial-asyncio depends on pyserial and this bumps pyserial-asyncio to 0.5 version release and pyserial to 3.5 version release.

https://github.com/pyserial/pyserial-asyncio/releases/tag/v0.5

https://github.com/pyserial/pyserial/releases/tag/v3.5
@codecov-io
Copy link

codecov-io commented Dec 11, 2020

Codecov Report

Merging #50 (1400c06) into dev (c5f6f7d) will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev      #50      +/-   ##
==========================================
- Coverage   99.14%   99.11%   -0.04%     
==========================================
  Files          37       37              
  Lines        3169     3169              
==========================================
- Hits         3142     3141       -1     
- Misses         27       28       +1     
Impacted Files Coverage Δ
zigpy_znp/zigbee/application.py 97.65% <0.00%> (-0.20%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5f6f7d...1400c06. Read the comment docs.

Copy link
Collaborator

@puddly puddly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zigpy-znp doesn't depend on a specific version of these packages so can you change the == to >=?

@Adminiuga
Copy link
Contributor

I'm thinking it is just better to leave it as is, without any specific versioning??? There was nothing new in 3.5 or 0.5 which any of the radio lib requires. For non ZHA related projects, it would install the latest version anyway. For ZHA it is just better to control it from one place -- ZHA, since there are other integrations which pin pyserial and pyserial-asyncio, and pinning like in this PR would break compatibility with HA and cause those running HA core issues

@puddly
Copy link
Collaborator

puddly commented Dec 11, 2020

I agree. Python doesn't install per-package dependencies so as long as some upstream library (i.e. ZHA) requires specific pyserial-asyncio and pyserial features, zigpy-znp will work with them.

The only real change that I'm aware of is pyserial/pyserial-asyncio#56, but apparently the new release broke Windows support: pyserial/pyserial-asyncio#69. May want to wait a few weeks to see if this report is valid and if other breakage occurs.

@puddly
Copy link
Collaborator

puddly commented Dec 11, 2020

For reference, here are all of the changes: pyserial/pyserial-asyncio@v0.4...v0.5
The other interesting issue fixed in this release is pyserial/pyserial-asyncio#53. I'm not sure what the consequences are of making all writes async like this, since to me it seems like this may introduce timing glitches due to the old behavior essentially passing through writes to the underlying serial object synchronously if there was no other buffered data.

Have you tested these new package versions under load? I'll have to do this with zigpy-znp.

@Adminiuga
Copy link
Contributor

running in my prod for a few days. haven't noticed anything abnormal, but it is well below heavy load.

Change from == to >= for pyserial 3.5 and pyserial-asyncio 0.5
@Hedda
Copy link
Contributor Author

Hedda commented Dec 14, 2020

zigpy-znp doesn't depend on a specific version of these packages so can you change the == to >=?

Changed from == to >= for pyserial 3.5 and pyserial-asyncio 0.5

@Hedda
Copy link
Contributor Author

Hedda commented Dec 14, 2020

I dont want to pin specific version

Did the PR home-assistant/core#44089 not in effect pin these to specific versions pyserial and pyserial-asyncio in Home Assistant?

Should maybe that be changed from == to >= in downstream Home Assistant core as well to not depend on specific versions?

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.

4 participants