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

proposed breaking change: Bleak init fuction #652

Closed
dlech opened this issue Oct 6, 2021 · 9 comments
Closed

proposed breaking change: Bleak init fuction #652

dlech opened this issue Oct 6, 2021 · 9 comments
Labels
enhancement New feature or request Opinions Appreciated Please add an opinion on your desired resolution on this issue!

Comments

@dlech
Copy link
Collaborator

dlech commented Oct 6, 2021

I've collected a number of cases for needing a Bleak init function. But first I should say that this would be a breaking change because it would require users to modify their code by adding one line like this:

import asyncio

from bleak import init as init_bleak

async def main():
    await init_bleak()  # <-- add this line
    ...


asyncio.run(main())

So why do we need such a function?

  • Allows detecting the presence of Bluetooth hardware before using other Bleak API:
  • Allows a place to customize Bleak startup:
    • Could select default adapter for BlueZ backend here
    • Could address STA vs. MTA issues in WinRT backend here (needed for UI integration). (no longer relevant)
  • Could be used to avoid import side effects (well-behaving Python module should not have side effects on import, like spawning processes, which we are currently doing): (no longer relevant)
    • This is also relevant for STA vs. MTA issues.
    • Would avoid the need hacks like:

      bleak/bleak/__init__.py

      Lines 19 to 20 in 41aa0e2

      _on_rtd = os.environ.get("READTHEDOCS") == "True"
      _on_ci = "CI" in os.environ
@dlech dlech added enhancement New feature or request Opinions Appreciated Please add an opinion on your desired resolution on this issue! labels Oct 6, 2021
@dlech
Copy link
Collaborator Author

dlech commented Oct 9, 2021

I suppose we could make this less breaking by issuing a warning and calling the init function the first time a scanner is started if it has not already been called.

@elupus
Copy link
Contributor

elupus commented Oct 16, 2021

Can't we switch to a factory function for the classes instead? We are contemplating a esphome backend for bleak to extend the range of homeassistant bluetooth device trackers for example. In that case we would have multiple backends active at the same time.

We could even do it backword compatible by overloading the new function.

class Scanner(object):
    def __new__(cls, host):
        if bluez:
            return object.__new__(BluezScanner)
        elif osx:
            return object.__new__(OsxScanner)
        elif host:
            return object.__new__(EspHome)

@dlech
Copy link
Collaborator Author

dlech commented Oct 16, 2021

Can't we switch to a factory function for the classes instead?

Yes, that would work, but is still a breaking change. And would probably require more than just adding one line to existing programs.

@elupus
Copy link
Contributor

elupus commented Oct 16, 2021

Non breaking change with hidden factory function. I suppressed whitespaces for it to be a bit easier to paste here. For example i've not modified the init.cli function that works like before.

diff --git a/bleak/__init__.py b/bleak/__init__.py
index dae0ed0..7a9952a 100644
--- a/bleak/__init__.py
+++ b/bleak/__init__.py
@@ -19,6 +19,7 @@ from bleak.uuids import register_uuids
 
 _on_rtd = os.environ.get("READTHEDOCS") == "True"
 _on_ci = "CI" in os.environ
+_sys_backend = None
 
 _logger = logging.getLogger(__name__)
 _logger.addHandler(logging.NullHandler())
@@ -30,7 +31,8 @@ if bool(os.environ.get("BLEAK_LOGGING", False)):
     _logger.addHandler(handler)
     _logger.setLevel(logging.DEBUG)
 
-if platform.system() == "Linux":
+def get_sys_backend():
+    if platform.system() == "Linux":
         if not _on_rtd and not _on_ci:
             # TODO: Check if BlueZ version 5.43 is sufficient.
             p = subprocess.Popen(["bluetoothctl", "--version"], stdout=subprocess.PIPE)
@@ -51,7 +53,9 @@ if platform.system() == "Linux":
         from bleak.backends.bluezdbus.client import (
             BleakClientBlueZDBus as BleakClient,
         )  # noqa: F401
-elif platform.system() == "Darwin":
+        return "bluezdbus"
+
+    elif platform.system() == "Darwin":
         try:
             from CoreBluetooth import CBPeripheral  # noqa: F401
         except Exception as ex:
@@ -63,8 +67,9 @@ elif platform.system() == "Darwin":
         from bleak.backends.corebluetooth.client import (
             BleakClientCoreBluetooth as BleakClient,
         )  # noqa: F401
+        return "corebluetooth"
 
-elif platform.system() == "Windows":
+    elif platform.system() == "Windows":
         # Requires Windows 10 Creators update at least, i.e. Window 10.0.16299
         _vtup = platform.win32_ver()[1].split(".")
         if int(_vtup[0]) != 10:
@@ -88,6 +93,8 @@ elif platform.system() == "Windows":
             from bleak.backends.winrt.client import (
                 BleakClientWinRT as BleakClient,
             )  # noqa: F401
+            return "winrt"
+
         except ImportError:
             from bleak.backends.dotnet.scanner import (
                 BleakScannerDotNet as BleakScanner,
@@ -95,10 +102,21 @@ elif platform.system() == "Windows":
             from bleak.backends.dotnet.client import (
                 BleakClientDotNet as BleakClient,
             )  # noqa: F401
-
-else:
+            return "dotnet"
+    else:
         raise BleakError(f"Unsupported platform: {platform.system()}")
 
+
+def get_sys_backend_cached():
+    global _sys_backend
+    if _sys_backend is None:
+        _sys_backend = get_sys_backend()
+
+    return _sys_backend
+
+from .backends.scanner import BaseBleakScanner as BleakScanner
+from .backends.client import BaseBleakClient as BleakClient
+
 # for backward compatibility
 discover = BleakScanner.discover
 
diff --git a/bleak/backends/client.py b/bleak/backends/client.py
index 5a1b088..13f3881 100644
--- a/bleak/backends/client.py
+++ b/bleak/backends/client.py
@@ -31,6 +31,18 @@ class BaseBleakClient(abc.ABC):
             argument, which will be this client object.
     """
 
+    def __new__(cls, **kwargs):
+        from .. import get_sys_backend_cached
+        backend = get_sys_backend_cached()
+        if backend == "bluezdbus":
+            from .bluezdbus.client import BleakClientBlueZDBus
+            ret = object.__new__(BleakClientBlueZDBus)
+        elif backend == "corebluetooth":
+            from .corebluetooth.client import BleakClientCoreBluetooth
+            ret = object.__new__(BleakClientCoreBluetooth)
+        ret.__init__(**kwargs)
+        return ret
+
     def __init__(self, address_or_ble_device: Union[BLEDevice, str], **kwargs):
         if isinstance(address_or_ble_device, BLEDevice):
             self.address = address_or_ble_device.address
diff --git a/bleak/backends/scanner.py b/bleak/backends/scanner.py
index 870a562..0e7dfe3 100644
--- a/bleak/backends/scanner.py
+++ b/bleak/backends/scanner.py
@@ -70,6 +70,18 @@ AdvertisementDataFilter = Callable[
 class BaseBleakScanner(abc.ABC):
     """Interface for Bleak Bluetooth LE Scanners"""
 
+    def __new__(cls, **kwargs):
+        from .. import get_sys_backend_cached
+        backend = get_sys_backend_cached()
+        if backend == "bluezdbus":
+            from .bluezdbus.scanner import BleakScannerBlueZDBus
+            ret = object.__new__(BleakScannerBlueZDBus)
+        elif backend == "corebluetooth":
+            from .corebluetooth.scanner import BleakScannerCoreBluetooth
+            ret = object.__new__(BleakScannerCoreBluetooth)
+        ret.__init__(**kwargs)
+        return ret
+
     def __init__(self, *args, **kwargs):
         super(BaseBleakScanner, self).__init__()
         self._callback: Optional[AdvertisementDataCallback] = None

@dlech
Copy link
Collaborator Author

dlech commented Oct 16, 2021

Thanks for the suggestion. However, some of the features that need to be added to this are async and we can't call async functions from __new__() or __init__().

@dlech
Copy link
Collaborator Author

dlech commented Oct 16, 2021

This is why I was thinking we could make a new global async init function. Users can call it explicitly if they need custom options or if they want to control exactly when it runs, but we could implicit call it if hasn't be run yet the first time we start scanning (from the async BleakScanner.start() method.

@elupus
Copy link
Contributor

elupus commented Oct 16, 2021

I think a breaking change with async factory functions make more sense then. You would still need to do detections to figure out which backend yo use right?

@dlech
Copy link
Collaborator Author

dlech commented Oct 16, 2021

You would still need to do detections to figure out which backend yo use right?

Yes, that would not change.

@dlech
Copy link
Collaborator Author

dlech commented Oct 3, 2022

Closing since most of the problems this would solve have been fixed other ways. There is still potentially a need for a BleakAdapter class as described in #992 (comment)

@dlech dlech closed this as completed Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Opinions Appreciated Please add an opinion on your desired resolution on this issue!
Projects
None yet
Development

No branches or pull requests

2 participants