Replace deprecated BLE API calls #12235

hugueskamba · 2020-01-10T10:03:11Z

Summary of changes

As the LegacyGap class has been deprecated, in favor of the Gap
class, all references of its references in the code base has been
replaced.

Impact of changes

Migration actions required

Documentation

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- @paul-szczepanek-arm

ciarmcom · 2020-01-10T12:00:36Z

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

0xc0170 · 2020-01-10T14:22:44Z

Even simple PRs should provide some Summary of changes . Please describe the PR.

what has been deprecated in BLE and what we should use - would it be useful to have in the commit msg ?

hugueskamba · 2020-01-13T07:40:54Z

Even simple PRs should provide some Summary of changes . Please describe the PR.

what has been deprecated in BLE and what we should use - would it be useful to have in the commit msg ?

Done.

0xc0170 · 2020-01-13T15:31:40Z

CI started

mbed-ci · 2020-01-13T16:03:23Z

Test run: FAILED

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

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

0xc0170 · 2020-01-14T08:33:41Z

Restarted entire pipeline to get test reports in logs

0xc0170 · 2020-01-14T08:59:28Z

We are investigating failures in CI, will restart once fixed

cc @VeliMattiLahtela

mbed-ci · 2020-01-14T09:52:56Z

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 4
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

0xc0170 · 2020-01-14T10:09:41Z

The failure is there, looks like it is related to the changes.

cc @ARMmbed/mbed-os-pan

hugueskamba · 2020-01-14T15:38:19Z

The failure is there, looks like it is related to the changes.

cc @ARMmbed/mbed-os-pan

@pan-
There seems to be a problem with the implementation of enablePrivacy_
The address is never initialised by the time Gap::enablePrivacy is called. And as far as I can tell, there is no function to initialize it either. I am working on adding something to initialize _non_private_address in features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NORDIC_SOFTDEVICE/TARGET_NRF51/source/nRF5xGap.cpp.

paul-szczepanek-arm · 2020-01-14T17:23:42Z

The init sequence needs to be amended if the set privacy call does not cause an HCI command to be sent. The sequence relies on each command causing a new HCI reply being generated.

As the `LegacyGap` class has been deprecated, in favor of the `Gap` class, all references of its references in the code base has been replaced.

hugueskamba · 2020-01-15T09:06:24Z

@pan- @paul-szczepanek-arm @LDong-Arm
Can you please review this. The cordio test passes with these changes. Especially whether the correct values for .use_non_resolvable_random_address and .resolution_strategy have been used. Also is setPeripheralPrivacyConfiguration or should I have used setCentralPrivacyConfiguration instead?
Thanks.

hugueskamba · 2020-01-15T12:52:00Z

those are only relevant if privacy is enabled (it only really enables privacy if one of the strategies requires it)

It appears it also is relevant when enablePrivacy is called with false as the initialization does not complete without the configuration.
Is there maybe an assumption that enablePrivacy(false) is only possible after a enablePrivacy(true) which would have done the configuration?

EDIT:
There is something in enablePrivacy that prevents any processing if the new state is equal to the current state. That will prevent enablePrivacy(false) to work at boot as the current state (disabled) is the same as the new state (disabled).

Doing a configuration is actually what allows the initialization to complete (not enablePrivacy) because the configuration ends up calling update_address_resolution_setting() which is what we expect enablePrivacy to call.

It was agreed that the currently deprecated API should be made Internal (usable in Mbed OS but not public) instead.

mbed-ci · 2020-01-16T14:49:55Z

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 5
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_cloud-client-pytest

0xc0170 · 2020-01-17T08:56:37Z

cloud test restarted

0xc0170 · 2020-01-17T10:00:45Z

@pan- @paul-szczepanek-arm Updated with tests passing, is this good to go in?

adbridge · 2020-01-20T15:33:05Z

NOTE the ci last ran several days ago and thus this will need to be run again before it can be merged, to ensure no conflicts etc due to other merges in the interim!

evedon · 2020-01-22T09:26:52Z

@adbridge Could you restart CI please?

adbridge · 2020-01-22T11:00:45Z

CI restarted

adbridge · 2020-01-22T11:01:24Z

@evedon this still needs approval from the Pan team..

mbed-ci · 2020-01-22T11:46:42Z

Test run: SUCCESS

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

adbridge · 2020-01-22T12:20:11Z

@evedon From Vincent:

I’m not seeing this getting approved; we discussed with Hugues last Thursday and there is no clear replacement for the deprecated function Gap::setAddress .
The function is deprecated for a good reason; it shouldn’t be part of the public API however it makes sense to have it available for drivers.
This whole PR changes the behaviour of BLE devices as it replaces a random static address with a private one. Meaning the address will change other time.

evedon · 2020-01-22T12:38:42Z

More work is required. @pan- will take over

mergify · 2020-01-25T11:54:36Z

This PR cannot be merged due to conflicts. Please rebase to resolve them.

hugueskamba · 2020-01-28T13:44:16Z

The PR https://github.com/ARMmbed/mbed-os/pull/12321/files replaces the deprecated PR with a new API.

ciarmcom requested review from pan-, paul-szczepanek-arm and a team January 10, 2020 12:00

ciarmcom added the needs: review label Jan 10, 2020

0xc0170 added needs: work and removed needs: review labels Jan 10, 2020

hugueskamba changed the title ~~Replace deprecated BLE APIs~~ Replace deprecated BLE API calls Jan 13, 2020

hugueskamba force-pushed the hk-replace-deprecated-ble branch from 3da0107 to fae6ab1 Compare January 13, 2020 07:40

0xc0170 approved these changes Jan 13, 2020

View reviewed changes

pan- approved these changes Jan 13, 2020

View reviewed changes

0xc0170 added needs: CI and removed needs: work labels Jan 13, 2020

0xc0170 added needs: work and removed needs: CI labels Jan 14, 2020

hugueskamba force-pushed the hk-replace-deprecated-ble branch from fae6ab1 to 260ef85 Compare January 14, 2020 15:34

Replace deprecated BLE API calls

dbf4cdb

As the `LegacyGap` class has been deprecated, in favor of the `Gap` class, all references of its references in the code base has been replaced.

hugueskamba force-pushed the hk-replace-deprecated-ble branch from 260ef85 to dbf4cdb Compare January 15, 2020 09:04

0xc0170 added needs: CI and removed needs: work labels Jan 16, 2020

0xc0170 added the release-version: 6.0.0-alpha-2 Second pre-release version of 6.0.0 label Jan 17, 2020

0xc0170 added needs: review and removed needs: CI labels Jan 17, 2020

0xc0170 requested a review from pan- January 20, 2020 15:09

mergify bot added needs: CI and removed needs: review labels Jan 22, 2020

mergify bot added ready for merge and removed needs: CI labels Jan 22, 2020

adbridge added do not merge and removed ready for merge release-version: 6.0.0-alpha-2 Second pre-release version of 6.0.0 labels Jan 22, 2020

mergify bot added the needs: work label Jan 25, 2020

pan- mentioned this pull request Jan 28, 2020

Ble driver set random static address #12321

Merged

hugueskamba closed this Jan 28, 2020

hugueskamba deleted the hk-replace-deprecated-ble branch January 28, 2020 13:45

Replace deprecated BLE API calls #12235

Replace deprecated BLE API calls #12235

Uh oh!

Conversation

hugueskamba commented Jan 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

Uh oh!

ciarmcom commented Jan 10, 2020

Uh oh!

0xc0170 commented Jan 10, 2020

Uh oh!

hugueskamba commented Jan 13, 2020

Uh oh!

0xc0170 commented Jan 13, 2020

Uh oh!

mbed-ci commented Jan 13, 2020

Test run: FAILED

Uh oh!

0xc0170 commented Jan 14, 2020

Uh oh!

0xc0170 commented Jan 14, 2020

Uh oh!

mbed-ci commented Jan 14, 2020

Test run: FAILED

Uh oh!

0xc0170 commented Jan 14, 2020

Uh oh!

hugueskamba commented Jan 14, 2020

Uh oh!

paul-szczepanek-arm commented Jan 14, 2020

Uh oh!

hugueskamba commented Jan 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hugueskamba commented Jan 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbed-ci commented Jan 16, 2020

Test run: FAILED

Uh oh!

0xc0170 commented Jan 17, 2020

Uh oh!

0xc0170 commented Jan 17, 2020

Uh oh!

adbridge commented Jan 20, 2020

Uh oh!

evedon commented Jan 22, 2020

Uh oh!

adbridge commented Jan 22, 2020

Uh oh!

adbridge commented Jan 22, 2020

Uh oh!

mbed-ci commented Jan 22, 2020

Test run: SUCCESS

Uh oh!

adbridge commented Jan 22, 2020

Uh oh!

evedon commented Jan 22, 2020

Uh oh!

mergify bot commented Jan 25, 2020

Uh oh!

hugueskamba commented Jan 28, 2020

Uh oh!

Uh oh!

hugueskamba commented Jan 10, 2020 •

edited

Loading

hugueskamba commented Jan 15, 2020 •

edited

Loading

hugueskamba commented Jan 15, 2020 •

edited

Loading