Merge pull request #2356 from tannewt/central_pairing

Add connection interval and debugging

12 files changed, 163 insertions, 23 deletions

diff --git a/.gitmodules b/.gitmodules
index bb62a5c39..764456cd6 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -76,7 +76,8 @@
 [submodule "lib/tinyusb"]
 	path = lib/tinyusb
 	url = https://github.com/hathach/tinyusb.git
-	branch = develop
+	branch = master
+	fetchRecurseSubmodules = false
 [submodule "tools/huffman"]
 	path = tools/huffman
 	url = https://github.com/tannewt/huffman.git
diff --git a/ports/nrf/bluetooth/ble_drv.c b/ports/nrf/bluetooth/ble_drv.c
index 16475e4b3..896fa0fb0 100644
--- a/ports/nrf/bluetooth/ble_drv.c
+++ b/ports/nrf/bluetooth/ble_drv.c
@@ -134,6 +134,9 @@ void SD_EVT_IRQHandler(void) {
         }
 
         ble_evt_t* event = (ble_evt_t *)m_ble_evt_buf;
+        #if CIRCUITPY_VERBOSE_BLE
+        mp_printf(&mp_plat_print, "BLE event: 0x%04x\n", event->header.evt_id);
+        #endif
 
         if (supervisor_bluetooth_hook(event)) {
             continue;
@@ -145,8 +148,15 @@ void SD_EVT_IRQHandler(void) {
             done = it->func(event, it->param) || done;
             it = it->next;
         }
+        #if CIRCUITPY_VERBOSE_BLE
+        if (event->header.evt_id == BLE_GATTS_EVT_WRITE) {
+            ble_gatts_evt_write_t* write_evt = &event->evt.gatts_evt.params.write;
+            mp_printf(&mp_plat_print, "Write to: UUID(0x%04x) handle %x of length %d auth %x\n", write_evt->uuid.uuid, write_evt->handle, write_evt->len, write_evt->auth_required);
+        }
         if (!done) {
-            //mp_printf(&mp_plat_print, "Unhandled ble event: 0x%04x\n", event->header.evt_id);
+            mp_printf(&mp_plat_print, "Unhandled ble event: 0x%04x\n", event->header.evt_id);
+
         }
+        #endif
     }
 }
diff --git a/ports/nrf/common-hal/_bleio/Adapter.c b/ports/nrf/common-hal/_bleio/Adapter.c
index 85c159c6d..00541c679 100644
--- a/ports/nrf/common-hal/_bleio/Adapter.c
+++ b/ports/nrf/common-hal/_bleio/Adapter.c
@@ -180,9 +180,18 @@ STATIC bool adapter_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
             connection->conn_handle = ble_evt->evt.gap_evt.conn_handle;
             connection->connection_obj = mp_const_none;
             connection->pair_status = PAIR_NOT_PAIRED;
+
             ble_drv_add_event_handler_entry(&connection->handler_entry, connection_on_ble_evt, connection);
             self->connection_objs = NULL;
 
+            // Save the current connection parameters.
+            memcpy(&connection->conn_params, &connected->conn_params, sizeof(ble_gap_conn_params_t));
+
+            #if CIRCUITPY_VERBOSE_BLE
+            ble_gap_conn_params_t *cp = &connected->conn_params;
+            mp_printf(&mp_plat_print, "conn params: min_ci %d max_ci %d s_l %d sup_timeout %d\n", cp->min_conn_interval, cp->max_conn_interval, cp->slave_latency, cp->conn_sup_timeout);
+            #endif
+
             // See if connection interval set by Central is out of range.
             // If so, negotiate our preferred range.
             ble_gap_conn_params_t conn_params;
diff --git a/ports/nrf/common-hal/_bleio/Connection.c b/ports/nrf/common-hal/_bleio/Connection.c
index 0e1e187c2..0720d5c47 100644
--- a/ports/nrf/common-hal/_bleio/Connection.c
+++ b/ports/nrf/common-hal/_bleio/Connection.c
@@ -70,8 +70,6 @@ static volatile bool m_discovery_successful;
 static bleio_service_obj_t *m_char_discovery_service;
 static bleio_characteristic_obj_t *m_desc_discovery_characteristic;
 
-bool dump_events = false;
-
 bool connection_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
     bleio_connection_internal_t *self = (bleio_connection_internal_t*)self_in;
 
@@ -84,16 +82,9 @@ bool connection_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
         return false;
     }
 
-    // For debugging.
-    if (dump_events) {
-        mp_printf(&mp_plat_print, "Connection event: 0x%04x\n", ble_evt->header.evt_id);
-    }
-
     switch (ble_evt->header.evt_id) {
         case BLE_GAP_EVT_DISCONNECTED:
             break;
-        case BLE_GAP_EVT_CONN_PARAM_UPDATE: // 0x12
-            break;
         case BLE_GAP_EVT_PHY_UPDATE_REQUEST: {
             ble_gap_phys_t const phys = {
                 .rx_phys = BLE_GAP_PHY_AUTO,
@@ -124,15 +115,61 @@ bool connection_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
             sd_ble_gatts_sys_attr_set(self->conn_handle, NULL, 0, 0);
             break;
 
-        case BLE_GATTS_EVT_HVN_TX_COMPLETE: // Capture this for now. 0x55
+        #if CIRCUITPY_VERBOSE_BLE
+        // Use read authorization to snoop on all reads when doing verbose debugging.
+        case BLE_GATTS_EVT_RW_AUTHORIZE_REQUEST: {
+
+            ble_gatts_evt_rw_authorize_request_t *request =
+                    &ble_evt->evt.gatts_evt.params.authorize_request;
+
+            mp_printf(&mp_plat_print, "Read %x offset %d ", request->request.read.handle, request->request.read.offset);
+            uint8_t value_bytes[22];
+            ble_gatts_value_t value;
+            value.offset = request->request.read.offset;
+            value.len = 22;
+            value.p_value = value_bytes;
+
+            sd_ble_gatts_value_get(self->conn_handle, request->request.read.handle, &value);
+            size_t len = value.len;
+            if (len > 22) {
+                len = 22;
+            }
+            for (uint8_t i = 0; i < len; i++) {
+                mp_printf(&mp_plat_print, " %02x", value_bytes[i]);
+            }
+            mp_printf(&mp_plat_print, "\n");
+            ble_gatts_rw_authorize_reply_params_t reply;
+            reply.type = request->type;
+            reply.params.read.gatt_status = BLE_GATT_STATUS_SUCCESS;
+            reply.params.read.update = false;
+            reply.params.read.offset = request->request.read.offset;
+            sd_ble_gatts_rw_authorize_reply(self->conn_handle, &reply);
             break;
+        }
+        #endif
 
+        case BLE_GATTS_EVT_HVN_TX_COMPLETE: // Capture this for now. 0x55
+            break;
         case BLE_GAP_EVT_CONN_PARAM_UPDATE_REQUEST: {
+            self->conn_params_updating = true;
             ble_gap_evt_conn_param_update_request_t *request =
                     &ble_evt->evt.gap_evt.params.conn_param_update_request;
             sd_ble_gap_conn_param_update(self->conn_handle, &request->conn_params);
             break;
         }
+        case BLE_GAP_EVT_CONN_PARAM_UPDATE: { // 0x12
+            ble_gap_evt_conn_param_update_t *result =
+                    &ble_evt->evt.gap_evt.params.conn_param_update;
+
+            #if CIRCUITPY_VERBOSE_BLE
+            ble_gap_conn_params_t *cp = &ble_evt->evt.gap_evt.params.conn_param_update.conn_params;
+            mp_printf(&mp_plat_print, "conn params updated: min_ci %d max_ci %d s_l %d sup_timeout %d\n", cp->min_conn_interval, cp->max_conn_interval, cp->slave_latency, cp->conn_sup_timeout);
+            #endif
+
+            memcpy(&self->conn_params, &result->conn_params, sizeof(ble_gap_conn_params_t));
+            self->conn_params_updating = false;
+            break;
+        }
         case BLE_GAP_EVT_SEC_PARAMS_REQUEST: {
             ble_gap_sec_keyset_t keyset = {
                 .keys_own = {
@@ -212,9 +249,9 @@ bool connection_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
 
         default:
             // For debugging.
-            if (dump_events) {
+            #if CIRCUITPY_VERBOSE_BLE
                 mp_printf(&mp_plat_print, "Unhandled connection event: 0x%04x\n", ble_evt->header.evt_id);
-            }
+            #endif
 
             return false;
     }
@@ -262,6 +299,25 @@ void common_hal_bleio_connection_pair(bleio_connection_internal_t *self, bool bo
     check_sec_status(self->sec_status);
 }
 
+mp_float_t common_hal_bleio_connection_get_connection_interval(bleio_connection_internal_t *self) {
+    while (self->conn_params_updating && !mp_hal_is_interrupted()) {
+        RUN_BACKGROUND_TASKS;
+    }
+    return 1.25f * self->conn_params.min_conn_interval;
+}
+
+void common_hal_bleio_connection_set_connection_interval(bleio_connection_internal_t *self, mp_float_t new_interval) {
+    self->conn_params_updating = true;
+    uint16_t interval = new_interval / 1.25f;
+    self->conn_params.min_conn_interval = interval;
+    self->conn_params.max_conn_interval = interval;
+    uint32_t status = NRF_ERROR_BUSY;
+    while (status == NRF_ERROR_BUSY) {
+        status = sd_ble_gap_conn_param_update(self->conn_handle, &self->conn_params);
+        RUN_BACKGROUND_TASKS;
+    }
+    check_nrf_error(status);
+}
 
 // service_uuid may be NULL, to discover all services.
 STATIC bool discover_next_services(bleio_connection_internal_t* connection, uint16_t start_handle, ble_uuid_t *service_uuid) {
@@ -600,6 +656,7 @@ STATIC void discover_remote_services(bleio_connection_internal_t *self, mp_obj_t
     ble_drv_remove_event_handler(discovery_on_ble_evt, self);
 
 }
+
 mp_obj_tuple_t *common_hal_bleio_connection_discover_remote_services(bleio_connection_obj_t *self, mp_obj_t service_uuids_whitelist) {
     discover_remote_services(self->connection, service_uuids_whitelist);
     // Convert to a tuple and then clear the list so the callee will take ownership.
@@ -609,7 +666,6 @@ mp_obj_tuple_t *common_hal_bleio_connection_discover_remote_services(bleio_conne
     return services_tuple;
 }
 
-
 uint16_t bleio_connection_get_conn_handle(bleio_connection_obj_t *self) {
     if (self == NULL || self->connection == NULL) {
         return BLE_CONN_HANDLE_INVALID;
diff --git a/ports/nrf/common-hal/_bleio/Connection.h b/ports/nrf/common-hal/_bleio/Connection.h
index d5548de45..1474a0a6c 100644
--- a/ports/nrf/common-hal/_bleio/Connection.h
+++ b/ports/nrf/common-hal/_bleio/Connection.h
@@ -63,6 +63,8 @@ typedef struct {
     uint8_t sec_status; // Internal security status.
     mp_obj_t connection_obj;
     ble_drv_evt_handler_entry_t handler_entry;
+    ble_gap_conn_params_t conn_params;
+    volatile bool conn_params_updating;
 } bleio_connection_internal_t;
 
 typedef struct {
diff --git a/ports/nrf/common-hal/_bleio/Service.c b/ports/nrf/common-hal/_bleio/Service.c
index 5918327c1..19288f747 100644
--- a/ports/nrf/common-hal/_bleio/Service.c
+++ b/ports/nrf/common-hal/_bleio/Service.c
@@ -119,6 +119,10 @@ void common_hal_bleio_service_add_characteristic(bleio_service_obj_t *self,
 
     bleio_attribute_gatts_set_security_mode(&char_attr_md.read_perm, characteristic->read_perm);
     bleio_attribute_gatts_set_security_mode(&char_attr_md.write_perm, characteristic->write_perm);
+    #if CIRCUITPY_VERBOSE_BLE
+    // Turn on read authorization so that we receive an event to print on every read.
+    char_attr_md.rd_auth = true;
+    #endif
 
     ble_gatts_attr_t char_attr = {
         .p_uuid = &char_uuid,
@@ -137,6 +141,9 @@ void common_hal_bleio_service_add_characteristic(bleio_service_obj_t *self,
     characteristic->cccd_handle = char_handles.cccd_handle;
     characteristic->sccd_handle = char_handles.sccd_handle;
     characteristic->handle = char_handles.value_handle;
+    #if CIRCUITPY_VERBOSE_BLE
+    mp_printf(&mp_plat_print, "Char handle %x user %x cccd %x sccd %x\n", characteristic->handle, characteristic->user_desc_handle, characteristic->cccd_handle, characteristic->sccd_handle);
+    #endif
 
     mp_obj_list_append(self->characteristic_list, MP_OBJ_FROM_PTR(characteristic));
 }
diff --git a/ports/nrf/common-hal/_bleio/__init__.c b/ports/nrf/common-hal/_bleio/__init__.c
index 7ba3dc8c1..4b2780ded 100644
--- a/ports/nrf/common-hal/_bleio/__init__.c
+++ b/ports/nrf/common-hal/_bleio/__init__.c
@@ -187,7 +187,11 @@ size_t common_hal_bleio_gattc_read(uint16_t handle, uint16_t conn_handle, uint8_
     read_info.done = false;
     ble_drv_add_event_handler(_on_gattc_read_rsp_evt, &read_info);
 
-    check_nrf_error(sd_ble_gattc_read(conn_handle, handle, 0));
+    uint32_t nrf_error = NRF_ERROR_BUSY;
+    while (nrf_error == NRF_ERROR_BUSY) {
+        nrf_error = sd_ble_gattc_read(conn_handle, handle, 0);
+    }
+    check_nrf_error(nrf_error);
 
     while (!read_info.done) {
         RUN_BACKGROUND_TASKS;
diff --git a/py/circuitpy_mpconfig.h b/py/circuitpy_mpconfig.h
index 3e83d0c51..000674c4f 100644
--- a/py/circuitpy_mpconfig.h
+++ b/py/circuitpy_mpconfig.h
@@ -666,4 +666,6 @@ void supervisor_run_background_tasks_if_tick(void);
 #define CIRCUITPY_FILESYSTEM_FLUSH_INTERVAL_MS 1000
 #define CIRCUITPY_BOOT_OUTPUT_FILE "/boot_out.txt"
 
+#define CIRCUITPY_VERBOSE_BLE 0
+
 #endif  // __INCLUDED_MPCONFIG_CIRCUITPY_H
diff --git a/shared-bindings/_bleio/Connection.c b/shared-bindings/_bleio/Connection.c
index e1e458c22..709f6c254 100644
--- a/shared-bindings/_bleio/Connection.c
+++ b/shared-bindings/_bleio/Connection.c
@@ -193,6 +193,46 @@ const mp_obj_property_t bleio_connection_paired_obj = {
                (mp_obj_t)&mp_const_none_obj },
 };
 
+
+//|   .. attribute:: connection_interval
+//|
+//|     Time between transmissions in milliseconds. Will be multiple of 1.25ms. Lower numbers
+//|     increase speed and decrease latency but increase power consumption.
+//|
+//|     When setting connection_interval, the peer may reject the new interval and
+//|     `connection_interval` will then remain the same.
+//|
+//|     Apple has additional guidelines that dictate should be a multiple of 15ms except if HID is
+//|     available. When HID is available Apple devices may accept 11.25ms intervals.
+//|
+//|
+STATIC mp_obj_t bleio_connection_get_connection_interval(mp_obj_t self_in) {
+    bleio_connection_obj_t *self = MP_OBJ_TO_PTR(self_in);
+
+    ensure_connected(self);
+    return mp_obj_new_float(common_hal_bleio_connection_get_connection_interval(self->connection));
+}
+STATIC MP_DEFINE_CONST_FUN_OBJ_1(bleio_connection_get_connection_interval_obj, bleio_connection_get_connection_interval);
+
+STATIC mp_obj_t bleio_connection_set_connection_interval(mp_obj_t self_in, mp_obj_t interval_in) {
+    bleio_connection_obj_t *self = MP_OBJ_TO_PTR(self_in);
+
+    mp_float_t interval = mp_obj_get_float(interval_in);
+
+    ensure_connected(self);
+    common_hal_bleio_connection_set_connection_interval(self->connection, interval);
+
+    return mp_const_none;
+}
+STATIC MP_DEFINE_CONST_FUN_OBJ_2(bleio_connection_set_connection_interval_obj, bleio_connection_set_connection_interval);
+
+const mp_obj_property_t bleio_connection_connection_interval_obj = {
+    .base.type = &mp_type_property,
+    .proxy = { (mp_obj_t)&bleio_connection_get_connection_interval_obj,
+               (mp_obj_t)&bleio_connection_set_connection_interval_obj,
+               (mp_obj_t)&mp_const_none_obj },
+};
+
 STATIC const mp_rom_map_elem_t bleio_connection_locals_dict_table[] = {
     // Methods
     { MP_ROM_QSTR(MP_QSTR_pair),                     MP_ROM_PTR(&bleio_connection_pair_obj) },
@@ -200,8 +240,10 @@ STATIC const mp_rom_map_elem_t bleio_connection_locals_dict_table[] = {
     { MP_ROM_QSTR(MP_QSTR_discover_remote_services), MP_ROM_PTR(&bleio_connection_discover_remote_services_obj) },
 
     // Properties
-    { MP_ROM_QSTR(MP_QSTR_connected),       MP_ROM_PTR(&bleio_connection_connected_obj) },
-    { MP_ROM_QSTR(MP_QSTR_paired),          MP_ROM_PTR(&bleio_connection_paired_obj) },
+    { MP_ROM_QSTR(MP_QSTR_connected),           MP_ROM_PTR(&bleio_connection_connected_obj) },
+    { MP_ROM_QSTR(MP_QSTR_paired),              MP_ROM_PTR(&bleio_connection_paired_obj) },
+    { MP_ROM_QSTR(MP_QSTR_connection_interval), MP_ROM_PTR(&bleio_connection_connection_interval_obj) },
+
 };
 
 STATIC MP_DEFINE_CONST_DICT(bleio_connection_locals_dict, bleio_connection_locals_dict_table);
diff --git a/shared-bindings/_bleio/Connection.h b/shared-bindings/_bleio/Connection.h
index f7eee180f..b0e26da5c 100644
--- a/shared-bindings/_bleio/Connection.h
+++ b/shared-bindings/_bleio/Connection.h
@@ -40,4 +40,7 @@ extern bool common_hal_bleio_connection_get_connected(bleio_connection_obj_t *se
 extern bool common_hal_bleio_connection_get_paired(bleio_connection_obj_t *self);
 extern mp_obj_tuple_t *common_hal_bleio_connection_discover_remote_services(bleio_connection_obj_t *self, mp_obj_t service_uuids_whitelist);
 
+mp_float_t common_hal_bleio_connection_get_connection_interval(bleio_connection_internal_t *self);
+void common_hal_bleio_connection_set_connection_interval(bleio_connection_internal_t *self, mp_float_t new_interval);
+
 #endif // MICROPY_INCLUDED_SHARED_BINDINGS_BLEIO_CONNECTION_H
diff --git a/shared-bindings/displayio/OnDiskBitmap.c b/shared-bindings/displayio/OnDiskBitmap.c
index bf00ef855..f7e2bf693 100644
--- a/shared-bindings/displayio/OnDiskBitmap.c
+++ b/shared-bindings/displayio/OnDiskBitmap.c
@@ -59,7 +59,7 @@
 //|
 //|   with open("/sample.bmp", "rb") as f:
 //|       odb = displayio.OnDiskBitmap(f)
-//|       face = displayio.TileGrid(odb, pixel_shader=displayio.ColorConverter(), position=(0,0))
+//|       face = displayio.TileGrid(odb, pixel_shader=displayio.ColorConverter())
 //|       splash.append(face)
 //|       # Wait for the image to load.
 //|       board.DISPLAY.wait_for_frame()
diff --git a/supervisor/shared/bluetooth.c b/supervisor/shared/bluetooth.c
index 02258de74..18caa2eb4 100644
--- a/supervisor/shared/bluetooth.c
+++ b/supervisor/shared/bluetooth.c
@@ -51,8 +51,12 @@ bleio_characteristic_obj_t supervisor_ble_length_characteristic;
 bleio_uuid_obj_t supervisor_ble_length_uuid;
 bleio_characteristic_obj_t supervisor_ble_contents_characteristic;
 bleio_uuid_obj_t supervisor_ble_contents_uuid;
+
+// This is the base UUID for CircuitPython services and characteristics.
 const uint8_t circuitpython_base_uuid[16] = {0x6e, 0x68, 0x74, 0x79, 0x50, 0x74, 0x69, 0x75, 0x63, 0x72, 0x69, 0x43, 0x00, 0x00, 0xaf, 0xad };
-uint8_t circuitpython_advertising_data[] = { 0x02, 0x01, 0x06, 0x02, 0x0a, 0x00, 0x11, 0x07, 0x6e, 0x68, 0x74, 0x79, 0x50, 0x74, 0x69, 0x75, 0x63, 0x72, 0x69, 0x43, 0x00, 0x01, 0xaf, 0xad, 0x06, 0x08, 0x43, 0x49, 0x52, 0x43, 0x55 }; 
+// This standard advertisement advertises the CircuitPython editing service and a CIRCUITPY short name.
+uint8_t circuitpython_advertising_data[] = { 0x02, 0x01, 0x06, 0x02, 0x0a, 0x00, 0x11, 0x07, 0x6e, 0x68, 0x74, 0x79, 0x50, 0x74, 0x69, 0x75, 0x63, 0x72, 0x69, 0x43, 0x00, 0x01, 0xaf, 0xad, 0x06, 0x08, 0x43, 0x49, 0x52, 0x43, 0x55 };
+// This scan response advertises the full CIRCUITPYXXXX device name.
 uint8_t circuitpython_scan_response_data[15] = {0x0e, 0x09, 0x43, 0x49, 0x52, 0x43, 0x55, 0x49, 0x54, 0x50, 0x59, 0x00, 0x00, 0x00, 0x00};
 mp_obj_list_t service_list;
 mp_obj_t service_list_items[1];
@@ -86,7 +90,7 @@ void supervisor_start_bluetooth(void) {
     characteristic_list.len = 0;
     characteristic_list.items = characteristic_list_items;
     mp_seq_clear(characteristic_list.items, 0, characteristic_list.alloc, sizeof(*characteristic_list.items));
-    
+
     _common_hal_bleio_service_construct(&supervisor_ble_service, &supervisor_ble_service_uuid, false /* is secondary */, &characteristic_list);
 
     // File length
@@ -225,7 +229,7 @@ void supervisor_bluetooth_background(void) {
     uint16_t current_length = ((uint16_t*) current_command)[0];
     if (current_length > 0 && current_length == current_offset) {
         uint16_t command = ((uint16_t *) current_command)[1];
-        
+
         if (command == 1) {
             uint16_t max_len = 20; //supervisor_ble_contents_characteristic.max_length;
             uint8_t buf[max_len];
@@ -274,7 +278,7 @@ void supervisor_bluetooth_background(void) {
                     f_write(&active_file, &data, 1, &actual);
                 }
             }
-            
+
             f_lseek(&active_file, offset);
             uint8_t* data = (uint8_t *) (current_command + 4);
             UINT written;