Skip to content

[Bugfix] get remote characteristic by uuid returning the wrong instance.#416

Merged
h2zero merged 1 commit intomasterfrom
bugfix/get-char
Mar 29, 2026
Merged

[Bugfix] get remote characteristic by uuid returning the wrong instance.#416
h2zero merged 1 commit intomasterfrom
bugfix/get-char

Conversation

@h2zero
Copy link
Copy Markdown
Owner

@h2zero h2zero commented Mar 29, 2026

This fixes a bug introduced when sorting the characteristic vector where the last item on in the vector was returned but the characteristc wasn't at the end anymore.

This reverts the previous sorting of the vector after retrieval and instead sorts as the characteristics are found and also provides a pointer to the characteristic from the retrieval callback.

Summary by CodeRabbit

  • Refactor
    • Optimized BLE characteristic discovery and retrieval process for improved performance and reliability.
    • Enhanced internal characteristic management to reduce latency during device connection and service discovery.

This fixes a bug introduced when sorting the characteristic vector where the last item on in the vector was
returned but the characteristc wasn't at the end anymore.

This reverts the previous sorting of the vector after retrieval and instead sorts as the characteristics are found and also
provides a pointer to the characteristic from the retrieval callback.
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 29, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f7e279dd-56f2-4b06-a989-bd9b13725692

📥 Commits

Reviewing files that changed from the base of the PR and between 67fa477 and 38e725b.

📒 Files selected for processing (2)
  • src/NimBLERemoteService.cpp
  • src/NimBLERemoteService.h

📝 Walkthrough

Walkthrough

Arr, shiver me timbers! This here PR be refactorin' the characteristic discovery mechanism, matey. 'Stead o' sortin' the characteristic vector after discovery, the code now inserts characteristics in sorted order by handle durin' discovery itself. The retrieveCharacteristics() method be extendedWithin an additional out-parameter fer accessin' discovered characteristics directly, eliminatin' the need to rely on vector size changes. Yarrr!

Changes

Cohort / File(s) Summary
Header Declaration
src/NimBLERemoteService.h
Extended retrieveCharacteristics() private method signature with optional out-parameter NimBLERemoteCharacteristic** ppChar fer returnin' discovered characteristic pointers.
Implementation Logic
src/NimBLERemoteService.cpp
Removed <algorithm> include and post-discovery sort; refactored characteristic discovery callback to insert in ascending order by def_handle; modified getCharacteristic() to use out-parameter approach; updated retrieveCharacteristics() implementation to populate out-parameter from task data upon success.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

Ye sorted vectors after the discovery quest complete,
Now insert in order, makin' the logic more neat!
An out-parameter fer the haul ye find,
No more size-checkin', that old grind!
Yarrr, the characteristics be shipshape and fine! 🏴‍☠️

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main bug fix: returning the wrong characteristic instance when querying by UUID.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bugfix/get-char

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@h2zero h2zero merged commit b11d34e into master Mar 29, 2026
66 checks passed
@h2zero h2zero deleted the bugfix/get-char branch March 29, 2026 20:21
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.

1 participant