More thorough documentation

[dhalbert]

The library could really use more thorough documentation, through more examples, overview text, a, detailed explanatory text. A Learn Guide is probably appropriate, but there should be more inline documentation too. The declarative nature of Service and Advertisement needs to be more thoroughly explained, for example.

[askpatrickw]

A link to the documentation in the Readme would be nice too.

https://circuitpython.readthedocs.io/projects/ble/en/latest/

[kevinjwalters]

Some other things that would be good to document and provide/cite examples:

• I don't yet understand exactly how prefix class variable works. I think this is what determines if an incoming packet matches the type of Advertisement - not clear if this is dependent on fields being identically ordered. AdafruitColor (in

  Adafruit_CircuitPython_BLE/adafruit_ble/advertising/adafruit.py

  Lines 43 to 68 in ffd1f1d

  	_MANUFACTURING_DATA_ADT = const(0xFF)
  	_ADAFRUIT_COMPANY_ID = const(0x0822)
  	_COLOR_DATA_ID = const(0x0000)
  	class AdafruitColor(Advertisement):
  	"""Broadcast a single RGB color."""
  	# This prefix matches all
  	prefix = struct.pack(
  	"<BBHBH",
  	0x6,
  	_MANUFACTURING_DATA_ADT,
  	_ADAFRUIT_COMPANY_ID,
  	struct.calcsize("<HI"),
  	_COLOR_DATA_ID,
  	)
  	manufacturer_data = LazyObjectField(
  	ManufacturerData,
  	"manufacturer_data",
  	advertising_data_type=_MANUFACTURING_DATA_ADT,
  	company_id=_ADAFRUIT_COMPANY_ID,
  	key_encoding="<H",
  	)
  	color = ManufacturerDataField(_COLOR_DATA_ID, "<I")
  	"""Color to broadcast as RGB integer."""

  ) uses the field ID value, _COLOR_DATA_ID, in the prefix so would the matching break if another field was added?
• I've not worked out yet if field order is predictable for Advertisement with the CP libraries. Even if the spec. regards packets as identical with different field orders it would still be useful to be able to explicitly order these, i.e. there's bound to be some semi-broken device that needs things in a specific order.
• equality tests for Advertisement objects:
  • there appears to be no __eq__ so these will return False even two Advertisements hold the same data
  • this could be documented with a suggestion to use bytes(obj1) == bytes(obj2) (but this does come back to the ordering question)
• Some guidelines on best practise for Adafruit's manufacturer data allocated for customer use (0xf000 to 0xffff (implied)). I'm not sure if I can use one id for my application as I need multiple fields and ideally I'd add a protocol version number too somewhere. And if the Advertisements have a protocol version then it would be nice to be able to choose to receive all versions or one version via something like an option to start_scan().
• ManufacturerDataField has a field_names feature that I have just discovered so I might be on my fourth design flip-flop...
• If variable length fields (e.g. strings) are allowed within the Manufacturer's data it would be good to have an example and one that deals with unicode/utf-8 in some way.
• Some recommendations on how to use sequence number in https://github.com/adafruit/Adafruit_CircuitPython_BLE_BroadcastNet/blob/c6328d5c7edf8a99ff719c3b1798cb4111bab397/adafruit_ble_broadcastnet.py#L84-L85 would be useful too, i.e. is there one counter for all Advertisements or is it per Advertisement type
• An explanation of the difference between these two color values which sound the same but have different ids:
  • AdafruitColor's color (0x0000):

    Adafruit_CircuitPython_BLE/adafruit_ble/advertising/adafruit.py

    Lines 67 to 68 in ffd1f1d

    	color = ManufacturerDataField(_COLOR_DATA_ID, "<I")
    	"""Color to broadcast as RGB integer."""

  • AdafruitSensorMeasurement's color (0x0a0e): https://github.com/adafruit/Adafruit_CircuitPython_BLE_BroadcastNet/blob/c6328d5c7edf8a99ff719c3b1798cb4111bab397/adafruit_ble_broadcastnet.py#L129-L130
• The order of multiple ManufacturerDataField in a ManufacturerData in an Advertisement appears to be determined by something like the keys() order which is linked to the key variables. I'm not proud of it but I'm manipulating the key numbers to get the order I want so I can do prefix matching in the style of AdafruitColor to match on the key of the "main" ManufacturerDataField.

[kevinjwalters]

I had this in a sub-class of Advertisement and this broke the parsing on received ads from start_scan():

    def __init__(self, enc_data=None, round=0):
        super().__init__()
        if enc_data is not None:
            self.enc_data = enc_data
        if round is not None:
            self.round = round

Sending

<RpsEncDataAdvertisement manufacturer_data=<ManufacturerData company_id=0822 data=0a 32 ff 41 42 43 44 45 46 47 48 03 31 ff 04 > >

got turned into

<RpsEncDataAdvertisement manufacturer_data=<ManufacturerData company_id=0822 data=03 31 ff 00 > >

on arrival. Note the final 00 bytes, that must be linked with round=0.

I changed it to this as a guess:

    def __init__(self, *, enc_data=None, round=None):

and that fixed it. This is not intuitive and needs documenting.

[dhalbert]

• I don't yet understand exactly how prefix class variable works. I think this is what determines if an incoming packet matches the type of Advertisement - not clear if this is dependent on fields being identically ordered.

I am working on an improvement on this to fix #82. You'll give a tuple of prefixes and it will combine them into a single concatenated prefix string to pass to _bleio. Also there will be more doc 😃 .

The order does not matter for matching purposes.

• I've not worked out yet if field order is predictable for Advertisement with the CP libraries. Even if the spec. regards packets as identical with different field orders it would still be useful to be able to explicitly order these, i.e. there's bound to be some semi-broken device that needs things in a specific order.

It is not predictable. So far we haven't found a device that cares.

• there appears to be no __eq__ so these will return False even two Advertisements hold the same data

Could you open an issue for to add __eq__? Thanks.

[dhalbert]

I had this in a sub-class of Advertisement and this broke the parsing on received ads from start_scan():

    def __init__(self, enc_data=None, round=0):
        super().__init__()
        if enc_data is not None:
            self.enc_data = enc_data
        if round is not None:
            self.round = round

Sending

<RpsEncDataAdvertisement manufacturer_data=<ManufacturerData company_id=0822 data=0a 32 ff 41 42 43 44 45 46 47 48 03 31 ff 04 > >

got turned into

<RpsEncDataAdvertisement manufacturer_data=<ManufacturerData company_id=0822 data=03 31 ff 00 > >

on arrival. Note the final 00 bytes, that must be linked with round=0.

I changed it to this as a guess:

    def __init__(self, *, enc_data=None, round=None):

and that fixed it. This is not intuitive and needs documenting.

This sounds like a bug: something is just checking falsey-ness instead of is None. Could you open an issue for this too, with a bit more detail or a pointer to the code? Thanks.

[kevinjwalters]

Some other things that would be good to document and provide/cite examples PART DEUX:

• description of how start_scan() behaves if lengthy processing/delays are added in the loop that retrieves data from it and recommendations about not doing this if appropriate.
• would be useful to have explicit documentation on whether a ManufacturerDataField can be data-less, i.e. just present or not present for a boolean flag. If this can be done would be good to show how the value is both set and then unset after setting. I can set these by assigning [] to the property, () does not work - I think this works by luck and is not intentional. I suspect receiving one isn't go to parse properly...
• Advertisement sub-classes are currently splattered with "<" formatting prefix for BLE's little-endian ordering. Wouldn't this detail of the underlying encoding be best handled by Advertisement or similar? I suppose there might be obscure cases where user wants to send data in a different order so permitting an override for those cases might make sense.
• Back on the field ordering, I have a working sub-class of Advertisement with a single manufacturer's field and prefix matching using same (questionnable) approach in AdafruitColor. If I set tx_power (part of Advertisement) that then breaks the prefix matching presumably due ordering issues.
• For optional fields in Advertisements it would be useful to have a recommended way to detect if the value was present in the incoming message. If None is returned when the value is not present that would be good to document.
• Some tips on delays in reading Advertisement from a start_scan would be useful. Users might be tempted to introduce processing into the loop that's reading Advertisements. My limited understanding here is a separate part of the chip handles the radio so there is true concurrency but there must still be some sort of buffer size issues when too much data accumulates if it's not being read? The case I'm thinking of is putting things up on the screen as displayio can surreptitiously throw in 100ms+ delays.
• Clarification on what should print(ad) for an Advertisement do. I have one here received from start_scan() with a manufacturer's field and it's not printed. See Ex1 below, __repr__ output is more useful than __str__ output when the data is received into an Advertisement as __str__ only shows the fields that get parsed out (I think).
• In start_scan() docs it says: "Advertisements and scan responses are filtered and returned separately" I am not sure what that means particularly the filtered part. I see two Advertisement per Advertisement sent by start_advertising() on another device.
• A lot of my prefix confusion has come about from assuming it was a single value matching against the stream of bytes that arrived over the air. I've seen the example and read the description in prefix building #82 and now understand it can be multiple prefixes concatenated.
• Would be useful to describe the matching algorithm when multiple classes are specified. The code in

  Adafruit_CircuitPython_BLE/adafruit_ble/__init__.py

  Lines 245 to 248 in b08096f

  	adv_type = Advertisement
  	for possible_type in advertisement_types:
  	if possible_type.matches(entry) and issubclass(possible_type, adv_type):
  	adv_type = possible_type

  appears to be checking all of them so it's not a simple (and efficient) first-matched approached. I think it's actually being more thorough and finds the deepest sub-class match.
• I originally assumed scan responses would only be requested if a new Advertisement sub class set a scan_response attribute but the code looks like it always sets them for short (<=31) - it makes an Advertisements with complete_name and tx_power.
• I'm now wondering how to receive scan response messages as start_scan(active=True) requests them... Brief discussion in circuitpython channel on Discord on 5-Jun-2020. The data does not appear to be combined into one Advertisement when listening for a specific ad, i.e. tx_power and complete_name are not populated and return None.
• I am not sure what uses the scan_response attribute (of type bool) in an Advertisement?
• I can do start_scan(timeout=1) ; start_scan(timeout=1) - I do not appear to have to run stop_scan() unless I break out of the associated for loop? Is that ok?
• A second start_advertising() call throws a _bleio.BluetoothError exception - developers may not be familiar with the _bleio library, is this as it was intended? I could track whether I am advertising but it seems easier here that I just add a try to my code to deal with this.
• Not a documentation issue, more potential RFE, should start_scan allow a buffer to be passed in similar to WaveFile's optional feature? That would allow early one-off allocation of the buffer and be useful for (less typical?) applications which make repeated use of start_scan.
• complete_name (property) (and anything else that's based on the underlying String) can throw a UnicodeError if malformed data is received - if that's the desired behaviour then perhaps some documentation or code examples showing how to deal with this is useful. There's an example of this occuring in Adafruit Forums: Pulse Oximeter Wireless Data Logger Example Getting Crazy Re although it's possible that particular one is caused by corruption on receiving device rather than sender putting out bad data.

Ex1:

>>> print(adv)
<Advertisement  >
>>> print(adv.data_dict[255])
b'"\x08\n0\xfeRPS\x00\x00\x00\x00\x00'
>>> print(adv.__repr__())
Advertisement(data=b"\x0e\xff\x22\x08\x0a\x30\xfe\x52\x50\x53\x00\x00\x00\x00\x00")

[kevinjwalters]

I mentioned how prefix matching works in https://learn.adafruit.com/rock-paper-scissors-circuitpython/advanced-game#advertisement-matching-3070937-39 - I've assumed it still checks the length field at the start of the packet but haven't checked that yet..