fix CCCD bonding store; avoid excessive bonding writes

8 files changed, 117 insertions, 114 deletions

diff --git a/ports/nrf/common-hal/_bleio/Characteristic.c b/ports/nrf/common-hal/_bleio/Characteristic.c
index a87cd6e18..4d5cab5c5 100644
--- a/ports/nrf/common-hal/_bleio/Characteristic.c
+++ b/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);
-}
diff --git a/ports/nrf/common-hal/_bleio/Characteristic.h b/ports/nrf/common-hal/_bleio/Characteristic.h
index 5759321aa..bb8f28495 100644
--- a/ports/nrf/common-hal/_bleio/Characteristic.h
+++ b/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;
diff --git a/ports/nrf/common-hal/_bleio/Connection.c b/ports/nrf/common-hal/_bleio/Connection.c
index 664805b66..b9a9cf2c4 100644
--- a/ports/nrf/common-hal/_bleio/Connection.c
+++ b/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;
         }
diff --git a/ports/nrf/common-hal/_bleio/bonding.c b/ports/nrf/common-hal/_bleio/bonding.c
index 0476d722b..56f77af85 100644
--- a/ports/nrf/common-hal/_bleio/bonding.c
+++ b/ports/nrf/common-hal/_bleio/bonding.c
@@ -33,6 +33,7 @@
 #include "shared-bindings/_bleio/__init__.h"
 #include "shared-bindings/_bleio/Adapter.h"
 #include "shared-bindings/nvm/ByteArray.h"
+#include "supervisor/shared/tick.h"
 
 #include "nrf_soc.h"
 #include "sd_mutex.h"
@@ -56,8 +57,8 @@ const uint32_t BONDING_FLAG = ('1' | '0' << 8 | 'D' << 16 | 'B' << 24);
 // Save both system and user service info.
 #define SYS_ATTR_FLAGS      (BLE_GATTS_SYS_ATTR_FLAG_SYS_SRVCS | BLE_GATTS_SYS_ATTR_FLAG_USR_SRVCS)
 
-STATIC bonding_block_t *bonding_unused_block = NULL;
-nrf_mutex_t queued_bonding_block_entries_mutex;
+STATIC nrf_mutex_t queued_bonding_block_entries_mutex;
+STATIC uint64_t block_queued_at_ticks_ms = 0;
 
 #if BONDING_DEBUG
 void bonding_print_block(bonding_block_t *block) {
@@ -79,6 +80,7 @@ STATIC size_t compute_block_size(uint16_t data_length) {
 }
 
 void bonding_erase_storage(void) {
+    BONDING_DEBUG_PRINTF("bonding_erase_storage()\n");
     // Erase all pages in the bonding area.
     for(uint32_t page_address = BONDING_PAGES_START_ADDR;
         page_address < BONDING_PAGES_END_ADDR;
@@ -90,8 +92,6 @@ void bonding_erase_storage(void) {
     uint32_t flag = BONDING_FLAG;
     sd_flash_write_sync((uint32_t *) BONDING_START_FLAG_ADDR, &flag, 1);
     sd_flash_write_sync((uint32_t *) BONDING_END_FLAG_ADDR, &flag, 1);
-    // First unused block is at the beginning.
-    bonding_unused_block = (bonding_block_t *) BONDING_DATA_START_ADDR;
 }
 
 // Given NULL to start or block address, return the address of the next valid block.
@@ -122,18 +122,16 @@ STATIC bonding_block_t *next_block(bonding_block_t *block) {
     }
 }
 
-// Find the block with given type and ediv value.
+// Find the block with given is_central, type and ediv value.
 // If type == BLOCK_UNUSED, ediv is ignored and the the sole unused block at the end is returned.
 // If not found, return NULL.
-STATIC bonding_block_t *find_block_with_keys(bool is_central, bonding_block_type_t type, uint16_t ediv) {
+STATIC bonding_block_t *find_candidate_block(bool is_central, bonding_block_type_t type, uint16_t ediv) {
     bonding_block_t *block = NULL;
-    BONDING_DEBUG_PRINTF("find_block_with_keys(): looking through blocks:\n");
     while (1) {
         block = next_block(block);
         if (block == NULL) {
             return NULL;
         }
-        BONDING_DEBUG_PRINT_BLOCK(block);
         if (block->type == BLOCK_INVALID) {
             // Skip discarded blocks.
             continue;
@@ -149,11 +147,22 @@ STATIC bonding_block_t *find_block_with_keys(bool is_central, bonding_block_type
     }
 }
 
+// Get an empty block large enough to store data_length data.
+STATIC bonding_block_t* find_unused_block(uint16_t data_length) {
+    bonding_block_t *unused_block = find_candidate_block(true, BLOCK_UNUSED, EDIV_INVALID);
+    // If no more room, erase all existing blocks and start over.
+    if (!unused_block ||
+        (uint8_t *) unused_block + compute_block_size(data_length) >= (uint8_t *) BONDING_DATA_END_ADDR) {
+        bonding_erase_storage();
+        unused_block = (bonding_block_t *) BONDING_DATA_START_ADDR;
+    }
+    return unused_block;
+}
+
 // Set the header word to all 0's, to mark the block as invalid.
 // We don't change data_length, so we can still skip over this block.
 STATIC void invalidate_block(bonding_block_t *block) {
     BONDING_DEBUG_PRINTF("invalidate_block()\n");
-    BONDING_DEBUG_PRINT_BLOCK(block);
     uint32_t zero = 0;
     sd_flash_write_sync((uint32_t *) block, &zero, 1);
 }
@@ -164,6 +173,10 @@ STATIC void queue_write_block(bool is_central, bonding_block_type_t type, uint16
         return;
     }
 
+    // No heap available, so never mind. This might be called between VM instantiations.
+    if (!gc_alloc_possible()) {
+        return;
+    }
     queued_bonding_block_entry_t* queued_entry =
         m_malloc_maybe(sizeof(queued_bonding_block_entry_t) + data_length, false);
 
@@ -181,31 +194,35 @@ STATIC void queue_write_block(bool is_central, bonding_block_type_t type, uint16
         memcpy(&queued_entry->block.data, data, data_length);
     }
 
+    // Note: blocks are added in LIFO order, for simplicity and speed.
+    // The assumption is that there won't be stale blocks on the
+    // list. The sys_attr blocks don't contain sys_attr data, just a
+    // request to store the latest value. The key blocks are assumed
+    // not to be superseded quickly.  If this assumption becomes
+    // invalid, the queue should be changed to FIFO.
+
     // Add this new element to the front of the list.
     sd_mutex_acquire_wait(&queued_bonding_block_entries_mutex);
     queued_entry->next = MP_STATE_VM(queued_bonding_block_entries);
     MP_STATE_VM(queued_bonding_block_entries) = queued_entry;
     sd_mutex_release(&queued_bonding_block_entries_mutex);
+
+    // Remember when we last queued a block, so we avoid excesive
+    // sys_attr writes.
+    block_queued_at_ticks_ms = supervisor_ticks_ms64();
 }
 
 // Write bonding block header.
-STATIC void write_block_header(bonding_block_t *block) {
-    // If no more room, erase all existing blocks and start over.
-    if (bonding_unused_block == NULL ||
-        (uint8_t *) bonding_unused_block + compute_block_size(block->data_length) >=
-        (uint8_t *)BONDING_DATA_END_ADDR) {
-        bonding_erase_storage();
-    }
-
-    sd_flash_write_sync((uint32_t *) bonding_unused_block, (uint32_t *) block, sizeof(bonding_block_t) / 4);
+STATIC void write_block_header(bonding_block_t *dest_block, bonding_block_t *source_block_header) {
+    sd_flash_write_sync((uint32_t *) dest_block, (uint32_t *) source_block_header, sizeof(bonding_block_t) / 4);
 }
 
 // Write variable-length data at end of bonding block.
-STATIC void write_block_data(uint8_t *data, uint16_t data_length) {
+STATIC void write_block_data(bonding_block_t *dest_block, uint8_t *data, uint16_t data_length) {
     // Minimize the number of writes. Datasheet says no more than two writes per word before erasing again.
 
     // Start writing after the current header.
-    uint32_t *flash_word_p = (uint32_t *) ((uint8_t *) bonding_unused_block + sizeof(bonding_block_t));
+    uint32_t *flash_word_p = (uint32_t *) ((uint8_t *) dest_block + sizeof(bonding_block_t));
     while (1) {
         uint32_t word = 0xffffffff;
         memcpy(&word, data, data_length >= 4 ? 4 : data_length);
@@ -218,43 +235,68 @@ STATIC void write_block_data(uint8_t *data, uint16_t data_length) {
         // Increment by word size.
         flash_word_p++;
     }
-    bonding_unused_block = (bonding_block_t *) flash_word_p;
 }
 
-STATIC bool write_sys_attr_block(bonding_block_t *block) {
-    BONDING_DEBUG_PRINTF("write_sys_attr_block()\n");
+STATIC void write_sys_attr_block(bonding_block_t *block) {
     uint16_t length = 0;
     // First find out how big a buffer we need, then fetch the data.
     if(sd_ble_gatts_sys_attr_get(block->conn_handle, NULL, &length, SYS_ATTR_FLAGS) != NRF_SUCCESS) {
-        return false;
+        return;
     }
 
     uint8_t sys_attr[length];
     if(sd_ble_gatts_sys_attr_get(block->conn_handle, sys_attr, &length, SYS_ATTR_FLAGS) != NRF_SUCCESS) {
-        return false;
+        return;
     }
 
     // Now we know the data size.
     block->data_length = length;
-    write_block_header(block);
-    write_block_data(sys_attr, length);
-    return true;
+
+    // Is there an existing sys_attr block that matches the current sys_attr data?
+    bonding_block_t *candidate_block = find_candidate_block(block->is_central, block->type, block->ediv);
+    if (candidate_block) {
+        if (length == candidate_block->data_length &&
+            memcmp(sys_attr, candidate_block->data, block->data_length) == 0) {
+            BONDING_DEBUG_PRINTF("Identical sys_attr block already stored.\n");
+            // Identical block found. No need to store again.
+            return;
+        }
+        // Data doesn't match. Invalidate block and store a new one.
+        invalidate_block(candidate_block);
+    }
+
+    bonding_block_t *new_block = find_unused_block(length);
+    write_block_header(new_block, block);
+    write_block_data(new_block, sys_attr, length);
+    return;
 }
 
-STATIC bool write_keys_block(bonding_block_t *block) {
-    BONDING_DEBUG_PRINTF("write_keys_block()\n");
+STATIC void write_keys_block(bonding_block_t *block) {
     if (block->data_length != sizeof(bonding_keys_t)) {
-        return false;
+        // Bad length.
+        return;
+    }
+
+    // Is there an existing keys block that matches?
+    bonding_block_t *candidate_block = find_candidate_block(block->is_central, block->type, block->ediv);
+    if (candidate_block) {
+        if (block->data_length == candidate_block->data_length &&
+            memcmp(block->data, candidate_block->data, block->data_length) == 0) {
+            BONDING_DEBUG_PRINTF("Identical keys block already stored.\n");
+            // Identical block found. No need to store again.
+            return;
+        }
+        // Data doesn't match. Invalidate block and store a new one.
+        invalidate_block(candidate_block);
     }
 
     bonding_keys_t *bonding_keys = (bonding_keys_t *) block->data;
-    BONDING_DEBUG_PRINT_KEYS(bonding_keys);
     block->ediv = block->is_central
         ? bonding_keys->peer_enc.master_id.ediv
         : bonding_keys->own_enc.master_id.ediv;
-    write_block_header(block);
-    write_block_data((uint8_t *) bonding_keys, sizeof(bonding_keys_t));
-    return true;
+    bonding_block_t *new_block = find_unused_block(sizeof(bonding_keys_t));
+    write_block_header(new_block, block);
+    write_block_data(new_block, (uint8_t *) bonding_keys, sizeof(bonding_keys_t));
 }
 
 
@@ -269,8 +311,6 @@ void bonding_reset(void) {
     if (BONDING_FLAG != *((uint32_t *) BONDING_START_FLAG_ADDR) ||
         BONDING_FLAG != *((uint32_t *) BONDING_END_FLAG_ADDR)) {
         bonding_erase_storage();
-    } else {
-        bonding_unused_block = find_block_with_keys(true, BLOCK_UNUSED, EDIV_INVALID);
     }
 }
 
@@ -282,6 +322,19 @@ void bonding_background(void) {
     if (!sd_en) {
         return;
     }
+
+    if (block_queued_at_ticks_ms == 0) {
+        // No writes have been queued yet.
+        return;
+    }
+
+    // Wait at least one second before writing a block, to consolidate writes
+    // that will be duplicates.
+    uint64_t current_ticks_ms = supervisor_ticks_ms64();
+    if (current_ticks_ms - block_queued_at_ticks_ms < 1000) {
+        return;
+    }
+
     // Get block at front of list.
     bonding_block_t *block = NULL;
     sd_mutex_acquire_wait(&queued_bonding_block_entries_mutex);
@@ -292,27 +345,10 @@ void bonding_background(void) {
     }
     sd_mutex_release(&queued_bonding_block_entries_mutex);
     if (!block) {
+        // List is empty.
         return;
     }
 
-    // Is there an existing block whose keys match?
-    BONDING_DEBUG_PRINTF("bonding_background(): processing queued block:\n");
-    BONDING_DEBUG_PRINT_BLOCK(block);
-    bonding_block_t *block_with_keys = find_block_with_keys(block->is_central, block->type, block->ediv);
-    if (block_with_keys) {
-        BONDING_DEBUG_PRINTF("bonding_background(): block with same keys found:\n");
-        BONDING_DEBUG_PRINT_BLOCK(block_with_keys);
-        if (block->data_length == block_with_keys->data_length &&
-            memcmp(block->data, block_with_keys->data, block->data_length) == 0) {
-            // Identical block found. No need to store again.
-            BONDING_DEBUG_PRINTF("bonding_background(): block is identical to block_with_keys\n");
-            return;
-        }
-        // Block keys match but data doesn't. Invalidate block and store a new one.
-        BONDING_DEBUG_PRINTF("bonding_background(): invalidating block_with_keys\n");
-        invalidate_block(block_with_keys);
-    }
-
     switch (block->type) {
         case BLOCK_SYS_ATTR:
             write_sys_attr_block(block);
@@ -323,27 +359,23 @@ void bonding_background(void) {
             break;
 
         default:
-            BONDING_DEBUG_PRINTF("unknown block type: %x\n", block->type);
             break;
     }
 }
 
 bool bonding_load_cccd_info(bool is_central, uint16_t conn_handle, uint16_t ediv) {
-    bonding_block_t *block = find_block_with_keys(is_central, BLOCK_SYS_ATTR, ediv);
+    bonding_block_t *block = find_candidate_block(is_central, BLOCK_SYS_ATTR, ediv);
     if (block == NULL) {
-        BONDING_DEBUG_PRINTF("bonding_load_cccd_info(): block not found, ediv: %04x\n", ediv);
         return false;
     }
 
-    BONDING_DEBUG_PRINTF("bonding_load_cccd_info(): block found, ediv: %04x\n", ediv);
     return NRF_SUCCESS ==
         sd_ble_gatts_sys_attr_set(conn_handle, block->data, block->data_length, SYS_ATTR_FLAGS);
 }
 
 bool bonding_load_keys(bool is_central, uint16_t ediv, bonding_keys_t *bonding_keys) {
-    bonding_block_t *block = find_block_with_keys(is_central, BLOCK_KEYS, ediv);
+    bonding_block_t *block = find_candidate_block(is_central, BLOCK_KEYS, ediv);
     if (block == NULL) {
-        BONDING_DEBUG_PRINTF("bonding_load_keys(): block not found, ediv: %04x\n", ediv);
         return false;
     }
     if (sizeof(bonding_keys_t) != block->data_length) {
@@ -351,15 +383,12 @@ bool bonding_load_keys(bool is_central, uint16_t ediv, bonding_keys_t *bonding_k
         return false;
     }
 
-    BONDING_DEBUG_PRINTF("bonding_load_keys(): block found, ediv: %04x\n", ediv);
     memcpy(bonding_keys, block->data, block->data_length);
-    BONDING_DEBUG_PRINT_KEYS(bonding_keys);
     return true;
 }
 
 void bonding_save_cccd_info(bool is_central, uint16_t conn_handle, uint16_t ediv) {
-    BONDING_DEBUG_PRINTF("bonding_save_cccd_info: is_central: %d, conn_handle: %04x, ediv: %04x\n",
-                         is_central, conn_handle, ediv);
+    BONDING_DEBUG_PRINTF("bonding_save_cccd_info()\n");
     queue_write_block(is_central, BLOCK_SYS_ATTR, ediv, conn_handle, NULL, 0);
 }
 
@@ -367,7 +396,5 @@ void bonding_save_keys(bool is_central, uint16_t conn_handle, bonding_keys_t *bo
     uint16_t const ediv = is_central
         ? bonding_keys->peer_enc.master_id.ediv
         : bonding_keys->own_enc.master_id.ediv;
-    BONDING_DEBUG_PRINTF("bonding_save_keys: is_central: %d, conn_handle: %04x, ediv: %04x\n",
-                         is_central, conn_handle, ediv);
     queue_write_block(is_central, BLOCK_KEYS, ediv, conn_handle, (uint8_t *) bonding_keys, sizeof(bonding_keys_t));
 }
diff --git a/py/gc.c b/py/gc.c
index 5e631f5ed..fef67f61b 100755
--- a/py/gc.c
+++ b/py/gc.c
@@ -464,6 +464,10 @@ void gc_info(gc_info_t *info) {
     GC_EXIT();
 }
 
+bool gc_alloc_possible(void) {
+    return MP_STATE_MEM(gc_pool_start) != 0;
+}
+
 // We place long lived objects at the end of the heap rather than the start. This reduces
 // fragmentation by localizing the heap churn to one portion of memory (the start of the heap.)
 void *gc_alloc(size_t n_bytes, bool has_finaliser, bool long_lived) {
diff --git a/py/gc.h b/py/gc.h
index cd63ba94c..2a0811f4e 100644
--- a/py/gc.h
+++ b/py/gc.h
@@ -58,6 +58,8 @@ void gc_collect_ptr(void *ptr);
 void gc_collect_root(void **ptrs, size_t len);
 void gc_collect_end(void);
 
+// Is the gc heap available?
+bool gc_alloc_possible(void);
 void *gc_alloc(size_t n_bytes, bool has_finaliser, bool long_lived);
 
 // Use this function to sweep the whole heap and run all finalisers
diff --git a/shared-bindings/_bleio/Characteristic.c b/shared-bindings/_bleio/Characteristic.c
index 0434368f2..2b1dc1f78 100644
--- a/shared-bindings/_bleio/Characteristic.c
+++ b/shared-bindings/_bleio/Characteristic.c
@@ -128,8 +128,7 @@ STATIC mp_obj_t bleio_characteristic_add_to_service(size_t n_args, const mp_obj_
     }
     mp_get_buffer_raise(initial_value, &initial_value_bufinfo, MP_BUFFER_READ);
 
-    // There may be some cleanup needed when a characteristic is gc'd, so enable finaliser.
-    bleio_characteristic_obj_t *characteristic = m_new_obj_with_finaliser(bleio_characteristic_obj_t);
+    bleio_characteristic_obj_t *characteristic = m_new_obj(bleio_characteristic_obj_t);
     characteristic->base.type = &bleio_characteristic_type;
 
     // Range checking on max_length arg is done by the common_hal layer, because
@@ -293,17 +292,7 @@ STATIC mp_obj_t bleio_characteristic_set_cccd(mp_uint_t n_args, const mp_obj_t *
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_KW(bleio_characteristic_set_cccd_obj, 1, bleio_characteristic_set_cccd);
 
-// Cleanup on gc.
-STATIC mp_obj_t bleio_characteristic_del(mp_obj_t self_in) {
-    bleio_characteristic_obj_t *self = MP_OBJ_TO_PTR(self_in);
-    common_hal_bleio_characteristic_del(self);
-    return mp_const_none;
-}
-STATIC MP_DEFINE_CONST_FUN_OBJ_1(bleio_characteristic_del_obj, bleio_characteristic_del);
-
-
 STATIC const mp_rom_map_elem_t bleio_characteristic_locals_dict_table[] = {
-    { MP_ROM_QSTR(MP_QSTR___del__),        MP_ROM_PTR(&bleio_characteristic_del_obj) },
     { MP_ROM_QSTR(MP_QSTR_add_to_service), MP_ROM_PTR(&bleio_characteristic_add_to_service_obj) },
     { MP_ROM_QSTR(MP_QSTR_properties),     MP_ROM_PTR(&bleio_characteristic_properties_obj) },
     { MP_ROM_QSTR(MP_QSTR_uuid),           MP_ROM_PTR(&bleio_characteristic_uuid_obj) },
diff --git a/shared-bindings/_bleio/Characteristic.h b/shared-bindings/_bleio/Characteristic.h
index 60ab57572..c4356fd4b 100644
--- a/shared-bindings/_bleio/Characteristic.h
+++ b/shared-bindings/_bleio/Characteristic.h
@@ -45,6 +45,5 @@ extern bleio_descriptor_obj_t *common_hal_bleio_characteristic_get_descriptor_li
 extern bleio_service_obj_t *common_hal_bleio_characteristic_get_service(bleio_characteristic_obj_t *self);
 extern void common_hal_bleio_characteristic_add_descriptor(bleio_characteristic_obj_t *self, bleio_descriptor_obj_t *descriptor);
 extern void common_hal_bleio_characteristic_set_cccd(bleio_characteristic_obj_t *self, bool notify, bool indicate);
-extern void common_hal_bleio_characteristic_del(bleio_characteristic_obj_t *self);
 
 #endif // MICROPY_INCLUDED_SHARED_BINDINGS_BLEIO_CHARACTERISTIC_H