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.

Sign up for GitHub

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

Jump to bottom

Created the Tutorial walkthrough file for ble_phy_central_example (IDFGH-9638) #10985

Closed
wants to merge 1 commit into from
Closed

Created the Tutorial walkthrough file for ble_phy_central_example (IDFGH-9638) #10985

wants to merge 1 commit into from

Conversation

abhi152
Copy link
Contributor

@abhi152 abhi152 commented Mar 14, 2023

No description provided.

@espressif-bot espressif-bot added the Status: Opened Issue is new label Mar 14, 2023
@github-actions github-actions bot changed the title Created the Tutorial walkthrough file for ble_phy_central_example Created the Tutorial walkthrough file for ble_phy_central_example (IDFGH-9638) Mar 14, 2023
@Alvin1Zhang
Copy link
Collaborator

Thanks for your contribution.

rahult-github
rahult-github suggested changes Mar 15, 2023
View reviewed changes
examples/bluetooth/nimble/ble_phy/phy_cent/tutorial/Ble_Phy_Central_Example_Walkthrough.md Outdated

## Includes

This example is located in the examples folder of the ESP-IDF under the [ble_phy/phy_cent/main](https://github.com/espressif/esp-idf/tree/master/examples/bluetooth/nimble/ble_phy/phy_cent/). The [main.c](https://github.com/espressif/esp-idf/tree/master/examples/bluetooth/nimble/ble_phy/phy_cent/main/main.c) file located in the main folder contains all the functionality that we are going to review. The header files contained in [main.c](https://github.com/espressif/esp-idf/tree/master/examples/bluetooth/nimble/ble_phy/phy_cent/main/main.c) are:
Copy link
Collaborator

Choose a reason for hiding this comment

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

The paths are absolute path. If the tutorial is backported to moved to other branch, this path will break. I suggest to instead use relative path, instead of absolute path.

examples/bluetooth/nimble/ble_phy/phy_cent/tutorial/Ble_Phy_Central_Example_Walkthrough.md Outdated

```

The main function starts by initializing the non-volatile storage library. This library allows to save key-value pairs in flash memory and is used by some components such as the Wi-Fi library to save the SSID and password:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is a BLE application, i suggest to put information related to BLE ( like passkey information) that gets stored in nvs.

examples/bluetooth/nimble/ble_phy/phy_cent/tutorial/Ble_Phy_Central_Example_Walkthrough.md Outdated

## Read Operation

`blecent_read` reads the supported LE PHY characteristic. If the peer does not support a required service, characteristic, or descriptor, then the peer lied when it claimed support for the alert notification service. When this happens, or if a GATT procedure fails this function immediately terminates the connection.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"Then the peer lied when it claimed ...." . This sounds too harsh in a tutorial.

Suggested wording change:
"If the peer does not support a required service, characteristic, or descriptor OR if a GATT procedure fails , then this function immediately terminates the connection."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @rahult-github for reviewing the walkthrough. I will do the all mentioned changes and modify this walkthrough

@CLAassistant
Copy link

CLAassistant commented Mar 15, 2023 •
edited
Loading

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@abhi152 abhi152 force-pushed the feature/walkthrough_for_nimble_examples branch 5 times, most recently from 39e488a to 3e74b9f Compare March 17, 2023 09:44
@rahult-github
Copy link
Collaborator

sha=73c06b5039f4382bfc21fb8bb1bf44355eb3ea89

@rahult-github rahult-github added the PR-Sync-Merge Pull request sync as merge commit label Mar 17, 2023
@abhi152 abhi152 force-pushed the feature/walkthrough_for_nimble_examples branch from 3e74b9f to c32c63e Compare March 21, 2023 08:48
@abhi152 abhi152 force-pushed the feature/walkthrough_for_nimble_examples branch from 7595817 to 3c9b54b Compare March 21, 2023 09:14
@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: Opened Issue is new labels Jul 6, 2023
@rahult-github
Copy link
Collaborator

PR merged . Closing

@tom-borcin tom-borcin added Resolution: Done Issue is done internally and removed Resolution: NA Issue resolution is unavailable labels Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rahult-github rahult-github rahult-github requested changes

Assignees
No one assigned
Labels
PR-Sync-Merge Pull request sync as merge commit Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@abhi152 @Alvin1Zhang @CLAassistant @rahult-github @espressif-bot @tom-borcin