Fix a few bugs

Author: dhalbert

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

@@ -804,15 +804,25 @@ void common_hal_bleio_adapter_stop_advertising(bleio_adapter_obj_t *self) {
     self->now_advertising = false;
     self->extended_advertising = false;
     self->circuitpython_advertising = false;
+
     int result = hci_le_set_advertising_enable(BT_HCI_LE_ADV_DISABLE);
-    // OK if we're already stopped.
-    if (result != BT_HCI_ERR_CMD_DISALLOWED) {
+    // OK if we're already stopped. There seems to be an ESP32 HCI bug:
+    // If advertising is already off, then LE_SET_ADV_ENABLE does not return a response.
+    if (result != HCI_RESPONSE_TIMEOUT) {
         check_hci_error(result);
     }
 
     //TODO startup CircuitPython advertising again.
 }
 
+// Note that something stopped advertising, such as a connection happening.
+//Don't ask the adapter to stop.
+void bleio_adapter_advertising_was_stopped(bleio_adapter_obj_t *self) {
+    self->now_advertising = false;
+    self->extended_advertising = false;
+    self->circuitpython_advertising = false;
+}
+
 bool common_hal_bleio_adapter_get_advertising(bleio_adapter_obj_t *self) {
     check_enabled(self);
 

devices/ble_hci/common-hal/_bleio/Adapter.h

@@ -88,6 +88,7 @@ typedef struct _bleio_adapter_obj_t {
 } bleio_adapter_obj_t;
 
 uint16_t bleio_adapter_add_attribute(bleio_adapter_obj_t *adapter, mp_obj_t *attribute);
+void bleio_adapter_advertising_was_stopped(bleio_adapter_obj_t *self);
 mp_obj_t* bleio_adapter_get_attribute(bleio_adapter_obj_t *adapter, uint16_t handle);
 uint16_t bleio_adapter_max_attribute_handle(bleio_adapter_obj_t *adapter);
 void bleio_adapter_background(bleio_adapter_obj_t* adapter);

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

@@ -48,11 +48,7 @@ void check_hci_error(hci_result_t result) {
         case HCI_OK:
             return;
 
-        case HCI_NO_RESPONSE:
-            mp_raise_bleio_BluetoothError(translate("No HCI command response received"));
-            return;
-
-        case HCI_READ_TIMEOUT:
+        case HCI_RESPONSE_TIMEOUT:
             mp_raise_bleio_BluetoothError(translate("Timeout waiting for HCI response"));
             return;
 

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

@@ -224,7 +224,6 @@ bool att_disconnect(uint16_t conn_handle) {
     }
 
     hci_disconnect(conn_handle);
-    hci_poll_for_incoming_pkt_timeout(timeout);
 
     // Confirm we're now disconnected.
     return !att_handle_is_connected(conn_handle);
@@ -508,13 +507,13 @@ void att_add_connection(uint16_t handle, uint8_t role, bt_addr_le_t *peer_addr,
 }
 
 
-void att_remove_connection(uint16_t handle, uint8_t reason) {
+void att_remove_connection(uint16_t conn_handle, uint8_t reason) {
     (void) reason;
     int peer_index = -1;
     int peer_count = 0;
 
     for (size_t i = 0; i < BLEIO_TOTAL_CONNECTION_COUNT; i++) {
-        if (bleio_connections[i].conn_handle == handle) {
+        if (bleio_connections[i].conn_handle == conn_handle) {
             peer_index = i;
         }
 
@@ -532,7 +531,7 @@ void att_remove_connection(uint16_t handle, uint8_t reason) {
 
         // Clear CCCD values on disconnect.
         size_t max_attribute_handle = bleio_adapter_max_attribute_handle(&common_hal_bleio_adapter_obj);
-        for (size_t i = 1; handle <= max_attribute_handle; i++) {
+        for (size_t handle = 1; handle <= max_attribute_handle; handle++) {
             mp_obj_t attribute_obj = bleio_adapter_get_attribute(&common_hal_bleio_adapter_obj, handle);
 
             uint16_t zero = 0;

devices/ble_hci/common-hal/_bleio/att.h

@@ -49,7 +49,7 @@ uint16_t att_conn_handle(bt_addr_le_t *addr);
 uint16_t att_mtu(uint16_t handle);
 void att_add_connection(uint16_t handle, uint8_t role, bt_addr_le_t *peer_addr, uint16_t interval, uint16_t latency, uint16_t supervision_timeout, uint8_t master_clock_accuracy);
 void att_process_data(uint16_t conn_handle, uint8_t dlen, uint8_t data[]);
-void att_remove_connection(uint16_t handle, uint8_t reason);
+void att_remove_connection(uint16_t conn_handle, uint8_t reason);
 void att_set_max_mtu(uint16_t max_mtu);
 void att_set_timeout(unsigned long timeout);
 void att_write_cmd(uint16_t conn_handle, uint16_t handle, const uint8_t* data, uint8_t data_len);

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

@@ -175,7 +175,6 @@ STATIC void process_evt_pkt(size_t pkt_len, uint8_t pkt_data[])
 
             att_remove_connection(disconn_complete->handle, disconn_complete->reason);
             //FIX L2CAPSignaling.removeConnection(disconn_complete->handle, disconn_complete->reason);
-            hci_le_set_advertising_enable(0x01);
             break;
         }
 
@@ -233,6 +232,12 @@ STATIC void process_evt_pkt(size_t pkt_len, uint8_t pkt_data[])
             uint8_t *le_evt =  pkt->params + sizeof (struct bt_hci_evt_le_meta_event);
 
             if (meta_evt->subevent == BT_HCI_EVT_LE_CONN_COMPLETE) {
+                // Advertising stops when connection occurs.
+                // We don't tell the adapter to stop, because stopping advertising
+                // when it's already stopped seems to exercise a bug in the ESP32 HCI code:
+                // It doesn't return a response.
+                bleio_adapter_advertising_was_stopped(&common_hal_bleio_adapter_obj);
+
                 struct bt_hci_evt_le_conn_complete *le_conn_complete =
                     (struct bt_hci_evt_le_conn_complete *) le_evt;
 
@@ -281,29 +286,11 @@ void bleio_hci_reset(void) {
     bleio_att_reset();
 }
 
-hci_result_t hci_poll_for_incoming_pkt_timeout(uint32_t timeout_msecs) {
-    uint64_t start = supervisor_ticks_ms64();
-
-    hci_result_t result = HCI_OK;
-
-    while (supervisor_ticks_ms64() - start < timeout_msecs) {
-        result = hci_poll_for_incoming_pkt();
-        RUN_BACKGROUND_TASKS;
-    }
-
-    return result;
-}
-
-
 hci_result_t hci_poll_for_incoming_pkt(void) {
     if (hci_poll_in_progress) {
         return HCI_OK;
     }
-    common_hal_mcu_disable_interrupts();
-    if (!hci_poll_in_progress) {
-        hci_poll_in_progress = true;
-    }
-    common_hal_mcu_enable_interrupts();
+    hci_poll_in_progress = true;
 
     // Assert RTS low to say we're ready to read data.
     common_hal_digitalio_digitalinout_set_value(common_hal_bleio_adapter_obj.rts_digitalinout, false);
@@ -431,7 +418,7 @@ STATIC hci_result_t send_command(uint16_t opcode, uint8_t params_len, void* para
     // Wait for a response. Note that other packets may be received that are not
     // command responses.
     uint64_t start = supervisor_ticks_ms64();
-    while (1) {
+    while (supervisor_ticks_ms64() - start < RESPONSE_TIMEOUT_MSECS) {
         result = hci_poll_for_incoming_pkt();
         if (result != HCI_OK) {
             // I/O error.
@@ -442,18 +429,14 @@ STATIC hci_result_t send_command(uint16_t opcode, uint8_t params_len, void* para
             // If this is definitely a response to the command that was sent,
             // return the status value, which will will be
             // BT_HCI_ERR_SUCCESS (0x00) if the command succeeded,
-            // or a BT_HCI_ERR_x value (> 0x00) if there ws a problem.
+            // or a BT_HCI_ERR_x value (> 0x00) if there was a problem.
             return cmd_response_status;
         }
-
-        if (supervisor_ticks_ms64() - start > RESPONSE_TIMEOUT_MSECS) {
-            return HCI_READ_TIMEOUT;
-        }
         RUN_BACKGROUND_TASKS;
     }
 
     // No I/O error, but no response sent back in time.
-    return HCI_NO_RESPONSE;
+    return HCI_RESPONSE_TIMEOUT;
 }
 
 hci_result_t hci_send_acl_pkt(uint16_t handle, uint8_t cid, uint8_t data_len, uint8_t *data) {
@@ -528,11 +511,6 @@ hci_result_t hci_read_rssi(uint16_t handle, int *rssi) {
     int result = send_command(BT_HCI_OP_READ_RSSI, sizeof(handle), &handle);
     if (result == HCI_OK) {
         struct bt_hci_rp_read_rssi *response = (struct bt_hci_rp_read_rssi *) cmd_response_data;
-        if (response->handle != handle) {
-            // Handle doesn't match.
-            return HCI_NO_RESPONSE;
-        }
-
         *rssi = response->rssi;
     }
 

devices/ble_hci/common-hal/_bleio/hci.h

@@ -32,12 +32,11 @@ typedef struct _bleio_adapter_obj_t bleio_adapter_obj_t;
 // or is it > 0 and is an HCI command status value (see hci_include/hci_err.h)
 typedef int hci_result_t;
 #define HCI_OK (0)
-#define HCI_NO_RESPONSE (-1)
-#define HCI_READ_TIMEOUT (-2)
-#define HCI_WRITE_TIMEOUT (-3)
-#define HCI_READ_ERROR (-4)
-#define HCI_WRITE_ERROR (-5)
-#define HCI_ATT_ERROR (-6)
+#define HCI_RESPONSE_TIMEOUT (-1)
+#define HCI_WRITE_TIMEOUT (-2)
+#define HCI_READ_ERROR (-3)
+#define HCI_WRITE_ERROR (-4)
+#define HCI_ATT_ERROR (-5)
 
 void bleio_hci_reset(void);
 
@@ -65,7 +64,6 @@ hci_result_t hci_le_set_scan_parameters(uint8_t scan_type, uint16_t interval, ui
 hci_result_t hci_le_set_scan_response_data(uint8_t length, uint8_t data[]);
 
 hci_result_t hci_poll_for_incoming_pkt(void);
-hci_result_t hci_poll_for_incoming_pkt_timeout(uint32_t timeout_msecs);
 
 hci_result_t hci_read_bd_addr(bt_addr_t *addr);
 hci_result_t hci_read_buffer_size(uint16_t *acl_max_len, uint8_t *sco_max_len, uint16_t *acl_max_num, uint16_t *sco_max_num);