Skip to content

fix(ble): Fix inconsistency in write callback order#12266

Merged
me-no-dev merged 2 commits intoespressif:masterfrom
lucasssvaz:ble/cb_order
Jan 21, 2026
Merged

fix(ble): Fix inconsistency in write callback order#12266
me-no-dev merged 2 commits intoespressif:masterfrom
lucasssvaz:ble/cb_order

Conversation

@lucasssvaz
Copy link
Member

Description of Change

This pull request refactors how the BLE characteristic write callback (onWrite) is invoked for both Bluedroid and NimBLE BLE stacks. The main improvement is to ensure that the onWrite callback is called before the write response is sent to the client, guaranteeing that the write has been processed when the client receives acknowledgment. This change also removes the deferred callback mechanism used for NimBLE, simplifying the codebase and aligning behavior between the two stacks.

Callback invocation timing improvements:

  • The onWrite callback in BLECharacteristic::handleGATTServerEvent is now invoked before sending the write response for both Bluedroid and NimBLE, ensuring the write is processed before acknowledgment is sent to the client. [1] [2] [3]

Codebase simplification and cleanup:

  • Removed the deferred write callback mechanism for NimBLE, including the processDeferredWriteCallback function and related struct definitions, since the callback is now invoked synchronously. [1] [2] [3]

Behavior alignment:

  • Ensured consistent callback timing between Bluedroid and NimBLE by removing the workaround that previously deferred the callback for NimBLE to mimic Bluedroid's behavior.

These changes improve reliability for BLE clients and simplify maintenance by making callback handling more predictable and consistent across BLE stacks.

Test Scenarios

Tested locally with NimBLE and Bluedroid

@lucasssvaz lucasssvaz self-assigned this Jan 20, 2026
@lucasssvaz lucasssvaz requested a review from SuGlider as a code owner January 20, 2026 21:42
@lucasssvaz lucasssvaz added the Area: BLE Issues related to BLE label Jan 20, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Jan 20, 2026

Warnings
⚠️

Some issues found for the commit messages in this PR:

  • the commit message "Update libraries/BLE/src/BLECharacteristic.cpp":
    • summary looks empty
    • type/action looks empty
  • the commit message "fix(BLE): Fix inconsistency in write callback order":
    • scope/component should be lowercase without whitespace, allowed special characters are _ / . , * - .

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 10 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).

👋 Hello lucasssvaz, 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.
- 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.

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 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.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against 15b1705

@github-actions
Copy link
Contributor

github-actions bot commented Jan 20, 2026

Test Results

 90 files   90 suites   31m 7s ⏱️
 67 tests  67 ✅ 0 💤 0 ❌
683 runs  683 ✅ 0 💤 0 ❌

Results for commit 15b1705.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

Memory usage test (comparing PR against master branch)

The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.

MemoryFLASH [bytes]FLASH [%]RAM [bytes]RAM [%]
TargetDECINCDECINCDECINCDECINC
ESP32💚 -1600.000.00000.000.00
ESP32C3💚 -2400💚 -0.040.00000.000.00
ESP32C5000.000.00000.000.00
ESP32C6000.000.00000.000.00
ESP32H2💚 -2400💚 -0.030.00000.000.00
ESP32P4💚 -2420💚 -0.030.00000.000.00
ESP32S3💚 -1920💚 -0.030.00000.000.00
Click to expand the detailed deltas report [usage change in BYTES]
TargetESP32ESP32C3ESP32C5ESP32C6ESP32H2ESP32P4ESP32S3
ExampleFLASHRAMFLASHRAMFLASHRAMFLASHRAMFLASHRAMFLASHRAMFLASHRAM
libraries/BLE/examples/Beacon_Scanner💚 -120💚 -2300----💚 -2380💚 -2300💚 -1920
libraries/BLE/examples/Client💚 -120💚 -2300----💚 -2380💚 -2300💚 -1640
libraries/BLE/examples/Client_Server💚 -160💚 -2380----💚 -2380💚 -2380--
libraries/BLE/examples/Client_multiconnect💚 -120💚 -2300----💚 -2300💚 -2380--
libraries/BLE/examples/Client_secure_static_passkey💚 -120💚 -2300----💚 -2300💚 -2380--
libraries/BLE/examples/EddystoneTLM_Beacon💚 -120💚 -2400----💚 -2320💚 -2320--
libraries/BLE/examples/EddystoneURL_Beacon💚 -120💚 -2300----💚 -2400💚 -2380--
libraries/BLE/examples/Notify💚 -120💚 -2380----💚 -2300💚 -2320--
libraries/BLE/examples/Scan💚 -160💚 -2380----💚 -2300💚 -2300--
libraries/BLE/examples/Server💚 -120💚 -2380----💚 -2300💚 -2320--
libraries/BLE/examples/Server_multiconnect💚 -120💚 -2320----💚 -2380💚 -2380--
libraries/BLE/examples/Server_secure_static_passkey💚 -120💚 -2400----💚 -2300💚 -2380--
libraries/BLE/examples/UART💚 -120💚 -2300----💚 -2400💚 -2380--
libraries/BLE/examples/Write💚 -120💚 -2320----💚 -2380💚 -2420--
libraries/BLE/examples/iBeacon💚 -120💚 -2300----💚 -2380💚 -2380--
libraries/BLE/examples/Server_secure_authorization--💚 -2300----💚 -2300----

@lucasssvaz lucasssvaz changed the title fix(BLE): Fix inconsistency in write callback order fix(ble): Fix inconsistency in write callback order Jan 21, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request refactors the BLE characteristic write callback timing for both Bluedroid and NimBLE stacks to ensure the onWrite callback is invoked before the write response is sent to the client. The PR also includes a bug fix for parameter access in the ESP_GATTS_EXEC_WRITE_EVT handler and removes the deferred callback mechanism that was used for NimBLE.

Changes:

  • Modified callback timing to invoke onWrite before sending write responses for both Bluedroid and NimBLE stacks
  • Fixed incorrect parameter access in ESP_GATTS_EXEC_WRITE_EVT handler (changed from param->write.* to param->exec_write.*)
  • Removed the deferred write callback mechanism for NimBLE, including the processDeferredWriteCallback function and DeferredWriteCallback struct

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
libraries/BLE/src/BLECharacteristic.h Removed DeferredWriteCallback struct and processDeferredWriteCallback method declaration
libraries/BLE/src/BLECharacteristic.cpp Changed callback timing for both stacks to invoke before response; fixed exec_write parameter access bug; removed deferred callback implementation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@lucasssvaz lucasssvaz added the Status: Pending Merge Pull Request is ready to be merged label Jan 21, 2026
@me-no-dev me-no-dev merged commit 69bc38f into espressif:master Jan 21, 2026
89 of 91 checks passed
@lucasssvaz lucasssvaz deleted the ble/cb_order branch January 21, 2026 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: BLE Issues related to BLE Status: Pending Merge Pull Request is ready to be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants