BLE Gap deprecation cleanup/rework #12730

LDong-Arm · 2020-03-31T13:49:24Z

Preceding PRs:

~~Remove Maxim and SoftDevice BLE stacks which do not support the latest APIs #12674~~
~~Fix deprecated Gap type usages mbed-os-example-ble#289 - examples update that can and should get in before this major PR.~~ (merged)

In conjunction with

Use new types for BLE address and Gap mbed-os-example-ble#290 - example update for address types.

Summary of changes

Remove BLE LegacyGap class and implementations. Map ble::Gap to global Gap.
Add non-deprecated APIs (originally found in LegacyGap) to Gap and implementations into GenericGap.
Remove/replace legacy BLE event signalling (callback chains) in favour of EventHandler, where possible.
Remove BLEProtocol:: and replace AddressType_t with own_address_type_t/peer_address_type_t, AddressByte_t with address_t, etc.
Replace BLE::Address_t (for accessing address types + values in whitelists) with the new equivalent whitelist_t::entry_t.
Rename ConnectionParams_t -> PreferredConnectionParams_t and use it only for Generic Access Service.
Fix misspelt central_privay_configuration_t -> central_privacy_configuration_t
Remove legacy type definitions and replace internal usages with new types, e.g. address types, disconnection reason, etc.

Notes:

setDeviceName(), getDeviceName(), setAppearance(), getAppearance() from LegacyGap are removed, as device name and appearance can be set via AdvertisingDataBuilder and saved by the application. But there were previously no deprecation warnings for them.
get/setPreferredConnectionParams() are dropped from Gap - updated versions will be added to Gatt where they should belong to.

Impact of changes

Legacy BLE Gap APIs and related type definitions are no longer available.
Some APIs have been updated to take parameters of new type definitions of address types + values.

Migration actions required

Applications based on up-to-date BLE APIs should be able to make most BLE function calls as before.
However as mentioned above some deprecated types (e.g. address) were not warned previously - their usages need to be replaced with corresponding new types. The best clue is compilation errors which can be manually fixed. The PR to update mbed-os-example-ble is an example of how to do this: Fix deprecated Gap type usages mbed-os-example-ble#289
Applications should set device name and appearance using AdvertisingDataBuilder and internally remember their values as needed, since APIs to get/set them are removed as explained above.

Documentation

API documentation is generated with Doxygen. Examples quoted in documentation (e.g. this) are based on ARMmbed/mbed-os-example-ble which requires an update.

Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[x] 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)
[x] Tests / results supplied as part of this PR

Manual testing

BLE examples compile & work as before using Use new types for BLE address and Gap mbed-os-example-ble#290
ble-cliapp is updated to work with this PR and used to manually test APIs (by referencing Gap tests in ble-tests)

Reviewers

features/FEATURE_BLE/ble/Gap.h

features/FEATURE_BLE/source/generic/GenericGap.tpp

features/FEATURE_BLE/ble/generic/GenericGap.h

ciarmcom · 2020-03-31T15:00:49Z

@LDong-Arm, thank you for your changes.
@ARMmbed/mbed-os-pan @ARMmbed/mbed-os-maintainers please review.

LDong-Arm · 2020-04-03T13:10:53Z

rebased

LDong-Arm · 2020-04-03T15:41:40Z

@pan- @evedon As per discussion, I have already made one agreed changed (remove get/set preferred connection params API) and created a ticket to reintroduce it in the correct form in the future.

As for the other change (use new address types), it's being worked on but should not take too long. I've marked this PR as WIP in the title.

features/FEATURE_BLE/ble/BLETypes.h

features/FEATURE_BLE/ble/gap/Gap.h

features/FEATURE_BLE/source/generic/GenericGap.tpp

pan-

Thanks Lingkai, it is very promising. I've noticed few improvements that could be applied here and there.

features/FEATURE_BLE/ble/gap/Types.h

pan- · 2020-04-06T18:07:47Z

features/FEATURE_BLE/source/generic/GenericGap.tpp

+ }
+ break;
+ case peer_address_type_t::RANDOM:
+ if (!is_random_static_address(entry.address)


The condition can be replace by a test to is_random_private_non_resolvable_address.

Checking is_random_private_non_resolvable_address alone doesn't guard against the scenario where the address value isn't even random.

It brings in another question: the old code only allowed public and random static, whereas the documentation of setWhitelist suggests resolvable private should also work. Were there any limitation that prevented us from whitelisting resolvable private addresses?

There's not much reason of adding a resolvable private address to the list as it change very regularly, you want to put the real address obtained during pairing in the whitelist, the controller should ensure derived resolvable addresses are check against this address.

I'll change it. With real addresses things would be simpler here.

features/FEATURE_BLE/source/generic/GenericGap.tpp

features/FEATURE_BLE/targets/TARGET_CORDIO/source/CordioGattServer.cpp

features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalGenericAccessService.h

LDong-Arm

@pan- Thanks for the review, I'm working on addressing your comments.

LDong-Arm · 2020-04-07T09:53:58Z

features/FEATURE_BLE/source/generic/GenericGap.tpp

+ }
+ break;
+ case peer_address_type_t::RANDOM:
+ if (!is_random_static_address(entry.address)


Checking is_random_private_non_resolvable_address alone doesn't guard against the scenario where the address value isn't even random.

It brings in another question: the old code only allowed public and random static, whereas the documentation of setWhitelist suggests resolvable private should also work. Were there any limitation that prevented us from whitelisting resolvable private addresses?

features/FEATURE_BLE/source/generic/GenericGap.tpp

mbed-ci · 2020-04-07T11:53:33Z

Test run: FAILED

Summary: 3 of 3 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM-lts
jenkins-ci/mbed-os-ci_build-GCC_ARM-lts
jenkins-ci/mbed-os-ci_unittests-lts

0xc0170 · 2020-04-07T14:19:55Z

CI lts not related to this PR, will be cleared and proper CI restarted after 5.15 PRs are all integrated

LDong-Arm · 2020-04-07T14:31:05Z

CI lts not related to this PR, will be cleared and proper CI restarted after 5.15 PRs are all integrated

Thanks. I'm still working on this PR and it's not ready for testing either.

LDong-Arm · 2020-04-07T14:46:12Z

@pan- I've addressed your comments, please re-review (the few commits since last review).

Regarding ConnectionParams_t - it's long been replaced by the full-fledged ConnectionParameters for actual connections. However the former is still used by Generic Access Service for a good reason - it's minimal and matches exactly what the specification requires for displaying preferred parameters. So I renamed it to PreferredConnectionParams_t.

In ARMmbed/mbed-os#12730 we have removed legacy address-related types (BLEProtocol:: namespace) in favour of new once in ble:: namespace. This commit updates examples so they continue to work the latest Mbed OS.

pan-

Thanks @LDong-Arm that looks very good, did you had the change to try it with ble-cliapp and the test suite ?

features/FEATURE_BLE/source/generic/GenericGap.tpp

features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalGenericAccessService.h

The callback on_connected() in ConnectionEventMonitorEventHandler contains a parameter ConnectionParams_t which is superceded by ConnectionParameters. Since it's not used, remove it.

For actual connections, full ConnectionParameters is used. But as per BLE specification, Generic Access Service can display preferred connection parameters which is a smaller subset and ConnectionParams_t matches exactly. Thus we rename/repurpose it to PreferredConnectionParams_t.

LDong-Arm · 2020-04-17T09:19:50Z

Rebased. As said before, this is pending manual testing.

evedon · 2020-04-23T10:18:29Z

@0xc0170 Could we run CI while we are doing manual tests?

adbridge · 2020-04-23T16:10:40Z

CI started

mbed-ci · 2020-04-23T16:38:39Z

Test run: FAILED

Summary: 2 of 3 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM

LDong-Arm · 2020-04-23T17:37:19Z

@adbridge @0xc0170 CI failed because this PR has a "chicken-and-egg" dependency on ARMmbed/mbed-os-example-ble#290

LDong-Arm · 2020-04-24T09:44:28Z

I have tested the individual APIs using an updated ble-cliapp + manual commands derived from ble-tests and created reports which are being reviewed by @pan- .

pan-

It looks good from all perspectives, thanks @LDong-Arm for your efforts on the test app and test suite!
This PR can be merged when CI pass.

LDong-Arm · 2020-04-24T15:46:26Z

ARMmbed/mbed-os-example-ble#290 is now merged, can we run CI on this PR and get it in so future tests won't break? Thanks

adbridge · 2020-04-24T15:50:45Z

CI started

mbed-ci · 2020-04-24T16:22:55Z

Test run: FAILED

Summary: 2 of 3 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM

LDong-Arm · 2020-04-24T16:38:02Z

Just fixed an issue in one of the examples: https://github.com/ARMmbed/mbed-os-example-ble/pull/294/files (merged).
Hopefully CI will pass once we re-run! 🙏

0xc0170 · 2020-04-27T08:30:45Z

CI started

mbed-ci · 2020-04-27T09:33:44Z

Test run: SUCCESS

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

In ARMmbed/mbed-os#12730 we have removed legacy address-related types (BLEProtocol:: namespace) in favour of new once in ble:: namespace. This commit updates examples so they continue to work the latest Mbed OS.

LDong-Arm commented Mar 31, 2020

View reviewed changes

features/FEATURE_BLE/ble/Gap.h Show resolved Hide resolved

LDong-Arm commented Mar 31, 2020

View reviewed changes

features/FEATURE_BLE/source/generic/GenericGap.tpp Show resolved Hide resolved

features/FEATURE_BLE/ble/generic/GenericGap.h Show resolved Hide resolved

ciarmcom requested review from a team March 31, 2020 15:00

ciarmcom added the needs: review label Mar 31, 2020

LDong-Arm mentioned this pull request Mar 31, 2020

Fix deprecated Gap type usages ARMmbed/mbed-os-example-ble#289

Merged

LDong-Arm changed the title ~~(Preceding PRs, WIP) BLE Gap deprecation cleanup/rework~~ (Preceding PRs) BLE Gap deprecation cleanup/rework Apr 1, 2020

0xc0170 added the needs: preceding PR label Apr 3, 2020

LDong-Arm force-pushed the gap_deprecation_cleanup branch from daa67a1 to 05d83b8 Compare April 3, 2020 13:10

LDong-Arm force-pushed the gap_deprecation_cleanup branch from 05d83b8 to 3480405 Compare April 3, 2020 14:34

LDong-Arm changed the title ~~(Preceding PRs) BLE Gap deprecation cleanup/rework~~ (WIP, Preceding PRs) BLE Gap deprecation cleanup/rework Apr 3, 2020

LDong-Arm force-pushed the gap_deprecation_cleanup branch 2 times, most recently from 8cf9023 to 2856adc Compare April 6, 2020 16:36

LDong-Arm commented Apr 6, 2020

View reviewed changes

features/FEATURE_BLE/ble/BLETypes.h Show resolved Hide resolved

features/FEATURE_BLE/ble/gap/Gap.h Show resolved Hide resolved

features/FEATURE_BLE/ble/gap/Gap.h Outdated Show resolved Hide resolved

features/FEATURE_BLE/source/generic/GenericGap.tpp Outdated Show resolved Hide resolved

pan- requested changes Apr 6, 2020

View reviewed changes

LDong-Arm commented Apr 7, 2020

View reviewed changes

LDong-Arm requested a review from pan- April 7, 2020 14:41

LDong-Arm mentioned this pull request Apr 7, 2020

Use new types for BLE address and Gap ARMmbed/mbed-os-example-ble#290

Merged

LDong-Arm changed the title ~~(WIP, Preceding PRs) BLE Gap deprecation cleanup/rework~~ (Preceding PR) BLE Gap deprecation cleanup/rework Apr 7, 2020

pan- approved these changes Apr 8, 2020

View reviewed changes

features/FEATURE_BLE/source/generic/GenericGap.tpp Show resolved Hide resolved

features/FEATURE_BLE/targets/TARGET_CORDIO/CordioPalGenericAccessService.h Show resolved Hide resolved

LDong-Arm added 6 commits April 17, 2020 09:59

Remove connection_params from internal on_connected()

7c49aeb

The callback on_connected() in ConnectionEventMonitorEventHandler contains a parameter ConnectionParams_t which is superceded by ConnectionParameters. Since it's not used, remove it.

BLE: correct misspelling: prefered -> preferred

76b92b8

Gap.tpp: remove misleading deprecation header

3c922ac

Remove a few leftovers in BLE namespace

a6810dd

BLE whitelist can only contain public or random static addresses

50928fb

LDong-Arm force-pushed the gap_deprecation_cleanup branch from 9d4e9b1 to 50928fb Compare April 17, 2020 09:18

LDong-Arm changed the title ~~(Manual tests pending) BLE Gap deprecation cleanup/rework~~ BLE Gap deprecation cleanup/rework Apr 24, 2020

pan- approved these changes Apr 24, 2020

View reviewed changes

0xc0170 added needs: CI and removed needs: work labels Apr 27, 2020

0xc0170 added BREAKING-CHANGE release-version: 6.0.0-beta-1 labels Apr 27, 2020

0xc0170 approved these changes Apr 27, 2020

View reviewed changes

0xc0170 merged commit 28ef753 into ARMmbed:master Apr 27, 2020

0xc0170 removed the needs: CI label Apr 27, 2020

tcoppex mentioned this pull request Feb 1, 2021

Update initialization to work with latest Arduino Mbed board packages tcoppex/mbed-ble-hid#6

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BLE Gap deprecation cleanup/rework #12730

BLE Gap deprecation cleanup/rework #12730

LDong-Arm commented Mar 31, 2020 •

edited

Loading

ciarmcom commented Mar 31, 2020

LDong-Arm commented Apr 3, 2020

LDong-Arm commented Apr 3, 2020

pan- left a comment

pan- Apr 6, 2020

LDong-Arm Apr 7, 2020

pan- Apr 8, 2020

LDong-Arm Apr 8, 2020

LDong-Arm Apr 8, 2020

LDong-Arm left a comment

LDong-Arm Apr 7, 2020

mbed-ci commented Apr 7, 2020

0xc0170 commented Apr 7, 2020

LDong-Arm commented Apr 7, 2020

LDong-Arm commented Apr 7, 2020 •

edited

Loading

pan- left a comment

LDong-Arm commented Apr 17, 2020

evedon commented Apr 23, 2020

adbridge commented Apr 23, 2020

mbed-ci commented Apr 23, 2020

LDong-Arm commented Apr 23, 2020

LDong-Arm commented Apr 24, 2020

pan- left a comment

LDong-Arm commented Apr 24, 2020

adbridge commented Apr 24, 2020

mbed-ci commented Apr 24, 2020

LDong-Arm commented Apr 24, 2020 •

edited

Loading

0xc0170 commented Apr 27, 2020

mbed-ci commented Apr 27, 2020

BLE Gap deprecation cleanup/rework #12730

BLE Gap deprecation cleanup/rework #12730

Conversation

LDong-Arm commented Mar 31, 2020 • edited Loading

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Mar 31, 2020

LDong-Arm commented Apr 3, 2020

LDong-Arm commented Apr 3, 2020

pan- left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LDong-Arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbed-ci commented Apr 7, 2020

0xc0170 commented Apr 7, 2020

LDong-Arm commented Apr 7, 2020

LDong-Arm commented Apr 7, 2020 • edited Loading

pan- left a comment

Choose a reason for hiding this comment

LDong-Arm commented Apr 17, 2020

evedon commented Apr 23, 2020

adbridge commented Apr 23, 2020

mbed-ci commented Apr 23, 2020

LDong-Arm commented Apr 23, 2020

LDong-Arm commented Apr 24, 2020

pan- left a comment

Choose a reason for hiding this comment

LDong-Arm commented Apr 24, 2020

adbridge commented Apr 24, 2020

mbed-ci commented Apr 24, 2020

LDong-Arm commented Apr 24, 2020 • edited Loading

0xc0170 commented Apr 27, 2020

mbed-ci commented Apr 27, 2020

LDong-Arm commented Mar 31, 2020 •

edited

Loading

LDong-Arm commented Apr 7, 2020 •

edited

Loading

LDong-Arm commented Apr 24, 2020 •

edited

Loading