Different behavior using HCITool backend

[alexwhittemore]

Not sure this QUITE counts as a bug, but a few additional notes in the readme might be helpful. Using the default backend, (deduplicated) advertisements always come in with scan response data, if there is any, so you can assume a given ad will either have a complete_name or not in your code. Using the hcitool backend, this isn't the case. The current ble_simpletest.py code at https://github.com/adafruit/Adafruit_CircuitPython_BLE/blob/master/examples/ble_simpletest.py takes this into account and works under both conditions. I'm not sure where I found this example, but the code I'd been working with before setting up hcitool doesn't:

from adafruit_ble import BLERadio

radio = BLERadio()
print("scanning")
found = set()
for entry in radio.start_scan(timeout=240, minimum_rssi=-80):
    addr = entry.address
    name = entry.complete_name
    if name and "MySensor" in name:
        print("Received ad for {}".format(name))
    if "09:17" in addr.string:
        print("Received nameless ad for {}".format(addr.string))
    if addr not in found and name and "MySensor" in name:
        print(entry)
    found.add(addr)

print("scan done")

Note I'm testing against known-name as well as known-MAC to illustrate that the nameless ad comes in first.

Mostly this is to illustrate how I came across a different subtle difference: With the non-hcitool backend, an unclean exit of the program like you'd often see while debugging doesn't seem to matter. Start it back up, everything's good. With hcitool, I find I have to run sudo hciconfig hci0 reset after every exit, or else the code above fails silently and just prints

scanning
scan done

before exiting.

As well, now that I'm testing code that DOESN'T rely on hcitool ad un-deduplication, I'm not sure how to "turn it off." That is, if I simply remove my user from the "bluetooth" group, _bleio still trys to use hcitool and hits a permissions error. I guess that if hcitool exists at all, Adafruit_Blinka_bleio will try to use it? I'd love an easy way to select between the two.

[alexwhittemore]

For a little clarification, one thing that seems to be tripping me up is the differing behavior around advertisement.scan_response in the example code of ble_simpletest.py:

Using the regular backend with deduplicated advertisements, any advertisement that's got a scan response seems to just have the scan response data folded in. For instance, advertisement.complete_name will be set to MyDeviceName and advertisement.scan_response will be None

Using the hcitool backend, those two parts get presented as separate advertisements, so the first will have advertisement.complete_name is None and the second will have advertisement.complete_name == MyDeviceName AND advertisement.scan_response set.

So I guess the point here is, if you look for your device by testing on advertisement.scan_response as the example sort of suggests, you'll find it using the hcitool backend but miss it without that. Sort of a subtle gotcha. But easy enough to workaround by testing for advertisement.complete_name knowing that the first packet of every advertising interval will come back with None until the scan response happens.

I imagine this has implications for "hcitool-or-not" cross compatibility looking for other data that only comes with a scan response, but I haven't run into them yet.

[tannewt]

The failing after an unclean exist issue sounds like it's worth a bug. We should be able to use a try: finally: to ensure clean up happens even when an exception is thrown.

The separation of scan response matches what circuitpython does and is the best way to do it because it is truer to what is transmitted over the air. The CircuitPython code can do merging as it sees fit rather than letting the host OS do it.

For README updates, please make a PR. A PR will be easier to discuss. Thanks!

[alexwhittemore]

The failing after an unclean exist issue sounds like it's worth a bug. We should be able to use a try: finally: to ensure clean up happens even when an exception is thrown.

Actually I'm not convinced now. I think I have to look into/characterize this a bit more first. This is all around scanning: some code appears to "fail" (in the sense that it returns immediately), other code doesn't FAIL, but behaves differently (for instance, a scan with a set timeout returns immediately with no results, but if you keep re-starting it with a loop, it WILL return ads when they happen to exist). Then after a reset, the timeout actually gets obeyed.

Anyway, like I said - I don't REALLY know what's actually going on, so I kind of need to figure that out first.

[dhalbert]

We could add an argument that says whether or not to de-duplicate the advertisements. Ultimately we like bluez to fix this, and there is some work going on in this area but it would be in a future kernel release.

I have also experienced the need to cycle Bluetooth off and on in order to reset things to a good state.

[alexwhittemore]

Ugh, what do y'all think:

I want to add a simple and obvious(ish) switch to pick explicitly between the "hcitool backend" and the original "bleak backend". Something like ble = BLERadio(use_hcitool=False), so that I can HAVE hcitool installed and functional, but rapidly switch while iterating code. To this point, I've been renaming hcitool with sudo mv /usr/bin/hcitool /usr/bin/hcitool_disabled which feels very silly.

It turns out, of course, doing this in code is QUITE easy:

ble = BLERadio()
ble._adapter._hcitool_is_usable = False # Comment out this line and hcitool will be used. Otherwise, normal Bleak.

On the one hand, it feels just dirty to access two layers of private variables to get the job done. And it's not like you can look at readthedocs and understand this is what you should do.

On the other hand, BLERadio() has no concept of hcitool or not, since blinka is supposed to transparently interpose. I COULD add a documented switch to _bleio.adapter, but that still remains obscured by a layer of private variable. Maybe some environment/global variable would make sense here? All of these options feel mediocre.

[tannewt]

environment, private access or global work for me. I don't want to add a kwarg because it won't match the CircuitPython impl. You are deep in the weeds so I think any option is fine.

[alexwhittemore]

Hmmm now that I'm thinking about it, I kind of like a global variable, global BLE_BACKEND="bleak" or BLE_BACKEND="hcitool". If undefined, default behavior (hcitool if able, bleak if not). If defined, use the selection, or except if unable (if hcitool is chosen, but unavailable).

I'll put together a PR.

[alexwhittemore]

Tried to fix this today:

global isn't an option, because globals only pertain to a given module. You'd have to set the value with something like adafruit_ble._bleio.adapter_.BLE_BACKEND = "hcitool" and that's not any cleaner than ble._adapter._hcitool_is_usable = False

environment is the only practical way to get explicit exception functionality along the lines of "you said hcitool, but it's not working, here's your warning" but setting actual environment variables before running is hardly intuitive, and setting them from python is also hardly intuitive.

As such, I just gave up for now and left it as in my example above with a couple layers of kind-of-opaque private access, and added a note in the readme about how to do that.

[alexwhittemore]

Agh I changed my mind. Leaving it as-is doesn't satisfy my desire to explicitly error out if it can't use the backend I expect it to. Updated the PR to allow private access to explicitly choose backend, which

1. Assumes "bleak" will always be OK if chosen
2. throws EnvironmentError if "hcitool" is chosen, but can't be used for whatever reason
3. Throws a ValueError if you enter an unknown string instead of "bleak" or "hcitool.