Gap::updateAdvertisingPayload should work for different length of data

[pan-]

The use of the function Gap::updateAdvertisingPayload can be improved:

• It is not possible to update a payload field if the new data have size different than the previous one.

gap.accumulateAdvertisingPayload(Gap::COMPLETE_LOCAL_NAME, "foo", 3);

// this call will return an error 
gap.accumulateAdvertisingPayload(Gap::COMPLETE_LOCAL_NAME, "foobar", 6);

• The user have to keep track of every field size if he want to update a field later

[rainierwolfcastle]

ARM Internal Ref: IOTSFW-1085

[andresag01]

It seems that this issue has been resolved partly. From the documentation, it seems that accumulateAdvertisingPayload() function is only meant for appending information to the packet payload and the updateAdvertisingPayload() is meant for replacing the value of a field with a specific AD type. However, the behaviour of the functions is actually as follows:

• updateAdvertisingPayload(): iterates over the already added payload searching for the data with the specified AD type to update. If the field is found the data will be updated with new data ONLY IF the new data has the exact same length as the old data. In any other cases it returns a failure code.
• accummulateAdvertisingPayload(): It checks whether the specified AD type was previously added to the payload. If it isnt then the data is appended to the end of the packet. However, in the case that it is, the behaviour if slightly more complicated:
  • If the specified AD type to accumulate is either of the following it REPLACES the existing value the new one regardless of whether the size is different:
    • FLAGS:
    • SHORTENED_LOCAL_NAME:
    • COMPLETE_LOCAL_NAME:
    • TX_POWER_LEVEL:
    • DEVICE_ID:
    • SLAVE_CONNECTION_INTERVAL_RANGE:
    • SERVICE_DATA:
    • APPEARANCE:
    • ADVERTISING_INTERVAL:
    • MANUFACTURER_SPECIFIC_DATA:
  • If the specified AD type to accumulate is either of the values below, it APPENDS the value to the existing data in the payload. Therefore, the call accumulateAdvertisingPayload(COMPLETE_LIST_16BIT_SERVICE_IDS, 0xAABB, 2) followed by a call to accumulateAdvertisingPayload(COMPLETE_LIST_16BIT_SERVICE_IDS, 0xCCDD, 2) yields a packet with payload [0x05, COMPLETE_LIST_16BIT_SERVICE_IDS, 0xAA, 0xBB, 0xCC, 0xDD].
    • INCOMPLETE_LIST_16BIT_SERVICE_IDS:
    • COMPLETE_LIST_16BIT_SERVICE_IDS:
    • INCOMPLETE_LIST_32BIT_SERVICE_IDS:
    • COMPLETE_LIST_32BIT_SERVICE_IDS:
    • INCOMPLETE_LIST_128BIT_SERVICE_IDS:
    • COMPLETE_LIST_128BIT_SERVICE_IDS:
    • LIST_128BIT_SOLICITATION_IDS:
  • For any other AD type value not specified in the cases above an error code will be returned when the AD type was previously added.

In my opinion, it seems that the accumulateAdvertisingPayload() function works more like an update (with some limitations) than the updateAdvertisingPayload(). Furthermore, the updateAdvertisingPayload() seems too limited. In my opinion we there are two ways to fix this issue:

1. We update the documentation explaining the behaviour in more detail.
2. We define what update and accumulate really mean and change the functions accordingly.

@pan- @rgrover @LiyouZhou: What are your thoughts on this?

[rgrover]

accumulate() should call update() when it finds the AD structure. The logic within accumulate() which deals with handling existing AD structures should move into update().

@marcuschangarm please review.

[marcuschangarm]

The most recent changes are here: fea34e7

I made the changes to GapAdvertisingData::addData because it required the least amount of overall changes.

Gap::updateAdvertisingPayload and GapAdvertisingData::updateData are obsolete at the moment.

What we should have is an advertising manager with a nice API to handle multiple advertising packets from different services and different advertisement intervals.