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

[h2zero]

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
  • Improved BLE characteristic discovery for faster, more reliable lookups.
  • Discoveries are inserted in-order to remove redundant sorting, reducing latency.
  • Characteristic queries now return results more directly, improving retry and cache behavior.

[coderabbitai]

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: 1c1eeb8d-6ecf-46d6-a9ee-b1e6581fd103

📥 Commits

Reviewing files that changed from the base of the PR and between bb4bdcc and 28e39e2.

📒 Files selected for processing (2)

• src/NimBLERemoteService.cpp
• src/NimBLERemoteService.h

🚧 Files skipped from review as they are similar to previous changes (1)

• src/NimBLERemoteService.h

---

📝 Walkthrough

Walkthrough

Arrr — The characteristic discovery flow was refactored: retrieveCharacteristics() gained an optional out-parameter to return a discovered characteristic pointer, discovery now inserts characteristics in ascending handle order during callback, and getCharacteristic() was simplified to use the new pattern and avoid post-discovery sorting.

Changes

Cohort / File(s)	Summary
Header Declaration src/NimBLERemoteService.h	Changed retrieveCharacteristics signature to accept an optional out-parameter: NimBLERemoteCharacteristic** ppChar = nullptr.
Discovery & Retrieval Logic src/NimBLERemoteService.cpp	Removed <algorithm> and the post-discovery std::sort. characteristicDiscCB() now inserts discovered characteristics in ascending def_handle order and sets the task buffer to the inserted pointer. retrieveCharacteristics() now fills *ppChar on success. getCharacteristic() returns on cache hit and calls the updated retrieval API to obtain characteristics directly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

Arrr, the handles march in tidy rows,
No sortin' after storms o' those,
A pointer handed, found at last,
Discovery swift, the chaos past. 🏴‍☠️

🚥 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 PR title accurately describes the main bugfix: correcting the return of the wrong characteristic instance after sorting.
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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/NimBLERemoteService.cpp`:
- Around line 154-161: The callback currently overwrites pTaskData->m_pBuf on
every discovered characteristic and uses the (C++17-only) return value of
emplace_back(), which causes inconsistent behavior and may not compile; fix by
only setting pTaskData->m_pBuf the first time a matching characteristic is
stored (i.e., check if m_pBuf is null before assigning) and replace the
emplace_back assignment with two steps: call pSvc->m_vChars.emplace_back(new
NimBLERemoteCharacteristic(pSvc, chr)); then set pTaskData->m_pBuf =
pSvc->m_vChars.back(); also ensure the insertion branch still assigns m_pBuf
when it was previously null (use (*pSvc->m_vChars.insert(...)) to get the
iterator result and set m_pBuf = *inserted_it if needed).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cc21dbaa-c550-4cd1-b5b2-fa71b5c29193

📥 Commits

Reviewing files that changed from the base of the PR and between cb02931 and bb4bdcc.

📒 Files selected for processing (2)

• src/NimBLERemoteService.cpp
• src/NimBLERemoteService.h