hbldh / bleak

A cross platform Bluetooth Low Energy Client for Python using asyncio
MIT License
1.56k stars 275 forks source link

corebluetooth scanner does not check on None vallue address #1523

Closed rrooggiieerr closed 2 weeks ago

rrooggiieerr commented 2 months ago

Description

The following exception was logged on my local Home Assistance instance:

2024-03-15 12:21:46.815 ERROR (MainThread) [homeassistant] Error doing job: Exception in callback CentralManagerDelegate.did_discover_peripheral(<CBCentralMan...x60000154d5e0>, <CBPeripheral... disconnected>, {
    kCBAdvD...rLevel = 12;
}, -103)
Traceback (most recent call last):
  File "/opt/homebrew/Cellar/python@3.11/3.11.8/Frameworks/Python.framework/Versions/3.11/lib/python3.11/asyncio/events.py", line 84, in _run
    self._context.run(self._callback, *self._args)
  File "/Users/rogier/workspaces/HomeAssistant/homeassistant-core/venv/lib/python3.11/site-packages/bleak/backends/corebluetooth/CentralManagerDelegate.py", line 265, in did_discover_peripheral
    callback(peripheral, advertisementData, RSSI)
  File "/Users/rogier/workspaces/HomeAssistant/homeassistant-core/venv/lib/python3.11/site-packages/bleak/backends/corebluetooth/scanner.py", line 133, in callback
    address = address_bytes.hex(":").upper()
              ^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'hex'

address_bytes is not checked on being None

What I Did

Just running Home Assistant

Logs

See above

dlech commented 2 months ago

What is the macOS version?

rrooggiieerr commented 2 months ago

Sonoma 14.3

dlech commented 2 months ago

I don't know what else could be done here besides raise an error with a more useful message. We can't allow None otherwise there would be no way to tell devices apart. This is using an undocumented feature of macOS which is subject to change or be removed without notice. Bleak doesn't use this by default, but Home Assistant is opting in to using the risky feature. It would probably be better if Home Assistant didn't do this to begin with.

rrooggiieerr commented 2 months ago

I understand the undocumented nature of the implementation. My thought would be to check on None value to prevent the AttributeError and just ignore the device if None. Something like:

                address_bytes: bytes = (
                    self._manager.central_manager.retrieveAddressForPeripheral_(p)
                )
                if address_bytes is None:
                    logger.warning("NoneType device address")
                    return
                address = address_bytes.hex(":").upper()
dlech commented 2 months ago

Is it only some devices but not all devices that return None?

rrooggiieerr commented 2 months ago

Seems to be only incidental. And I don't know which device. I'm receiving successful updates from my BLE devices

dlech commented 2 months ago

If that is the case, then I would take a pull request with the change you suggest. It probably just needs to be logger.debug though and maybe we could include the peripheral ID in the message so we can see what device it is. Maybe it just takes a while to resolve the Bluetooth address and subsequent calls return non-None.

logger.debug("Could not get Bluetooth address for %s. Ignoring this device.", p.identifier().UUIDString())
rrooggiieerr commented 2 months ago

Ok, fair enough. But I don't have a bleak development environment setup, and don't like to create an untested pull request, even though I'm quite confident it would resolve the matter. Also it's quite difficult to test since the sporadic nature of the None type address. But if you're fine with an untested pull request I'll go ahead.

dlech commented 2 months ago

Can you test by editing your Home Assistant install?

rrooggiieerr commented 2 months ago

I would not know how to reproduce the issue, it happened only once in the last 8 hours