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 bluetooth tracker asyncio usage #94695

Merged
merged 3 commits into from Jun 16, 2023
Merged

Conversation

d03n3rfr1tz3
Copy link
Contributor

@d03n3rfr1tz3 d03n3rfr1tz3 commented Jun 15, 2023

Breaking change

Proposed change

This PR only fixes the compatibility of the bluetooth_tracker component with Python 3.11, by correcting the usage of asyncio.wait(). As its not allowed to pass coroutines into it anymore, therefore a task needs to be created.

Beware, this PR does not fix the requirement of PyBluez 0.22 and its setup problem.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

@home-assistant
Copy link

Hi @d03n3rfr1tz3

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

Comment on lines 8 to 9
"requirements": ["bt-proximity==0.2.1", "git+https://github.com/pybluez/pybluez.git@master#pybluez"],
"version": "1.0.0"
Copy link
Member

Choose a reason for hiding this comment

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

We can't accept requirements that aren't packaged releases.

I suggest splitting this PR so the device tracker change can be merged

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I expected that and removed that one commit.

Do you know if, besides of HACS or waiting and praying for the owner of PyBluez releasing another official version, there is anything that can be done?

Copy link
Member

Choose a reason for hiding this comment

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

Fork PyBluez and create a new GitHub org to maintain it

Copy link
Member

Choose a reason for hiding this comment

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

If you are interested in maintaining it, I would try to reach out to the existing repo owners first

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well ok. I'm not really interested in maintaining it (there are already new maintainers), mostly because its not my main language/environment. Just saw (like some others), that it got updated since the last release of 0.23 in 2019 and is even compatible to Python 3.11. sadly the PyPi release is kinda stale since 2021, because the new maintainers on GitHub dont have access to the PyPi account for example.

Copy link
Member

@bdraco bdraco Jun 16, 2023

Choose a reason for hiding this comment

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

It looks like its a fragmented mess https://pypi.org/search/?q=pybluez with no fork clearly alive or published a release in the last 6 months

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.

Please see above

@home-assistant home-assistant bot marked this pull request as draft June 15, 2023 22:39
@home-assistant
Copy link

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

fixes the error "Passing coroutines is forbidden, use tasks explicitly", caused by passing an async function into asyncio.wait directly instead of creating a task for it.
@d03n3rfr1tz3 d03n3rfr1tz3 changed the title Fix bluetooth tracker and bumped pybluez 0.22 to master branch (0.30) Fix bluetooth tracker asyncio usage Jun 15, 2023
@bdraco bdraco added this to the 2023.6.3 milestone Jun 16, 2023
@d03n3rfr1tz3 d03n3rfr1tz3 marked this pull request as ready for review June 16, 2023 00:32
@home-assistant home-assistant bot requested a review from bdraco June 16, 2023 00:32
@bdraco
Copy link
Member

bdraco commented Jun 16, 2023

Please update the proposed change text to reflect what this PR does

@balloob balloob merged commit 3440c16 into home-assistant:dev Jun 16, 2023
18 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jun 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants