Merge pull request #2562 from dhalbert/ble-fixes

nrf: track vm_used_ble better

ports/nrf/bluetooth/ble_drv.c

@@ -145,6 +145,9 @@ void SD_EVT_IRQHandler(void) {
         ble_drv_evt_handler_entry_t *it = MP_STATE_VM(ble_drv_evt_handler_entries);
         bool done = false;
         while (it != NULL) {
+            #if CIRCUITPY_VERBOSE_BLE
+            // mp_printf(&mp_plat_print, "  calling handler: 0x%08lx, param: 0x%08lx\n", it->func-1, it->param);
+            #endif
             done = it->func(event, it->param) || done;
             it = it->next;
         }

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

@@ -437,6 +437,7 @@ mp_obj_t common_hal_bleio_adapter_start_scan(bleio_adapter_obj_t *self, uint8_t*
         .active = active
     };
     uint32_t err_code;
+    vm_used_ble = true;
     err_code = sd_ble_gap_scan_start(&scan_params, sd_data);
 
     if (err_code != NRF_SUCCESS) {
@@ -511,6 +512,7 @@ mp_obj_t common_hal_bleio_adapter_connect(bleio_adapter_obj_t *self, bleio_addre
     ble_drv_add_event_handler(connect_on_ble_evt, &event_info);
     event_info.done = false;
 
+    vm_used_ble = true;
     uint32_t err_code = sd_ble_gap_connect(&addr, &scan_params, &conn_params, BLE_CONN_CFG_TAG_CUSTOM);
 
     if (err_code != NRF_SUCCESS) {
@@ -615,6 +617,7 @@ uint32_t _common_hal_bleio_adapter_start_advertising(bleio_adapter_obj_t *self,
         return err_code;
     }
 
+    vm_used_ble = true;
     err_code = sd_ble_gap_adv_start(adv_handle, BLE_CONN_CFG_TAG_CUSTOM);
     if (err_code != NRF_SUCCESS) {
         return err_code;
@@ -709,6 +712,11 @@ void bleio_adapter_reset(bleio_adapter_obj_t* adapter) {
     adapter->connection_objs = NULL;
     for (size_t i = 0; i < BLEIO_TOTAL_CONNECTION_COUNT; i++) {
         bleio_connection_internal_t *connection = &bleio_connections[i];
+        // Disconnect all connections with Python state cleanly. Keep any supervisor-only connections.
+        if (connection->connection_obj != mp_const_none &&
+            connection->conn_handle != BLE_CONN_HANDLE_INVALID) {
+            common_hal_bleio_connection_disconnect(connection);
+        }
         connection->connection_obj = mp_const_none;
     }
 }

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

@@ -91,13 +91,14 @@ void check_sec_status(uint8_t sec_status) {
 void bleio_reset() {
     bleio_adapter_reset(&common_hal_bleio_adapter_obj);
     if (!vm_used_ble) {
+        // No user-code BLE operations were done, so we can maintain the supervisor state.
         return;
     }
     if (common_hal_bleio_adapter_get_enabled(&common_hal_bleio_adapter_obj)) {
         common_hal_bleio_adapter_set_enabled(&common_hal_bleio_adapter_obj, false);
     }
-    supervisor_start_bluetooth();
     bonding_reset();
+    supervisor_start_bluetooth();
 }
 
 // The singleton _bleio.Adapter object, bound to _bleio.adapter
@@ -195,14 +196,17 @@ size_t common_hal_bleio_gattc_read(uint16_t handle, uint16_t conn_handle, uint8_
     while (nrf_error == NRF_ERROR_BUSY) {
         nrf_error = sd_ble_gattc_read(conn_handle, handle, 0);
     }
+    if (nrf_error != NRF_SUCCESS) {
+        ble_drv_remove_event_handler(_on_gattc_read_rsp_evt, &read_info);
         check_nrf_error(nrf_error);
+    }
 
     while (!read_info.done) {
         RUN_BACKGROUND_TASKS;
     }
-    check_gatt_status(read_info.status);
 
     ble_drv_remove_event_handler(_on_gattc_read_rsp_evt, &read_info);
+    check_gatt_status(read_info.status);
     return read_info.final_len;
 }
 

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

@@ -267,15 +267,16 @@ void bonding_background(void) {
     for (size_t i = 0; i < BLEIO_TOTAL_CONNECTION_COUNT; i++) {
         bleio_connection_internal_t *connection = &bleio_connections[i];
 
-        uint64_t current_ticks_ms = supervisor_ticks_ms64();
         // Wait at least one second before saving CCCD, to consolidate
         // writes that involve multiple CCCDs. For instance, for HID,
         // three CCCD's are set in short succession by the HID client.
-        if (connection->do_bond_cccds &&
-            current_ticks_ms - connection->do_bond_cccds_request_time >= 1000) {
+        if (connection->do_bond_cccds) {
+            uint64_t current_ticks_ms = supervisor_ticks_ms64();
+            if (current_ticks_ms - connection->do_bond_cccds_request_time >= 1000) {
                 write_sys_attr_block(connection);
                 connection->do_bond_cccds = false;
             }
+        }
 
         if (connection->do_bond_keys) {
             write_keys_block(connection);

ports/nrf/common-hal/microcontroller/__init__.c

@@ -37,8 +37,10 @@
 #include "shared-bindings/microcontroller/Processor.h"
 
 #include "supervisor/filesystem.h"
+#include "supervisor/port.h"
 #include "supervisor/shared/safe_mode.h"
 #include "nrfx_glue.h"
+#include "nrf_nvic.h"
 
 // This routine should work even when interrupts are disabled. Used by OneWire
 // for precise timing.
@@ -46,10 +48,38 @@ void common_hal_mcu_delay_us(uint32_t delay) {
     NRFX_DELAY_US(delay);
 }
 
+static volatile uint32_t nesting_count = 0;
+static uint8_t is_nested_critical_region;
+static uint8_t sd_is_enabled = false;
 void common_hal_mcu_disable_interrupts() {
+    sd_softdevice_is_enabled(&sd_is_enabled);
+    if (sd_is_enabled) {
+        sd_nvic_critical_region_enter(&is_nested_critical_region);
+    } else {
+        __disable_irq();
+        __DMB();
+        nesting_count++;
+    }
 }
 
 void common_hal_mcu_enable_interrupts() {
+    // Don't check here if SD is enabled, because we'll crash if interrupts
+    // were turned off and sd_softdevice_is_enabled is called.
+    if (sd_is_enabled) {
+        sd_nvic_critical_region_exit(is_nested_critical_region);
+    } else {
+        if (nesting_count == 0) {
+            // This is very very bad because it means there was mismatched disable/enables so we
+            // crash.
+            reset_into_safe_mode(HARD_CRASH);
+        }
+        nesting_count--;
+        if (nesting_count > 0) {
+            return;
+        }
+        __DMB();
+        __enable_irq();
+    }
 }
 
 void common_hal_mcu_on_next_reset(mcu_runmode_t runmode) {