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

Ble5 long range wiring API #2298

Merged
merged 9 commits into from
Mar 24, 2021
Merged

Ble5 long range wiring API #2298

merged 9 commits into from
Mar 24, 2021

Conversation

mgolu
Copy link
Contributor

@mgolu mgolu commented Mar 10, 2021

This builds on the community-submitted PR(#2287) to add BLE5 long range and does the following:

HAL:

  • Add check when setting Advertising parameters, that the PHY is properly selected (for advertising, only one PHY can be chosen).
  • When setting the extended scan parameter, check if Coded PHY is selected either by itself or bitwise with another PHY. In either of those cases, extended must be set to 1.

Wiring:

  • Add setScanPhy() API
  • Add setAdvertisingPhy() API

Integration/Application tests run on 2 Tracker Ones, which can communicate with each other and switch PHYs

Usage example:

Scanner:
BLE.setScanPhy(BlePhy::CODED | BlePhy::MBPS1);

Advertiser:
BLE.setAdvertisingPhy(BlePhy::CODED);

Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

Copy link
Member

@XuGuohui XuGuohui left a comment

Choose a reason for hiding this comment

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

Good job 👍! We also need to add API test and on-device unit test for QA. I'm glad to help with this if you're not familiar with the test framework.

hal/src/nRF52840/ble_hal.cpp Outdated Show resolved Hide resolved
hal/src/nRF52840/ble_hal.cpp Show resolved Hide resolved
wiring/inc/spark_wiring_ble.h Outdated Show resolved Hide resolved
wiring/src/spark_wiring_ble.cpp Outdated Show resolved Hide resolved
wiring/src/spark_wiring_ble.cpp Show resolved Hide resolved
@XuGuohui XuGuohui added ble Bluetooth Low Energy feature needs review labels Mar 11, 2021
@XuGuohui XuGuohui added this to the 3.1.0 milestone Mar 11, 2021
wiring/src/spark_wiring_ble.cpp Show resolved Hide resolved
wiring/src/spark_wiring_ble.cpp Show resolved Hide resolved
@XuGuohui XuGuohui added ready to merge PR has been reviewed and tested and removed needs review labels Mar 24, 2021
@XuGuohui XuGuohui merged commit de61653 into develop Mar 24, 2021
@XuGuohui XuGuohui deleted the ble5_long_range_wiring branch March 24, 2021 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ble Bluetooth Low Energy feature ready to merge PR has been reviewed and tested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants