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?

Adafruit_CircuitPython_BLE/adafruit_ble/advertising/standard.py

Lines 162 to 168 in 2f79150

	def __init__(self, *services, entry=None):
	super().__init__(entry=entry)
	if services:
	self.services.extend(services)
	self.connectable = True
	self.flags.general_discovery = True
	self.flags.le_only = True

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:

Adafruit_CircuitPython_BLE/adafruit_ble/advertising/__init__.py

Line 241 in 2f79150

self.data_dict = {}

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: adafruit/Adafruit_CircuitPython_BLE_Adafruit#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.

Adafruit_CircuitPython_BLE/adafruit_ble/advertising/__init__.py

Lines 180 to 190 in 2f79150

	def __get__(self, obj, cls):
	if obj is None:
	return self
	# Return None if our object is immutable and the data is not present.
	if not obj.mutable and self._adt not in obj.data_dict:
	return None
	# Instantiate the object.
	bound_obj = self._cls(obj, advertising_data_type=self._adt, **self._kwargs)
	setattr(obj, self._attribute_name, bound_obj)
	obj.data_dict[self._adt] = bound_obj
	return bound_obj

(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:

Adafruit_CircuitPython_BLE/adafruit_ble/advertising/__init__.py

Lines 239 to 253 in 2f79150

	def __init__(self, *, entry=None):
	"""Create an empty advertising packet or one from a ScanEntry."""
	self.data_dict = {}
	self.address = None
	self._rssi = None
	self.connectable = False
	self.mutable = True
	self.scan_response = False
	if entry:
	self.data_dict = decode_data(entry.advertisement_bytes)
	self.address = entry.address
	self._rssi = entry.rssi # pylint: disable=protected-access
	self.connectable = entry.connectable
	self.scan_response = entry.scan_response
	self.mutable = False

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.