Advertising scan fails

[kschmelzer]

After updating to the latest version, my script began crashing with the following stack:

...
File "<redacted>/lib/python3.7/site-packages/adafruit_ble/__init__.py", line 268, in start_scan
    advertisement = adv_type(entry=entry)
File "<redacted>/lib/python3.7/site-packages/adafruit_ble/advertising/standard.py", line 167, in __init__
    self.flags.general_discovery = True
AttributeError: 'NoneType' object has no attribute 'general_discovery'

Perhaps related to the lazy object instantiation of self.flags from the parent class.

Reverting to version 7.3.4 fixes the problem.

[dhalbert]

@tannewt so it needs not to set flags if entry is not None? https://github.com/adafruit/Adafruit_CircuitPython_BLE/blob/2f7915018a3fa94597177eb51fb5ab9d5b66ddde/adafruit_ble/advertising/standard.py#L162-L168 and will also need to be fixed in similar places, not just here.

[tannewt]

@dhalbert Ya, I think generally you want to avoid setting anything if entry is provided. Otherwise you may override settings from the scan entry.

[kschmelzer]

I might be missing something, but it looks like if entry is none then in Advertisement.__init__ Advertisement.data_dict does not get set. If Advertisement.data_dict is not set when accessing Advertisement.flags, the LazyObjectField.__get__ method returns none which would cause a similar situation.

[dhalbert]

It gets set to an empty dict: https://github.com/adafruit/Adafruit_CircuitPython_BLE/blob/2f7915018a3fa94597177eb51fb5ab9d5b66ddde/adafruit_ble/advertising/__init__.py#L241

It used to be that Advertisement.__init__() was only used to construct outgoing Advertisements. Advertisement.from_entry() was used to construct Advertisements from incoming (received) advertisements. That changed in #123. We fixed some of the cases where we should just use the incoming advertisement values, but we missed some. I found one above, and there are more in this library. There were also some elsewhere: https://github.com/adafruit/Adafruit_CircuitPython_BLE_Adafruit/pull/12, etc.

[tannewt]

Note that from_entry still called __init__ to make the object and then it overrode settings. This is the cleanest way I think.

[kschmelzer]

@dhalbert , Sorry, when I say that the data_dict isn't set, I mean that it is declared as an empty dictionary, not set from the values from an entry. When __get__ is called, if the Advertisement object is not mutable and _adv is not in data_dict it returns none. Since data_dict is getting set to {}, _adv would not be found and __get__ returns none. It is the only way I see that flags could be an object of NoneType.

I think that overriding the flags.general_discovery attribute is okay only if entry is not none. If entry is none, I don't see how flags can be a valid object.

[kschmelzer]

I think the PR you linked is more efficient, but it does not fix the root cause.

[dhalbert]

When __get__ is called, if the Advertisement object is not mutable and _adv is not in data_dict it returns None. https://github.com/adafruit/Adafruit_CircuitPython_BLE/blob/2f7915018a3fa94597177eb51fb5ab9d5b66ddde/adafruit_ble/advertising/__init__.py#L180-L190

(I had to study this a bit.) That's true, but if entry is None, then in fact self.mutable is set to True in the constructor, and the if on line 184 above will be false. So __get()__ will not return None, and the flags object will get instantiated when referenced. self.mutable is False only when an entry is given: https://github.com/adafruit/Adafruit_CircuitPython_BLE/blob/2f7915018a3fa94597177eb51fb5ab9d5b66ddde/adafruit_ble/advertising/__init__.py#L239-L253

The "policy" is that if you create the Advertisement yourself, it's mutable, but if it's received, then it's immutable.

Here's some REPL testing showing it's working. This is with #125.

>>> from adafruit_ble.advertising import Advertisement
>>> from adafruit_ble.advertising.standard import ProvideServicesAdvertisement
>>> a = Advertisement()
>>> a.flags
<adafruit_ble.advertising.AdvertisingFlags object at 0x7fae72441970>
>>> a = Advertisement()  # make another one
>>> a.flags.general_discovery = True    # it set something in flags
>>> psa = ProvideServicesAdvertisement()
>>> psa.flags.general_discovery = True    # also works

[kschmelzer]

Thank you @dhalbert , so, if I understand it correctly, the only way that flags can be none is in the condition that entry is not none but advertising_type, or _adt, is not in the original entry.advertisement_bytes. So a bad entry object?

I will do some more testing on my side to see if I can find the exact reason.

[dhalbert]

Right, so there may be something odd with your ScanEntry.

[dhalbert]

Automatically closed when I merged the pull request, but if you are still having issues, we of course can reopen.