Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify functions that manipulate adv payload #153

Merged
merged 5 commits into from
Dec 21, 2015

Conversation

andresag01
Copy link

Modify the functions addData() and updateData() to correctly update the payload
information for a specified AD type if that type was already present in the
payload. For addData() if the AD type is not found, it is added to the payload.
In contrast, in updateData() if the AD type is not found an error is returned.

Documentation was updated accordingly.
@rgrover @pan- @LiyouZhou

Andres Amaya Garcia added 2 commits December 18, 2015 13:53
Modify the functions addData() and updateData() to correctly update the payload
information for a specified AD type if that type was already present in the
payload. For addData() if the AD type is not found, it is added to the payload.
In contrast, in updateData() if the AD type is not found an error is returned.

Documentation was updated accordingly.
byteIndex += (currentADV->len + 1); /* Advance by len+1; '+1' is needed to span the len field itself. */
if (field) {
// Field type already exist, either add to field or replace
return updateField(advDataType, payload, len, field);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't do what is expected if AD type is 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 or LIST_128BIT_SOLICITATION_IDS.

It will not update the field, it will append the new data at the end of the existing one.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added the comments to document this.

@rgrover
Copy link
Contributor

rgrover commented Dec 21, 2015

Looks good to me.
@pan- please take over the pull request.

* advertising buffer to overflow. BLE_ERROR_NONE is returned
* on success.
*
* @note When the specified AD type is INCOMPLETE_LIST_16BIT_SERVICE_IDS,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will always feel weird for me. There is no way to update a list of services UUIDS. The only way is to memorize all the fields, cleanup the whole structure fill the structure with the previous fields except the one to update then add the new one. It is a lot of work.

@andresag01
Copy link
Author

@pan- @rgrover : How should proceed with regards to @pan- comment?

@rgrover
Copy link
Contributor

rgrover commented Dec 21, 2015

let's resolve this over a chat. writing comments on github is slow.

Andres Amaya Garcia added 2 commits December 21, 2015 10:38
Accumulate and update advertising payload now differ in their implementations.
Accumulate updates the previous value, if it is UUID then the previously added
values are kept and the new one is simple appended. In contrast, update
replaces the previous value in all cases.
@andresag01
Copy link
Author

@pan- I have modified the pull request as per your comments.

pan- added a commit that referenced this pull request Dec 21, 2015
Modify functions that manipulate adv payload
@pan- pan- merged commit 3036b05 into ARMmbed:develop Dec 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants