[Bugfix] Persist in-place CCCD/CSFC updates to NVS

[r3wretrhy]

Fixes #1192.

ble_store_config_persist_cccds() only compares the NVS and RAM entry counts. When ble_store_config_write_cccd() overwrites an existing entry in place (same peer and handle, new flags), the counts stay equal and nothing is written to NVS, so the old value comes back after a reboot. persist_csfcs() has the same gap.

This adds an equal-count branch to both: it looks for the NVS entry that no longer matches any RAM entry and rewrites it with the RAM entry that has the same key (peer address + characteristic handle for CCCDs, peer address for CSFCs). If every NVS entry still matches, nothing is written, so a repeated identical write doesn't touch flash. Add and delete paths are unchanged.

Espressif's esp-nimble fixed this in espressif/esp-nimble@4e54569 (nimble-1.10.0-idf) with an equal-count update branch in every persist_*() function. That version is built on their newer shared-NVS-handle code, so this is a smaller port that fits the current file, limited to CCCDs and CSFCs: they have a simple key and are what the issue hits. The sec/IRK/RPA paths carry extra bond-count ordering upstream, so I'd rather do those in a separate PR if you want them.

Testing (no hardware):

• Host harness (not committed): builds the real ble_store_nvs.c against an in-memory NVS, uses the same find/overwrite/append logic as ble_store_config_write_cccd()/_write_csfc(), and "reboots" by clearing the RAM db and calling ble_store_config_conf_init(). With the sequence from the issue (bonding persists 0x80 placeholders for handles 29/36, host then subscribes with 0x01), master restores 29=0x80 and 36=0x80, the same as the issue shows; with this change they restore as 0x01. Also checked: unsubscribe in place (0x1 -> 0x0) survives a reboot, an identical rewrite does 0 NVS writes, appending a second peer still persists, and a CSFC in-place update survives a reboot. Built with ASan/UBSan, no reports.
• pio run with examples/NimBLE_Server for esp32dev and esp32-s3-devkitc-1 (espressif32 / Arduino, -Wall -Wextra): ble_store_nvs.c compiles with no warnings on master and with this change. With my local arduino-esp32 3.3.12 package the final link fails on mbedtls_cipher_cmac_* from ble_sm_alg.c on master as well, so I didn't get a linked image here.

Summary by CodeRabbit

• Bug Fixes
  • Fixed an issue where saved Bluetooth connection settings could become inconsistent with the device’s current settings when both contained the same number of entries. The saved values are now updated to match the current settings when a mismatch is detected, helping ensure those settings remain consistent across restarts.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 415ee2e3-a4f4-462e-9bb4-7298a29739ed

📥 Commits

Reviewing files that changed from the base of the PR and between c22a209 and ee5a412.

📒 Files selected for processing (1)

• src/nimble/nimble/host/store/config/src/ble_store_nvs.c

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

Arr, CCCD and CSFC persistence now checks for changed entries when RAM and NVS counts are equal and nonzero. It matches entries by store key and updates an unmatched NVS value from RAM.

Changes

CCCD and CSFC persistence

Layer / File(s)	Summary
Detect and persist changed entries src/nimble/nimble/host/store/config/src/ble_store_nvs.c	CCCD keys match by peer address and characteristic value handle. CSFC keys match by peer address. The NVS scan skips missing keys, returns BLE_HS_ESTORE_FAIL on read failures, and succeeds if no stale entry needs an update. Equal, nonzero CCCD and CSFC counts now invoke this synchronization. Arr.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to ee5a4

The change addresses in-place subscription persistence. No newly introduced merge-blocking issue is established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 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 change: persisting in-place CCCD and CSFC updates to NVS, arrr.
Linked Issues check	✅ Passed	Arr, #1192 requires persistence of in-place CCCD updates. The change adds equal-count update handling in ble_store_config_persist_cccds() and rewrites the unmatched NVS entry from the RAM entry with…
Out of Scope Changes check	✅ Passed	Arr, the CCCD change directly addresses #1192. The CSFC change is also connected: #1192 identifies the same count-only persistence pattern in CSFC and other store objects. The supplied change summary …

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

_{Arr, equal counts once hid the change,
Now stale NVS values can rearrange.
Peer and handle guide the search,
Fresh RAM values reach their berth.
CCCDs and CSFCs sail in sync.}

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

[h2zero]

Thanks!