Skip to content

Fix deprecated Gap type usages #289

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

Merged
merged 1 commit into from
Apr 7, 2020

Conversation

LDong-Arm
Copy link
Contributor

@LDong-Arm LDong-Arm commented Mar 31, 2020

This PR replaces usages of deprecated types in Gap:: that are not warned by the compiler but will not be available anymore once ARMmbed/mbed-os#12730 is merged. (Note: This does not depend on that PR.)

Types replaced:

  • Gap::Phy_t -> ble::phy_t
  • Gap::Handle_t -> ble::connection_handle_t
  • GapAdvertisingData::LE_GENERAL_DISCOVERABLE -> ble::adv_data_flags_t(value).getGeneralDiscoverable()
  • Gap::CentralPrivacyConfiguration_t -> ble::central_privacy_configuration_t

The legacy types exist in the legacy Gap class only which will be removed from mbed-os. The current Gap APIs only use the new types.

@LDong-Arm
Copy link
Contributor Author

LDong-Arm commented Mar 31, 2020

Hi @ARMmbed/mbed-os-pan, @evedon, this PR makes sure examples continue to work after ARMmbed/mbed-os#12730 is merged. The example updates are trivial and there's no dependency on the mbed-os PR though.

Copy link

@evedon evedon left a comment

Choose a reason for hiding this comment

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

Changes look good but could you add more details in the PR description on the migration actions?

@LDong-Arm
Copy link
Contributor Author

Changes look good but could you add more details in the PR description on the migration actions?

Thanks for the review, I've updated the description.

@LDong-Arm LDong-Arm changed the title Fix deprecated Gap type usages WIP: Fix deprecated Gap type usages Apr 3, 2020
@LDong-Arm LDong-Arm changed the title WIP: Fix deprecated Gap type usages Fix deprecated Gap type usages Apr 3, 2020
@LDong-Arm
Copy link
Contributor Author

@jamesbeyond It looks like the CI result is not reported back to GitHub?
@pan- Can we get this in?

Thanks

@jamesbeyond
Copy link
Contributor

jamesbeyond commented Apr 6, 2020

@jamesbeyond It looks like the CI result is not reported back to GitHub?
@pan- Can we get this in?

Thanks

@LDong-Arm
Can you rebase? then the new CI will kicks in

@LDong-Arm LDong-Arm force-pushed the fix_deprecated_types branch from 902e703 to 38d1c7e Compare April 6, 2020 16:25
@jamesbeyond
Copy link
Contributor

now CI works 😉

@LDong-Arm
Copy link
Contributor Author

@jamesbeyond Thanks!

@LDong-Arm
Copy link
Contributor Author

@pan- @evedon In addition to this, I'll make another example update PR to match device address related API changes. That will have a two-way dependency with the mbed-os Gap PR and they'll need to get in at the same time.

Copy link
Member

@pan- pan- left a comment

Choose a reason for hiding this comment

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

LGTM

@pan- pan- merged commit 21bfa30 into ARMmbed:master Apr 7, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Apr 8, 2020

This is now causing PRs failing (master Mbed OS)

./source/main.cpp:411:14: error: no type named 'central_privacy_configuration_t' in namespace 'ble'; did you mean 'central_privay_configuration_t'?

@LDong-Arm
Copy link
Contributor Author

The mbed-os master branch has long had a typo central_privay_configuration_t (should be privacy). I corrected the spelling both here which caused mbed-os CI to fail...

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.

5 participants