* [PATCH BlueZ 1/4] shared/gatt-client: Speed up discovery
@ 2015-06-08 13:35 Luiz Augusto von Dentz
2015-06-08 13:35 ` [PATCH BlueZ 2/4] shared/gatt-client: Consolidate service changed registration Luiz Augusto von Dentz
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2015-06-08 13:35 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This tune the discovery to take advantage of the cached database whenever
possible, so instead of clearing the whole db if the device is not paired
the code now make the remote services active once they are discovered
and with that bt_gatt_client can then skip discovering characteristics
and descriptors of services that have not changed since last connection
which improves the reconnecting speed for any device regardless if the
device was paired or not.
---
src/device.c | 18 +++++++-----------
src/gatt-client.c | 17 +++++------------
src/shared/gatt-client.c | 44 ++++++++++++++++++++++++++++----------------
src/shared/gatt-db.c | 33 +++++++++++++++++++++++++++------
4 files changed, 67 insertions(+), 45 deletions(-)
diff --git a/src/device.c b/src/device.c
index ce96ab5..3ef0340 100644
--- a/src/device.c
+++ b/src/device.c
@@ -541,17 +541,6 @@ static void gatt_client_cleanup(struct btd_device *device)
bt_gatt_client_set_ready_handler(device->client, NULL, NULL, NULL);
bt_gatt_client_unref(device->client);
device->client = NULL;
-
- /*
- * TODO: Once GATT over BR/EDR is properly supported, we should check
- * the bonding state for the correct bearer based on the transport over
- * which GATT is being done.
- */
- if (device->le_state.bonded)
- return;
-
- gatt_db_clear(device->db);
- device->gatt_cache_used = false;
}
static void gatt_server_cleanup(struct btd_device *device)
@@ -2862,6 +2851,9 @@ static void dev_probe_gatt(struct btd_profile *p, void *user_data)
return;
}
+ /* Mark service as active to skip discovering it again */
+ gatt_db_service_set_active(data->cur_attr, true);
+
/* Mark service as claimed */
gatt_db_service_set_claimed(data->cur_attr, true);
@@ -2890,6 +2882,8 @@ static void dev_probe_gatt_profile(struct gatt_db_attribute *attr,
/* Don't probe the profiles if a matching service already exists. */
if (find_service_with_uuid(data->dev->services, data->cur_uuid)) {
+ /* Mark service as active to skip discovering it again */
+ gatt_db_service_set_active(data->cur_attr, true);
/* Mark the service as claimed by the existing profile. */
gatt_db_service_set_claimed(data->cur_attr, true);
return;
@@ -4181,6 +4175,8 @@ static void register_gatt_services(struct btd_device *device)
device_svc_resolved(device, device->bdaddr_type, 0);
}
+static void gatt_client_init(struct btd_device *device);
+
static void gatt_client_ready_cb(bool success, uint8_t att_ecode,
void *user_data)
{
diff --git a/src/gatt-client.c b/src/gatt-client.c
index 162bcac..3356ee4 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -1535,6 +1535,9 @@ static struct service *service_create(struct gatt_db_attribute *attr,
DBG("Exported GATT service: %s", service->path);
+ /* Set service active so we can skip discovering next time */
+ gatt_db_service_set_active(attr, true);
+
return service;
}
@@ -1941,22 +1944,12 @@ void btd_gatt_client_disconnected(struct btd_gatt_client *client)
DBG("Device disconnected. Cleaning up.");
/*
- * Remove all services. We'll recreate them when a new bt_gatt_client
- * becomes ready. Leave the services around if the device is bonded.
* TODO: Once GATT over BR/EDR is properly supported, we should pass the
* correct bdaddr_type based on the transport over which GATT is being
* done.
*/
- if (!device_is_bonded(client->device, BDADDR_LE_PUBLIC)) {
- DBG("Device not bonded. Removing exported services.");
- queue_remove_all(client->services, NULL, NULL,
- unregister_service);
- } else {
- DBG("Device is bonded. Keeping exported services up.");
- queue_foreach(client->all_notify_clients, clear_notify_id,
- NULL);
- queue_foreach(client->services, cancel_ops, client->gatt);
- }
+ queue_foreach(client->all_notify_clients, clear_notify_id, NULL);
+ queue_foreach(client->services, cancel_ops, client->gatt);
bt_gatt_client_unref(client->gatt);
client->gatt = NULL;
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 7bc3b71..feb30f6 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -892,13 +892,21 @@ static void discover_secondary_cb(bool success, uint8_t att_ecode,
attr = gatt_db_insert_service(client->db, start, &uuid, false,
end - start + 1);
if (!attr) {
- util_debug(client->debug_callback, client->debug_data,
- "Failed to create service");
- success = false;
- goto done;
+ gatt_db_clear_range(client->db, start, end);
+ attr = gatt_db_insert_service(client->db, start, &uuid,
+ false, end - start + 1);
+ if (!attr) {
+ util_debug(client->debug_callback,
+ client->debug_data,
+ "Failed to store service");
+ success = false;
+ goto done;
+ }
}
- queue_push_tail(op->pending_svcs, attr);
+ /* Skip if service already active */
+ if (!gatt_db_service_get_active(attr))
+ queue_push_tail(op->pending_svcs, attr);
}
next:
@@ -906,8 +914,10 @@ next:
attr = queue_pop_head(op->pending_svcs);
/* Complete with success if queue is empty */
- if (!attr)
+ if (!attr) {
+ success = true;
goto done;
+ }
/*
* Store the service in the tmp queue to be reused during
@@ -981,13 +991,21 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
attr = gatt_db_insert_service(client->db, start, &uuid, true,
end - start + 1);
if (!attr) {
- util_debug(client->debug_callback, client->debug_data,
+ gatt_db_clear_range(client->db, start, end);
+ attr = gatt_db_insert_service(client->db, start, &uuid,
+ true, end - start + 1);
+ if (!attr) {
+ util_debug(client->debug_callback,
+ client->debug_data,
"Failed to store service");
- success = false;
- goto done;
+ success = false;
+ goto done;
+ }
}
- queue_push_tail(op->pending_svcs, attr);
+ /* Skip if service already active */
+ if (!gatt_db_service_get_active(attr))
+ queue_push_tail(op->pending_svcs, attr);
}
secondary:
@@ -1044,12 +1062,6 @@ static void exchange_mtu_cb(bool success, uint8_t att_ecode, void *user_data)
"MTU exchange complete, with MTU: %u",
bt_att_get_mtu(client->att));
- /* Don't do discovery if the database was pre-populated */
- if (!gatt_db_isempty(client->db)) {
- op->complete_func(op, true, 0);
- return;
- }
-
client->discovery_req = bt_gatt_discover_all_primary_services(
client->att, NULL,
discover_primary_cb,
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index 2b2090c..5e1537e 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -486,7 +486,8 @@ bool gatt_db_clear_range(struct gatt_db *db, uint16_t start_handle,
return true;
}
-static bool find_insert_loc(struct gatt_db *db, uint16_t start, uint16_t end,
+static struct gatt_db_service *find_insert_loc(struct gatt_db *db,
+ uint16_t start, uint16_t end,
struct gatt_db_service **after)
{
const struct queue_entry *services_entry;
@@ -503,19 +504,19 @@ static bool find_insert_loc(struct gatt_db *db, uint16_t start, uint16_t end,
gatt_db_service_get_handles(service, &cur_start, &cur_end);
if (start >= cur_start && start <= cur_end)
- return false;
+ return service;
if (end >= cur_start && end <= cur_end)
- return false;
+ return service;
if (end < cur_start)
- return true;
+ return NULL;
*after = service;
services_entry = services_entry->next;
}
- return true;
+ return NULL;
}
struct gatt_db_attribute *gatt_db_insert_service(struct gatt_db *db,
@@ -534,8 +535,28 @@ struct gatt_db_attribute *gatt_db_insert_service(struct gatt_db *db,
if (num_handles < 1 || (handle + num_handles - 1) > UINT16_MAX)
return NULL;
- if (!find_insert_loc(db, handle, handle + num_handles - 1, &after))
+ service = find_insert_loc(db, handle, handle + num_handles - 1, &after);
+ if (service) {
+ const bt_uuid_t *type;
+ bt_uuid_t value;
+
+ if (primary)
+ type = &primary_service_uuid;
+ else
+ type = &secondary_service_uuid;
+
+ gatt_db_attribute_get_service_uuid(service->attributes[0],
+ &value);
+
+ /* Check if service match */
+ if (!bt_uuid_cmp(&service->attributes[0]->uuid, type) &&
+ !bt_uuid_cmp(&value, uuid) &&
+ service->num_handles == num_handles &&
+ service->attributes[0]->handle == handle)
+ return service->attributes[0];
+
return NULL;
+ }
service = gatt_db_service_create(uuid, handle, primary, num_handles);
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH BlueZ 2/4] shared/gatt-client: Consolidate service changed registration
2015-06-08 13:35 [PATCH BlueZ 1/4] shared/gatt-client: Speed up discovery Luiz Augusto von Dentz
@ 2015-06-08 13:35 ` Luiz Augusto von Dentz
2015-06-08 13:35 ` [PATCH BlueZ 3/4] shared/gatt-client: Simplify ready flag Luiz Augusto von Dentz
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2015-06-08 13:35 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This rework the code using service changed registration so it becomes
reusable by different part of code.
---
src/shared/gatt-client.c | 495 +++++++++++++++++++++++------------------------
1 file changed, 239 insertions(+), 256 deletions(-)
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index feb30f6..ee3f984 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1085,28 +1085,193 @@ struct service_changed_op {
uint16_t end_handle;
};
-static void service_changed_reregister_cb(uint16_t att_ecode, void *user_data)
+static void process_service_changed(struct bt_gatt_client *client,
+ uint16_t start_handle,
+ uint16_t end_handle);
+static void service_changed_cb(uint16_t value_handle, const uint8_t *value,
+ uint16_t length, void *user_data);
+
+static void complete_notify_request(void *data)
{
- struct bt_gatt_client *client = user_data;
+ struct notify_data *notify_data = data;
+
+ /* Increment the per-characteristic ref count of notify handlers */
+ __sync_fetch_and_add(¬ify_data->chrc->notify_count, 1);
+
+ notify_data->att_id = 0;
+ notify_data->callback(0, notify_data->user_data);
+}
+
+static bool notify_data_write_ccc(struct notify_data *notify_data, bool enable,
+ bt_att_response_func_t callback)
+{
+ uint8_t pdu[4];
+ unsigned int att_id;
+
+ assert(notify_data->chrc->ccc_handle);
+ memset(pdu, 0, sizeof(pdu));
+ put_le16(notify_data->chrc->ccc_handle, pdu);
+
+ if (enable) {
+ /* Try to enable notifications and/or indications based on
+ * whatever the characteristic supports.
+ */
+ if (notify_data->chrc->properties & BT_GATT_CHRC_PROP_NOTIFY)
+ pdu[2] = 0x01;
+
+ if (notify_data->chrc->properties & BT_GATT_CHRC_PROP_INDICATE)
+ pdu[2] |= 0x02;
+
+ if (!pdu[2])
+ return false;
+ }
+
+ att_id = bt_att_send(notify_data->client->att, BT_ATT_OP_WRITE_REQ,
+ pdu, sizeof(pdu), callback,
+ notify_data_ref(notify_data),
+ notify_data_unref);
+ notify_data->chrc->ccc_write_id = notify_data->att_id = att_id;
+
+ return !!att_id;
+}
+
+static uint8_t process_error(const void *pdu, uint16_t length)
+{
+ const struct bt_att_pdu_error_rsp *error_pdu;
+
+ if (!pdu || length != sizeof(struct bt_att_pdu_error_rsp))
+ return 0;
+
+ error_pdu = pdu;
+
+ return error_pdu->ecode;
+}
+
+static void enable_ccc_callback(uint8_t opcode, const void *pdu,
+ uint16_t length, void *user_data)
+{
+ struct notify_data *notify_data = user_data;
+ uint16_t att_ecode;
+
+ assert(!notify_data->chrc->notify_count);
+ assert(notify_data->chrc->ccc_write_id);
+
+ notify_data->chrc->ccc_write_id = 0;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP) {
+ att_ecode = process_error(pdu, length);
+
+ /* Failed to enable. Complete the current request and move on to
+ * the next one in the queue. If there was an error sending the
+ * write request, then just move on to the next queued entry.
+ */
+ queue_remove(notify_data->client->notify_list, notify_data);
+ notify_data->callback(att_ecode, notify_data->user_data);
+
+ while ((notify_data = queue_pop_head(
+ notify_data->chrc->reg_notify_queue))) {
+
+ if (notify_data_write_ccc(notify_data, true,
+ enable_ccc_callback))
+ return;
+ }
- if (!att_ecode) {
- util_debug(client->debug_callback, client->debug_data,
- "Re-registered handler for \"Service Changed\" after "
- "change in GATT service");
- client->svc_chngd_registered = true;
return;
}
- util_debug(client->debug_callback, client->debug_data,
- "Failed to register handler for \"Service Changed\"");
- client->svc_chngd_ind_id = 0;
+ /* Success! Report success for all remaining requests. */
+ bt_gatt_client_ref(notify_data->client);
+
+ complete_notify_request(notify_data);
+ queue_remove_all(notify_data->chrc->reg_notify_queue, NULL, NULL,
+ complete_notify_request);
+
+ bt_gatt_client_unref(notify_data->client);
}
-static void process_service_changed(struct bt_gatt_client *client,
- uint16_t start_handle,
- uint16_t end_handle);
-static void service_changed_cb(uint16_t value_handle, const uint8_t *value,
- uint16_t length, void *user_data);
+static bool match_notify_chrc_value_handle(const void *a, const void *b)
+{
+ const struct notify_chrc *chrc = a;
+ uint16_t value_handle = PTR_TO_UINT(b);
+
+ return chrc->value_handle == value_handle;
+}
+
+static unsigned int register_notify(struct bt_gatt_client *client,
+ uint16_t handle,
+ bt_gatt_client_register_callback_t callback,
+ bt_gatt_client_notify_callback_t notify,
+ void *user_data,
+ bt_gatt_client_destroy_func_t destroy)
+{
+ struct notify_data *notify_data;
+ struct notify_chrc *chrc = NULL;
+
+ /* Check if a characteristic ref count has been started already */
+ chrc = queue_find(client->notify_chrcs, match_notify_chrc_value_handle,
+ UINT_TO_PTR(handle));
+
+ if (!chrc) {
+ /*
+ * Create an entry if the characteristic is known and has a CCC
+ * descriptor.
+ */
+ chrc = notify_chrc_create(client, handle);
+ if (!chrc)
+ return 0;
+ }
+
+ /* Fail if we've hit the maximum allowed notify sessions */
+ if (chrc->notify_count == INT_MAX)
+ return 0;
+
+ notify_data = new0(struct notify_data, 1);
+ if (!notify_data)
+ return 0;
+
+ notify_data->client = client;
+ notify_data->ref_count = 1;
+ notify_data->chrc = chrc;
+ notify_data->callback = callback;
+ notify_data->notify = notify;
+ notify_data->user_data = user_data;
+ notify_data->destroy = destroy;
+
+ /* Add the handler to the bt_gatt_client's general list */
+ queue_push_tail(client->notify_list, notify_data);
+
+ /* Assign an ID to the handler. */
+ if (client->next_reg_id < 1)
+ client->next_reg_id = 1;
+
+ notify_data->id = client->next_reg_id++;
+
+ /*
+ * If a write to the CCC descriptor is in progress, then queue this
+ * request.
+ */
+ if (chrc->ccc_write_id) {
+ queue_push_tail(chrc->reg_notify_queue, notify_data);
+ return notify_data->id;
+ }
+
+ /*
+ * If the ref count is not zero, then notifications are already enabled.
+ */
+ if (chrc->notify_count > 0 || !chrc->ccc_handle) {
+ complete_notify_request(notify_data);
+ return notify_data->id;
+ }
+
+ /* Write to the CCC descriptor */
+ if (!notify_data_write_ccc(notify_data, true, enable_ccc_callback)) {
+ queue_remove(client->notify_list, notify_data);
+ free(notify_data);
+ return 0;
+ }
+
+ return notify_data->id;
+}
static void get_first_attribute(struct gatt_db_attribute *attrib,
void *user_data)
@@ -1119,6 +1284,61 @@ static void get_first_attribute(struct gatt_db_attribute *attrib,
*stored = attrib;
}
+static void service_changed_register_cb(uint16_t att_ecode, void *user_data)
+{
+ bool success;
+ struct bt_gatt_client *client = user_data;
+
+ if (att_ecode) {
+ util_debug(client->debug_callback, client->debug_data,
+ "Failed to register handler for \"Service Changed\"");
+ success = false;
+ client->svc_chngd_ind_id = 0;
+ goto done;
+ }
+
+ client->svc_chngd_registered = true;
+ success = true;
+ util_debug(client->debug_callback, client->debug_data,
+ "Registered handler for \"Service Changed\": %u",
+ client->svc_chngd_ind_id);
+
+done:
+ if (!client->ready) {
+ client->ready = success;
+ notify_client_ready(client, success, att_ecode);
+ }
+}
+
+static bool register_service_changed(struct bt_gatt_client *client)
+{
+ bt_uuid_t uuid;
+ struct gatt_db_attribute *attr = NULL;
+
+ bt_uuid16_create(&uuid, SVC_CHNGD_UUID);
+
+ if (client->svc_chngd_ind_id)
+ return true;
+
+ gatt_db_find_by_type(client->db, 0x0001, 0xffff, &uuid,
+ get_first_attribute, &attr);
+ if (!attr)
+ return true;
+
+ /*
+ * Register an indication handler for the "Service Changed"
+ * characteristic and report ready only if the handler is registered
+ * successfully.
+ */
+ client->svc_chngd_ind_id = register_notify(client,
+ gatt_db_attribute_get_handle(attr),
+ service_changed_register_cb,
+ service_changed_cb,
+ client, NULL);
+
+ return client->svc_chngd_ind_id ? true : false;
+}
+
static void service_changed_complete(struct discovery_op *op, bool success,
uint8_t att_ecode)
{
@@ -1126,8 +1346,6 @@ static void service_changed_complete(struct discovery_op *op, bool success,
struct service_changed_op *next_sc_op;
uint16_t start_handle = op->start;
uint16_t end_handle = op->end;
- struct gatt_db_attribute *attr = NULL;
- bt_uuid_t uuid;
client->in_svc_chngd = false;
@@ -1153,23 +1371,7 @@ static void service_changed_complete(struct discovery_op *op, bool success,
return;
}
- bt_uuid16_create(&uuid, SVC_CHNGD_UUID);
-
- gatt_db_find_by_type(client->db, start_handle, end_handle, &uuid,
- get_first_attribute, &attr);
- if (!attr)
- return;
-
- /* The GATT service was modified. Re-register the handler for
- * indications from the "Service Changed" characteristic.
- */
- client->svc_chngd_registered = false;
- client->svc_chngd_ind_id = bt_gatt_client_register_notify(client,
- gatt_db_attribute_get_handle(attr),
- service_changed_reregister_cb,
- service_changed_cb,
- client, NULL);
- if (client->svc_chngd_ind_id)
+ if (register_service_changed(client))
return;
util_debug(client->debug_callback, client->debug_data,
@@ -1262,69 +1464,21 @@ static void service_changed_cb(uint16_t value_handle, const uint8_t *value,
queue_push_tail(client->svc_chngd_queue, op);
}
-static void service_changed_register_cb(uint16_t att_ecode, void *user_data)
-{
- bool success;
- struct bt_gatt_client *client = user_data;
-
- if (att_ecode) {
- util_debug(client->debug_callback, client->debug_data,
- "Failed to register handler for \"Service Changed\"");
- success = false;
- client->svc_chngd_ind_id = 0;
- goto done;
- }
-
- client->svc_chngd_registered = true;
- client->ready = true;
- success = true;
- util_debug(client->debug_callback, client->debug_data,
- "Registered handler for \"Service Changed\": %u",
- client->svc_chngd_ind_id);
-
-done:
- notify_client_ready(client, success, att_ecode);
-}
-
static void init_complete(struct discovery_op *op, bool success,
uint8_t att_ecode)
{
struct bt_gatt_client *client = op->client;
- struct gatt_db_attribute *attr = NULL;
- bt_uuid_t uuid;
client->in_init = false;
if (!success)
goto fail;
- bt_uuid16_create(&uuid, SVC_CHNGD_UUID);
-
- gatt_db_find_by_type(client->db, 0x0001, 0xffff, &uuid,
- get_first_attribute, &attr);
- if (!attr) {
+ if (register_service_changed(client)) {
client->ready = true;
goto done;
}
- /* Register an indication handler for the "Service Changed"
- * characteristic and report ready only if the handler is registered
- * successfully. Temporarily set "ready" to true so that we can register
- * the handler using the existing framework.
- */
- client->ready = true;
- client->svc_chngd_ind_id = bt_gatt_client_register_notify(client,
- gatt_db_attribute_get_handle(attr),
- service_changed_register_cb,
- service_changed_cb,
- client, NULL);
-
- if (!client->svc_chngd_registered)
- client->ready = false;
-
- if (client->svc_chngd_ind_id)
- return;
-
util_debug(client->debug_callback, client->debug_data,
"Failed to register handler for \"Service Changed\"");
success = false;
@@ -1379,104 +1533,6 @@ struct pdu_data {
uint16_t length;
};
-static void complete_notify_request(void *data)
-{
- struct notify_data *notify_data = data;
-
- /* Increment the per-characteristic ref count of notify handlers */
- __sync_fetch_and_add(¬ify_data->chrc->notify_count, 1);
-
- notify_data->att_id = 0;
- notify_data->callback(0, notify_data->user_data);
-}
-
-static bool notify_data_write_ccc(struct notify_data *notify_data, bool enable,
- bt_att_response_func_t callback)
-{
- uint8_t pdu[4];
- unsigned int att_id;
-
- assert(notify_data->chrc->ccc_handle);
- memset(pdu, 0, sizeof(pdu));
- put_le16(notify_data->chrc->ccc_handle, pdu);
-
- if (enable) {
- /* Try to enable notifications and/or indications based on
- * whatever the characteristic supports.
- */
- if (notify_data->chrc->properties & BT_GATT_CHRC_PROP_NOTIFY)
- pdu[2] = 0x01;
-
- if (notify_data->chrc->properties & BT_GATT_CHRC_PROP_INDICATE)
- pdu[2] |= 0x02;
-
- if (!pdu[2])
- return false;
- }
-
- att_id = bt_att_send(notify_data->client->att, BT_ATT_OP_WRITE_REQ,
- pdu, sizeof(pdu), callback,
- notify_data_ref(notify_data),
- notify_data_unref);
- notify_data->chrc->ccc_write_id = notify_data->att_id = att_id;
-
- return !!att_id;
-}
-
-static uint8_t process_error(const void *pdu, uint16_t length)
-{
- const struct bt_att_pdu_error_rsp *error_pdu;
-
- if (!pdu || length != sizeof(struct bt_att_pdu_error_rsp))
- return 0;
-
- error_pdu = pdu;
-
- return error_pdu->ecode;
-}
-
-static void enable_ccc_callback(uint8_t opcode, const void *pdu,
- uint16_t length, void *user_data)
-{
- struct notify_data *notify_data = user_data;
- uint16_t att_ecode;
-
- assert(!notify_data->chrc->notify_count);
- assert(notify_data->chrc->ccc_write_id);
-
- notify_data->chrc->ccc_write_id = 0;
-
- if (opcode == BT_ATT_OP_ERROR_RSP) {
- att_ecode = process_error(pdu, length);
-
- /* Failed to enable. Complete the current request and move on to
- * the next one in the queue. If there was an error sending the
- * write request, then just move on to the next queued entry.
- */
- queue_remove(notify_data->client->notify_list, notify_data);
- notify_data->callback(att_ecode, notify_data->user_data);
-
- while ((notify_data = queue_pop_head(
- notify_data->chrc->reg_notify_queue))) {
-
- if (notify_data_write_ccc(notify_data, true,
- enable_ccc_callback))
- return;
- }
-
- return;
- }
-
- /* Success! Report success for all remaining requests. */
- bt_gatt_client_ref(notify_data->client);
-
- complete_notify_request(notify_data);
- queue_remove_all(notify_data->chrc->reg_notify_queue, NULL, NULL,
- complete_notify_request);
-
- bt_gatt_client_unref(notify_data->client);
-}
-
static void disable_ccc_callback(uint8_t opcode, const void *pdu,
uint16_t length, void *user_data)
{
@@ -2900,14 +2956,6 @@ unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
return id;
}
-static bool match_notify_chrc_value_handle(const void *a, const void *b)
-{
- const struct notify_chrc *chrc = a;
- uint16_t value_handle = PTR_TO_UINT(b);
-
- return chrc->value_handle == value_handle;
-}
-
unsigned int bt_gatt_client_register_notify(struct bt_gatt_client *client,
uint16_t chrc_value_handle,
bt_gatt_client_register_callback_t callback,
@@ -2915,79 +2963,14 @@ unsigned int bt_gatt_client_register_notify(struct bt_gatt_client *client,
void *user_data,
bt_gatt_client_destroy_func_t destroy)
{
- struct notify_data *notify_data;
- struct notify_chrc *chrc = NULL;
-
if (!client || !client->db || !chrc_value_handle || !callback)
return 0;
if (!bt_gatt_client_is_ready(client) || client->in_svc_chngd)
return 0;
- /* Check if a characteristic ref count has been started already */
- chrc = queue_find(client->notify_chrcs, match_notify_chrc_value_handle,
- UINT_TO_PTR(chrc_value_handle));
-
- if (!chrc) {
- /*
- * Create an entry if the characteristic is known and has a CCC
- * descriptor.
- */
- chrc = notify_chrc_create(client, chrc_value_handle);
- if (!chrc)
- return 0;
- }
-
- /* Fail if we've hit the maximum allowed notify sessions */
- if (chrc->notify_count == INT_MAX)
- return 0;
-
- notify_data = new0(struct notify_data, 1);
- if (!notify_data)
- return 0;
-
- notify_data->client = client;
- notify_data->ref_count = 1;
- notify_data->chrc = chrc;
- notify_data->callback = callback;
- notify_data->notify = notify;
- notify_data->user_data = user_data;
- notify_data->destroy = destroy;
-
- /* Add the handler to the bt_gatt_client's general list */
- queue_push_tail(client->notify_list, notify_data);
-
- /* Assign an ID to the handler. */
- if (client->next_reg_id < 1)
- client->next_reg_id = 1;
-
- notify_data->id = client->next_reg_id++;
-
- /*
- * If a write to the CCC descriptor is in progress, then queue this
- * request.
- */
- if (chrc->ccc_write_id) {
- queue_push_tail(chrc->reg_notify_queue, notify_data);
- return notify_data->id;
- }
-
- /*
- * If the ref count is not zero, then notifications are already enabled.
- */
- if (chrc->notify_count > 0 || !chrc->ccc_handle) {
- complete_notify_request(notify_data);
- return notify_data->id;
- }
-
- /* Write to the CCC descriptor */
- if (!notify_data_write_ccc(notify_data, true, enable_ccc_callback)) {
- queue_remove(client->notify_list, notify_data);
- free(notify_data);
- return 0;
- }
-
- return notify_data->id;
+ return register_notify(client, chrc_value_handle, callback, notify,
+ user_data, destroy);
}
bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client,
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH BlueZ 3/4] shared/gatt-client: Simplify ready flag
2015-06-08 13:35 [PATCH BlueZ 1/4] shared/gatt-client: Speed up discovery Luiz Augusto von Dentz
2015-06-08 13:35 ` [PATCH BlueZ 2/4] shared/gatt-client: Consolidate service changed registration Luiz Augusto von Dentz
@ 2015-06-08 13:35 ` Luiz Augusto von Dentz
2015-06-08 13:35 ` [PATCH BlueZ 4/4] shared/gatt-client: Don't discovery secondaries if there is no primary Luiz Augusto von Dentz
2015-06-18 19:46 ` [PATCH BlueZ 1/4] shared/gatt-client: Speed up discovery Arman Uguray
3 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2015-06-08 13:35 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This makes notify_client_ready takes care of setting ready flag so the
caller code don't have to set it before calling notify_client_ready.
---
src/shared/gatt-client.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index ee3f984..724fc53 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1031,10 +1031,11 @@ done:
static void notify_client_ready(struct bt_gatt_client *client, bool success,
uint8_t att_ecode)
{
- if (!client->ready_callback)
+ if (!client->ready_callback || client->ready)
return;
bt_gatt_client_ref(client);
+ client->ready = success;
client->ready_callback(success, att_ecode, client->ready_data);
bt_gatt_client_unref(client);
}
@@ -1304,10 +1305,7 @@ static void service_changed_register_cb(uint16_t att_ecode, void *user_data)
client->svc_chngd_ind_id);
done:
- if (!client->ready) {
- client->ready = success;
- notify_client_ready(client, success, att_ecode);
- }
+ notify_client_ready(client, success, att_ecode);
}
static bool register_service_changed(struct bt_gatt_client *client)
@@ -1474,10 +1472,8 @@ static void init_complete(struct discovery_op *op, bool success,
if (!success)
goto fail;
- if (register_service_changed(client)) {
- client->ready = true;
+ if (register_service_changed(client))
goto done;
- }
util_debug(client->debug_callback, client->debug_data,
"Failed to register handler for \"Service Changed\"");
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH BlueZ 4/4] shared/gatt-client: Don't discovery secondaries if there is no primary
2015-06-08 13:35 [PATCH BlueZ 1/4] shared/gatt-client: Speed up discovery Luiz Augusto von Dentz
2015-06-08 13:35 ` [PATCH BlueZ 2/4] shared/gatt-client: Consolidate service changed registration Luiz Augusto von Dentz
2015-06-08 13:35 ` [PATCH BlueZ 3/4] shared/gatt-client: Simplify ready flag Luiz Augusto von Dentz
@ 2015-06-08 13:35 ` Luiz Augusto von Dentz
2015-06-18 19:46 ` [PATCH BlueZ 1/4] shared/gatt-client: Speed up discovery Arman Uguray
3 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2015-06-08 13:35 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
The spec state that a secondary needs to referenced by at least one
primary, Version 4.2 [Vol 1, Part A] page 101:
'A secondary service is a service that provides auxiliary
functionality of a device and is referenced from at least one
primary service on the device.'
---
src/shared/gatt-client.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 724fc53..7e9d550 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1009,6 +1009,15 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
}
secondary:
+ /*
+ * Version 4.2 [Vol 1, Part A] page 101:
+ * A secondary service is a service that provides auxiliary
+ * functionality of a device and is referenced from at least one
+ * primary service on the device.
+ */
+ if (queue_isempty(op->pending_svcs))
+ goto done;
+
/* Discover secondary services */
client->discovery_req = bt_gatt_discover_secondary_services(client->att,
NULL, op->start, op->end,
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH BlueZ 1/4] shared/gatt-client: Speed up discovery
2015-06-08 13:35 [PATCH BlueZ 1/4] shared/gatt-client: Speed up discovery Luiz Augusto von Dentz
` (2 preceding siblings ...)
2015-06-08 13:35 ` [PATCH BlueZ 4/4] shared/gatt-client: Don't discovery secondaries if there is no primary Luiz Augusto von Dentz
@ 2015-06-18 19:46 ` Arman Uguray
2015-06-18 20:57 ` Luiz Augusto von Dentz
3 siblings, 1 reply; 6+ messages in thread
From: Arman Uguray @ 2015-06-18 19:46 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: BlueZ development
Hi Luiz,
> On Mon, Jun 8, 2015 at 6:35 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This tune the discovery to take advantage of the cached database whenever
> possible, so instead of clearing the whole db if the device is not paired
> the code now make the remote services active once they are discovered
> and with that bt_gatt_client can then skip discovering characteristics
> and descriptors of services that have not changed since last connection
> which improves the reconnecting speed for any device regardless if the
> device was paired or not.
IIRC there's no guarantee that the client will receive a Service
Changed indication on reconnection if the devices are not bonded. How
does this guarantee that the service doesn't need re-discovery?
> ---
> src/device.c | 18 +++++++-----------
> src/gatt-client.c | 17 +++++------------
> src/shared/gatt-client.c | 44 ++++++++++++++++++++++++++++----------------
> src/shared/gatt-db.c | 33 +++++++++++++++++++++++++++------
> 4 files changed, 67 insertions(+), 45 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index ce96ab5..3ef0340 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -541,17 +541,6 @@ static void gatt_client_cleanup(struct btd_device *device)
> bt_gatt_client_set_ready_handler(device->client, NULL, NULL, NULL);
> bt_gatt_client_unref(device->client);
> device->client = NULL;
> -
> - /*
> - * TODO: Once GATT over BR/EDR is properly supported, we should check
> - * the bonding state for the correct bearer based on the transport over
> - * which GATT is being done.
> - */
> - if (device->le_state.bonded)
> - return;
> -
> - gatt_db_clear(device->db);
> - device->gatt_cache_used = false;
> }
>
> static void gatt_server_cleanup(struct btd_device *device)
> @@ -2862,6 +2851,9 @@ static void dev_probe_gatt(struct btd_profile *p, void *user_data)
> return;
> }
>
> + /* Mark service as active to skip discovering it again */
> + gatt_db_service_set_active(data->cur_attr, true);
> +
> /* Mark service as claimed */
> gatt_db_service_set_claimed(data->cur_attr, true);
>
> @@ -2890,6 +2882,8 @@ static void dev_probe_gatt_profile(struct gatt_db_attribute *attr,
>
> /* Don't probe the profiles if a matching service already exists. */
> if (find_service_with_uuid(data->dev->services, data->cur_uuid)) {
> + /* Mark service as active to skip discovering it again */
> + gatt_db_service_set_active(data->cur_attr, true);
> /* Mark the service as claimed by the existing profile. */
> gatt_db_service_set_claimed(data->cur_attr, true);
> return;
> @@ -4181,6 +4175,8 @@ static void register_gatt_services(struct btd_device *device)
> device_svc_resolved(device, device->bdaddr_type, 0);
> }
>
> +static void gatt_client_init(struct btd_device *device);
> +
> static void gatt_client_ready_cb(bool success, uint8_t att_ecode,
> void *user_data)
> {
> diff --git a/src/gatt-client.c b/src/gatt-client.c
> index 162bcac..3356ee4 100644
> --- a/src/gatt-client.c
> +++ b/src/gatt-client.c
> @@ -1535,6 +1535,9 @@ static struct service *service_create(struct gatt_db_attribute *attr,
>
> DBG("Exported GATT service: %s", service->path);
>
> + /* Set service active so we can skip discovering next time */
> + gatt_db_service_set_active(attr, true);
> +
> return service;
> }
>
> @@ -1941,22 +1944,12 @@ void btd_gatt_client_disconnected(struct btd_gatt_client *client)
> DBG("Device disconnected. Cleaning up.");
>
> /*
> - * Remove all services. We'll recreate them when a new bt_gatt_client
> - * becomes ready. Leave the services around if the device is bonded.
> * TODO: Once GATT over BR/EDR is properly supported, we should pass the
> * correct bdaddr_type based on the transport over which GATT is being
> * done.
> */
> - if (!device_is_bonded(client->device, BDADDR_LE_PUBLIC)) {
> - DBG("Device not bonded. Removing exported services.");
> - queue_remove_all(client->services, NULL, NULL,
> - unregister_service);
> - } else {
> - DBG("Device is bonded. Keeping exported services up.");
> - queue_foreach(client->all_notify_clients, clear_notify_id,
> - NULL);
> - queue_foreach(client->services, cancel_ops, client->gatt);
> - }
> + queue_foreach(client->all_notify_clients, clear_notify_id, NULL);
> + queue_foreach(client->services, cancel_ops, client->gatt);
>
> bt_gatt_client_unref(client->gatt);
> client->gatt = NULL;
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index 7bc3b71..feb30f6 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -892,13 +892,21 @@ static void discover_secondary_cb(bool success, uint8_t att_ecode,
> attr = gatt_db_insert_service(client->db, start, &uuid, false,
> end - start + 1);
> if (!attr) {
> - util_debug(client->debug_callback, client->debug_data,
> - "Failed to create service");
> - success = false;
> - goto done;
> + gatt_db_clear_range(client->db, start, end);
> + attr = gatt_db_insert_service(client->db, start, &uuid,
> + false, end - start + 1);
> + if (!attr) {
> + util_debug(client->debug_callback,
> + client->debug_data,
> + "Failed to store service");
> + success = false;
> + goto done;
> + }
> }
>
> - queue_push_tail(op->pending_svcs, attr);
> + /* Skip if service already active */
> + if (!gatt_db_service_get_active(attr))
> + queue_push_tail(op->pending_svcs, attr);
> }
>
> next:
> @@ -906,8 +914,10 @@ next:
> attr = queue_pop_head(op->pending_svcs);
>
> /* Complete with success if queue is empty */
> - if (!attr)
> + if (!attr) {
> + success = true;
> goto done;
> + }
>
> /*
> * Store the service in the tmp queue to be reused during
> @@ -981,13 +991,21 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
> attr = gatt_db_insert_service(client->db, start, &uuid, true,
> end - start + 1);
> if (!attr) {
> - util_debug(client->debug_callback, client->debug_data,
> + gatt_db_clear_range(client->db, start, end);
> + attr = gatt_db_insert_service(client->db, start, &uuid,
> + true, end - start + 1);
> + if (!attr) {
> + util_debug(client->debug_callback,
> + client->debug_data,
> "Failed to store service");
> - success = false;
> - goto done;
> + success = false;
> + goto done;
> + }
> }
>
> - queue_push_tail(op->pending_svcs, attr);
> + /* Skip if service already active */
> + if (!gatt_db_service_get_active(attr))
> + queue_push_tail(op->pending_svcs, attr);
> }
>
> secondary:
> @@ -1044,12 +1062,6 @@ static void exchange_mtu_cb(bool success, uint8_t att_ecode, void *user_data)
> "MTU exchange complete, with MTU: %u",
> bt_att_get_mtu(client->att));
>
> - /* Don't do discovery if the database was pre-populated */
> - if (!gatt_db_isempty(client->db)) {
> - op->complete_func(op, true, 0);
> - return;
> - }
> -
> client->discovery_req = bt_gatt_discover_all_primary_services(
> client->att, NULL,
> discover_primary_cb,
> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
> index 2b2090c..5e1537e 100644
> --- a/src/shared/gatt-db.c
> +++ b/src/shared/gatt-db.c
> @@ -486,7 +486,8 @@ bool gatt_db_clear_range(struct gatt_db *db, uint16_t start_handle,
> return true;
> }
>
> -static bool find_insert_loc(struct gatt_db *db, uint16_t start, uint16_t end,
> +static struct gatt_db_service *find_insert_loc(struct gatt_db *db,
> + uint16_t start, uint16_t end,
> struct gatt_db_service **after)
> {
> const struct queue_entry *services_entry;
> @@ -503,19 +504,19 @@ static bool find_insert_loc(struct gatt_db *db, uint16_t start, uint16_t end,
> gatt_db_service_get_handles(service, &cur_start, &cur_end);
>
> if (start >= cur_start && start <= cur_end)
> - return false;
> + return service;
>
> if (end >= cur_start && end <= cur_end)
> - return false;
> + return service;
>
> if (end < cur_start)
> - return true;
> + return NULL;
>
> *after = service;
> services_entry = services_entry->next;
> }
>
> - return true;
> + return NULL;
> }
>
> struct gatt_db_attribute *gatt_db_insert_service(struct gatt_db *db,
> @@ -534,8 +535,28 @@ struct gatt_db_attribute *gatt_db_insert_service(struct gatt_db *db,
> if (num_handles < 1 || (handle + num_handles - 1) > UINT16_MAX)
> return NULL;
>
> - if (!find_insert_loc(db, handle, handle + num_handles - 1, &after))
> + service = find_insert_loc(db, handle, handle + num_handles - 1, &after);
> + if (service) {
> + const bt_uuid_t *type;
> + bt_uuid_t value;
> +
> + if (primary)
> + type = &primary_service_uuid;
> + else
> + type = &secondary_service_uuid;
> +
> + gatt_db_attribute_get_service_uuid(service->attributes[0],
> + &value);
> +
> + /* Check if service match */
> + if (!bt_uuid_cmp(&service->attributes[0]->uuid, type) &&
> + !bt_uuid_cmp(&value, uuid) &&
> + service->num_handles == num_handles &&
> + service->attributes[0]->handle == handle)
> + return service->attributes[0];
> +
> return NULL;
> + }
>
> service = gatt_db_service_create(uuid, handle, primary, num_handles);
>
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks,
Arman
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH BlueZ 1/4] shared/gatt-client: Speed up discovery
2015-06-18 19:46 ` [PATCH BlueZ 1/4] shared/gatt-client: Speed up discovery Arman Uguray
@ 2015-06-18 20:57 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2015-06-18 20:57 UTC (permalink / raw)
To: Arman Uguray; +Cc: BlueZ development
Hi Arman,
On Thu, Jun 18, 2015 at 10:46 PM, Arman Uguray <armansito@chromium.org> wrote:
> Hi Luiz,
>
>> On Mon, Jun 8, 2015 at 6:35 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> This tune the discovery to take advantage of the cached database whenever
>> possible, so instead of clearing the whole db if the device is not paired
>> the code now make the remote services active once they are discovered
>> and with that bt_gatt_client can then skip discovering characteristics
>> and descriptors of services that have not changed since last connection
>> which improves the reconnecting speed for any device regardless if the
>> device was paired or not.
>
> IIRC there's no guarantee that the client will receive a Service
> Changed indication on reconnection if the devices are not bonded. How
> does this guarantee that the service doesn't need re-discovery?
The services will always be rediscovered, what we don't rediscover is
its attributes in case the service range have not changed.
>> ---
>> src/device.c | 18 +++++++-----------
>> src/gatt-client.c | 17 +++++------------
>> src/shared/gatt-client.c | 44 ++++++++++++++++++++++++++++----------------
>> src/shared/gatt-db.c | 33 +++++++++++++++++++++++++++------
>> 4 files changed, 67 insertions(+), 45 deletions(-)
>>
>> diff --git a/src/device.c b/src/device.c
>> index ce96ab5..3ef0340 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -541,17 +541,6 @@ static void gatt_client_cleanup(struct btd_device *device)
>> bt_gatt_client_set_ready_handler(device->client, NULL, NULL, NULL);
>> bt_gatt_client_unref(device->client);
>> device->client = NULL;
>> -
>> - /*
>> - * TODO: Once GATT over BR/EDR is properly supported, we should check
>> - * the bonding state for the correct bearer based on the transport over
>> - * which GATT is being done.
>> - */
>> - if (device->le_state.bonded)
>> - return;
>> -
>> - gatt_db_clear(device->db);
>> - device->gatt_cache_used = false;
>> }
>>
>> static void gatt_server_cleanup(struct btd_device *device)
>> @@ -2862,6 +2851,9 @@ static void dev_probe_gatt(struct btd_profile *p, void *user_data)
>> return;
>> }
>>
>> + /* Mark service as active to skip discovering it again */
>> + gatt_db_service_set_active(data->cur_attr, true);
>> +
>> /* Mark service as claimed */
>> gatt_db_service_set_claimed(data->cur_attr, true);
>>
>> @@ -2890,6 +2882,8 @@ static void dev_probe_gatt_profile(struct gatt_db_attribute *attr,
>>
>> /* Don't probe the profiles if a matching service already exists. */
>> if (find_service_with_uuid(data->dev->services, data->cur_uuid)) {
>> + /* Mark service as active to skip discovering it again */
>> + gatt_db_service_set_active(data->cur_attr, true);
>> /* Mark the service as claimed by the existing profile. */
>> gatt_db_service_set_claimed(data->cur_attr, true);
>> return;
>> @@ -4181,6 +4175,8 @@ static void register_gatt_services(struct btd_device *device)
>> device_svc_resolved(device, device->bdaddr_type, 0);
>> }
>>
>> +static void gatt_client_init(struct btd_device *device);
>> +
>> static void gatt_client_ready_cb(bool success, uint8_t att_ecode,
>> void *user_data)
>> {
>> diff --git a/src/gatt-client.c b/src/gatt-client.c
>> index 162bcac..3356ee4 100644
>> --- a/src/gatt-client.c
>> +++ b/src/gatt-client.c
>> @@ -1535,6 +1535,9 @@ static struct service *service_create(struct gatt_db_attribute *attr,
>>
>> DBG("Exported GATT service: %s", service->path);
>>
>> + /* Set service active so we can skip discovering next time */
>> + gatt_db_service_set_active(attr, true);
>> +
>> return service;
>> }
>>
>> @@ -1941,22 +1944,12 @@ void btd_gatt_client_disconnected(struct btd_gatt_client *client)
>> DBG("Device disconnected. Cleaning up.");
>>
>> /*
>> - * Remove all services. We'll recreate them when a new bt_gatt_client
>> - * becomes ready. Leave the services around if the device is bonded.
>> * TODO: Once GATT over BR/EDR is properly supported, we should pass the
>> * correct bdaddr_type based on the transport over which GATT is being
>> * done.
>> */
>> - if (!device_is_bonded(client->device, BDADDR_LE_PUBLIC)) {
>> - DBG("Device not bonded. Removing exported services.");
>> - queue_remove_all(client->services, NULL, NULL,
>> - unregister_service);
>> - } else {
>> - DBG("Device is bonded. Keeping exported services up.");
>> - queue_foreach(client->all_notify_clients, clear_notify_id,
>> - NULL);
>> - queue_foreach(client->services, cancel_ops, client->gatt);
>> - }
>> + queue_foreach(client->all_notify_clients, clear_notify_id, NULL);
>> + queue_foreach(client->services, cancel_ops, client->gatt);
>>
>> bt_gatt_client_unref(client->gatt);
>> client->gatt = NULL;
>> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
>> index 7bc3b71..feb30f6 100644
>> --- a/src/shared/gatt-client.c
>> +++ b/src/shared/gatt-client.c
>> @@ -892,13 +892,21 @@ static void discover_secondary_cb(bool success, uint8_t att_ecode,
>> attr = gatt_db_insert_service(client->db, start, &uuid, false,
>> end - start + 1);
>> if (!attr) {
>> - util_debug(client->debug_callback, client->debug_data,
>> - "Failed to create service");
>> - success = false;
>> - goto done;
>> + gatt_db_clear_range(client->db, start, end);
>> + attr = gatt_db_insert_service(client->db, start, &uuid,
>> + false, end - start + 1);
>> + if (!attr) {
>> + util_debug(client->debug_callback,
>> + client->debug_data,
>> + "Failed to store service");
>> + success = false;
>> + goto done;
>> + }
>> }
>>
>> - queue_push_tail(op->pending_svcs, attr);
>> + /* Skip if service already active */
>> + if (!gatt_db_service_get_active(attr))
>> + queue_push_tail(op->pending_svcs, attr);
>> }
>>
>> next:
>> @@ -906,8 +914,10 @@ next:
>> attr = queue_pop_head(op->pending_svcs);
>>
>> /* Complete with success if queue is empty */
>> - if (!attr)
>> + if (!attr) {
>> + success = true;
>> goto done;
>> + }
>>
>> /*
>> * Store the service in the tmp queue to be reused during
>> @@ -981,13 +991,21 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
>> attr = gatt_db_insert_service(client->db, start, &uuid, true,
>> end - start + 1);
>> if (!attr) {
>> - util_debug(client->debug_callback, client->debug_data,
>> + gatt_db_clear_range(client->db, start, end);
>> + attr = gatt_db_insert_service(client->db, start, &uuid,
>> + true, end - start + 1);
>> + if (!attr) {
>> + util_debug(client->debug_callback,
>> + client->debug_data,
>> "Failed to store service");
>> - success = false;
>> - goto done;
>> + success = false;
>> + goto done;
>> + }
>> }
>>
>> - queue_push_tail(op->pending_svcs, attr);
>> + /* Skip if service already active */
>> + if (!gatt_db_service_get_active(attr))
>> + queue_push_tail(op->pending_svcs, attr);
>> }
>>
>> secondary:
>> @@ -1044,12 +1062,6 @@ static void exchange_mtu_cb(bool success, uint8_t att_ecode, void *user_data)
>> "MTU exchange complete, with MTU: %u",
>> bt_att_get_mtu(client->att));
>>
>> - /* Don't do discovery if the database was pre-populated */
>> - if (!gatt_db_isempty(client->db)) {
>> - op->complete_func(op, true, 0);
>> - return;
>> - }
>> -
>> client->discovery_req = bt_gatt_discover_all_primary_services(
>> client->att, NULL,
>> discover_primary_cb,
>> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
>> index 2b2090c..5e1537e 100644
>> --- a/src/shared/gatt-db.c
>> +++ b/src/shared/gatt-db.c
>> @@ -486,7 +486,8 @@ bool gatt_db_clear_range(struct gatt_db *db, uint16_t start_handle,
>> return true;
>> }
>>
>> -static bool find_insert_loc(struct gatt_db *db, uint16_t start, uint16_t end,
>> +static struct gatt_db_service *find_insert_loc(struct gatt_db *db,
>> + uint16_t start, uint16_t end,
>> struct gatt_db_service **after)
>> {
>> const struct queue_entry *services_entry;
>> @@ -503,19 +504,19 @@ static bool find_insert_loc(struct gatt_db *db, uint16_t start, uint16_t end,
>> gatt_db_service_get_handles(service, &cur_start, &cur_end);
>>
>> if (start >= cur_start && start <= cur_end)
>> - return false;
>> + return service;
>>
>> if (end >= cur_start && end <= cur_end)
>> - return false;
>> + return service;
>>
>> if (end < cur_start)
>> - return true;
>> + return NULL;
>>
>> *after = service;
>> services_entry = services_entry->next;
>> }
>>
>> - return true;
>> + return NULL;
>> }
>>
>> struct gatt_db_attribute *gatt_db_insert_service(struct gatt_db *db,
>> @@ -534,8 +535,28 @@ struct gatt_db_attribute *gatt_db_insert_service(struct gatt_db *db,
>> if (num_handles < 1 || (handle + num_handles - 1) > UINT16_MAX)
>> return NULL;
>>
>> - if (!find_insert_loc(db, handle, handle + num_handles - 1, &after))
>> + service = find_insert_loc(db, handle, handle + num_handles - 1, &after);
>> + if (service) {
>> + const bt_uuid_t *type;
>> + bt_uuid_t value;
>> +
>> + if (primary)
>> + type = &primary_service_uuid;
>> + else
>> + type = &secondary_service_uuid;
>> +
>> + gatt_db_attribute_get_service_uuid(service->attributes[0],
>> + &value);
>> +
>> + /* Check if service match */
>> + if (!bt_uuid_cmp(&service->attributes[0]->uuid, type) &&
>> + !bt_uuid_cmp(&value, uuid) &&
>> + service->num_handles == num_handles &&
>> + service->attributes[0]->handle == handle)
>> + return service->attributes[0];
The code above does what I mentioned, it first locates the service in
region than attempts to match against the service found since the odds
are very small that the range stay the same but its attributes
arrangement changes over time.
>> +
>> return NULL;
>> + }
>>
>> service = gatt_db_service_create(uuid, handle, primary, num_handles);
>>
>> --
>> 2.1.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> Thanks,
> Arman
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-06-18 20:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-08 13:35 [PATCH BlueZ 1/4] shared/gatt-client: Speed up discovery Luiz Augusto von Dentz
2015-06-08 13:35 ` [PATCH BlueZ 2/4] shared/gatt-client: Consolidate service changed registration Luiz Augusto von Dentz
2015-06-08 13:35 ` [PATCH BlueZ 3/4] shared/gatt-client: Simplify ready flag Luiz Augusto von Dentz
2015-06-08 13:35 ` [PATCH BlueZ 4/4] shared/gatt-client: Don't discovery secondaries if there is no primary Luiz Augusto von Dentz
2015-06-18 19:46 ` [PATCH BlueZ 1/4] shared/gatt-client: Speed up discovery Arman Uguray
2015-06-18 20:57 ` Luiz Augusto von Dentz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.