Merge pull request #9325 from tannewt/esp_ble_follow_up

Improve ESP BLE and turn on BLE workflow

.gitmodules

@@ -143,7 +143,7 @@
 [submodule "ports/espressif/esp-idf"]
 	path = ports/espressif/esp-idf
 	url = https://github.com/adafruit/esp-idf.git
-	branch = circuitpython-v5.1.3
+	branch = circuitpython-v5.2.2
 [submodule "ports/espressif/esp-protocols"]
 	path = ports/espressif/esp-protocols
 	url = https://github.com/espressif/esp-protocols.git

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

@@ -37,6 +37,9 @@ bool vm_used_ble;
 //     }
 // }
 
+void common_hal_bleio_init(void) {
+}
+
 void bleio_user_reset() {
     // HCI doesn't support the BLE workflow so just do a full reset.
     bleio_reset();

locale/circuitpython.pot

@@ -113,7 +113,7 @@ msgstr ""
 #: ports/raspberrypi/common-hal/rp2pio/StateMachine.c
 #: ports/raspberrypi/common-hal/usb_host/Port.c
 #: shared-bindings/digitalio/DigitalInOut.c
-#: shared-bindings/microcontroller/Pin.c
+#: shared-bindings/microcontroller/Pin.c shared-module/max3421e/Max3421E.c
 msgid "%q in use"
 msgstr ""
 
@@ -1114,10 +1114,12 @@ msgstr ""
 msgid "Input/output error"
 msgstr ""
 
+#: ports/espressif/common-hal/_bleio/__init__.c
 #: ports/nordic/common-hal/_bleio/__init__.c
 msgid "Insufficient authentication"
 msgstr ""
 
+#: ports/espressif/common-hal/_bleio/__init__.c
 #: ports/nordic/common-hal/_bleio/__init__.c
 msgid "Insufficient encryption"
 msgstr ""
@@ -1154,6 +1156,7 @@ msgstr ""
 #: ports/atmel-samd/common-hal/alarm/pin/PinAlarm.c
 #: ports/atmel-samd/common-hal/countio/Counter.c
 #: ports/atmel-samd/common-hal/frequencyio/FrequencyIn.c
+#: ports/atmel-samd/common-hal/max3421e/Max3421E.c
 #: ports/atmel-samd/common-hal/ps2io/Ps2.c
 #: ports/atmel-samd/common-hal/pulseio/PulseIn.c
 #: ports/atmel-samd/common-hal/rotaryio/IncrementalEncoder.c
@@ -1285,6 +1288,10 @@ msgstr ""
 msgid "MAC address was invalid"
 msgstr ""
 
+#: ports/espressif/common-hal/_bleio/Characteristic.c
+msgid "MITM security not supported"
+msgstr ""
+
 #: shared-bindings/is31fl3741/IS31FL3741.c
 msgid "Mapping must be a tuple"
 msgstr ""
@@ -2139,6 +2146,7 @@ msgstr ""
 msgid "Unknown BLE error: %d"
 msgstr ""
 
+#: ports/espressif/common-hal/max3421e/Max3421E.c
 #: ports/raspberrypi/common-hal/wifi/__init__.c
 #, c-format
 msgid "Unknown error code %d"
@@ -3810,7 +3818,7 @@ msgstr ""
 msgid "pop from empty %q"
 msgstr ""
 
-#: shared-bindings/socketpool/Socket.c shared-bindings/ssl/SSLSocket.c
+#: shared-bindings/socketpool/Socket.c
 msgid "port must be >= 0"
 msgstr ""
 

main.c

@@ -1006,10 +1006,6 @@ int __attribute__((used)) main(void) {
     supervisor_status_bar_init();
     #endif
 
-    #if CIRCUITPY_BLEIO
-    // Early init so that a reset press can cause BLE public advertising.
-    supervisor_bluetooth_init();
-    #endif
 
     #if !INTERNAL_FLASH_FILESYSTEM
     // Set up anything that might need to get done before we try to use SPI flash
@@ -1027,6 +1023,12 @@ int __attribute__((used)) main(void) {
         set_safe_mode(SAFE_MODE_NO_CIRCUITPY);
     }
 
+    #if CIRCUITPY_BLEIO
+    // Early init so that a reset press can cause BLE public advertising. Need the filesystem to
+    // read settings.toml.
+    supervisor_bluetooth_init();
+    #endif
+
     #if CIRCUITPY_ALARM
     // Record which alarm woke us up, if any.
     // common_hal_alarm_record_wake_alarm() should return a static, non-heap object

ports/espressif/Makefile

@@ -229,11 +229,13 @@ LDFLAGS += \
 	-Tesp32c6.rom.newlib.ld \
 	-Tesp32c6.rom.coexist.ld \
 	-Tesp32c6.rom.heap.ld \
+	-Tesp32c6.rom.systimer.ld \
 	-Tesp32c6.rom.wdt.ld
 else ifeq ($(IDF_TARGET),esp32h2)
 LDFLAGS += \
 	-Tesp32h2.rom.heap.ld \
 	-Tesp32h2.rom.newlib.ld \
+	-Tesp32h2.rom.systimer.ld \
 	-Tesp32h2.rom.wdt.ld
 else ifeq ($(IDF_TARGET),esp32s2)
 LDFLAGS += \

ports/espressif/boards/adafruit_feather_esp32c6_4mbflash_nopsram/mpconfigboard.h

@@ -11,7 +11,9 @@
 #define MICROPY_HW_BOARD_NAME       "Adafruit Feather ESP32-C6 4MB Flash No PSRAM"
 #define MICROPY_HW_MCU_NAME         "ESP32C6"
 
-#define MICROPY_HW_NEOPIXEL (&pin_GPIO9)
+// Don't use the neopixel for status because we can't use it at the same time as
+// the boot button.
+// #define MICROPY_HW_NEOPIXEL (&pin_GPIO9)
 
 #define MICROPY_HW_LED_STATUS (&pin_GPIO15)
 

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

@@ -39,6 +39,7 @@
 #include "common-hal/_bleio/Connection.h"
 
 #include "esp_bt.h"
+#include "esp_mac.h"
 #include "esp_nimble_hci.h"
 #include "nvs_flash.h"
 
@@ -48,11 +49,6 @@
 
 bleio_connection_internal_t bleio_connections[BLEIO_TOTAL_CONNECTION_COUNT];
 
-// static void bluetooth_adapter_background(void *data) {
-//     supervisor_bluetooth_background();
-//     bleio_background();
-// }
-
 bool ble_active = false;
 
 static void nimble_host_task(void *param) {
@@ -63,7 +59,7 @@ static void nimble_host_task(void *param) {
 static TaskHandle_t cp_task = NULL;
 
 static void _on_sync(void) {
-    int rc = ble_hs_util_ensure_addr(0);
+    int rc = ble_hs_util_ensure_addr(false);
     assert(rc == 0);
 
     xTaskNotifyGive(cp_task);
@@ -72,6 +68,8 @@ static void _on_sync(void) {
 // All examples have this. It'd make sense in a header.
 void ble_store_config_init(void);
 
+char default_ble_name[] = { 'C', 'I', 'R', 'C', 'U', 'I', 'T', 'P', 'Y', 0, 0, 0, 0, 0, 0, 0};
+
 void common_hal_bleio_adapter_set_enabled(bleio_adapter_obj_t *self, bool enabled) {
     const bool is_enabled = common_hal_bleio_adapter_get_enabled(self);
 
@@ -81,15 +79,6 @@ void common_hal_bleio_adapter_set_enabled(bleio_adapter_obj_t *self, bool enable
     }
 
     if (enabled) {
-        esp_err_t err = nvs_flash_init();
-        if (err == ESP_ERR_NVS_NO_FREE_PAGES || err == ESP_ERR_NVS_NEW_VERSION_FOUND) {
-            // NVS partition was truncated and needs to be erased
-            // Retry nvs_flash_init
-            ESP_ERROR_CHECK(nvs_flash_erase());
-            err = nvs_flash_init();
-        }
-        ESP_ERROR_CHECK(err);
-
         CHECK_ESP_RESULT(nimble_port_init());
 
         // ble_hs_cfg.reset_cb = blecent_on_reset;
@@ -104,8 +93,8 @@ void common_hal_bleio_adapter_set_enabled(bleio_adapter_obj_t *self, bool enable
         ble_hs_cfg.sm_our_key_dist |= BLE_SM_PAIR_KEY_DIST_ENC;
         ble_hs_cfg.sm_their_key_dist |= BLE_SM_PAIR_KEY_DIST_ENC;
 
-        ble_hs_cfg.sm_mitm = 1;
-        ble_hs_cfg.sm_sc = 1;
+        ble_hs_cfg.sm_mitm = 0;
+        ble_hs_cfg.sm_sc = 0;
         /* Stores the IRK */
         ble_hs_cfg.sm_our_key_dist |= BLE_SM_PAIR_KEY_DIST_ID;
         ble_hs_cfg.sm_their_key_dist |= BLE_SM_PAIR_KEY_DIST_ID;
@@ -122,7 +111,17 @@ void common_hal_bleio_adapter_set_enabled(bleio_adapter_obj_t *self, bool enable
         } else
         #endif
         {
-            ble_svc_gap_device_name_set("CIRCUITPY");
+            uint8_t mac[6];
+            esp_read_mac(mac, ESP_MAC_BT);
+            mp_int_t len = sizeof(default_ble_name) - 1;
+            default_ble_name[len - 6] = nibble_to_hex_lower[mac[3] >> 4 & 0xf];
+            default_ble_name[len - 5] = nibble_to_hex_lower[mac[3] & 0xf];
+            default_ble_name[len - 4] = nibble_to_hex_lower[mac[4] >> 4 & 0xf];
+            default_ble_name[len - 3] = nibble_to_hex_lower[mac[4] & 0xf];
+            default_ble_name[len - 2] = nibble_to_hex_lower[mac[5] >> 4 & 0xf];
+            default_ble_name[len - 1] = nibble_to_hex_lower[mac[5] & 0xf];
+            default_ble_name[len] = '\0'; // for now we add null for compatibility with C ASCIIZ strings
+            ble_svc_gap_device_name_set(default_ble_name);
         }
 
         // Clear all of the internal connection objects.
@@ -180,6 +179,14 @@ bool common_hal_bleio_adapter_set_address(bleio_adapter_obj_t *self, bleio_addre
     return result == 0;
 }
 
+uint16_t bleio_adapter_get_name(char *buf, uint16_t len) {
+    const char *name = ble_svc_gap_device_name();
+    uint16_t full_len = strlen(name);
+    memcpy(buf, name, MIN(full_len, len));
+
+    return full_len;
+}
+
 mp_obj_str_t *common_hal_bleio_adapter_get_name(bleio_adapter_obj_t *self) {
     const char *name = ble_svc_gap_device_name();
 
@@ -401,7 +408,8 @@ mp_obj_t common_hal_bleio_adapter_connect(bleio_adapter_obj_t *self, bleio_addre
             _connect_event, self));
 
     int error_code;
-    CHECK_NOTIFY(xTaskNotifyWait(0, 0, (uint32_t *)&error_code, 200));
+    // Wait an extra 50 ms to give the connect method the opportunity to time out.
+    CHECK_NOTIFY(xTaskNotifyWait(0, 0, (uint32_t *)&error_code, pdMS_TO_TICKS(timeout * 1000 + 50)));
     // Negative values are error codes, connection handle otherwise.
     if (error_code < 0) {
         CHECK_BLE_ERROR(-error_code);
@@ -487,6 +495,7 @@ static int _advertising_event(struct ble_gap_event *event, void *self_in) {
             #endif
             break;
     }
+    background_callback_add_core(&bleio_background_callback);
     return 0;
 }
 
@@ -499,6 +508,8 @@ uint32_t _common_hal_bleio_adapter_start_advertising(bleio_adapter_obj_t *self,
     if (ble_gap_adv_active() && !self->user_advertising) {
         return BLE_HS_EBUSY;
     }
+    // Override anonymous because it isn't working with the ESP-IDF.
+    anonymous = false;
 
     uint32_t rc;
 
@@ -521,12 +532,19 @@ uint32_t _common_hal_bleio_adapter_start_advertising(bleio_adapter_obj_t *self,
     bool extended = advertising_data_len > BLE_ADV_LEGACY_DATA_MAX_LEN ||
         scan_response_data_len > BLE_ADV_LEGACY_DATA_MAX_LEN;
 
+    bool scannable = scan_response_data_len > 0;
+    bool legacy_pdu = !extended && !anonymous;
+    if (legacy_pdu && connectable) {
+        // Connectable legacy advertisements are always scannable too.
+        scannable = true;
+    }
+
     struct ble_gap_ext_adv_params adv_params = {
         .connectable = connectable,
-        .scannable = scan_response_data_len > 0,
+        .scannable = scannable,
         .directed = directed_to != NULL,
         .high_duty_directed = high_duty_directed,
-        .legacy_pdu = !extended,
+        .legacy_pdu = legacy_pdu,
         .anonymous = anonymous,
         .include_tx_power = extended,
         .scan_req_notif = false,
@@ -712,14 +730,46 @@ mp_obj_t common_hal_bleio_adapter_get_connections(bleio_adapter_obj_t *self) {
     return self->connection_objs;
 }
 
+#define NIMBLE_NVS_PEER_SEC_KEY                  "peer_sec"
+#define NIMBLE_NVS_OUR_SEC_KEY                   "our_sec"
+#define NIMBLE_NVS_CCCD_SEC_KEY                  "cccd_sec"
+#define NIMBLE_NVS_PEER_RECORDS_KEY              "p_dev_rec"
+#define NIMBLE_NVS_NAMESPACE                     "nimble_bond"
+
+// Implement bonding control ourselves when the adapter isn't enabled so that it
+// can run when BLE is off.
 void common_hal_bleio_adapter_erase_bonding(bleio_adapter_obj_t *self) {
-    ble_store_clear();
+    if (common_hal_bleio_adapter_get_enabled(self)) {
+        ble_store_clear();
+    } else {
+        nvs_handle_t nimble_handle;
+        esp_err_t err = nvs_open(NIMBLE_NVS_NAMESPACE, NVS_READWRITE, &nimble_handle);
+        if (err != ESP_OK) {
+            return;
+        }
+        nvs_erase_all(nimble_handle);
+        nvs_commit(nimble_handle);
+        nvs_close(nimble_handle);
+    }
 }
 
 bool common_hal_bleio_adapter_is_bonded_to_central(bleio_adapter_obj_t *self) {
-    int count;
-    ble_store_util_count(BLE_STORE_OBJ_TYPE_PEER_SEC, &count);
-    return count > 0;
+    if (common_hal_bleio_adapter_get_enabled(self)) {
+        int count;
+        ble_store_util_count(BLE_STORE_OBJ_TYPE_PEER_SEC, &count);
+        return count > 0;
+    }
+    nvs_handle_t nimble_handle;
+    esp_err_t err = nvs_open(NIMBLE_NVS_NAMESPACE, NVS_READONLY, &nimble_handle);
+    if (err != ESP_OK) {
+        return false;
+    }
+    err = nvs_find_key(nimble_handle, "peer_sec_1", NULL);
+    nvs_close(nimble_handle);
+    if (err == ESP_OK) {
+        return true;
+    }
+    return false;
 }
 
 void bleio_adapter_gc_collect(bleio_adapter_obj_t *adapter) {

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

@@ -17,7 +17,6 @@
 
 #include "common-hal/_bleio/Adapter.h"
 #include "common-hal/_bleio/Service.h"
-// #include "common-hal/_bleio/bonding.h"
 
 
 static int characteristic_on_ble_gap_evt(struct ble_gap_event *event, void *param);
@@ -57,6 +56,22 @@ void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self,
     if ((props & CHAR_PROP_WRITE_NO_RESPONSE) != 0) {
         self->flags |= BLE_GATT_CHR_F_WRITE_NO_RSP;
     }
+    if (read_perm == SECURITY_MODE_ENC_WITH_MITM || write_perm == SECURITY_MODE_ENC_WITH_MITM ||
+        read_perm == SECURITY_MODE_SIGNED_WITH_MITM || write_perm == SECURITY_MODE_SIGNED_WITH_MITM) {
+        mp_raise_NotImplementedError(MP_ERROR_TEXT("MITM security not supported"));
+    }
+    if (read_perm == SECURITY_MODE_ENC_NO_MITM) {
+        self->flags |= BLE_GATT_CHR_F_READ_ENC;
+    }
+    if (read_perm == SECURITY_MODE_SIGNED_NO_MITM) {
+        self->flags |= BLE_GATT_CHR_F_READ_AUTHEN;
+    }
+    if (write_perm == SECURITY_MODE_ENC_NO_MITM) {
+        self->flags |= BLE_GATT_CHR_F_WRITE_ENC;
+    }
+    if (write_perm == SECURITY_MODE_SIGNED_NO_MITM) {
+        self->flags |= BLE_GATT_CHR_F_WRITE_AUTHEN;
+    }
 
     if (initial_value_bufinfo != NULL) {
         // Copy the initial value if it's on the heap. Otherwise it's internal and we may not be able
@@ -72,6 +87,12 @@ void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self,
             self->current_value = initial_value_bufinfo->buf;
             assert(self->current_value_len == max_length);
         }
+    } else {
+        self->current_value = port_malloc(max_length, false);
+        if (self->current_value != NULL) {
+            self->current_value_alloc = max_length;
+            self->current_value_len = 0;
+        }
     }
 
     if (gc_alloc_possible()) {
@@ -294,6 +315,7 @@ int bleio_characteristic_access_cb(uint16_t conn_handle, uint16_t attr_handle,
                         bleio_packet_buffer_extend(MP_OBJ_FROM_PTR(self->observer), conn_handle, self->current_value, self->current_value_len);
                     }
                 }
+                background_callback_add_core(&bleio_background_callback);
                 return rc;
             }
             return BLE_ATT_ERR_UNLIKELY;

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

@@ -91,14 +91,18 @@ int bleio_connection_event_cb(struct ble_gap_event *event, void *connection_in)
         case BLE_GAP_EVENT_NOTIFY_TX:
             MP_FALLTHROUGH;
         case BLE_GAP_EVENT_SUBSCRIBE:
-            return ble_event_run_handlers(event);
+            int status = ble_event_run_handlers(event);
+            background_callback_add_core(&bleio_background_callback);
+            return status;
 
         default:
             #if CIRCUITPY_VERBOSE_BLE
             mp_printf(&mp_plat_print, "Unhandled connection event: %d\n", event->type);
             #endif
-            return 0;
+            break;
     }
+
+    background_callback_add_core(&bleio_background_callback);
     return 0;
 }