Keep the BLE workflow connection alive across VM restarts

devices/ble_hci/common-hal/_bleio/__init__.c

@@ -38,6 +38,13 @@ bool vm_used_ble;
 // }
 
 void common_hal_bleio_init(void) {
+    // Called on every import of _bleio. This is the only initialization HCI gets:
+    // unlike other ports, nothing runs at board startup, because HCI is unusable
+    // until user code supplies an adapter (UART, etc.) anyway.
+    // Create a UUID object for all CCCD's.
+    cccd_uuid.base.type = &bleio_uuid_type;
+    common_hal_bleio_uuid_construct(&cccd_uuid, BLE_UUID_CCCD, NULL);
+    bleio_hci_reset();
 }
 
 void bleio_user_reset(void) {
@@ -47,10 +54,6 @@ void bleio_user_reset(void) {
 
 // Turn off BLE on a reset or reload.
 void bleio_reset(void) {
-    // Create a UUID object for all CCCD's.
-    cccd_uuid.base.type = &bleio_uuid_type;
-    common_hal_bleio_uuid_construct(&cccd_uuid, BLE_UUID_CCCD, NULL);
-
     bleio_hci_reset();
 
     if (!common_hal_bleio_adapter_get_enabled(&common_hal_bleio_adapter_obj)) {

main.c

@@ -121,12 +121,6 @@ uint8_t value_out = 0;
 #include "supervisor/shared/settings.h"
 #endif
 
-static void reset_devices(void) {
-    #if CIRCUITPY_BLEIO_HCI
-    bleio_reset();
-    #endif
-}
-
 static uint8_t *_heap;
 static uint8_t *_pystack;
 static volatile bool _vm_is_running = false;
@@ -374,9 +368,6 @@ static void cleanup_after_vm(mp_obj_t exception) {
         }
     }
 
-    // Reset port-independent devices, like CIRCUITPY_BLEIO_HCI.
-    reset_devices();
-
     #if CIRCUITPY_ATEXIT
     atexit_reset();
     #endif
@@ -390,7 +381,7 @@ static void cleanup_after_vm(mp_obj_t exception) {
     memorymonitor_reset();
     #endif
 
-    // Disable user related BLE state that uses the micropython heap.
+    // Disable user related BLE state that uses the VM heap. Leave BLE workflow running if it's in use.
     #if CIRCUITPY_BLEIO
     bleio_user_reset();
     #endif
@@ -775,8 +766,10 @@ static bool __attribute__((noinline)) run_code_py(safe_mode_t safe_mode, bool *s
 
     // Done waiting, start the board back up.
 
-    // We delay resetting BLE until after the wait in case we're transferring
-    // more files over.
+    // Resetting BLE is delayed until here, after the wait, in case files were being
+    // transferred over the BLE workflow during the wait. The reset restarts the BLE
+    // stack, dropping the workflow connection, only if user code created GATT
+    // services; otherwise it is a no-op and the connection continues.
     #if CIRCUITPY_BLEIO
     bleio_reset();
     #endif
@@ -992,8 +985,9 @@ static int run_repl(safe_mode_t safe_mode) {
     #endif
     cleanup_after_vm(MP_OBJ_SENTINEL);
 
-    // Also reset bleio. The above call omits it in case workflows should continue. In this case,
-    // we're switching straight to another VM so we want to reset.
+    // Also reset bleio, which cleanup_after_vm() above omits so workflows can
+    // continue between VMs. As in run_code_py(), this restarts the BLE stack only if
+    // user code created GATT services.
     #if CIRCUITPY_BLEIO
     bleio_reset();
     #endif
@@ -1085,8 +1079,6 @@ int __attribute__((used)) main(void) {
 
     // Reset everything and prep MicroPython to run boot.py.
     reset_port();
-    // Port-independent devices, like CIRCUITPY_BLEIO_HCI.
-    reset_devices();
     reset_board();
 
     // displays init after filesystem, since they could share the flash SPI

ports/espressif/common-hal/_bleio/Adapter.c

@@ -309,7 +309,7 @@ static int _mtu_reply(uint16_t conn_handle,
     return 0;
 }
 
-static void _new_connection(uint16_t conn_handle) {
+static void _new_connection(uint16_t conn_handle, bool user_owned) {
     // Set the tx_power for the connection higher than the advertisement.
     esp_ble_tx_power_set(conn_handle, ESP_PWR_LVL_N0);
 
@@ -332,6 +332,7 @@ static void _new_connection(uint16_t conn_handle) {
     connection->connection_obj = mp_const_none;
     connection->pair_status = PAIR_NOT_PAIRED;
     connection->mtu = 0;
+    connection->user_owned = user_owned;
 
     ble_gattc_exchange_mtu(conn_handle, _mtu_reply, connection);
 
@@ -349,7 +350,8 @@ static int _connect_event(struct ble_gap_event *event, void *self_in) {
         case BLE_GAP_EVENT_CONNECT:
             if (event->connect.status == 0) {
                 // This triggers an MTU exchange. Its reply will exit the loop waiting for a connection.
-                _new_connection(event->connect.conn_handle);
+                // Only user code connects in the central role.
+                _new_connection(event->connect.conn_handle, true);
                 // Set connections objs back to NULL since we have a new
                 // connection and need a new tuple.
                 self->connection_objs = NULL;
@@ -468,7 +470,8 @@ static int _advertising_event(struct ble_gap_event *event, void *self_in) {
 
             #if !MYNEWT_VAL(BLE_EXT_ADV)
             if (event->connect.status == NIMBLE_OK) {
-                _new_connection(event->connect.conn_handle);
+                // The connection belongs to whoever started the advertising it answered.
+                _new_connection(event->connect.conn_handle, self->user_advertising);
                 // Set connections objs back to NULL since we have a new
                 // connection and need a new tuple.
                 self->connection_objs = NULL;
@@ -481,7 +484,8 @@ static int _advertising_event(struct ble_gap_event *event, void *self_in) {
         case BLE_GAP_EVENT_ADV_COMPLETE:
             #if MYNEWT_VAL(BLE_EXT_ADV)
             if (event->adv_complete.reason == NIMBLE_OK) {
-                _new_connection(event->adv_complete.conn_handle);
+                // The connection belongs to whoever started the advertising it answered.
+                _new_connection(event->adv_complete.conn_handle, self->user_advertising);
                 // Set connections objs back to NULL since we have a new
                 // connection and need a new tuple.
                 self->connection_objs = NULL;

ports/espressif/common-hal/_bleio/Connection.h

@@ -31,6 +31,11 @@ typedef enum {
 typedef struct {
     uint16_t conn_handle;
     bool is_central;
+    // True if user code initiated or accepted this connection: it connected in the
+    // central role, or a central answered user code's advertising. User-owned
+    // connections are disconnected when the VM resets; the BLE workflow connection
+    // is not user-owned and stays up.
+    bool user_owned;
     // Remote services discovered when this peripheral is acting as a client.
     mp_obj_list_t *remote_service_list;
     // The advertising data and scan response buffers are held by us, not by the SD, so we must

ports/espressif/common-hal/_bleio/__init__.c

@@ -24,6 +24,8 @@
 #include "common-hal/_bleio/__init__.h"
 #include "common-hal/_bleio/ble_events.h"
 
+#include "services/gatt/ble_svc_gatt.h"
+
 #include "nvs_flash.h"
 
 static volatile int _completion_status;
@@ -35,10 +37,29 @@ void bleio_user_reset(void) {
     if (!common_hal_bleio_adapter_get_enabled(&common_hal_bleio_adapter_obj)) {
         return;
     }
-    // Stop any user scanning or advertising, and stop all connections.
-    // TODO: Don't stop BLE workflow connection.
-    bleio_adapter_reset(&common_hal_bleio_adapter_obj);
+    // Stop any user scanning or advertising.
+    common_hal_bleio_adapter_stop_scan(&common_hal_bleio_adapter_obj);
+    common_hal_bleio_adapter_stop_advertising(&common_hal_bleio_adapter_obj);
+
+    // Disconnect the connections that user code initiated or accepted with its own
+    // advertising. Keep the BLE workflow connection if present.
+    //
+    // Remove each connection's pointers into the VM heap before disconnecting:
+    // the heap is about to go away, and a disconnect completes asynchronously.
+    for (size_t i = 0; i < BLEIO_TOTAL_CONNECTION_COUNT; i++) {
+        bleio_connection_internal_t *connection = &bleio_connections[i];
+        connection->connection_obj = mp_const_none;
+        connection->remote_service_list = NULL;
+        if (connection->conn_handle != BLEIO_HANDLE_INVALID && connection->user_owned) {
+            common_hal_bleio_connection_disconnect(connection);
+        }
+    }
+
+    // Now clear the adapter's remaining heap pointer, since it will be stale
+    // when the VM stops.
+    common_hal_bleio_adapter_obj.connection_objs = NULL;
 
+    // Also clean up event handlers that are on the heap.
     ble_event_remove_heap_handlers();
 
     // Maybe start advertising the BLE workflow.
@@ -52,11 +73,31 @@ void bleio_reset(void) {
         return;
     }
 
+    // The stop/start cycle below clears user-created services from the GATT
+    // table, and drops every connection, BLE workflow included. So run it
+    // only when user code created services. All other user BLE state has already
+    // been torn down individually by bleio_user_reset().
+    //
+    // TODO: ESP-IDF NimBLE can delete individual services (ble_gatts_delete_svc(),
+    // used in Service.c), so bleio_user_reset() could delete user services one by
+    // one and never restart the BLE stack at all. For now this port matches
+    // nordic, whose SoftDevice can only clear services with a full cycle.
+    if (!bleio_get_user_services_created()) {
+        return;
+    }
+    bleio_clear_user_services_created();
+
     supervisor_stop_bluetooth();
     ble_event_reset();
     bleio_adapter_reset(&common_hal_bleio_adapter_obj);
     common_hal_bleio_adapter_set_enabled(&common_hal_bleio_adapter_obj, false);
     supervisor_start_bluetooth();
+
+    // The stop/start above rebuilt the GATT table, so now any bonded peers' cached
+    // tables are stale. Signal Service Changed over the whole handle range.
+    // Without Service Changed, a bonded host trusts its cache indefinitely and can
+    // look up characteristics at stale handles.
+    ble_svc_gatt_changed(0x0001, 0xffff);
 }
 
 // The singleton _bleio.Adapter object, bound to _bleio.adapter

ports/nordic/boards/pca10100/mpconfigboard.mk

@@ -10,3 +10,4 @@ INTERNAL_FLASH_FILESYSTEM = 1
 CIRCUITPY_ONEWIREIO = 0
 CIRCUITPY_AUDIOMIXER = 0
 CIRCUITPY_RAINBOWIO = 0
+CIRCUITPY_USB_MIDI = 0

ports/nordic/common-hal/_bleio/Adapter.c

@@ -244,6 +244,10 @@ static bool adapter_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
             connection->connection_obj = mp_const_none;
             connection->pair_status = PAIR_NOT_PAIRED;
             connection->mtu = 0;
+            // Only user code connects in the central role, and a peripheral
+            // connection belongs to whoever started the advertising it answered.
+            connection->user_owned = connected->role == BLE_GAP_ROLE_CENTRAL ||
+                self->advertising_started_by_user;
             // Clear leftover bond state; connection slots are recycled. The
             // SoftDevice fills in only the keys the new peer distributes, so a
             // stale keyset could mix the previous peer's keys into this peer's
@@ -356,6 +360,9 @@ void common_hal_bleio_adapter_set_enabled(bleio_adapter_obj_t *self, bool enable
             ble_drv_remove_event_handler(connection_on_ble_evt, connection);
             connection->conn_handle = BLE_CONN_HANDLE_INVALID;
         }
+        // The SoftDevice's GATT table is empty after an enable.
+        bleio_gatts_min_handle = 0xFFFF;
+        bleio_gatts_max_handle = 0;
         self->background_callback.fun = bluetooth_adapter_background;
         self->background_callback.data = self;
         bleio_adapter_reset_name(self);
@@ -716,6 +723,9 @@ uint32_t _common_hal_bleio_adapter_start_advertising(bleio_adapter_obj_t *self,
     if (self->current_advertising_data != NULL && self->current_advertising_data == self->advertising_data) {
         return NRF_ERROR_BUSY;
     }
+    // The supervisor calls this internal function directly; user code arrives via
+    // common_hal_bleio_adapter_start_advertising(), which overrides this to true.
+    self->advertising_started_by_user = false;
 
     // If the current advertising data isn't owned by the adapter then it must be an internal
     // advertisement that we should stop.
@@ -896,6 +906,7 @@ void common_hal_bleio_adapter_start_advertising(bleio_adapter_obj_t *self, bool
         tx_power,
         directed_to));
     self->user_advertising = true;
+    self->advertising_started_by_user = true;
 }
 
 void common_hal_bleio_adapter_stop_advertising(bleio_adapter_obj_t *self) {

ports/nordic/common-hal/_bleio/Adapter.h

@@ -39,6 +39,11 @@ typedef struct {
     ble_drv_evt_handler_entry_t advertising_handler_entry;
     background_callback_t background_callback;
     bool user_advertising;
+    // Whether the current (or most recent) advertising was started by user code
+    // rather than the supervisor. Unlike user_advertising, this is not cleared when
+    // advertising stops, so the connected-event handler can still read it: the
+    // advertising event handler runs first and has already stopped the advertising.
+    bool advertising_started_by_user;
     // Cached local address returned by common_hal_bleio_adapter_get_address().
     // Stored inline so it never needs to allocate, even when read before the
     // heap is available (e.g. from bleio_adapter_reset_name).

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

@@ -240,6 +240,8 @@ void common_hal_bleio_characteristic_add_descriptor(bleio_characteristic_obj_t *
     };
 
     check_nrf_error(sd_ble_gatts_descriptor_add(self->handle, &desc_attr, &descriptor->handle));
+    bleio_gatts_min_handle = MIN(bleio_gatts_min_handle, descriptor->handle);
+    bleio_gatts_max_handle = MAX(bleio_gatts_max_handle, descriptor->handle);
 
     mp_obj_list_append(MP_OBJ_FROM_PTR(self->descriptor_list),
         MP_OBJ_FROM_PTR(descriptor));

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

@@ -289,15 +289,23 @@ bool connection_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
                 // mode >=1 and/or level >=1 means encryption is set up
                 self->pair_status = PAIR_NOT_PAIRED;
             } else {
-                if (bonding_load_cccd_info(self->is_central, self->conn_handle, self->ediv)) {
-                    // Did an sd_ble_gatts_sys_attr_set() with the stored sys_attr values.
-                    // Indicate ATTR table change because we may have reloaded since the peer last
-                    // connected.
-                    sd_ble_gatts_service_changed(self->conn_handle, 0xC, 0xFFFF);
-                } else {
-                    // No matching bonding found, so use fresh system attributes.
+                if (!bonding_load_cccd_info(self->is_central, self->conn_handle, self->ediv)) {
+                    // No stored system attributes (CCCD values) found, so use fresh ones.
                     sd_ble_gatts_sys_attr_set(self->conn_handle, NULL, 0, 0);
                 }
+                if (self->ediv != EDIV_INVALID && bleio_gatts_min_handle <= bleio_gatts_max_handle) {
+                    // The peer reconnected using a stored bond, and the GATT table may
+                    // have changed since it last connected: a reload can add or remove
+                    // user services. Indicate Service Changed so the peer discards its
+                    // cached table. This must not be conditional on the sys-attr load
+                    // above succeeding: a peer that missed the indication would trust a
+                    // stale cache indefinitely, and saved system attributes are the
+                    // less reliable of the two records. The error is ignored if the
+                    // peer hasn't subscribed to indications yet. Both handles must be
+                    // within the application-populated range: the SoftDevice rejects
+                    // 0xFFFF, and also 0xC, which is inside its own GATT service.
+                    (void)sd_ble_gatts_service_changed(self->conn_handle, bleio_gatts_min_handle, bleio_gatts_max_handle);
+                }
                 self->pair_status = PAIR_PAIRED;
             }
             break;

ports/nordic/common-hal/_bleio/Connection.h

@@ -31,6 +31,11 @@ typedef enum {
 typedef struct {
     uint16_t conn_handle;
     bool is_central;
+    // True if user code initiated or accepted this connection: it connected in the
+    // central role, or a central answered user code's advertising. User-owned
+    // connections are disconnected when the VM resets; the BLE workflow connection
+    // is not user-owned and stays up.
+    bool user_owned;
     // Remote services discovered when this peripheral is acting as a client.
     mp_obj_list_t *remote_service_list;
     // The advertising data and scan response buffers are held by us, not by the SD, so we must

ports/nordic/common-hal/_bleio/Service.c

@@ -46,6 +46,8 @@ uint32_t _common_hal_bleio_service_construct(bleio_service_obj_t *self, bleio_uu
     uint32_t result = sd_ble_gatts_service_add(service_type, &nordic_uuid, &self->handle);
     // Do a service changed indication to all connected peers.
     if (result == NRF_SUCCESS) {
+        bleio_gatts_min_handle = MIN(bleio_gatts_min_handle, self->handle);
+        bleio_gatts_max_handle = MAX(bleio_gatts_max_handle, self->handle);
         _indicate_service_change(self->handle, self->handle);
     }
 
@@ -169,6 +171,8 @@ void common_hal_bleio_service_add_characteristic(bleio_service_obj_t *self,
     _expand_range(char_handles.cccd_handle, &start, &end);
     _expand_range(char_handles.sccd_handle, &start, &end);
     _expand_range(char_handles.user_desc_handle, &start, &end);
+    bleio_gatts_min_handle = MIN(bleio_gatts_min_handle, start);
+    bleio_gatts_max_handle = MAX(bleio_gatts_max_handle, end);
     _indicate_service_change(start, end);
 
     #if CIRCUITPY_VERBOSE_BLE