From 109d42bb85ab1007f349d1378e1bbf10c3810a5c Mon Sep 17 00:00:00 2001 From: liqigan Date: Tue, 10 Jan 2023 19:52:05 +0800 Subject: [PATCH] fix HID Host bug when handling the two consecutive connection request Closes https://github.com/espressif/esp-idf/issues/10504 --- .../bt/host/bluedroid/bta/hh/bta_hh_act.c | 3 +-- .../bt/host/bluedroid/bta/hh/bta_hh_utils.c | 1 + .../bt/host/bluedroid/stack/hid/hidh_api.c | 19 +++++++++++++++---- .../bt/host/bluedroid/stack/hid/hidh_conn.c | 7 +++++++ .../bluedroid/stack/hid/include/hid_int.h | 1 + components/esp_hid/src/bt_hidh.c | 18 ++++++++---------- components/esp_hid/src/esp_hidh.c | 13 ++++++++++++- 7 files changed, 45 insertions(+), 17 deletions(-) diff --git a/components/bt/host/bluedroid/bta/hh/bta_hh_act.c b/components/bt/host/bluedroid/bta/hh/bta_hh_act.c index 3edce53bc6..604b5e95b5 100644 --- a/components/bt/host/bluedroid/bta/hh/bta_hh_act.c +++ b/components/bt/host/bluedroid/bta/hh/bta_hh_act.c @@ -459,8 +459,7 @@ void bta_hh_sdp_cmpl(tBTA_HH_DEV_CB *p_cb, tBTA_HH_DATA *p_data) /* move state machine W4_CONN ->IDLE */ bta_hh_sm_execute(p_cb, BTA_HH_API_CLOSE_EVT, NULL); - /* if this is an outgoing connection to an unknown device, clean up cb */ - if (p_cb->app_id == 0 && !p_cb->incoming_conn) { + if (p_cb->app_id == 0) { /* clean up device control block */ bta_hh_clean_up_kdev(p_cb); } diff --git a/components/bt/host/bluedroid/bta/hh/bta_hh_utils.c b/components/bt/host/bluedroid/bta/hh/bta_hh_utils.c index 8ca8b09c45..6dc3ec02dd 100644 --- a/components/bt/host/bluedroid/bta/hh/bta_hh_utils.c +++ b/components/bt/host/bluedroid/bta/hh/bta_hh_utils.c @@ -88,6 +88,7 @@ UINT8 bta_hh_find_cb(BD_ADDR bda) for (xx = 0; xx < BTA_HH_MAX_DEVICE; xx++) { if (!bta_hh_cb.kdev[xx].in_use) { bdcpy(bta_hh_cb.kdev[xx].addr, bda); + bta_hh_cb.kdev[xx].in_use = TRUE; break; } } diff --git a/components/bt/host/bluedroid/stack/hid/hidh_api.c b/components/bt/host/bluedroid/stack/hid/hidh_api.c index 188aa0a4ee..ea8e73a75c 100644 --- a/components/bt/host/bluedroid/stack/hid/hidh_api.c +++ b/components/bt/host/bluedroid/stack/hid/hidh_api.c @@ -378,6 +378,7 @@ tHID_STATUS HID_HostAddDev ( BD_ADDR addr, UINT16 attr_mask, UINT8 *handle ) if (!hh_cb.devices[i].in_use) { hh_cb.devices[i].in_use = TRUE; + hh_cb.devices[i].delay_remove = FALSE; memcpy( hh_cb.devices[i].addr, addr, sizeof( BD_ADDR ) ) ; hh_cb.devices[i].state = HID_DEV_NO_CONN; hh_cb.devices[i].conn_tries = 0 ; @@ -443,10 +444,20 @@ tHID_STATUS HID_HostRemoveDev ( UINT8 dev_handle ) } HID_HostCloseDev( dev_handle ) ; - hh_cb.devices[dev_handle].in_use = FALSE; - hh_cb.devices[dev_handle].conn.conn_state = HID_CONN_STATE_UNUSED; - hh_cb.devices[dev_handle].conn.ctrl_cid = hh_cb.devices[dev_handle].conn.intr_cid = 0; - hh_cb.devices[dev_handle].attr_mask = 0; + + if (hh_cb.devices[dev_handle].conn.conn_state == HID_CONN_STATE_DISCONNECTING_INTR || + hh_cb.devices[dev_handle].conn.conn_state == HID_CONN_STATE_DISCONNECTING_CTRL) { + // delay the remove action, to close the control and the interrupt channel + hh_cb.devices[dev_handle].delay_remove = TRUE; + } else { + HIDH_TRACE_WARNING("%s dev_handle:%d conn_state:%d", __func__, dev_handle, + hh_cb.devices[dev_handle].conn.conn_state); + hh_cb.devices[dev_handle].in_use = FALSE; + hh_cb.devices[dev_handle].conn.conn_state = HID_CONN_STATE_UNUSED; + hh_cb.devices[dev_handle].conn.ctrl_cid = hh_cb.devices[dev_handle].conn.intr_cid = 0; + hh_cb.devices[dev_handle].attr_mask = 0; + } + return HID_SUCCESS; } diff --git a/components/bt/host/bluedroid/stack/hid/hidh_conn.c b/components/bt/host/bluedroid/stack/hid/hidh_conn.c index d94d9d1fa0..42d170ddd0 100644 --- a/components/bt/host/bluedroid/stack/hid/hidh_conn.c +++ b/components/bt/host/bluedroid/stack/hid/hidh_conn.c @@ -110,6 +110,7 @@ tHID_STATUS hidh_conn_reg (void) for (xx = 0; xx < HID_HOST_MAX_DEVICES; xx++) { hh_cb.devices[xx].in_use = FALSE ; + hh_cb.devices[xx].delay_remove = FALSE; hh_cb.devices[xx].conn.conn_state = HID_CONN_STATE_UNUSED; } @@ -681,6 +682,12 @@ static void hidh_l2cif_disconnect_cfm (UINT16 l2cap_cid, UINT16 result) if ((p_hcon->ctrl_cid == 0) && (p_hcon->intr_cid == 0)) { hh_cb.devices[dhandle].state = HID_DEV_NO_CONN; p_hcon->conn_state = HID_CONN_STATE_UNUSED; + // removes the device from list devices that host has to manage + if (hh_cb.devices[dhandle].delay_remove) { + hh_cb.devices[dhandle].in_use = FALSE; + hh_cb.devices[dhandle].delay_remove = FALSE; + hh_cb.devices[dhandle].attr_mask = 0; + } hh_cb.callback( dhandle, hh_cb.devices[dhandle].addr, HID_HDEV_EVT_CLOSE, p_hcon->disc_reason, NULL ) ; } } diff --git a/components/bt/host/bluedroid/stack/hid/include/hid_int.h b/components/bt/host/bluedroid/stack/hid/include/hid_int.h index d926e3a891..b42089662d 100644 --- a/components/bt/host/bluedroid/stack/hid/include/hid_int.h +++ b/components/bt/host/bluedroid/stack/hid/include/hid_int.h @@ -34,6 +34,7 @@ enum { HID_DEV_NO_CONN, HID_DEV_CONNECTED }; typedef struct per_device_ctb { BOOLEAN in_use; + BOOLEAN delay_remove; BD_ADDR addr; /* BD-Addr of the host device */ UINT16 attr_mask; /* 0x01- virtual_cable; 0x02- normally_connectable; 0x03- reconn_initiate; 0x04- sdp_disable; */ diff --git a/components/esp_hid/src/bt_hidh.c b/components/esp_hid/src/bt_hidh.c index ff583ed300..6f0410e302 100644 --- a/components/esp_hid/src/bt_hidh.c +++ b/components/esp_hid/src/bt_hidh.c @@ -244,17 +244,15 @@ static void esp_hh_cb(esp_hidh_cb_event_t event, esp_hidh_cb_param_t *param) break; } else { ESP_LOGD(TAG, "incoming device connect"); - if (param->open.status == ESP_HIDH_OK) { - if ((dev = hidh_dev_ctor(param->open.bd_addr)) == NULL) { - ESP_LOGE(TAG, "%s create device failed!", __func__); - param->open.status = ESP_HIDH_ERR_NO_RES; - break; - } - esp_hidh_dev_lock(dev); - dev->opened = false; // not opened by ourself - dev->is_orig = false; - esp_hidh_dev_unlock(dev); + if ((dev = hidh_dev_ctor(param->open.bd_addr)) == NULL) { + ESP_LOGE(TAG, "%s create device failed!", __func__); + param->open.status = ESP_HIDH_ERR_NO_RES; + break; } + esp_hidh_dev_lock(dev); + dev->opened = false; // not opened by ourself + dev->is_orig = false; + esp_hidh_dev_unlock(dev); } } diff --git a/components/esp_hid/src/esp_hidh.c b/components/esp_hid/src/esp_hidh.c index 8931f07fe6..5102105d6d 100644 --- a/components/esp_hid/src/esp_hidh.c +++ b/components/esp_hid/src/esp_hidh.c @@ -11,6 +11,9 @@ #include #include #include "esp_event_base.h" +#if CONFIG_BT_HID_HOST_ENABLED +#include "esp_hidh_api.h" +#endif /* CONFIG_BT_HID_HOST_ENABLED */ ESP_EVENT_DEFINE_BASE(ESP_HIDH_EVENTS); #define ESP_HIDH_DELAY_FREE_TO 100000 // us @@ -801,7 +804,15 @@ void esp_hidh_post_process_event_handler(void *event_handler_arg, esp_event_base switch (event) { case ESP_HIDH_OPEN_EVENT: if (param->open.status != ESP_OK) { - esp_hidh_dev_free_inner(param->open.dev); + esp_hidh_dev_t *dev = param->open.dev; + if (dev) { +#if CONFIG_BT_HID_HOST_ENABLED + if (esp_hidh_dev_transport_get(dev) == ESP_HID_TRANSPORT_BT && param->open.status == ESP_HIDH_ERR_SDP) { + break; + } +#endif /* CONFIG_BT_HID_HOST_ENABLED */ + esp_hidh_dev_free_inner(dev); + } } break; case ESP_HIDH_CLOSE_EVENT: