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

fix: typing for generic BleakClient classes and the retry_bluetooth_connection_error decorator #86

Merged
merged 4 commits into from
Feb 25, 2023

Conversation

kaechele
Copy link
Contributor

This should be the correct way of dealing with both of these types and mypy.

Using a bound TypeVar we can ensure that any client class we are dealing
with is either BleakClient or a descendant of it and that type then
stays consistent throughout the lifecycle.

Signed-off-by: Felix Kaechele <felix@kaechele.ca>
Use TypeVar together with ParamVar to drop the use of the unsafe cast
operation.

Signed-off-by: Felix Kaechele <felix@kaechele.ca>
@codecov
Copy link

codecov bot commented Feb 25, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@58f9958). Click here to learn what that means.
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 84c2036 differs from pull request most recent head c65cb02. Consider uploading reports for the commit c65cb02 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #86   +/-   ##
=======================================
  Coverage        ?   78.15%           
=======================================
  Files           ?        4           
  Lines           ?      412           
  Branches        ?       93           
=======================================
  Hits            ?      322           
  Misses          ?       61           
  Partials        ?       29           
Impacted Files Coverage Δ
src/bleak_retry_connector/__init__.py 77.36% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@kaechele
Copy link
Contributor Author

Python 3.9 test fails due to missing ParamSpec support. Can we drop Python 3.9 now that Home Assistant only supports 3.10 and 3.11 or should we postpone that change until 3.9 support can be dropped?

Lint stage fails due to antiquated pre-commit-hooks that are now broken. See my other PR.

@bdraco
Copy link
Member

bdraco commented Feb 25, 2023

Please drop python 3.9 in another PR. It should be marked as a BREAKING CHANGE

@kaechele
Copy link
Contributor Author

Will do. Preparing that PR now.

@bdraco
Copy link
Member

bdraco commented Feb 25, 2023

Will take a look tomorrow if Wi-Fi works inflight

@kaechele
Copy link
Contributor Author

Thanks! Safe travels!

async def _async_wrap_bluetooth_connection_error_retry(
*args: Any, **kwargs: Any
) -> Any:
async def _async_wrap_bluetooth_connection_error_retry( # type: ignore[return]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The mypy ignore here is needed because mypy expects the function to also return outside of the for loop. But there isn't anything sensible we could return there.

@bdraco bdraco changed the title Typing Fixes for generic BleakClient classes and the retry_bluetooth_connection_error decorator fix: typing for generic BleakClient classes and the retry_bluetooth_connection_error decorator Feb 25, 2023
Copy link
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

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

Thanks @kaechele

@bdraco bdraco merged commit 8ddf242 into Bluetooth-Devices:main Feb 25, 2023
@bdraco
Copy link
Member

bdraco commented Feb 25, 2023

I’ll fix the lint issues after I make my connection 🛬

@bdraco
Copy link
Member

bdraco commented Feb 25, 2023

3.0.0 published.

If you plan on bumping HA, I can take a look when I land ~9hrs if I'm not too tired.

@kaechele
Copy link
Contributor Author

No rush. My integration I'm working on is still a while out.
Thanks for the collaboration 🙂

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