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

Doc/optimize doc for ble (IDFGH-13260) #14193

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

esp-zbw
Copy link

@esp-zbw esp-zbw commented Jul 16, 2024

Add summary list of examples on ble and ble_50
Modified api about ble_gap: esp_gap_ble_api.h, btc_gap_ble.h

@CLAassistant
Copy link

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.

Copy link

github-actions bot commented Jul 16, 2024

Warnings
⚠️

Some issues found for the commit messages in this PR:

  • the commit message "doc(bt/bluedroid): Add readme files for BLE examples":
    • type/action should be one of [change, ci, docs, feat, fix, refactor, remove, revert, test]
  • the commit message "modified notes about esp_gap_ble_api":
    • summary looks empty
    • type/action looks empty

Please fix these commit messages - here are some basic tips:

  • follow Conventional Commits style
  • correct format of commit message should be: <type/action>(<scope/component>): <summary>, for example fix(esp32): Fixed startup timeout issue
  • allowed types are: change,ci,docs,feat,fix,refactor,remove,revert,test
  • sufficiently descriptive message summary should be between 20 to 72 characters and start with upper case letter
  • avoid Jira references in commit messages (unavailable/irrelevant for our customers)

TIP: Install pre-commit hooks and run this check when committing (uses the Conventional Precommit Linter).

Messages
📖 This PR seems to be quite large (total lines of code: 5778), you might consider splitting it into smaller PRs

👋 Hello esp-zbw, we appreciate your contribution to this project!


📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project.

Click to see more instructions ...


This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- Addressing info messages (📖) is strongly recommended; they're less critical but valuable.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...


We do welcome contributions in the form of bug reports, feature requests and pull requests via this public GitHub repository.

This GitHub project is public mirror of our internal git repository

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved, we synchronize it into our internal git repository.
4. In the internal git repository we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
5. If the change is approved and passes the tests it is merged into the default branch.
5. On next sync from the internal git repository merged change will appear in this public GitHub repository.

Generated by 🚫 dangerJS against 41643ad

@espressif-bot espressif-bot added the Status: Opened Issue is new label Jul 16, 2024
@github-actions github-actions bot changed the title Doc/optimize doc for ble Doc/optimize doc for ble (IDFGH-13260) Jul 16, 2024
BTC_GAP_BLE_ACT_CONFIG_LOCAL_PRIVACY, //Bluetooth GAP action type for configuring local privacy.
BTC_GAP_BLE_ACT_CONFIG_LOCAL_ICON, //Bluetooth GAP action type for configuring local icon.
BTC_GAP_BLE_ACT_UPDATE_WHITE_LIST, //Bluetooth GAP action type for updating white list.
BTC_GAP_BLE_ACT_CLEAR_WHITE_LIST, // Bluetooth GAP action type for clearing white list.
Copy link
Collaborator

Choose a reason for hiding this comment

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

与上面保持一致

Copy link
Collaborator

Choose a reason for hiding this comment

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

文档需要符合 doxygen 语法

#endif //#if (BLE_FEAT_PERIODIC_ADV_SYNC_TRANSFER == TRUE)
BTC_GAP_BLE_PERIODIC_ADV_RECV_ENABLE, //Bluetooth GAP action type for enabling periodic advertising receive.
BTC_GAP_BLE_PERIODIC_ADV_SYNC_TRANS, //Bluetooth GAP action type for periodic advertising synchronization transfer.
BTC_GAP_BLE_PERIODIC_ADV_SET_INFO_TRANS,// Bluetooth GAP action type for periodic advertising set information transfer.
Copy link
Collaborator

Choose a reason for hiding this comment

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

与上面一致

esp_bd_addr_t remote_device;
uint16_t tx_data_length;
esp_bd_addr_t remote_device; //Remote device address
uint16_t tx_data_length; //Transmit data length,For BLE 4.x without Data Length Extension (DLE) support, common default maximum value is 27 bytes (if no negotiated MTU extension).
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里描述不正确
see: BLUETOOTH SPECIFICATION Version 4.2 [Vol 2, Part E] page 1021

@@ -199,7 +247,8 @@ typedef union {
uint8_t *raw_scan_rsp;
uint32_t raw_scan_rsp_len;
} cfg_scan_rsp_data_raw;
#endif // #if (BLE_42_FEATURE_SUPPORT == TRUE)
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

#endif // #if (BLE_42_FEATURE_SUPPORT == TRUE)

Copy link
Collaborator

@weiyuhannnn weiyuhannnn left a comment

Choose a reason for hiding this comment

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

@esp-zbw Thanks for your hardworking on reviewing. I have left comments. Some of them are applied to the whole document. Also, I would suggest building the docs locally to preview the document.

# Bluedroid host Examples for BLE_50

Note: To use examples in this directory, you need to have BLE enabled in configuration and Bluedroid selected as the host stack.

Copy link
Collaborator

Choose a reason for hiding this comment

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

BLE 5.0 features have to be enabled, right? If I remember correctly, there is one option in memuconfig to enable ble 5.0 features. please double-check.

# Bluedroid host Examples for BLE

Note: To use examples in this directory, you need to have BLE enabled in configuration and Bluedroid selected as the host stack.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest mentioning how to enable BLE and select Bluedroid host. Maybe list the parameters in menuconfig

# Example Layout

The examples are grouped into subdirectories by category. Each category directory contains one or more example projects:
* `ble_ancs` Demonstrates how to use the ESP-IDF BLE ANCS (Apple Notification Center Service) on ESP32 series chips to access notifications generated on iOS devices via a Bluetooth low-energy link, and includes instructions for setting the correct chip target, authorizing access to ANCS characteristics, and building, flashing, and monitoring the project. See the [ble_ancs](./ble_ancs/README.md) about example.
Copy link
Collaborator

Choose a reason for hiding this comment

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

See the README for more details about this example

* `ble_ancs` Demonstrates how to use the ESP-IDF BLE ANCS (Apple Notification Center Service) on ESP32 series chips to access notifications generated on iOS devices via a Bluetooth low-energy link, and includes instructions for setting the correct chip target, authorizing access to ANCS characteristics, and building, flashing, and monitoring the project. See the [ble_ancs](./ble_ancs/README.md) about example.
* `ble_compatibility_test` Demonstrates how to use the ESP32 series chips to perform Eddystone-compatible BLE scanning of eddystone frames, including UID and URL, aimed at improving proximity-based experiences on both Android and iOS platforms. See the [ble_compatibility_test](./ble_compatibility_test/README.md) about example.
* `ble_eddystone` Demonstrates how to use the ESP32 series chips to perform Eddystone-compatible BLE scanning of eddystone frames, including UID and URL, aimed at improving proximity-based experiences on both Android and iOS platforms. See the [ble_eddystone](./ble_eddystone/README.md) about example.
* `ble_hid_device_demo` Demonstrates how to implement a BLE HID device profile with four different reports (Mouse, Keyboard and LED, Consumer Devices, Vendor devices) on ESP32 series chips, allowing users to set the correct chip target and configure the project according to their application scenarios. See the [ble_hid_device_demo](./ble_hid_device_demo/README.md) about example.
Copy link
Collaborator

Choose a reason for hiding this comment

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

with four different reports? is this word used correctly?

# Example Layout

The examples are grouped into subdirectories by category. Each category directory contains one or more example projects:
* `ble_ancs` Demonstrates how to use the ESP-IDF BLE ANCS (Apple Notification Center Service) on ESP32 series chips to access notifications generated on iOS devices via a Bluetooth low-energy link, and includes instructions for setting the correct chip target, authorizing access to ANCS characteristics, and building, flashing, and monitoring the project. See the [ble_ancs](./ble_ancs/README.md) about example.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest not mentioning ESP32 series chips in this readme. It may be confusing that these examples could be supported on ESP32 chips only.


/**
* @brief BLE SRK keys
* @brief bluetooth low energy signature resolving keys
Copy link
Collaborator

Choose a reason for hiding this comment

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

Signature Resolving Key (SRK)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please check the term in the spec. Some of them are capitalized. Include the abbreviation.

uint32_t counter; /*!< The counter value. */
uint16_t div; /*!< The diversifier value. */
uint8_t sec_level; /*!< The security level of the security link. */
esp_bt_octet16_t csrk; /*!< The connection signature resolving key value. */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Connection Signature Resolving Key (CSRK)



/**
* @brief union type of the security key value
* the union to the bluetooth low energy key value type
Copy link
Collaborator

Choose a reason for hiding this comment

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

? I am not able to understand this explanation


/**
* @brief structure type of the ble local oob data value
* @brief structure type of the bluetooth low energy local oob data value
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out Of Band (OOB)

/**
* @brief union associated with ble security
esp_bd_addr_t bd_addr; /*!< Basic rate address peer device. */
bool key_present; /*!< Valid link key value in key element */
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the boolean type, please indicate which behavior corresponds to true and which corresponds to false.
For example:
True if xxxxx; false otherwise

@esp-zbw esp-zbw force-pushed the doc/optimize_doc_for_ble branch 2 times, most recently from cdaa7e5 to 1fbb50d Compare July 25, 2024 07:24
@esp-zbw esp-zbw force-pushed the doc/optimize_doc_for_ble branch 3 times, most recently from ef57320 to 7e7d245 Compare July 26, 2024 10:31
@esp-zbw esp-zbw force-pushed the doc/optimize_doc_for_ble branch 3 times, most recently from fe8e31c to 1e9465e Compare August 15, 2024 09:29
@esp-zbw esp-zbw force-pushed the doc/optimize_doc_for_ble branch 6 times, most recently from 3660b2c to 1ad1d7b Compare August 21, 2024 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Opened Issue is new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants