fix CCCD bonding store; avoid excessive bonding writes

Author: dhalbert

ports/nrf/common-hal/_bleio/Characteristic.c

@@ -83,29 +83,6 @@ STATIC void characteristic_gatts_notify_indicate(uint16_t handle, uint16_t conn_
     }
 }
 
-STATIC bool characteristic_on_ble_evt(ble_evt_t *ble_evt, void *param) {
-    bleio_characteristic_obj_t *self = (bleio_characteristic_obj_t *) param;
-    switch (ble_evt->header.evt_id) {
-        case BLE_GATTS_EVT_WRITE: {
-            // A client wrote to this server characteristic.
-            // If we are bonded, stored the CCCD value.
-            if (self->service != MP_OBJ_NULL) {
-                bleio_connection_obj_t *connection = self->service->connection;
-                uint16_t conn_handle = bleio_connection_get_conn_handle(connection);
-                if (conn_handle != BLE_CONN_HANDLE_INVALID &&
-                    common_hal_bleio_connection_get_paired(connection) &&
-                    ble_evt->evt.gatts_evt.params.write.handle == self->cccd_handle) {
-                    bonding_save_cccd_info(
-                        connection->connection->is_central, conn_handle, connection->connection->ediv);
-                }
-            }
-            break;
-        }
-    }
-
-    return true;
-}
-
 void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self, bleio_service_obj_t *service, uint16_t handle, bleio_uuid_obj_t *uuid, bleio_characteristic_properties_t props, bleio_attribute_security_mode_t read_perm, bleio_attribute_security_mode_t write_perm, mp_int_t max_length, bool fixed_length, mp_buffer_info_t *initial_value_bufinfo) {
     self->service = service;
     self->uuid = uuid;
@@ -132,9 +109,6 @@ void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self,
     if (initial_value_bufinfo != NULL) {
         common_hal_bleio_characteristic_set_value(self, initial_value_bufinfo);
     }
-
-    self->handler_entry.next = NULL;
-////////////////    ble_drv_add_event_handler_entry(&self->handler_entry, characteristic_on_ble_evt, self);
 }
 
 bleio_descriptor_obj_t *common_hal_bleio_characteristic_get_descriptor_list(bleio_characteristic_obj_t *self) {
@@ -288,8 +262,3 @@ void common_hal_bleio_characteristic_set_cccd(bleio_characteristic_obj_t *self,
     }
 
 }
-
-void common_hal_bleio_characteristic_del(bleio_characteristic_obj_t *self) {
-    // Remove from event handler list, since the evt handler entry is built-in and not a heap object.
-    ble_drv_remove_event_handler(characteristic_on_ble_evt, self);
-}

ports/nrf/common-hal/_bleio/Characteristic.h

@@ -47,7 +47,6 @@ typedef struct _bleio_characteristic_obj {
     bleio_attribute_security_mode_t read_perm;
     bleio_attribute_security_mode_t write_perm;
     bleio_descriptor_obj_t *descriptor_list;
-    ble_drv_evt_handler_entry_t handler_entry;
     uint16_t user_desc_handle;
     uint16_t cccd_handle;
     uint16_t sccd_handle;

ports/nrf/common-hal/_bleio/Connection.c

@@ -123,6 +123,17 @@ bool connection_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
             break;
         }
 
+        case BLE_GATTS_EVT_WRITE:
+            // A client wrote a value.
+            // If we are bonded and it's a CCCD (UUID 0x2902), store the CCCD value.
+            if (self->conn_handle != BLE_CONN_HANDLE_INVALID &&
+                self->pair_status == PAIR_PAIRED &&
+                ble_evt->evt.gatts_evt.params.write.uuid.type == BLE_UUID_TYPE_BLE &&
+                ble_evt->evt.gatts_evt.params.write.uuid.uuid == 0x2902) {
+                bonding_save_cccd_info(self->is_central, self->conn_handle, self->ediv);
+            }
+            break;
+
         case BLE_GATTS_EVT_SYS_ATTR_MISSING:
             sd_ble_gatts_sys_attr_set(self->conn_handle, NULL, 0, 0);
             break;
@@ -223,7 +234,7 @@ bool connection_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
 
         case BLE_GAP_EVT_AUTH_STATUS: { // 0x19
             CONNECTION_DEBUG_PRINTF("BLE_GAP_EVT_AUTH_STATUS\n");
-            // Pairing process completed
+            // Key exchange completed.
             ble_gap_evt_auth_status_t* status = &ble_evt->evt.gap_evt.params.auth_status;
             self->sec_status = status->auth_status;
             if (status->auth_status == BLE_GAP_SEC_STATUS_SUCCESS) {
@@ -264,8 +275,10 @@ bool connection_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
         }
 
         case BLE_GAP_EVT_CONN_SEC_UPDATE: { // 0x1a
-            CONNECTION_DEBUG_PRINTF("BLE_GAP_EVT_CONN_SEC_UPDATE\n");
+            // We get this both on first-time pairing and on subsequent pairings using stored keys.
             ble_gap_conn_sec_t* conn_sec = &ble_evt->evt.gap_evt.params.conn_sec_update.conn_sec;
+            CONNECTION_DEBUG_PRINTF("BLE_GAP_EVT_CONN_SEC_UPDATE, sm: %d, lv: %d\n",
+                                    conn_sec->sec_mode.sm, conn_sec->sec_mode.lv);
             if (conn_sec->sec_mode.sm <= 1 && conn_sec->sec_mode.lv <= 1) {
                 // Security setup did not succeed:
                 // mode 0, level 0 means no access
@@ -282,6 +295,7 @@ bool connection_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
                     CONNECTION_DEBUG_PRINTF("bonding_load_cccd_info() failed\n");
                     sd_ble_gatts_sys_attr_set(self->conn_handle, NULL, 0, 0);
                 }
+                self->pair_status = PAIR_PAIRED;
             }
             break;
         }