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

Cordio configure identity address #13622

Merged

Conversation

pan-
Copy link
Member

@pan- pan- commented Sep 16, 2020

Summary of changes

This adds new API to the cordio stack to set the identity address. It also change the random static address in GAP if it doesn't match the address in the security DB.

Impact of changes

Migration actions required

Documentation


Pull request type

[] 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)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers


@ciarmcom
Copy link
Member

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

0xc0170
0xc0170 previously approved these changes Sep 16, 2020
@mergify mergify bot added needs: CI and removed needs: review labels Sep 16, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Sep 17, 2020

CI started

@mbed-ci
Copy link

mbed-ci commented Sep 17, 2020

Jenkins CI Test : ❌ FAILED

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-ARM ✔️

@mergify mergify bot added needs: work and removed needs: CI labels Sep 17, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Sep 17, 2020

Failure unrelated, but I recall seeing it few days ago.

@jeromecoutant Wasn't this fixed or we got the recent changes that increased the size and causing it to fail?

@jeromecoutant
Copy link
Collaborator

@jeromecoutant Wasn't this fixed or we got the recent changes that increased the size and causing it to fail?

yes, should be fixed by #13587

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 17, 2020

Thanks, I did not realize this is for the feature branch. @pan- please rebase the feature branch

// The code should replace the random static address with the identity
// address if this is the case.
if (_db->get_local_identity_address() != gap.getRandomStaticAddress()) {
ble_error_t err =gap.setRandomStaticAddress(_db->get_local_identity_address());

Choose a reason for hiding this comment

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

missing space after =

@pan- pan- changed the base branch from feature-ble-host-privacy to master September 21, 2020 09:19
@pan- pan- changed the base branch from master to feature-ble-host-privacy September 21, 2020 09:20
@pan- pan- force-pushed the cordio-configure-identity-address branch from 4b524e6 to e2d1bf1 Compare September 21, 2020 09:21
@mergify mergify bot dismissed 0xc0170’s stale review September 21, 2020 09:22

Pull request has been modified.

…platform.

The function to get it has been removed as this operation is driven by the security manager.
@pan- pan- force-pushed the cordio-configure-identity-address branch from e2d1bf1 to a675ce6 Compare September 21, 2020 09:22
This also ensure the random static address used by gap is the correct one.
@pan- pan- force-pushed the cordio-configure-identity-address branch from a675ce6 to fd3bda3 Compare September 21, 2020 09:25
@pan-
Copy link
Member Author

pan- commented Sep 21, 2020

Thanks for your review @paul-szczepanek-arm . I fixed the typo and rebased the branch.

@pan- pan- merged commit d6c5b2f into ARMmbed:feature-ble-host-privacy Sep 21, 2020
@pan- pan- mentioned this pull request Oct 5, 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