This reverts commit d6cafa1f0c3ea1989f8a580e52f418b0998a3552.
In commit d6cafa1f0c3e ("input/hog: Remove HID device after HoG device
disconnects"), the bt_hog structure is destroyed in order to fix a bug
where the UHID connection is not destroyed. This fix has the cost of
increasing reconnection time because every reconnection would need to
re-read the report map again. An improvement to this fix is, instead of
removing the bt_hog structure, we can just destroy the UHID with
UHID_DESTROY event and use the existing bt_hog structure to keep the
cache of the report map to avoid re-reading the report map at
reconnection.
Reviewed-by: Alain Michaud <[email protected]>
---
profiles/input/hog.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/profiles/input/hog.c b/profiles/input/hog.c
index 91de4c70f..d50b82321 100644
--- a/profiles/input/hog.c
+++ b/profiles/input/hog.c
@@ -207,8 +207,6 @@ static int hog_disconnect(struct btd_service *service)
struct hog_device *dev = btd_service_get_user_data(service);
bt_hog_detach(dev->hog);
- bt_hog_unref(dev->hog);
- dev->hog = NULL;
btd_service_disconnecting_complete(service, 0);
--
2.29.2
To optimize BLE HID devices reconnection response, we can cache the
report map so that the subsequent reconnections do not need round trip
time to read the report map.
Reviewed-by: Alain Michaud <[email protected]>
---
profiles/input/hog-lib.c | 151 ++++++++++++++++++++++++++++++++-------
1 file changed, 127 insertions(+), 24 deletions(-)
diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
index ee811d301..1f132aa4c 100644
--- a/profiles/input/hog-lib.c
+++ b/profiles/input/hog-lib.c
@@ -95,6 +95,13 @@ struct bt_hog {
struct queue *bas;
GSList *instances;
struct queue *gatt_op;
+ struct gatt_db *gatt_db;
+ struct gatt_db_attribute *report_map_attr;
+};
+
+struct report_map {
+ uint8_t value[HOG_REPORT_MAP_MAX_SIZE];
+ size_t length;
};
struct report {
@@ -924,33 +931,16 @@ static char *item2string(char *str, uint8_t *buf, uint8_t len)
return str;
}
-static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
- gpointer user_data)
+static void uhid_create(struct bt_hog *hog, uint8_t *report_map,
+ ssize_t report_map_len)
{
- struct gatt_request *req = user_data;
- struct bt_hog *hog = req->user_data;
- uint8_t value[HOG_REPORT_MAP_MAX_SIZE];
+ uint8_t *value = report_map;
struct uhid_event ev;
- ssize_t vlen;
+ ssize_t vlen = report_map_len;
char itemstr[20]; /* 5x3 (data) + 4 (continuation) + 1 (null) */
int i, err;
GError *gerr = NULL;
- destroy_gatt_req(req);
-
- DBG("HoG inspecting report map");
-
- if (status != 0) {
- error("Report Map read failed: %s", att_ecode2str(status));
- return;
- }
-
- vlen = dec_read_resp(pdu, plen, value, sizeof(value));
- if (vlen < 0) {
- error("ATT protocol error");
- return;
- }
-
DBG("Report MAP:");
for (i = 0; i < vlen;) {
ssize_t ilen = 0;
@@ -1022,6 +1012,46 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
DBG("HoG created uHID device");
}
+static void db_report_map_write_value_cb(struct gatt_db_attribute *attr,
+ int err, void *user_data)
+{
+ if (err)
+ error("Error writing report map value to gatt db");
+}
+
+static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
+ gpointer user_data)
+{
+ struct gatt_request *req = user_data;
+ struct bt_hog *hog = req->user_data;
+ uint8_t value[HOG_REPORT_MAP_MAX_SIZE];
+ ssize_t vlen;
+
+ destroy_gatt_req(req);
+
+ DBG("HoG inspecting report map");
+
+ if (status != 0) {
+ error("Report Map read failed: %s", att_ecode2str(status));
+ return;
+ }
+
+ vlen = dec_read_resp(pdu, plen, value, sizeof(value));
+ if (vlen < 0) {
+ error("ATT protocol error");
+ return;
+ }
+
+ uhid_create(hog, value, vlen);
+
+ /* Cache the report map if gatt_db is available */
+ if (hog->report_map_attr) {
+ gatt_db_attribute_write(hog->report_map_attr, 0, value, vlen, 0,
+ NULL, db_report_map_write_value_cb,
+ NULL);
+ }
+}
+
static void info_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
gpointer user_data)
{
@@ -1184,6 +1214,8 @@ static void hog_free(void *data)
g_free(hog->name);
g_free(hog->primary);
queue_destroy(hog->gatt_op, (void *) destroy_gatt_req);
+ if (hog->gatt_db)
+ gatt_db_unref(hog->gatt_db);
g_free(hog);
}
@@ -1269,12 +1301,32 @@ static void foreach_hog_external(struct gatt_db_attribute *attr,
external_report_reference_cb, hog);
}
+static void db_report_map_read_value_cb(struct gatt_db_attribute *attrib,
+ int err, const uint8_t *value,
+ size_t length, void *user_data)
+{
+ struct report_map *map = user_data;
+
+ if (err) {
+ error("Error reading report map from gatt db %s",
+ strerror(-err));
+ return;
+ }
+
+ if (!length)
+ return;
+
+ map->length = length < sizeof(map->value) ? length : sizeof(map->value);
+ memcpy(map->value, value, map->length);
+}
+
static void foreach_hog_chrc(struct gatt_db_attribute *attr, void *user_data)
{
struct bt_hog *hog = user_data;
bt_uuid_t uuid, report_uuid, report_map_uuid, info_uuid;
bt_uuid_t proto_mode_uuid, ctrlpt_uuid;
uint16_t handle, value_handle;
+ struct report_map report_map = {0};
gatt_db_attribute_get_char_data(attr, &handle, &value_handle, NULL,
NULL, &uuid);
@@ -1288,8 +1340,28 @@ static void foreach_hog_chrc(struct gatt_db_attribute *attr, void *user_data)
bt_uuid16_create(&report_map_uuid, HOG_REPORT_MAP_UUID);
if (!bt_uuid_cmp(&report_map_uuid, &uuid)) {
- read_char(hog, hog->attrib, value_handle, report_map_read_cb,
- hog);
+
+ if (hog->gatt_db) {
+ /* Try to read the cache of report map if available */
+ hog->report_map_attr = gatt_db_get_attribute(
+ hog->gatt_db,
+ value_handle);
+ gatt_db_attribute_read(hog->report_map_attr, 0,
+ BT_ATT_OP_READ_REQ, NULL,
+ db_report_map_read_value_cb,
+ &report_map);
+ }
+
+ if (report_map.length) {
+ /* Report map found in the cache, straight to creating
+ * UHID to optimize reconnection.
+ */
+ uhid_create(hog, report_map.value, report_map.length);
+ } else {
+ read_char(hog, hog->attrib, value_handle,
+ report_map_read_cb, hog);
+ }
+
gatt_db_service_foreach_desc(attr, foreach_hog_external, hog);
return;
}
@@ -1417,6 +1489,8 @@ struct bt_hog *bt_hog_new(int fd, const char *name, uint16_t vendor,
hog->dis = bt_dis_new(db);
bt_dis_set_notification(hog->dis, dis_notify, hog);
}
+
+ hog->gatt_db = gatt_db_ref(db);
}
return bt_hog_ref(hog);
@@ -1612,9 +1686,14 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt)
hog->primary->range.start,
hog->primary->range.end, NULL,
char_discovered_cb, hog);
- return true;
}
+ if (!hog->uhid_created)
+ return true;
+
+ /* If UHID is already created, set up the report value handlers to
+ * optimize reconnection.
+ */
for (l = hog->reports; l; l = l->next) {
struct report *r = l->data;
@@ -1627,6 +1706,29 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt)
return true;
}
+static void uhid_destroy(struct bt_hog *hog)
+{
+ int err;
+ struct uhid_event ev;
+
+ if (!hog->uhid_created)
+ return;
+
+ bt_uhid_unregister_all(hog->uhid);
+
+ memset(&ev, 0, sizeof(ev));
+ ev.type = UHID_DESTROY;
+
+ err = bt_uhid_send(hog->uhid, &ev);
+
+ if (err < 0) {
+ error("bt_uhid_send: %s", strerror(-err));
+ return;
+ }
+
+ hog->uhid_created = false;
+}
+
void bt_hog_detach(struct bt_hog *hog)
{
GSList *l;
@@ -1660,6 +1762,7 @@ void bt_hog_detach(struct bt_hog *hog)
queue_foreach(hog->gatt_op, (void *) cancel_gatt_req, NULL);
g_attrib_unref(hog->attrib);
hog->attrib = NULL;
+ uhid_destroy(hog);
}
int bt_hog_set_control_point(struct bt_hog *hog, bool suspend)
--
2.29.2
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=401857
---Test result---
##############################
Test: CheckPatch - PASS
##############################
Test: CheckGitLint - PASS
##############################
Test: CheckBuild - PASS
##############################
Test: MakeCheck - PASS
---
Regards,
Linux Bluetooth