Disable CCCD persistence to prevent flash corruption

This commit intentionally disables the saving of CCCD (Client Characteristic
Configuration Descriptor) values to flash storage while maintaining API
compatibility. This change addresses a critical flash corruption issue that
has been affecting BLE keyboard users.

Problem:
--------
Users have reported that their BLE keyboards fail to reconnect after sleep,
requiring re-pairing. Investigation revealed that bonding data files were
becoming corrupted, particularly during the wake-from-sleep sequence.

Root Cause:
-----------
1. When a device wakes from sleep, the BLE connection is re-established
2. The host automatically re-writes CCCD values to enable notifications
3. These writes are queued via ada_callback() to execute asynchronously
4. The wake period is unstable - connections often drop with errors 0x08
   (BLE_HCI_CONNECTION_TIMEOUT) or 0x22 (BLE_HCI_STATUS_CODE_LMP_RESPONSE_TIMEOUT)
5. When disconnection occurs during flash write, the operation is interrupted
6. The flash_cache layer silently ignores write errors, resulting in corrupted data

Why Disable CCCD Saving:
------------------------
1. UNNECESSARY: For HID devices, CCCD values are constant - notifications must
   always be enabled for keyboard functionality. Every host writes the same value.

2. DANGEROUS: The post-wake period creates a perfect storm for corruption:
   unstable connections + automatic CCCD writes + poor error handling

3. MINIMAL IMPACT: Hosts automatically re-enable notifications on connection.
   The only theoretical downside is a 1-2 second delay for non-HID services
   like Battery Service, which is negligible.

Implementation:
---------------
- bond_save_cccd() now returns true immediately without writing to flash
- bond_load_cccd() remains functional for compatibility and migration
- BLEGatt.cpp call site preserved to maintain API compatibility
- Extensive comments document the rationale and trade-offs

This eliminates a major corruption vector while having no functional impact
on keyboard operation. The change is backward compatible - existing devices
will load stored CCCDs one final time, then stop saving new values.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>

Author: obra

libraries/Bluefruit52Lib/src/BLEGatt.cpp

@@ -142,6 +142,12 @@ void BLEGatt::_eventHandler(ble_evt_t* evt)
       // Save CCCD if paired
       if ( conn->secured() && (evt_id == BLE_GATTS_EVT_WRITE) && (req_handle == chr->handles().cccd_handle) )
       {
+        /* NOTE: This call to saveCccd() is intentionally left in place even though the underlying
+         * implementation has been disabled. This preserves API compatibility and avoids the need
+         * to modify this widely-used code path. The actual bond_save_cccd() function now returns
+         * immediately without performing any flash operations. See bond_save_cccd() in bonding.cpp
+         * for detailed explanation of why CCCD saving has been disabled.
+         */
         conn->saveCccd();
       }
     }

libraries/Bluefruit52Lib/src/utility/bonding.cpp

@@ -258,12 +258,71 @@ static void bond_save_cccd_dfr (uint8_t role, uint16_t conn_hdl, ble_gap_addr_t
 
 bool bond_save_cccd (uint8_t role, uint16_t conn_hdl, ble_gap_addr_t const* id_addr)
 {
-  // queue to execute in Ada Callback thread
-  return ada_callback(id_addr, sizeof(ble_gap_addr_t), bond_save_cccd_dfr, role, conn_hdl, id_addr);
+  /* DISABLED: CCCD (Client Characteristic Configuration Descriptor) saving has been intentionally disabled
+   * to prevent flash storage corruption issues.
+   *
+   * BACKGROUND:
+   * CCCDs control whether notifications/indications are enabled for BLE characteristics. The original
+   * implementation saved these values to flash storage to persist them across connections. However,
+   * this approach has several critical issues:
+   *
+   * 1. CORRUPTION RISK: The flash write operations occur via deferred callbacks (ada_callback) which
+   *    can be interrupted by BLE disconnections. When disconnections happen during flash writes
+   *    (especially common during the unstable period after device wake), the flash storage can
+   *    become corrupted. The underlying flash_cache layer does not properly handle write errors,
+   *    leading to partial writes being silently accepted as successful.
+   *
+   * 2. UNNECESSARY FOR HID: For HID devices like keyboards, CCCD values are effectively constant.
+   *    The HID Input Report characteristics MUST have notifications enabled for the device to
+   *    function. Every host will write the same value (0x0001) during connection setup. Saving
+   *    and restoring these values provides no functional benefit.
+   *
+   * 3. WAKE-UP VULNERABILITY: The highest risk period is immediately after device wake from sleep.
+   *    During this time:
+   *    - The BLE connection is re-establishing and unstable
+   *    - The host automatically re-writes CCCD values to restore notifications
+   *    - These writes trigger flash operations during the most vulnerable period
+   *    - Disconnections are common as connection parameters are renegotiated
+   *
+   * 4. MINIMAL IMPACT: Other services that might use CCCDs (Battery Service, DFU, etc.) will
+   *    simply have their notifications re-enabled by the host on each connection. This happens
+   *    automatically and quickly, with no user-visible impact.
+   *
+   * TRADE-OFFS:
+   * - Benefit: Eliminates a major source of flash corruption
+   * - Benefit: Reduces unnecessary flash wear
+   * - Benefit: Simpler, more reliable operation
+   * - Cost: Theoretical ~1-2 second delay for non-HID notification enable (negligible in practice)
+   *
+   * FUTURE CONSIDERATIONS:
+   * If CCCD persistence becomes necessary for specific use cases, implement:
+   * 1. Proper error handling in the flash_cache layer
+   * 2. Connection stability checks before allowing writes
+   * 3. Write verification and retry logic
+   * 4. Selective saving only for non-HID services
+   */
+  
+  // Return true to indicate "success" - prevents error propagation while doing nothing
+  return true;
+  
+  // Original implementation disabled:
+  // return ada_callback(id_addr, sizeof(ble_gap_addr_t), bond_save_cccd_dfr, role, conn_hdl, id_addr);
 }
 
 bool bond_load_cccd(uint8_t role, uint16_t conn_hdl, ble_gap_addr_t const* id_addr)
 {
+  /* NOTE: While bond_save_cccd() has been disabled to prevent corruption, we keep bond_load_cccd()
+   * functional to maintain compatibility with existing stored data and to avoid breaking the
+   * API for applications that might call this function.
+   *
+   * Since we no longer save CCCD data, this function will typically return false (not loaded),
+   * which causes the BLE stack to use default values. This is the desired behavior - the host
+   * will simply re-enable any required notifications during connection setup.
+   *
+   * Keeping this function operational also provides a migration path: devices with existing
+   * stored CCCD data will load it one final time, after which no new data will be saved.
+   */
+  
   bool loaded = false;
 
   char filename[BOND_FNAME_LEN];

libraries/Bluefruit52Lib/src/utility/bonding.h

@@ -62,6 +62,8 @@ void bond_remove_key(uint8_t role, ble_gap_addr_t const* id_addr);
 bool bond_save_keys (uint8_t role, uint16_t conn_hdl, bond_keys_t const* bkeys);
 bool bond_load_keys(uint8_t role, ble_gap_addr_t* peer_addr, bond_keys_t* bkeys);
 
+// CCCD persistence functions - save is disabled to prevent flash corruption
+// See implementation in bonding.cpp for detailed explanation
 bool bond_save_cccd (uint8_t role, uint16_t conn_hdl, ble_gap_addr_t const* id_addr);
 bool bond_load_cccd (uint8_t role, uint16_t conn_hdl, ble_gap_addr_t const* id_addr);