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

BLE: fix direct advertising on Cordio #13060

Merged
merged 1 commit into from
Jun 10, 2020

Conversation

paul-szczepanek-arm
Copy link
Member

@paul-szczepanek-arm paul-szczepanek-arm commented Jun 3, 2020

Summary of changes

Fix support for direct advertising on Cordio.

Impact of changes

Migration actions required

Documentation

none


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@pan-


@ciarmcom ciarmcom requested review from pan- and a team June 3, 2020 09:00
@ciarmcom
Copy link
Member

ciarmcom commented Jun 3, 2020

@paul-szczepanek-arm, thank you for your changes.
@pan- @ARMmbed/mbed-os-pan @ARMmbed/mbed-os-maintainers please review.

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

Would it be worth describing what is this fixing (adding) in the commit message? I don't see the needed context for example.

if (enable) {
uint16_t *durations_ms = new uint16_t[number_of_sets];
advertising_handle_t handles[DM_NUM_ADV_SETS];;
Copy link
Member

Choose a reason for hiding this comment

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

Could you remove the extra ; ?

@ciarmcom ciarmcom requested review from pan- and a team June 3, 2020 15:00
@ciarmcom
Copy link
Member

ciarmcom commented Jun 3, 2020

@paul-szczepanek-arm, thank you for your changes.
@pan- @ARMmbed/mbed-os-pan @ARMmbed/mbed-os-maintainers please review.

To start direct connectable advertising on the cordio stack, the programmer should call the function DmConnAccept instead of the function DmAdvStart .
This functions expect the target address and address type as parameter, which are passed to the controller when the programmer sets the advertising parameters and not known when advertising_enable is called.
Therefore, this information should be kept in memory when advertising parameters are set and retrieved when advertising is enable to choose the right call to start (or stop) advertising.
Timeout of direct advertising is also handled in an uncommon way, a connection timeout is received. Similarly, DmConnClose should be called to stop connectable direct advertising.

The state is kept in an array of direct_adv_cb_t. Each items contains a peer address, the peer address type, the connection handle and the advertising handle as well as a state which indicate if the advertising is running, pending or not used.
When advertising parameters are set, the state is updated to match the target address or disable direct advertising management for the advertising set being configured.
When advertising is enabled, the pal dispatch the operation to the right calls (DmAdvStart/DmConnAccept or DmAdvStop/DmConnClose).
When an advertising timeout happen or a connection is made, the pal cleans any direct advertising state of this advertising set
@0xc0170
Copy link
Contributor

0xc0170 commented Jun 9, 2020

CI started

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 9, 2020

Also restarted Travis to report the status

@mbed-ci
Copy link

mbed-ci commented Jun 9, 2020

Test run: SUCCESS

Summary: 6 of 6 test jobs passed
Build number : 1
Build artifacts

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 10, 2020

All green but the incorrect status reported from mergify makes it red. I'll merge as it can't be overwritten.

@0xc0170 0xc0170 added the release-type: patch Indentifies a PR as containing just a patch label Jun 10, 2020
@0xc0170 0xc0170 merged commit d971d4e into ARMmbed:master Jun 10, 2020
@mergify mergify bot removed the ready for merge label Jun 10, 2020
@adbridge adbridge added release-version: 6.1.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch labels Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants