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

Late-import base36 and QR code libraries; remove SUPPORT_QR_CODE flag #470

Merged
merged 1 commit into from
Apr 1, 2024

Conversation

akx
Copy link
Contributor

@akx akx commented Nov 27, 2023

Some tests run locally for home-assistant/core would fail with a cryptic "base36 is not defined" error if that library was not installed.

This PR:

  • makes that error message cleaner – it's now a RuntimeError that has the import error as a reason
  • makes pyhap not import base36 or QRCode at all until they are needed
    • in doing that removes the SUPPORT_QR_CODE flag; based on GitHub Code Search there are no external users for that flag that I can see.

@akx
Copy link
Contributor Author

akx commented Mar 11, 2024

Ping @ikalchev :)

@ikalchev
Copy link
Owner

ikalchev commented Apr 1, 2024

Sorry for the huge delay on that!

@ikalchev ikalchev merged commit 4bc2cb0 into ikalchev:dev Apr 1, 2024
@@ -253,7 +255,15 @@ def setup_message(self):
Installation through `pip install HAP-python[QRCode]`
"""
pincode = self.driver.state.pincode.decode()
if SUPPORT_QR_CODE:
try:
from qrcode import QRCode
Copy link
Contributor

Choose a reason for hiding this comment

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

This does blocking IO in the event loop

@@ -190,6 +186,12 @@ def xhm_uri(self) -> str:

:rtype: str
"""
try:
import base36
Copy link
Contributor

Choose a reason for hiding this comment

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

This does blocking IO in the event loop

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.

3 participants