* [PATCH BlueZ v2 01/14] core: gatt: Expose charac. extended properties.
2015-01-08 5:48 [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
@ 2015-01-08 5:48 ` Arman Uguray
2015-01-08 5:48 ` [PATCH BlueZ v2 02/14] shared/gatt-client: Make read/write cancelable Arman Uguray
` (13 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Arman Uguray @ 2015-01-08 5:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch reads the extended properties for characteristics that have
the necessary descriptor. Updating the "Characteristics" property is
delayed for services until the extended properties have been read for
all of their characteristics that have them.
---
src/gatt-client.c | 155 +++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 119 insertions(+), 36 deletions(-)
diff --git a/src/gatt-client.c b/src/gatt-client.c
index 47faf86..8bbb03d 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -42,6 +42,10 @@
#include "gatt-client.h"
#include "dbus-common.h"
+#ifndef NELEM
+#define NELEM(x) (sizeof(x) / sizeof((x)[0]))
+#endif
+
#define GATT_SERVICE_IFACE "org.bluez.GattService1"
#define GATT_CHARACTERISTIC_IFACE "org.bluez.GattCharacteristic1"
#define GATT_DESCRIPTOR_IFACE "org.bluez.GattDescriptor1"
@@ -64,6 +68,8 @@ struct service {
char *path;
struct queue *chrcs;
bool chrcs_ready;
+ struct queue *pending_ext_props;
+ guint idle_id;
};
struct characteristic {
@@ -72,6 +78,8 @@ struct characteristic {
uint16_t handle;
uint16_t value_handle;
uint8_t props;
+ uint16_t ext_props;
+ uint16_t ext_props_handle;
bt_uuid_t uuid;
char *path;
@@ -92,6 +100,15 @@ struct descriptor {
bool in_write;
};
+static bool uuid_cmp(const bt_uuid_t *uuid, uint16_t u16)
+{
+ bt_uuid_t uuid16;
+
+ bt_uuid16_create(&uuid16, u16);
+
+ return bt_uuid_cmp(uuid, &uuid16) == 0;
+}
+
static gboolean descriptor_get_uuid(const GDBusPropertyTable *property,
DBusMessageIter *iter, void *data)
{
@@ -492,7 +509,6 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
uint8_t *value = NULL;
size_t value_len = 0;
bool result;
- bt_uuid_t uuid;
if (desc->in_write)
return btd_error_in_progress(msg);
@@ -505,8 +521,7 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
* descriptors. We achieve this through the StartNotify and StopNotify
* methods on GattCharacteristic1.
*/
- bt_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
- if (bt_uuid_cmp(&desc->uuid, &uuid))
+ if (uuid_cmp(&desc->uuid, GATT_CLIENT_CHARAC_CFG_UUID))
return btd_error_not_permitted(msg, "Write not permitted");
/*
@@ -590,6 +605,9 @@ static struct descriptor *descriptor_create(struct gatt_db_attribute *attr,
DBG("Exported GATT characteristic descriptor: %s", desc->path);
+ if (uuid_cmp(&desc->uuid, GATT_CHARAC_EXT_PROPER_UUID))
+ chrc->ext_props_handle = desc->handle;
+
return desc;
}
@@ -668,10 +686,12 @@ static gboolean characteristic_get_notifying(const GDBusPropertyTable *property,
return TRUE;
}
-static struct {
+struct chrc_prop_data {
uint8_t prop;
char *str;
-} properties[] = {
+};
+
+struct chrc_prop_data chrc_props[] = {
/* Default Properties */
{ BT_GATT_CHRC_PROP_BROADCAST, "broadcast" },
{ BT_GATT_CHRC_PROP_READ, "read" },
@@ -680,7 +700,13 @@ static struct {
{ BT_GATT_CHRC_PROP_NOTIFY, "notify" },
{ BT_GATT_CHRC_PROP_INDICATE, "indicate" },
{ BT_GATT_CHRC_PROP_AUTH, "authenticated-signed-writes" },
- { },
+ { BT_GATT_CHRC_PROP_EXT_PROP, "extended-properties" }
+};
+
+struct chrc_prop_data chrc_ext_props[] = {
+ /* Extended Properties */
+ { BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE, "reliable-write" },
+ { BT_GATT_CHRC_EXT_PROP_WRITABLE_AUX, "writable-auxiliaries" }
};
static gboolean characteristic_get_flags(const GDBusPropertyTable *property,
@@ -688,20 +714,21 @@ static gboolean characteristic_get_flags(const GDBusPropertyTable *property,
{
struct characteristic *chrc = data;
DBusMessageIter array;
- int i;
+ unsigned i;
dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "s", &array);
- for (i = 0; properties[i].str; i++) {
- if (chrc->props & properties[i].prop)
+ for (i = 0; i < NELEM(chrc_props); i++) {
+ if (chrc->props & chrc_props[i].prop)
dbus_message_iter_append_basic(&array, DBUS_TYPE_STRING,
- &properties[i].str);
+ &chrc_props[i].str);
}
- /*
- * TODO: Handle extended properties if the descriptor is
- * present.
- */
+ for (i = 0; i < NELEM(chrc_ext_props); i++) {
+ if (chrc->ext_props & chrc_ext_props[i].prop)
+ dbus_message_iter_append_basic(&array, DBUS_TYPE_STRING,
+ &chrc_ext_props[i].str);
+ }
dbus_message_iter_close_container(iter, &array);
@@ -1085,6 +1112,7 @@ static void service_free(void *data)
struct service *service = data;
queue_destroy(service->chrcs, NULL); /* List should be empty here */
+ queue_destroy(service->pending_ext_props, NULL);
g_free(service->path);
free(service);
}
@@ -1106,6 +1134,13 @@ static struct service *service_create(struct gatt_db_attribute *attr,
return NULL;
}
+ service->pending_ext_props = queue_new();
+ if (!service->pending_ext_props) {
+ queue_destroy(service->chrcs, NULL);
+ free(service);
+ return NULL;
+ }
+
service->client = client;
gatt_db_attribute_get_service_data(attr, &service->start_handle,
@@ -1142,12 +1177,24 @@ static void unregister_service(void *data)
DBG("Removing GATT service: %s", service->path);
+ if (service->idle_id)
+ g_source_remove(service->idle_id);
+
queue_remove_all(service->chrcs, NULL, NULL, unregister_characteristic);
g_dbus_unregister_interface(btd_get_dbus_connection(), service->path,
GATT_SERVICE_IFACE);
}
+static void notify_chrcs(struct service *service)
+{
+ service->chrcs_ready = true;
+
+ g_dbus_emit_property_changed(btd_get_dbus_connection(), service->path,
+ GATT_SERVICE_IFACE,
+ "Characteristics");
+}
+
struct export_data {
void *root;
bool failed;
@@ -1171,6 +1218,46 @@ static void export_desc(struct gatt_db_attribute *attr, void *user_data)
queue_push_tail(charac->descs, desc);
}
+static void read_ext_props_cb(bool success, uint8_t att_ecode,
+ const uint8_t *value, uint16_t length,
+ void *user_data)
+{
+ struct characteristic *chrc = user_data;
+ struct service *service = chrc->service;
+
+ if (!success) {
+ error("Failed to obtain extended properties - error: 0x%02x",
+ att_ecode);
+ return;
+ }
+
+ if (!value || length != 2) {
+ error("Malformed extended properties value");
+ return;
+ }
+
+ chrc->ext_props = get_le16(value);
+ if (chrc->ext_props)
+ g_dbus_emit_property_changed(btd_get_dbus_connection(),
+ service->path,
+ GATT_SERVICE_IFACE, "Flags");
+
+ queue_remove(service->pending_ext_props, chrc);
+
+ if (queue_isempty(service->pending_ext_props))
+ notify_chrcs(service);
+}
+
+static void read_ext_props(void *data, void *user_data)
+{
+ struct characteristic *chrc = data;
+
+ bt_gatt_client_read_value(chrc->service->client->gatt,
+ chrc->ext_props_handle,
+ read_ext_props_cb,
+ chrc, NULL);
+}
+
static bool create_descriptors(struct gatt_db_attribute *attr,
struct characteristic *charac)
{
@@ -1204,6 +1291,9 @@ static void export_char(struct gatt_db_attribute *attr, void *user_data)
queue_push_tail(service->chrcs, charac);
+ if (charac->ext_props_handle)
+ queue_push_tail(service->pending_ext_props, charac);
+
return;
fail:
@@ -1220,28 +1310,20 @@ static bool create_characteristics(struct gatt_db_attribute *attr,
gatt_db_service_foreach_char(attr, export_char, &data);
- return !data.failed;
-}
-
-static void notify_chrcs(void *data, void *user_data)
-{
- struct service *service = data;
+ if (data.failed)
+ return false;
- service->chrcs_ready = true;
+ /* Obtain extended properties */
+ queue_foreach(service->pending_ext_props, read_ext_props, NULL);
- g_dbus_emit_property_changed(btd_get_dbus_connection(), service->path,
- GATT_SERVICE_IFACE,
- "Characteristics");
+ return true;
}
static gboolean set_chrcs_ready(gpointer user_data)
{
- struct btd_gatt_client *client = user_data;
-
- if (!client->gatt)
- return FALSE;
+ struct service *service = user_data;
- queue_foreach(client->services, notify_chrcs, NULL);
+ notify_chrcs(service);
return FALSE;
}
@@ -1265,6 +1347,14 @@ static void export_service(struct gatt_db_attribute *attr, void *user_data)
}
queue_push_tail(client->services, service);
+
+ /*
+ * Asynchronously update the "Characteristics" property of the service.
+ * If there are any pending reads to obtain the value of the "Extended
+ * Properties" descriptor then wait until they are complete.
+ */
+ if (!service->chrcs_ready && queue_isempty(service->pending_ext_props))
+ service->idle_id = g_idle_add(set_chrcs_ready, service);
}
static void create_services(struct btd_gatt_client *client)
@@ -1272,13 +1362,6 @@ static void create_services(struct btd_gatt_client *client)
DBG("Exporting objects for GATT services: %s", client->devaddr);
gatt_db_foreach_service(client->db, NULL, export_service, client);
-
- /*
- * Asynchronously update the "Characteristics" property of each service.
- * We do this so that users have a way to know that all characteristics
- * of a service have been exported.
- */
- g_idle_add(set_chrcs_ready, client);
}
struct btd_gatt_client *btd_gatt_client_new(struct btd_device *device)
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH BlueZ v2 02/14] shared/gatt-client: Make read/write cancelable
2015-01-08 5:48 [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
2015-01-08 5:48 ` [PATCH BlueZ v2 01/14] core: gatt: Expose charac. extended properties Arman Uguray
@ 2015-01-08 5:48 ` Arman Uguray
2015-01-08 5:48 ` [PATCH BlueZ v2 03/14] shared/gatt-client: Make long-write cancelable Arman Uguray
` (12 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Arman Uguray @ 2015-01-08 5:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
The shared/gatt-client functions for initiating read/write procedures
currently have no way to cancel them. The functions simple return a
boolean result to indicate success/failure. This patch fixes this by
returning a request id from these methods and adds the bt_gatt_cancel_*
functions to cancel a request with one of these ids.
bt_gatt_client internally manages its own set of request ids and maps
them to the underlying ATT request id returned by bt_att_send to
correctly cancel GATT procedures that span across multiple ATT protocol
PDUs.
---
src/shared/gatt-client.c | 298 +++++++++++++++++++++++++++++++++++++----------
src/shared/gatt-client.h | 14 ++-
2 files changed, 244 insertions(+), 68 deletions(-)
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 10dfcbb..362fc32 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -65,7 +65,8 @@ struct bt_gatt_client {
bool in_init;
bool ready;
- /* Queue of long write requests. An error during "prepare write"
+ /*
+ * Queue of long write requests. An error during "prepare write"
* requests can result in a cancel through "execute write". To prevent
* cancelation of prepared writes to the wrong attribute and multiple
* requests to the same attribute that may result in a corrupted final
@@ -80,15 +81,75 @@ struct bt_gatt_client {
int next_reg_id;
unsigned int disc_id, notify_id, ind_id;
- /* Handles of the GATT Service and the Service Changed characteristic
+ /*
+ * Handles of the GATT Service and the Service Changed characteristic
* value handle. These will have the value 0 if they are not present on
* the remote peripheral.
*/
unsigned int svc_chngd_ind_id;
struct queue *svc_chngd_queue; /* Queued service changed events */
bool in_svc_chngd;
+
+ /*
+ * List of pending read/write operations. For operations that span
+ * across multiple PDUs, this list provides a mapping from an operation
+ * id to an ATT request id.
+ */
+ struct queue *pending_requests;
+ unsigned int next_request_id;
+};
+
+struct request {
+ struct bt_gatt_client *client;
+ bool removed;
+ int ref_count;
+ unsigned int id;
+ unsigned int att_id;
+ void *data;
+ void (*destroy)(void *);
};
+static struct request *request_ref(struct request *req)
+{
+ __sync_fetch_and_add(&req->ref_count, 1);
+
+ return req;
+}
+
+static struct request *request_create(struct bt_gatt_client *client)
+{
+ struct request *req;
+
+ req = new0(struct request, 1);
+ if (!req)
+ return NULL;
+
+ if (client->next_request_id < 1)
+ client->next_request_id = 1;
+
+ queue_push_tail(client->pending_requests, req);
+ req->client = client;
+ req->id = client->next_request_id++;
+
+ return request_ref(req);
+}
+
+static void request_unref(void *data)
+{
+ struct request *req = data;
+
+ if (__sync_sub_and_fetch(&req->ref_count, 1))
+ return;
+
+ if (req->destroy)
+ req->destroy(req->data);
+
+ if (!req->removed)
+ queue_remove(req->client->pending_requests, req);
+
+ free(req);
+}
+
struct notify_chrc {
uint16_t value_handle;
uint16_t ccc_handle;
@@ -1416,6 +1477,8 @@ static void long_write_op_unref(void *data);
static void bt_gatt_client_free(struct bt_gatt_client *client)
{
+ bt_gatt_client_cancel_all(client);
+
if (client->ready_destroy)
client->ready_destroy(client->ready_data);
@@ -1435,6 +1498,7 @@ static void bt_gatt_client_free(struct bt_gatt_client *client)
queue_destroy(client->long_write_queue, long_write_op_unref);
queue_destroy(client->notify_list, notify_data_unref);
queue_destroy(client->notify_chrcs, notify_chrc_free);
+ queue_destroy(client->pending_requests, request_unref);
free(client);
}
@@ -1490,6 +1554,10 @@ struct bt_gatt_client *bt_gatt_client_new(struct gatt_db *db,
if (!client->notify_chrcs)
goto fail;
+ client->pending_requests = queue_new();
+ if (!client->pending_requests)
+ goto fail;
+
client->notify_id = bt_att_register(att, BT_ATT_OP_HANDLE_VAL_NOT,
notify_cb, client, NULL);
if (!client->notify_id)
@@ -1600,6 +1668,49 @@ uint16_t bt_gatt_client_get_mtu(struct bt_gatt_client *client)
return bt_att_get_mtu(client->att);
}
+static bool match_req_id(const void *a, const void *b)
+{
+ const struct request *req = a;
+ unsigned int id = PTR_TO_UINT(b);
+
+ return req->id == id;
+}
+
+bool bt_gatt_client_cancel(struct bt_gatt_client *client, unsigned int id)
+{
+ struct request *req;
+
+ if (!client || !id || !client->att)
+ return false;
+
+ req = queue_remove_if(client->pending_requests, match_req_id,
+ UINT_TO_PTR(id));
+ if (!req)
+ return false;
+
+ req->removed = true;
+
+ return bt_att_cancel(client->att, req->att_id);
+}
+
+static void cancel_request(void *data)
+{
+ struct request *req = data;
+
+ req->removed = true;
+ bt_att_cancel(req->client->att, req->att_id);
+}
+
+bool bt_gatt_client_cancel_all(struct bt_gatt_client *client)
+{
+ if (!client || !client->att)
+ return false;
+
+ queue_remove_all(client->pending_requests, NULL, NULL, cancel_request);
+
+ return true;
+}
+
struct read_op {
bt_gatt_client_read_callback_t callback;
void *user_data;
@@ -1619,7 +1730,8 @@ static void destroy_read_op(void *data)
static void read_cb(uint8_t opcode, const void *pdu, uint16_t length,
void *user_data)
{
- struct read_op *op = user_data;
+ struct request *req = user_data;
+ struct read_op *op = req->data;
bool success;
uint8_t att_ecode = 0;
const uint8_t *value = NULL;
@@ -1646,42 +1758,56 @@ done:
op->callback(success, att_ecode, value, length, op->user_data);
}
-bool bt_gatt_client_read_value(struct bt_gatt_client *client,
+unsigned int bt_gatt_client_read_value(struct bt_gatt_client *client,
uint16_t value_handle,
bt_gatt_client_read_callback_t callback,
void *user_data,
bt_gatt_client_destroy_func_t destroy)
{
+ struct request *req;
struct read_op *op;
uint8_t pdu[2];
if (!client)
- return false;
+ return 0;
op = new0(struct read_op, 1);
if (!op)
- return false;
+ return 0;
+
+ req = request_create(client);
+ if (!req) {
+ free(op);
+ return 0;
+ }
op->callback = callback;
op->user_data = user_data;
op->destroy = destroy;
+ req->data = op;
+ req->destroy = destroy_read_op;
+
put_le16(value_handle, pdu);
- if (!bt_att_send(client->att, BT_ATT_OP_READ_REQ, pdu, sizeof(pdu),
- read_cb, op,
- destroy_read_op)) {
- free(op);
- return false;
+ req->att_id = bt_att_send(client->att, BT_ATT_OP_READ_REQ,
+ pdu, sizeof(pdu),
+ read_cb, req,
+ request_unref);
+ if (!req->att_id) {
+ op->destroy = NULL;
+ request_unref(req);
+ return 0;
}
- return true;
+ return req->id;
}
static void read_multiple_cb(uint8_t opcode, const void *pdu, uint16_t length,
void *user_data)
{
- struct read_op *op = user_data;
+ struct request *req = user_data;
+ struct read_op *op = req->data;
uint8_t att_ecode;
bool success;
@@ -1704,43 +1830,57 @@ static void read_multiple_cb(uint8_t opcode, const void *pdu, uint16_t length,
op->callback(success, att_ecode, pdu, length, op->user_data);
}
-bool bt_gatt_client_read_multiple(struct bt_gatt_client *client,
+unsigned int bt_gatt_client_read_multiple(struct bt_gatt_client *client,
uint16_t *handles, uint8_t num_handles,
bt_gatt_client_read_callback_t callback,
void *user_data,
bt_gatt_client_destroy_func_t destroy)
{
uint8_t pdu[num_handles * 2];
+ struct request *req;
struct read_op *op;
int i;
if (!client)
- return false;
+ return 0;
if (num_handles < 2)
- return false;
+ return 0;
if (num_handles * 2 > bt_att_get_mtu(client->att) - 1)
- return false;
+ return 0;
op = new0(struct read_op, 1);
if (!op)
- return false;
+ return 0;
+
+ req = request_create(client);
+ if (!client) {
+ free(op);
+ return 0;
+ }
op->callback = callback;
op->user_data = user_data;
op->destroy = destroy;
+ req->data = op;
+ req->destroy = destroy_read_op;
+
for (i = 0; i < num_handles; i++)
put_le16(handles[i], pdu + (2 * i));
- if (!bt_att_send(client->att, BT_ATT_OP_READ_MULT_REQ, pdu, sizeof(pdu),
- read_multiple_cb, op, destroy_read_op)) {
- free(op);
- return false;
+ req->att_id = bt_att_send(client->att, BT_ATT_OP_READ_MULT_REQ,
+ pdu, sizeof(pdu),
+ read_multiple_cb, req,
+ request_unref);
+ if (!req->att_id) {
+ op->destroy = NULL;
+ request_unref(req);
+ return 0;
}
- return true;
+ return req->id;
}
struct read_long_op {
@@ -1791,20 +1931,10 @@ static void destroy_blob(void *data)
free(blob);
}
-static struct read_long_op *read_long_op_ref(struct read_long_op *op)
-{
- __sync_fetch_and_add(&op->ref_count, 1);
-
- return op;
-}
-
-static void read_long_op_unref(void *data)
+static void destroy_read_long_op(void *data)
{
struct read_long_op *op = data;
- if (__sync_sub_and_fetch(&op->ref_count, 1))
- return;
-
if (op->destroy)
op->destroy(op->user_data);
@@ -1853,7 +1983,8 @@ done:
static void read_long_cb(uint8_t opcode, const void *pdu,
uint16_t length, void *user_data)
{
- struct read_long_op *op = user_data;
+ struct request *req = user_data;
+ struct read_long_op *op = req->data;
struct blob *blob;
bool success;
uint8_t att_ecode = 0;
@@ -1889,14 +2020,16 @@ static void read_long_cb(uint8_t opcode, const void *pdu,
put_le16(op->value_handle, pdu);
put_le16(op->offset, pdu + 2);
- if (bt_att_send(op->client->att, BT_ATT_OP_READ_BLOB_REQ,
+ req->att_id = bt_att_send(op->client->att,
+ BT_ATT_OP_READ_BLOB_REQ,
pdu, sizeof(pdu),
read_long_cb,
- read_long_op_ref(op),
- read_long_op_unref))
+ request_ref(req),
+ request_unref);
+ if (req->att_id)
return;
- read_long_op_unref(op);
+ request_unref(req);
success = false;
goto done;
}
@@ -1908,26 +2041,34 @@ done:
complete_read_long_op(op, success, att_ecode);
}
-bool bt_gatt_client_read_long_value(struct bt_gatt_client *client,
+unsigned int bt_gatt_client_read_long_value(struct bt_gatt_client *client,
uint16_t value_handle, uint16_t offset,
bt_gatt_client_read_callback_t callback,
void *user_data,
bt_gatt_client_destroy_func_t destroy)
{
+ struct request *req;
struct read_long_op *op;
uint8_t pdu[4];
if (!client)
- return false;
+ return 0;
op = new0(struct read_long_op, 1);
if (!op)
- return false;
+ return 0;
op->blobs = queue_new();
if (!op->blobs) {
free(op);
- return false;
+ return 0;
+ }
+
+ req = request_create(client);
+ if (!req) {
+ queue_destroy(op->blobs, free);
+ free(op);
+ return 0;
}
op->client = client;
@@ -1938,26 +2079,32 @@ bool bt_gatt_client_read_long_value(struct bt_gatt_client *client,
op->user_data = user_data;
op->destroy = destroy;
+ req->data = op;
+ req->destroy = destroy_read_long_op;
+
put_le16(value_handle, pdu);
put_le16(offset, pdu + 2);
- if (!bt_att_send(client->att, BT_ATT_OP_READ_BLOB_REQ, pdu, sizeof(pdu),
- read_long_cb,
- read_long_op_ref(op),
- read_long_op_unref)) {
- queue_destroy(op->blobs, free);
- free(op);
- return false;
+ req->att_id = bt_att_send(client->att, BT_ATT_OP_READ_BLOB_REQ,
+ pdu, sizeof(pdu),
+ read_long_cb, req,
+ request_unref);
+ if (!req->att_id) {
+ op->destroy = NULL;
+ request_unref(req);
+ return 0;
}
- return true;
+ return req->id;
}
-bool bt_gatt_client_write_without_response(struct bt_gatt_client *client,
+unsigned int bt_gatt_client_write_without_response(
+ struct bt_gatt_client *client,
uint16_t value_handle,
bool signed_write,
const uint8_t *value, uint16_t length) {
uint8_t pdu[2 + length];
+ struct request *req;
if (!client)
return 0;
@@ -1966,11 +2113,22 @@ bool bt_gatt_client_write_without_response(struct bt_gatt_client *client,
if (signed_write)
return 0;
+ req = request_create(client);
+ if (!req)
+ return 0;
+
put_le16(value_handle, pdu);
memcpy(pdu + 2, value, length);
- return bt_att_send(client->att, BT_ATT_OP_WRITE_CMD, pdu, sizeof(pdu),
+ req->att_id = bt_att_send(client->att, BT_ATT_OP_WRITE_CMD,
+ pdu, sizeof(pdu),
NULL, NULL, NULL);
+ if (!req->att_id) {
+ request_unref(req);
+ return 0;
+ }
+
+ return req->id;
}
struct write_op {
@@ -1992,7 +2150,8 @@ static void destroy_write_op(void *data)
static void write_cb(uint8_t opcode, const void *pdu, uint16_t length,
void *user_data)
{
- struct write_op *op = user_data;
+ struct request *req = user_data;
+ struct write_op *op = req->data;
bool success = true;
uint8_t att_ecode = 0;
@@ -2010,38 +2169,51 @@ done:
op->callback(success, att_ecode, op->user_data);
}
-bool bt_gatt_client_write_value(struct bt_gatt_client *client,
+unsigned int bt_gatt_client_write_value(struct bt_gatt_client *client,
uint16_t value_handle,
const uint8_t *value, uint16_t length,
bt_gatt_client_callback_t callback,
void *user_data,
bt_gatt_client_destroy_func_t destroy)
{
+ struct request *req;
struct write_op *op;
uint8_t pdu[2 + length];
if (!client)
- return false;
+ return 0;
op = new0(struct write_op, 1);
if (!op)
- return false;
+ return 0;
+
+ req = request_create(client);
+ if (!req) {
+ free(op);
+ return 0;
+ }
op->callback = callback;
op->user_data = user_data;
op->destroy = destroy;
+ req->data = op;
+ req->destroy = destroy_write_op;
+
put_le16(value_handle, pdu);
memcpy(pdu + 2, value, length);
- if (!bt_att_send(client->att, BT_ATT_OP_WRITE_REQ, pdu, sizeof(pdu),
- write_cb, op,
- destroy_write_op)) {
- free(op);
- return false;
+ req->att_id = bt_att_send(client->att, BT_ATT_OP_WRITE_REQ,
+ pdu, sizeof(pdu),
+ write_cb, req,
+ request_unref);
+ if (!req->att_id) {
+ op->destroy = NULL;
+ request_unref(req);
+ return 0;
}
- return true;
+ return req->id;
}
struct long_write_op {
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 42eeaec..74d781c 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -72,27 +72,31 @@ bool bt_gatt_client_set_debug(struct bt_gatt_client *client,
uint16_t bt_gatt_client_get_mtu(struct bt_gatt_client *client);
-bool bt_gatt_client_read_value(struct bt_gatt_client *client,
+bool bt_gatt_client_cancel(struct bt_gatt_client *client, unsigned int id);
+bool bt_gatt_client_cancel_all(struct bt_gatt_client *client);
+
+unsigned int bt_gatt_client_read_value(struct bt_gatt_client *client,
uint16_t value_handle,
bt_gatt_client_read_callback_t callback,
void *user_data,
bt_gatt_client_destroy_func_t destroy);
-bool bt_gatt_client_read_long_value(struct bt_gatt_client *client,
+unsigned int bt_gatt_client_read_long_value(struct bt_gatt_client *client,
uint16_t value_handle, uint16_t offset,
bt_gatt_client_read_callback_t callback,
void *user_data,
bt_gatt_client_destroy_func_t destroy);
-bool bt_gatt_client_read_multiple(struct bt_gatt_client *client,
+unsigned int bt_gatt_client_read_multiple(struct bt_gatt_client *client,
uint16_t *handles, uint8_t num_handles,
bt_gatt_client_read_callback_t callback,
void *user_data,
bt_gatt_client_destroy_func_t destroy);
-bool bt_gatt_client_write_without_response(struct bt_gatt_client *client,
+unsigned int bt_gatt_client_write_without_response(
+ struct bt_gatt_client *client,
uint16_t value_handle,
bool signed_write,
const uint8_t *value, uint16_t length);
-bool bt_gatt_client_write_value(struct bt_gatt_client *client,
+unsigned int bt_gatt_client_write_value(struct bt_gatt_client *client,
uint16_t value_handle,
const uint8_t *value, uint16_t length,
bt_gatt_client_callback_t callback,
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH BlueZ v2 03/14] shared/gatt-client: Make long-write cancelable
2015-01-08 5:48 [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
2015-01-08 5:48 ` [PATCH BlueZ v2 01/14] core: gatt: Expose charac. extended properties Arman Uguray
2015-01-08 5:48 ` [PATCH BlueZ v2 02/14] shared/gatt-client: Make read/write cancelable Arman Uguray
@ 2015-01-08 5:48 ` Arman Uguray
2015-01-08 5:48 ` [PATCH BlueZ v2 04/14] core: gatt: Cancel pending reads/writes Arman Uguray
` (11 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Arman Uguray @ 2015-01-08 5:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch extends the GATT request ids in bt_gatt_client to
bt_gatt_client_write_long_value.
---
src/shared/gatt-client.c | 162 ++++++++++++++++++++++++++++++-----------------
src/shared/gatt-client.h | 2 +-
2 files changed, 104 insertions(+), 60 deletions(-)
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 362fc32..083bcd2 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -101,6 +101,7 @@ struct bt_gatt_client {
struct request {
struct bt_gatt_client *client;
+ bool long_write;
bool removed;
int ref_count;
unsigned int id;
@@ -1473,8 +1474,6 @@ static void notify_cb(uint8_t opcode, const void *pdu, uint16_t length,
bt_gatt_client_unref(client);
}
-static void long_write_op_unref(void *data);
-
static void bt_gatt_client_free(struct bt_gatt_client *client)
{
bt_gatt_client_cancel_all(client);
@@ -1495,7 +1494,7 @@ static void bt_gatt_client_free(struct bt_gatt_client *client)
gatt_db_unref(client->db);
queue_destroy(client->svc_chngd_queue, free);
- queue_destroy(client->long_write_queue, long_write_op_unref);
+ queue_destroy(client->long_write_queue, request_unref);
queue_destroy(client->notify_list, notify_data_unref);
queue_destroy(client->notify_chrcs, notify_chrc_free);
queue_destroy(client->pending_requests, request_unref);
@@ -1676,9 +1675,16 @@ static bool match_req_id(const void *a, const void *b)
return req->id == id;
}
+static void cancel_long_write_cb(uint8_t opcode, const void *pdu, uint16_t len,
+ void *user_data)
+{
+ /* Do nothing */
+}
+
bool bt_gatt_client_cancel(struct bt_gatt_client *client, unsigned int id)
{
struct request *req;
+ uint8_t pdu = 0x00;
if (!client || !id || !client->att)
return false;
@@ -1690,15 +1696,48 @@ bool bt_gatt_client_cancel(struct bt_gatt_client *client, unsigned int id)
req->removed = true;
- return bt_att_cancel(client->att, req->att_id);
+ if (!bt_att_cancel(client->att, req->att_id) && !req->long_write)
+ return false;
+
+ /* If this was a long-write, we need to abort all prepared writes */
+ if (!req->long_write)
+ return true;
+
+ if (!req->att_id)
+ queue_remove(client->long_write_queue, req);
+ else
+ bt_att_send(client->att, BT_ATT_OP_EXEC_WRITE_REQ,
+ &pdu, sizeof(pdu),
+ cancel_long_write_cb,
+ NULL, NULL);
+
+ if (queue_isempty(client->long_write_queue))
+ client->in_long_write = false;
+
+ return true;
}
static void cancel_request(void *data)
{
struct request *req = data;
+ uint8_t pdu = 0x00;
req->removed = true;
bt_att_cancel(req->client->att, req->att_id);
+
+ if (!req->long_write)
+ return;
+
+ if (!req->att_id)
+ queue_remove(req->client->long_write_queue, req);
+
+ if (queue_isempty(req->client->long_write_queue))
+ req->client->in_long_write = false;
+
+ bt_att_send(req->client->att, BT_ATT_OP_EXEC_WRITE_REQ,
+ &pdu, sizeof(pdu),
+ cancel_long_write_cb,
+ NULL, NULL);
}
bool bt_gatt_client_cancel_all(struct bt_gatt_client *client)
@@ -2218,7 +2257,6 @@ unsigned int bt_gatt_client_write_value(struct bt_gatt_client *client,
struct long_write_op {
struct bt_gatt_client *client;
- int ref_count;
bool reliable;
bool success;
uint8_t att_ecode;
@@ -2234,20 +2272,10 @@ struct long_write_op {
bt_gatt_client_destroy_func_t destroy;
};
-static struct long_write_op *long_write_op_ref(struct long_write_op *op)
-{
- __sync_fetch_and_add(&op->ref_count, 1);
-
- return op;
-}
-
-static void long_write_op_unref(void *data)
+static void long_write_op_free(void *data)
{
struct long_write_op *op = data;
- if (__sync_sub_and_fetch(&op->ref_count, 1))
- return;
-
if (op->destroy)
op->destroy(op->user_data);
@@ -2257,11 +2285,12 @@ static void long_write_op_unref(void *data)
static void prepare_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
void *user_data);
-static void complete_write_long_op(struct long_write_op *op, bool success,
+static void complete_write_long_op(struct request *req, bool success,
uint8_t att_ecode, bool reliable_error);
-static void handle_next_prep_write(struct long_write_op *op)
+static void handle_next_prep_write(struct request *req)
{
+ struct long_write_op *op = req->data;
bool success = true;
uint8_t *pdu;
@@ -2275,12 +2304,13 @@ static void handle_next_prep_write(struct long_write_op *op)
put_le16(op->offset + op->index, pdu + 2);
memcpy(pdu + 4, op->value + op->index, op->cur_length);
- if (!bt_att_send(op->client->att, BT_ATT_OP_PREP_WRITE_REQ,
+ req->att_id = bt_att_send(op->client->att, BT_ATT_OP_PREP_WRITE_REQ,
pdu, op->cur_length + 4,
prepare_write_cb,
- long_write_op_ref(op),
- long_write_op_unref)) {
- long_write_op_unref(op);
+ request_ref(req),
+ request_unref);
+ if (!req->att_id) {
+ request_unref(req);
success = false;
}
@@ -2294,34 +2324,36 @@ static void handle_next_prep_write(struct long_write_op *op)
return;
done:
- complete_write_long_op(op, success, 0, false);
+ complete_write_long_op(req, success, 0, false);
}
static void start_next_long_write(struct bt_gatt_client *client)
{
- struct long_write_op *op;
+ struct request *req;
if (queue_isempty(client->long_write_queue)) {
client->in_long_write = false;
return;
}
- op = queue_pop_head(client->long_write_queue);
- if (!op)
+ req = queue_pop_head(client->long_write_queue);
+ if (!req)
return;
- handle_next_prep_write(op);
+ handle_next_prep_write(req);
- /* send_next_prep_write adds an extra ref. Unref here to clean up if
+ /*
+ * send_next_prep_write adds an extra ref. Unref here to clean up if
* necessary, since we also added a ref before pushing to the queue.
*/
- long_write_op_unref(op);
+ request_unref(req);
}
static void execute_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
void *user_data)
{
- struct long_write_op *op = user_data;
+ struct request *req = user_data;
+ struct long_write_op *op = req->data;
bool success = op->success;
uint8_t att_ecode = op->att_ecode;
@@ -2338,9 +2370,10 @@ static void execute_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
start_next_long_write(op->client);
}
-static void complete_write_long_op(struct long_write_op *op, bool success,
+static void complete_write_long_op(struct request *req, bool success,
uint8_t att_ecode, bool reliable_error)
{
+ struct long_write_op *op = req->data;
uint8_t pdu;
op->success = success;
@@ -2352,14 +2385,16 @@ static void complete_write_long_op(struct long_write_op *op, bool success,
else
pdu = 0x00; /* Cancel */
- if (bt_att_send(op->client->att, BT_ATT_OP_EXEC_WRITE_REQ,
+ req->att_id = bt_att_send(op->client->att, BT_ATT_OP_EXEC_WRITE_REQ,
&pdu, sizeof(pdu),
execute_write_cb,
- long_write_op_ref(op),
- long_write_op_unref))
+ request_ref(req),
+ request_unref);
+ if (req->att_id)
return;
- long_write_op_unref(op);
+
+ request_unref(req);
success = false;
if (op->callback)
@@ -2371,7 +2406,8 @@ static void complete_write_long_op(struct long_write_op *op, bool success,
static void prepare_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
void *user_data)
{
- struct long_write_op *op = user_data;
+ struct request *req = user_data;
+ struct long_write_op *op = req->data;
bool success = true;
bool reliable_error = false;
uint8_t att_ecode = 0;
@@ -2422,15 +2458,15 @@ static void prepare_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
op->index = next_index;
op->cur_length = MIN(op->length - op->index,
bt_att_get_mtu(op->client->att) - 5);
- handle_next_prep_write(op);
+ handle_next_prep_write(req);
return;
}
done:
- complete_write_long_op(op, success, att_ecode, reliable_error);
+ complete_write_long_op(req, success, att_ecode, reliable_error);
}
-bool bt_gatt_client_write_long_value(struct bt_gatt_client *client,
+unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
bool reliable,
uint16_t value_handle, uint16_t offset,
const uint8_t *value, uint16_t length,
@@ -2438,30 +2474,37 @@ bool bt_gatt_client_write_long_value(struct bt_gatt_client *client,
void *user_data,
bt_gatt_client_destroy_func_t destroy)
{
+ struct request *req;
struct long_write_op *op;
uint8_t *pdu;
- bool status;
if (!client)
- return false;
+ return 0;
if ((size_t)(length + offset) > UINT16_MAX)
- return false;
+ return 0;
/* Don't allow writing a 0-length value using this procedure. The
* upper-layer should use bt_gatt_write_value for that instead.
*/
if (!length || !value)
- return false;
+ return 0;
op = new0(struct long_write_op, 1);
if (!op)
- return false;
+ return 0;
op->value = malloc(length);
if (!op->value) {
free(op);
- return false;
+ return 0;
+ }
+
+ req = request_create(client);
+ if (!req) {
+ free(op->value);
+ free(op);
+ return 0;
}
memcpy(op->value, value, length);
@@ -2476,40 +2519,41 @@ bool bt_gatt_client_write_long_value(struct bt_gatt_client *client,
op->user_data = user_data;
op->destroy = destroy;
+ req->data = op;
+ req->destroy = long_write_op_free;
+ req->long_write = true;
+
if (client->in_long_write) {
- queue_push_tail(client->long_write_queue,
- long_write_op_ref(op));
- return true;
+ queue_push_tail(client->long_write_queue, req);
+ return req->id;
}
pdu = malloc(op->cur_length + 4);
if (!pdu) {
free(op->value);
free(op);
- return false;
+ return 0;
}
put_le16(value_handle, pdu);
put_le16(offset, pdu + 2);
memcpy(pdu + 4, op->value, op->cur_length);
- status = bt_att_send(client->att, BT_ATT_OP_PREP_WRITE_REQ,
+ req->att_id = bt_att_send(client->att, BT_ATT_OP_PREP_WRITE_REQ,
pdu, op->cur_length + 4,
- prepare_write_cb,
- long_write_op_ref(op),
- long_write_op_unref);
-
+ prepare_write_cb, req,
+ request_unref);
free(pdu);
- if (!status) {
- free(op->value);
- free(op);
- return false;
+ if (!req->att_id) {
+ op->destroy = NULL;
+ request_unref(req);
+ return 0;
}
client->in_long_write = true;
- return true;
+ return req->id;
}
static bool match_notify_chrc_value_handle(const void *a, const void *b)
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 74d781c..9a00feb 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -102,7 +102,7 @@ unsigned int bt_gatt_client_write_value(struct bt_gatt_client *client,
bt_gatt_client_callback_t callback,
void *user_data,
bt_gatt_client_destroy_func_t destroy);
-bool bt_gatt_client_write_long_value(struct bt_gatt_client *client,
+unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
bool reliable,
uint16_t value_handle, uint16_t offset,
const uint8_t *value, uint16_t length,
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH BlueZ v2 04/14] core: gatt: Cancel pending reads/writes
2015-01-08 5:48 [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
` (2 preceding siblings ...)
2015-01-08 5:48 ` [PATCH BlueZ v2 03/14] shared/gatt-client: Make long-write cancelable Arman Uguray
@ 2015-01-08 5:48 ` Arman Uguray
2015-01-08 5:48 ` [PATCH BlueZ v2 05/14] shared/gatt-db: Add gatt_db_attribute_reset Arman Uguray
` (10 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Arman Uguray @ 2015-01-08 5:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
Exported characteristic and descriptor structures now keep track of
pending read/write procedure ids and cancel the operation when they are
free'd.
---
src/gatt-client.c | 157 +++++++++++++++++++++++++++++++-----------------------
1 file changed, 89 insertions(+), 68 deletions(-)
diff --git a/src/gatt-client.c b/src/gatt-client.c
index 8bbb03d..a118870 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -83,8 +83,8 @@ struct characteristic {
bt_uuid_t uuid;
char *path;
- bool in_read;
- bool in_write;
+ unsigned int read_id;
+ unsigned int write_id;
struct queue *descs;
};
@@ -96,8 +96,8 @@ struct descriptor {
bt_uuid_t uuid;
char *path;
- bool in_read;
- bool in_write;
+ unsigned int read_id;
+ unsigned int write_id;
};
static bool uuid_cmp(const bt_uuid_t *uuid, uint16_t u16)
@@ -339,7 +339,7 @@ static void desc_read_cb(bool success, uint8_t att_ecode,
if (!success) {
DBusMessage *reply = create_gatt_dbus_error(op->msg, att_ecode);
- desc->in_read = false;
+ desc->read_id = 0;
g_dbus_send_message(btd_get_dbus_connection(), reply);
return;
}
@@ -354,16 +354,18 @@ static void desc_read_cb(bool success, uint8_t att_ecode,
*/
if (length == bt_gatt_client_get_mtu(service->client->gatt) - 1) {
op->offset += length;
- if (bt_gatt_client_read_long_value(service->client->gatt,
+ desc->read_id = bt_gatt_client_read_long_value(
+ service->client->gatt,
desc->handle,
op->offset,
desc_read_cb,
async_dbus_op_ref(op),
- async_dbus_op_unref))
+ async_dbus_op_unref);
+ if (desc->read_id)
return;
}
- desc->in_read = false;
+ desc->read_id = 0;
/* Read the stored data from db */
gatt_db_attribute_read(desc->attr, 0, 0, NULL, read_op_cb, op);
@@ -376,7 +378,7 @@ static DBusMessage *descriptor_read_value(DBusConnection *conn,
struct bt_gatt_client *gatt = desc->chrc->service->client->gatt;
struct async_dbus_op *op;
- if (desc->in_read)
+ if (desc->read_id)
return btd_error_in_progress(msg);
op = new0(struct async_dbus_op, 1);
@@ -386,12 +388,12 @@ static DBusMessage *descriptor_read_value(DBusConnection *conn,
op->msg = dbus_message_ref(msg);
op->data = desc;
- if (bt_gatt_client_read_value(gatt, desc->handle, desc_read_cb,
+ desc->read_id = bt_gatt_client_read_value(gatt, desc->handle,
+ desc_read_cb,
async_dbus_op_ref(op),
- async_dbus_op_unref)) {
- desc->in_read = true;
+ async_dbus_op_unref);
+ if (desc->read_id)
return NULL;
- }
async_dbus_op_free(op);
@@ -435,13 +437,14 @@ static void write_cb(bool success, uint8_t att_ecode, void *user_data)
write_result_cb(success, false, att_ecode, user_data);
}
-static bool start_long_write(DBusMessage *msg, uint16_t handle,
+static unsigned int start_long_write(DBusMessage *msg, uint16_t handle,
struct bt_gatt_client *gatt,
bool reliable, const uint8_t *value,
size_t value_len, void *data,
async_dbus_op_complete_t complete)
{
struct async_dbus_op *op;
+ unsigned int id;
op = new0(struct async_dbus_op, 1);
if (!op)
@@ -451,24 +454,25 @@ static bool start_long_write(DBusMessage *msg, uint16_t handle,
op->data = data;
op->complete = complete;
- if (bt_gatt_client_write_long_value(gatt, reliable, handle,
+ id = bt_gatt_client_write_long_value(gatt, reliable, handle,
0, value, value_len,
write_result_cb, op,
- async_dbus_op_free))
- return true;
+ async_dbus_op_free);
- async_dbus_op_free(op);
+ if (!id)
+ async_dbus_op_free(op);
- return false;
+ return id;
}
-static bool start_write_request(DBusMessage *msg, uint16_t handle,
+static unsigned int start_write_request(DBusMessage *msg, uint16_t handle,
struct bt_gatt_client *gatt,
const uint8_t *value, size_t value_len,
void *data,
async_dbus_op_complete_t complete)
{
struct async_dbus_op *op;
+ unsigned int id;
op = new0(struct async_dbus_op, 1);
if (!op)
@@ -478,21 +482,20 @@ static bool start_write_request(DBusMessage *msg, uint16_t handle,
op->data = data;
op->complete = complete;
- if (bt_gatt_client_write_value(gatt, handle, value, value_len,
+ id = bt_gatt_client_write_value(gatt, handle, value, value_len,
write_cb, op,
- async_dbus_op_free))
- return true;
+ async_dbus_op_free);
+ if (!id)
+ async_dbus_op_free(op);
- async_dbus_op_free(op);
-
- return false;
+ return id;
}
static bool desc_write_complete(void *data)
{
struct descriptor *desc = data;
- desc->in_write = false;
+ desc->write_id = false;
/*
* The descriptor might have been unregistered during the read. Return
@@ -508,9 +511,8 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
struct bt_gatt_client *gatt = desc->chrc->service->client->gatt;
uint8_t *value = NULL;
size_t value_len = 0;
- bool result;
- if (desc->in_write)
+ if (desc->write_id)
return btd_error_in_progress(msg);
if (!parse_value_arg(msg, &value, &value_len))
@@ -529,19 +531,19 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
* write.
*/
if (value_len <= (unsigned) bt_gatt_client_get_mtu(gatt) - 3)
- result = start_write_request(msg, desc->handle, gatt, value,
+ desc->write_id = start_write_request(msg, desc->handle,
+ gatt, value,
value_len, desc,
desc_write_complete);
else
- result = start_long_write(msg, desc->handle, gatt, false, value,
+ desc->write_id = start_long_write(msg, desc->handle,
+ gatt, false, value,
value_len, desc,
desc_write_complete);
- if (!result)
+ if (!desc->write_id)
return btd_error_failed(msg, "Failed to initiate write");
- desc->in_write = true;
-
return NULL;
}
@@ -614,9 +616,18 @@ static struct descriptor *descriptor_create(struct gatt_db_attribute *attr,
static void unregister_descriptor(void *data)
{
struct descriptor *desc = data;
+ struct bt_gatt_client *gatt = desc->chrc->service->client->gatt;
DBG("Removing GATT descriptor: %s", desc->path);
+ if (desc->read_id)
+ bt_gatt_client_cancel(gatt, desc->read_id);
+
+ if (desc->write_id)
+ bt_gatt_client_cancel(gatt, desc->write_id);
+
+ desc->chrc = NULL;
+
g_dbus_unregister_interface(btd_get_dbus_connection(), desc->path,
GATT_DESCRIPTOR_IFACE);
}
@@ -757,7 +768,7 @@ static void chrc_read_cb(bool success, uint8_t att_ecode, const uint8_t *value,
if (!success) {
DBusMessage *reply = create_gatt_dbus_error(op->msg, att_ecode);
- chrc->in_read = false;
+ chrc->read_id = 0;
g_dbus_send_message(btd_get_dbus_connection(), reply);
return ;
}
@@ -772,16 +783,18 @@ static void chrc_read_cb(bool success, uint8_t att_ecode, const uint8_t *value,
*/
if (length == bt_gatt_client_get_mtu(service->client->gatt) - 1) {
op->offset += length;
- if (bt_gatt_client_read_long_value(service->client->gatt,
+ chrc->read_id = bt_gatt_client_read_long_value(
+ service->client->gatt,
chrc->value_handle,
op->offset,
chrc_read_cb,
async_dbus_op_ref(op),
- async_dbus_op_unref))
+ async_dbus_op_unref);
+ if (chrc->read_id)
return;
}
- chrc->in_read = false;
+ chrc->read_id = 0;
/* Read the stored data from db */
gatt_db_attribute_read(chrc->attr, 0, 0, NULL, read_op_cb, op);
@@ -794,7 +807,7 @@ static DBusMessage *characteristic_read_value(DBusConnection *conn,
struct bt_gatt_client *gatt = chrc->service->client->gatt;
struct async_dbus_op *op;
- if (chrc->in_read)
+ if (chrc->read_id)
return btd_error_in_progress(msg);
op = new0(struct async_dbus_op, 1);
@@ -804,12 +817,12 @@ static DBusMessage *characteristic_read_value(DBusConnection *conn,
op->msg = dbus_message_ref(msg);
op->data = chrc;
- if (bt_gatt_client_read_value(gatt, chrc->value_handle, chrc_read_cb,
- async_dbus_op_ref(op),
- async_dbus_op_unref)) {
- chrc->in_read = true;
+ chrc->read_id = bt_gatt_client_read_value(gatt, chrc->value_handle,
+ chrc_read_cb,
+ async_dbus_op_ref(op),
+ async_dbus_op_unref);
+ if (chrc->read_id)
return NULL;
- }
async_dbus_op_free(op);
@@ -820,7 +833,7 @@ static bool chrc_write_complete(void *data)
{
struct characteristic *chrc = data;
- chrc->in_write = false;
+ chrc->write_id = false;
/*
* The characteristic might have been unregistered during the read.
@@ -836,17 +849,14 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
struct bt_gatt_client *gatt = chrc->service->client->gatt;
uint8_t *value = NULL;
size_t value_len = 0;
+ bool supported = false;
- if (chrc->in_write)
+ if (chrc->write_id)
return btd_error_in_progress(msg);
if (!parse_value_arg(msg, &value, &value_len))
return btd_error_invalid_args(msg);
- if (!(chrc->props & (BT_GATT_CHRC_PROP_WRITE |
- BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP)))
- return btd_error_not_supported(msg);
-
/*
* Decide which write to use based on characteristic properties. For now
* we don't perform signed writes since gatt-client doesn't support them
@@ -860,39 +870,43 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
*/
if (chrc->props & BT_GATT_CHRC_PROP_WRITE) {
uint16_t mtu;
- bool result;
+ supported = true;
mtu = bt_gatt_client_get_mtu(gatt);
if (!mtu)
return btd_error_failed(msg, "No ATT transport");
if (value_len <= (unsigned) mtu - 3)
- result = start_write_request(msg, chrc->value_handle,
- gatt, value,
- value_len, chrc,
- chrc_write_complete);
+ chrc->write_id = start_write_request(msg,
+ chrc->value_handle,
+ gatt, value, value_len,
+ chrc, chrc_write_complete);
else
- result = start_long_write(msg, chrc->value_handle, gatt,
- false, value, value_len, chrc,
- chrc_write_complete);
+ chrc->write_id = start_long_write(msg,
+ chrc->value_handle, gatt,
+ false, value, value_len,
+ chrc, chrc_write_complete);
- if (result)
- goto done_async;
+ if (chrc->write_id)
+ return NULL;
}
- if ((chrc->props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP) &&
- bt_gatt_client_write_without_response(gatt,
+ if (!(chrc->props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP))
+ goto fail;
+
+ supported = true;
+ chrc->write_id = bt_gatt_client_write_without_response(gatt,
chrc->value_handle,
false, value,
- value_len))
+ value_len);
+ if (chrc->write_id)
return dbus_message_new_method_return(msg);
- return btd_error_failed(msg, "Failed to initiate write");
-
-done_async:
- chrc->in_write = true;
+fail:
+ if (supported)
+ return btd_error_failed(msg, "Failed to initiate write");
- return NULL;
+ return btd_error_not_supported(msg);
}
static DBusMessage *characteristic_start_notify(DBusConnection *conn,
@@ -1024,9 +1038,16 @@ static struct characteristic *characteristic_create(
static void unregister_characteristic(void *data)
{
struct characteristic *chrc = data;
+ struct bt_gatt_client *gatt = chrc->service->client->gatt;
DBG("Removing GATT characteristic: %s", chrc->path);
+ if (chrc->read_id)
+ bt_gatt_client_cancel(gatt, chrc->read_id);
+
+ if (chrc->write_id)
+ bt_gatt_client_cancel(gatt, chrc->write_id);
+
queue_remove_all(chrc->descs, NULL, NULL, unregister_descriptor);
g_dbus_unregister_interface(btd_get_dbus_connection(), chrc->path,
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH BlueZ v2 05/14] shared/gatt-db: Add gatt_db_attribute_reset
2015-01-08 5:48 [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
` (3 preceding siblings ...)
2015-01-08 5:48 ` [PATCH BlueZ v2 04/14] core: gatt: Cancel pending reads/writes Arman Uguray
@ 2015-01-08 5:48 ` Arman Uguray
2015-01-08 5:48 ` [PATCH BlueZ v2 06/14] core: gatt: Reset value in db when caching Arman Uguray
` (9 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Arman Uguray @ 2015-01-08 5:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch adds the gatt_db_attribute_reset function, which clears the
value of an attribute that is stored in the database.
---
src/shared/gatt-db.c | 15 +++++++++++++++
src/shared/gatt-db.h | 2 ++
2 files changed, 17 insertions(+)
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index eac7948..9a9cadc 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -1544,3 +1544,18 @@ bool gatt_db_attribute_write_result(struct gatt_db_attribute *attrib,
return true;
}
+
+bool gatt_db_attribute_reset(struct gatt_db_attribute *attrib)
+{
+ if (!attrib)
+ return false;
+
+ if (!attrib->value || !attrib->value_len)
+ return true;
+
+ free(attrib->value);
+ attrib->value = NULL;
+ attrib->value_len = 0;
+
+ return true;
+}
diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
index 2edd13f..1f4005e 100644
--- a/src/shared/gatt-db.h
+++ b/src/shared/gatt-db.h
@@ -207,3 +207,5 @@ bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset,
bool gatt_db_attribute_write_result(struct gatt_db_attribute *attrib,
unsigned int id, int err);
+
+bool gatt_db_attribute_reset(struct gatt_db_attribute *attrib);
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH BlueZ v2 06/14] core: gatt: Reset value in db when caching
2015-01-08 5:48 [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
` (4 preceding siblings ...)
2015-01-08 5:48 ` [PATCH BlueZ v2 05/14] shared/gatt-db: Add gatt_db_attribute_reset Arman Uguray
@ 2015-01-08 5:48 ` Arman Uguray
2015-01-08 5:48 ` [PATCH BlueZ v2 07/14] core: gatt: Issue long write for reliable-write Arman Uguray
` (8 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Arman Uguray @ 2015-01-08 5:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
After an attribute value is read, changes in the attribute value length
may cause incorrect bytes to remain in the database if the value is not
properly truncated. This patch addresses this by resetting the attribute
value before storing an updated value.
---
src/gatt-client.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/gatt-client.c b/src/gatt-client.c
index a118870..46af263 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -344,6 +344,7 @@ static void desc_read_cb(bool success, uint8_t att_ecode,
return;
}
+ gatt_db_attribute_reset(desc->attr);
gatt_db_attribute_write(desc->attr, 0, value, length, 0, NULL,
write_descriptor_cb, desc);
@@ -773,6 +774,7 @@ static void chrc_read_cb(bool success, uint8_t att_ecode, const uint8_t *value,
return ;
}
+ gatt_db_attribute_reset(chrc->attr);
gatt_db_attribute_write(chrc->attr, 0, value, length, op->offset, NULL,
write_characteristic_cb, chrc);
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH BlueZ v2 07/14] core: gatt: Issue long write for reliable-write
2015-01-08 5:48 [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
` (5 preceding siblings ...)
2015-01-08 5:48 ` [PATCH BlueZ v2 06/14] core: gatt: Reset value in db when caching Arman Uguray
@ 2015-01-08 5:48 ` Arman Uguray
2015-01-08 5:48 ` [PATCH BlueZ v2 08/14] core: gatt: Handle Service Changed Arman Uguray
` (7 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Arman Uguray @ 2015-01-08 5:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
If the characteristic has the "reliable-write" extended property,
GattCharacteristic1.WriteValue will now start a reliable long-write
procedure.
---
src/gatt-client.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/gatt-client.c b/src/gatt-client.c
index 46af263..77e3539 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -870,6 +870,15 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
* - If value is larger than MTU - 3: long-write
* * "write-without-response" property set -> write command.
*/
+ if ((chrc->ext_props & BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE)) {
+ supported = true;
+ chrc->write_id = start_long_write(msg, chrc->value_handle, gatt,
+ true, value, value_len,
+ chrc, chrc_write_complete);
+ if (chrc->write_id)
+ return NULL;
+ }
+
if (chrc->props & BT_GATT_CHRC_PROP_WRITE) {
uint16_t mtu;
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH BlueZ v2 08/14] core: gatt: Handle Service Changed.
2015-01-08 5:48 [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
` (6 preceding siblings ...)
2015-01-08 5:48 ` [PATCH BlueZ v2 07/14] core: gatt: Issue long write for reliable-write Arman Uguray
@ 2015-01-08 5:48 ` Arman Uguray
2015-01-08 5:48 ` [PATCH BlueZ v2 09/14] core: device: Fix GATT profile probing Arman Uguray
` (6 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Arman Uguray @ 2015-01-08 5:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch adds handling for Service Changed events. All exported
objects that match attributes within the changed handle range are
unregistered and new ones get exported based on the newly discovered
services.
---
src/device.c | 4 ++--
src/gatt-client.c | 35 +++++++++++++++++++++++++++++++----
2 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/src/device.c b/src/device.c
index e22f92d..1236698 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2703,7 +2703,7 @@ static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data)
if (l)
service_accept(l->data);
- store_services(device);
+ store_device_info(device);
btd_gatt_client_service_added(device->client_dbus, attr);
}
@@ -2776,7 +2776,7 @@ static void gatt_service_removed(struct gatt_db_attribute *attr,
g_free(prim);
- store_services(device);
+ store_device_info(device);
btd_gatt_client_service_removed(device->client_dbus, attr);
}
diff --git a/src/gatt-client.c b/src/gatt-client.c
index 77e3539..d792670 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -1220,6 +1220,11 @@ static void unregister_service(void *data)
static void notify_chrcs(struct service *service)
{
+
+ if (service->chrcs_ready ||
+ !queue_isempty(service->pending_ext_props))
+ return;
+
service->chrcs_ready = true;
g_dbus_emit_property_changed(btd_get_dbus_connection(), service->path,
@@ -1276,8 +1281,7 @@ static void read_ext_props_cb(bool success, uint8_t att_ecode,
queue_remove(service->pending_ext_props, chrc);
- if (queue_isempty(service->pending_ext_props))
- notify_chrcs(service);
+ notify_chrcs(service);
}
static void read_ext_props(void *data, void *user_data)
@@ -1459,13 +1463,36 @@ void btd_gatt_client_ready(struct btd_gatt_client *client)
void btd_gatt_client_service_added(struct btd_gatt_client *client,
struct gatt_db_attribute *attrib)
{
- /* TODO */
+ if (!client)
+ return;
+
+ export_service(attrib, client);
+}
+
+static bool match_service_handle(const void *a, const void *b)
+{
+ const struct service *service = a;
+ uint16_t start_handle = PTR_TO_UINT(b);
+
+ return service->start_handle == start_handle;
}
void btd_gatt_client_service_removed(struct btd_gatt_client *client,
struct gatt_db_attribute *attrib)
{
- /* TODO */
+ uint16_t start_handle, end_handle;
+
+ if (!client || !attrib)
+ return;
+
+ gatt_db_attribute_get_service_handles(attrib, &start_handle,
+ &end_handle);
+
+ DBG("GATT Services Removed - start: 0x%04x, end: 0x%04x", start_handle,
+ end_handle);
+ queue_remove_all(client->services, match_service_handle,
+ UINT_TO_PTR(start_handle),
+ unregister_service);
}
void btd_gatt_client_disconnected(struct btd_gatt_client *client)
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH BlueZ v2 09/14] core: device: Fix GATT profile probing
2015-01-08 5:48 [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
` (7 preceding siblings ...)
2015-01-08 5:48 ` [PATCH BlueZ v2 08/14] core: gatt: Handle Service Changed Arman Uguray
@ 2015-01-08 5:48 ` Arman Uguray
2015-01-12 22:04 ` Luiz Augusto von Dentz
2015-01-08 5:48 ` [PATCH BlueZ v2 10/14] profiles/gap: Fix probe/accept behavior Arman Uguray
` (5 subsequent siblings)
14 siblings, 1 reply; 18+ messages in thread
From: Arman Uguray @ 2015-01-08 5:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch fixes a recently introduced issue which changed the behavior
of profile probing for GATT-based profiles to create a btd_service
instance for all discovered GATT services to be on a per-UUID basis
rather than per-service.
---
src/device.c | 176 ++++++++++++++++++++++-------------------------------------
1 file changed, 64 insertions(+), 112 deletions(-)
diff --git a/src/device.c b/src/device.c
index 1236698..a9dc22d 100644
--- a/src/device.c
+++ b/src/device.c
@@ -295,21 +295,15 @@ static GSList *find_service_with_state(GSList *list,
return NULL;
}
-static GSList *find_service_with_gatt_handles(GSList *list,
- uint16_t start_handle,
- uint16_t end_handle)
+static GSList *find_service_with_uuid(GSList *list, char *uuid)
{
GSList *l;
- uint16_t svc_start, svc_end;
for (l = list; l != NULL; l = g_slist_next(l)) {
struct btd_service *service = l->data;
+ struct btd_profile *profile = btd_service_get_profile(service);
- if (!btd_service_get_gatt_handles(service, &svc_start,
- &svc_end))
- continue;
-
- if (svc_start == start_handle && svc_end == end_handle)
+ if (bt_uuid_strcmp(profile->remote_uuid, uuid) == 0)
return l;
}
@@ -2420,6 +2414,7 @@ static void add_primary(struct gatt_db_attribute *attr, void *user_data)
static void device_add_uuids(struct btd_device *device, GSList *uuids)
{
GSList *l;
+ bool changed = false;
for (l = uuids; l != NULL; l = g_slist_next(l)) {
GSList *match = g_slist_find_custom(device->uuids, l->data,
@@ -2427,12 +2422,14 @@ static void device_add_uuids(struct btd_device *device, GSList *uuids)
if (match)
continue;
+ changed = true;
device->uuids = g_slist_insert_sorted(device->uuids,
g_strdup(l->data),
bt_uuid_strcmp);
}
- g_dbus_emit_property_changed(dbus_conn, device->path,
+ if (changed)
+ g_dbus_emit_property_changed(dbus_conn, device->path,
DEVICE_INTERFACE, "UUIDs");
}
@@ -2444,8 +2441,6 @@ struct gatt_probe_data {
GSList *uuids;
struct gatt_db_attribute *cur_attr;
char cur_uuid[MAX_LEN_UUID_STR];
- uint16_t start_handle, end_handle;
- GSList *valid_services;
};
static bool device_match_profile(struct btd_device *device,
@@ -2473,8 +2468,7 @@ static void dev_probe_gatt(struct btd_profile *p, void *user_data)
if (!p->remote_uuid || bt_uuid_strcmp(p->remote_uuid, data->cur_uuid))
return;
- service = service_create_gatt(data->dev, p, data->start_handle,
- data->end_handle);
+ service = service_create(data->dev, p);
if (!service)
return;
@@ -2487,72 +2481,6 @@ static void dev_probe_gatt(struct btd_profile *p, void *user_data)
gatt_db_service_set_active(data->cur_attr, false);
data->dev->services = g_slist_append(data->dev->services, service);
-
- if (data->all_services)
- data->valid_services = g_slist_append(data->valid_services,
- service);
-}
-
-static bool match_existing_service(struct gatt_probe_data *data)
-{
- struct btd_device *dev = data->dev;
- struct btd_service *service;
- struct btd_profile *profile;
- uint16_t start, end;
- GSList *l, *tmp;
-
- /*
- * Check if the profiles should be probed for the service in the
- * database.
- */
- for (l = dev->services; l != NULL;) {
- service = l->data;
- profile = btd_service_get_profile(service);
-
- /* If this is local or non-GATT based service, then skip. */
- if (!profile->remote_uuid ||
- !btd_service_get_gatt_handles(service, &start, &end)) {
- l = g_slist_next(l);
- continue;
- }
-
- /* Skip this service if the handle ranges don't overlap. */
- if (start > data->end_handle || end < data->start_handle) {
- l = g_slist_next(l);
- continue;
- }
-
- /*
- * If there is an exact match, then there's no need to probe the
- * profiles. An exact match is when the service handles AND the
- * service UUID match.
- */
- if (start == data->start_handle && end == data->end_handle &&
- !bt_uuid_strcmp(profile->remote_uuid, data->cur_uuid)) {
- if (data->all_services)
- data->valid_services = g_slist_append(
- data->valid_services, service);
- return true;
- }
-
- /*
- * The handles overlap but there is no exact match. This means
- * that the service is no longer valid. Remove it.
- *
- * TODO: The following code is fairly inefficient, especially
- * when we consider all the extra searches that we're already
- * doing. Consider changing the services list to a GList.
- */
- tmp = l->next;
- dev->services = g_slist_delete_link(dev->services, l);
- dev->pending = g_slist_remove(dev->pending, service);
- service_remove(service);
-
- l = tmp;
- }
-
- /* No match found */
- return false;
}
static void dev_probe_gatt_profile(struct gatt_db_attribute *attr,
@@ -2562,22 +2490,27 @@ static void dev_probe_gatt_profile(struct gatt_db_attribute *attr,
bt_uuid_t uuid;
GSList *l = NULL;
- gatt_db_attribute_get_service_data(attr, &data->start_handle,
- &data->end_handle, NULL,
- &uuid);
+ gatt_db_attribute_get_service_uuid(attr, &uuid);
bt_uuid_to_string(&uuid, data->cur_uuid, sizeof(data->cur_uuid));
data->cur_attr = attr;
- /* Don't probe the profiles if a matching service already exists. */
- if (!match_existing_service(data))
- btd_profile_foreach(dev_probe_gatt, data);
-
- if (data->all_services) {
+ /*
+ * If we're probing for all services, store the UUID since device->uuids
+ * was cleared.
+ */
+ if (data->all_services)
data->uuids = g_slist_append(data->uuids,
g_strdup(data->cur_uuid));
+
+ /* Don't probe the profiles if a matching service already exists. */
+ if (find_service_with_uuid(data->dev->services, data->cur_uuid))
+ return;
+
+ btd_profile_foreach(dev_probe_gatt, data);
+
+ if (data->all_services)
return;
- }
l = g_slist_append(l, g_strdup(data->cur_uuid));
device_add_uuids(data->dev, l);
@@ -2600,12 +2533,15 @@ static void remove_invalid_services(struct gatt_probe_data *data)
{
struct btd_device *dev = data->dev;
struct btd_service *service;
+ struct btd_profile *profile;
GSList *l, *tmp;
for (l = dev->services; l != NULL;) {
service = l->data;
+ profile = btd_service_get_profile(service);
- if (g_slist_find(data->valid_services, service)) {
+ if (g_slist_find_custom(dev->uuids, profile->remote_uuid,
+ bt_uuid_strcmp)) {
l = g_slist_next(l);
continue;
}
@@ -2639,11 +2575,15 @@ static void device_probe_gatt_profiles(struct btd_device *device)
gatt_db_foreach_service(device->db, NULL, dev_probe_gatt_profile,
&data);
+
+ /* Clear the UUIDs list */
+ g_slist_free_full(device->uuids, g_free);
+ device->uuids = NULL;
+
device_add_uuids(device, data.uuids);
g_slist_free_full(data.uuids, g_free);
remove_invalid_services(&data);
- g_slist_free(data.valid_services);
}
static void device_accept_gatt_profiles(struct btd_device *device)
@@ -2657,13 +2597,15 @@ static void device_accept_gatt_profiles(struct btd_device *device)
static void device_remove_gatt_profile(struct btd_device *device,
struct gatt_db_attribute *attr)
{
- uint16_t start, end;
struct btd_service *service;
+ bt_uuid_t uuid;
+ char uuid_str[MAX_LEN_UUID_STR];
GSList *l;
- gatt_db_attribute_get_service_handles(attr, &start, &end);
+ gatt_db_attribute_get_service_uuid(attr, &uuid);
+ bt_uuid_to_string(&uuid, uuid_str, sizeof(uuid_str));
- l = find_service_with_gatt_handles(device->services, start, end);
+ l = find_service_with_uuid(device->services, uuid_str);
if (!l)
return;
@@ -2677,13 +2619,16 @@ static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data)
{
struct btd_device *device = user_data;
GSList *new_service = NULL;
+ bt_uuid_t uuid;
+ char uuid_str[MAX_LEN_UUID_STR];
uint16_t start, end;
GSList *l;
if (!bt_gatt_client_is_ready(device->client))
return;
- gatt_db_attribute_get_service_handles(attr, &start, &end);
+ gatt_db_attribute_get_service_data(attr, &start, &end, NULL, &uuid);
+ bt_uuid_to_string(&uuid, uuid_str, sizeof(uuid_str));
DBG("start: 0x%04x, end: 0x%04x", start, end);
@@ -2695,14 +2640,19 @@ static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data)
if (!new_service)
return;
- device_register_primaries(device, new_service, -1);
+ l = find_service_with_uuid(device->services, uuid_str);
- device_probe_gatt_profile(device, attr);
+ device_register_primaries(device, new_service, -1);
- l = find_service_with_gatt_handles(device->services, start, end);
- if (l)
+ /*
+ * If the profile was probed for the first time then call accept on
+ * the service.
+ */
+ if (!l && (l = find_service_with_uuid(device->services, uuid_str)))
service_accept(l->data);
+ device_probe_gatt_profile(device, attr);
+
store_device_info(device);
btd_gatt_client_service_added(device->client_dbus, attr);
@@ -2750,24 +2700,26 @@ static void gatt_service_removed(struct gatt_db_attribute *attr,
prim = l->data;
device->primaries = g_slist_delete_link(device->primaries, l);
- /*
- * If this happend since the db was cleared for a non-bonded device,
- * then don't remove the btd_service just yet. We do this so that we can
- * avoid re-probing the profile if the same GATT service is found on the
- * device on re-connection. However, if the device is marked as
- * temporary, then we remove it anyway.
- */
- if (device->client || device->temporary == TRUE)
- device_remove_gatt_profile(device, attr);
+ /* Get the UUID entry to be removed below */
+ l = g_slist_find_custom(device->uuids, prim->uuid, bt_uuid_strcmp);
/*
- * Remove the corresponding UUIDs entry, only if this is the last
- * service with this UUID.
+ * Remove the corresponding UUIDs entry and the profile, only if this
+ * is the last service with this UUID.
*/
- l = g_slist_find_custom(device->uuids, prim->uuid, bt_uuid_strcmp);
-
if (!g_slist_find_custom(device->primaries, prim->uuid,
prim_uuid_cmp)) {
+ /*
+ * If this happened since the db was cleared for a non-bonded
+ * device, then don't remove the btd_service just yet. We do
+ * this so that we can avoid re-probing the profile if the same
+ * GATT service is found on the device on re-connection.
+ * However, if the device is marked as temporary, then we remove
+ * it anyway.
+ */
+ if (device->client || device->temporary == TRUE)
+ device_remove_gatt_profile(device, attr);
+
g_free(l->data);
device->uuids = g_slist_delete_link(device->uuids, l);
g_dbus_emit_property_changed(dbus_conn, device->path,
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH BlueZ v2 09/14] core: device: Fix GATT profile probing
2015-01-08 5:48 ` [PATCH BlueZ v2 09/14] core: device: Fix GATT profile probing Arman Uguray
@ 2015-01-12 22:04 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 18+ messages in thread
From: Luiz Augusto von Dentz @ 2015-01-12 22:04 UTC (permalink / raw)
To: Arman Uguray; +Cc: linux-bluetooth@vger.kernel.org
Hi Arman,
On Thu, Jan 8, 2015 at 3:48 AM, Arman Uguray <armansito@chromium.org> wrote:
> This patch fixes a recently introduced issue which changed the behavior
> of profile probing for GATT-based profiles to create a btd_service
> instance for all discovered GATT services to be on a per-UUID basis
> rather than per-service.
> ---
> src/device.c | 176 ++++++++++++++++++++++-------------------------------------
> 1 file changed, 64 insertions(+), 112 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index 1236698..a9dc22d 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -295,21 +295,15 @@ static GSList *find_service_with_state(GSList *list,
> return NULL;
> }
>
> -static GSList *find_service_with_gatt_handles(GSList *list,
> - uint16_t start_handle,
> - uint16_t end_handle)
> +static GSList *find_service_with_uuid(GSList *list, char *uuid)
> {
> GSList *l;
> - uint16_t svc_start, svc_end;
>
> for (l = list; l != NULL; l = g_slist_next(l)) {
> struct btd_service *service = l->data;
> + struct btd_profile *profile = btd_service_get_profile(service);
>
> - if (!btd_service_get_gatt_handles(service, &svc_start,
> - &svc_end))
> - continue;
> -
> - if (svc_start == start_handle && svc_end == end_handle)
> + if (bt_uuid_strcmp(profile->remote_uuid, uuid) == 0)
> return l;
> }
>
> @@ -2420,6 +2414,7 @@ static void add_primary(struct gatt_db_attribute *attr, void *user_data)
> static void device_add_uuids(struct btd_device *device, GSList *uuids)
> {
> GSList *l;
> + bool changed = false;
>
> for (l = uuids; l != NULL; l = g_slist_next(l)) {
> GSList *match = g_slist_find_custom(device->uuids, l->data,
> @@ -2427,12 +2422,14 @@ static void device_add_uuids(struct btd_device *device, GSList *uuids)
> if (match)
> continue;
>
> + changed = true;
> device->uuids = g_slist_insert_sorted(device->uuids,
> g_strdup(l->data),
> bt_uuid_strcmp);
> }
>
> - g_dbus_emit_property_changed(dbus_conn, device->path,
> + if (changed)
> + g_dbus_emit_property_changed(dbus_conn, device->path,
> DEVICE_INTERFACE, "UUIDs");
> }
>
> @@ -2444,8 +2441,6 @@ struct gatt_probe_data {
> GSList *uuids;
> struct gatt_db_attribute *cur_attr;
> char cur_uuid[MAX_LEN_UUID_STR];
> - uint16_t start_handle, end_handle;
> - GSList *valid_services;
> };
>
> static bool device_match_profile(struct btd_device *device,
> @@ -2473,8 +2468,7 @@ static void dev_probe_gatt(struct btd_profile *p, void *user_data)
> if (!p->remote_uuid || bt_uuid_strcmp(p->remote_uuid, data->cur_uuid))
> return;
>
> - service = service_create_gatt(data->dev, p, data->start_handle,
> - data->end_handle);
> + service = service_create(data->dev, p);
> if (!service)
> return;
>
> @@ -2487,72 +2481,6 @@ static void dev_probe_gatt(struct btd_profile *p, void *user_data)
> gatt_db_service_set_active(data->cur_attr, false);
>
> data->dev->services = g_slist_append(data->dev->services, service);
> -
> - if (data->all_services)
> - data->valid_services = g_slist_append(data->valid_services,
> - service);
> -}
> -
> -static bool match_existing_service(struct gatt_probe_data *data)
> -{
> - struct btd_device *dev = data->dev;
> - struct btd_service *service;
> - struct btd_profile *profile;
> - uint16_t start, end;
> - GSList *l, *tmp;
> -
> - /*
> - * Check if the profiles should be probed for the service in the
> - * database.
> - */
> - for (l = dev->services; l != NULL;) {
> - service = l->data;
> - profile = btd_service_get_profile(service);
> -
> - /* If this is local or non-GATT based service, then skip. */
> - if (!profile->remote_uuid ||
> - !btd_service_get_gatt_handles(service, &start, &end)) {
> - l = g_slist_next(l);
> - continue;
> - }
> -
> - /* Skip this service if the handle ranges don't overlap. */
> - if (start > data->end_handle || end < data->start_handle) {
> - l = g_slist_next(l);
> - continue;
> - }
> -
> - /*
> - * If there is an exact match, then there's no need to probe the
> - * profiles. An exact match is when the service handles AND the
> - * service UUID match.
> - */
> - if (start == data->start_handle && end == data->end_handle &&
> - !bt_uuid_strcmp(profile->remote_uuid, data->cur_uuid)) {
> - if (data->all_services)
> - data->valid_services = g_slist_append(
> - data->valid_services, service);
> - return true;
> - }
> -
> - /*
> - * The handles overlap but there is no exact match. This means
> - * that the service is no longer valid. Remove it.
> - *
> - * TODO: The following code is fairly inefficient, especially
> - * when we consider all the extra searches that we're already
> - * doing. Consider changing the services list to a GList.
> - */
> - tmp = l->next;
> - dev->services = g_slist_delete_link(dev->services, l);
> - dev->pending = g_slist_remove(dev->pending, service);
> - service_remove(service);
> -
> - l = tmp;
> - }
> -
> - /* No match found */
> - return false;
> }
>
> static void dev_probe_gatt_profile(struct gatt_db_attribute *attr,
> @@ -2562,22 +2490,27 @@ static void dev_probe_gatt_profile(struct gatt_db_attribute *attr,
> bt_uuid_t uuid;
> GSList *l = NULL;
>
> - gatt_db_attribute_get_service_data(attr, &data->start_handle,
> - &data->end_handle, NULL,
> - &uuid);
> + gatt_db_attribute_get_service_uuid(attr, &uuid);
> bt_uuid_to_string(&uuid, data->cur_uuid, sizeof(data->cur_uuid));
>
> data->cur_attr = attr;
>
> - /* Don't probe the profiles if a matching service already exists. */
> - if (!match_existing_service(data))
> - btd_profile_foreach(dev_probe_gatt, data);
> -
> - if (data->all_services) {
> + /*
> + * If we're probing for all services, store the UUID since device->uuids
> + * was cleared.
> + */
> + if (data->all_services)
> data->uuids = g_slist_append(data->uuids,
> g_strdup(data->cur_uuid));
> +
> + /* Don't probe the profiles if a matching service already exists. */
> + if (find_service_with_uuid(data->dev->services, data->cur_uuid))
> + return;
> +
> + btd_profile_foreach(dev_probe_gatt, data);
> +
> + if (data->all_services)
> return;
> - }
>
> l = g_slist_append(l, g_strdup(data->cur_uuid));
> device_add_uuids(data->dev, l);
> @@ -2600,12 +2533,15 @@ static void remove_invalid_services(struct gatt_probe_data *data)
> {
> struct btd_device *dev = data->dev;
> struct btd_service *service;
> + struct btd_profile *profile;
> GSList *l, *tmp;
>
> for (l = dev->services; l != NULL;) {
> service = l->data;
> + profile = btd_service_get_profile(service);
>
> - if (g_slist_find(data->valid_services, service)) {
> + if (g_slist_find_custom(dev->uuids, profile->remote_uuid,
> + bt_uuid_strcmp)) {
> l = g_slist_next(l);
> continue;
> }
> @@ -2639,11 +2575,15 @@ static void device_probe_gatt_profiles(struct btd_device *device)
>
> gatt_db_foreach_service(device->db, NULL, dev_probe_gatt_profile,
> &data);
> +
> + /* Clear the UUIDs list */
> + g_slist_free_full(device->uuids, g_free);
> + device->uuids = NULL;
> +
This would actually cause duplicated UUIDs to be emitted, this
probably won't work properly for dual-mode devices either since the
whole list is cleared, also it is probably better to use a list to
track the removed ones so you don't need to do a double lookup at the
end, to do that copy the uuids list at the start and remove them once
they are found (we might be better off with a different UUID list for
Gatt based profiles in this case) which mean what remains at the must
have been removed.
> device_add_uuids(device, data.uuids);
> g_slist_free_full(data.uuids, g_free);
>
> remove_invalid_services(&data);
> - g_slist_free(data.valid_services);
> }
>
> static void device_accept_gatt_profiles(struct btd_device *device)
> @@ -2657,13 +2597,15 @@ static void device_accept_gatt_profiles(struct btd_device *device)
> static void device_remove_gatt_profile(struct btd_device *device,
> struct gatt_db_attribute *attr)
> {
> - uint16_t start, end;
> struct btd_service *service;
> + bt_uuid_t uuid;
> + char uuid_str[MAX_LEN_UUID_STR];
> GSList *l;
>
> - gatt_db_attribute_get_service_handles(attr, &start, &end);
> + gatt_db_attribute_get_service_uuid(attr, &uuid);
> + bt_uuid_to_string(&uuid, uuid_str, sizeof(uuid_str));
>
> - l = find_service_with_gatt_handles(device->services, start, end);
> + l = find_service_with_uuid(device->services, uuid_str);
> if (!l)
> return;
>
> @@ -2677,13 +2619,16 @@ static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data)
> {
> struct btd_device *device = user_data;
> GSList *new_service = NULL;
> + bt_uuid_t uuid;
> + char uuid_str[MAX_LEN_UUID_STR];
> uint16_t start, end;
> GSList *l;
>
> if (!bt_gatt_client_is_ready(device->client))
> return;
>
> - gatt_db_attribute_get_service_handles(attr, &start, &end);
> + gatt_db_attribute_get_service_data(attr, &start, &end, NULL, &uuid);
> + bt_uuid_to_string(&uuid, uuid_str, sizeof(uuid_str));
>
> DBG("start: 0x%04x, end: 0x%04x", start, end);
>
> @@ -2695,14 +2640,19 @@ static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data)
> if (!new_service)
> return;
>
> - device_register_primaries(device, new_service, -1);
> + l = find_service_with_uuid(device->services, uuid_str);
>
> - device_probe_gatt_profile(device, attr);
> + device_register_primaries(device, new_service, -1);
>
> - l = find_service_with_gatt_handles(device->services, start, end);
> - if (l)
> + /*
> + * If the profile was probed for the first time then call accept on
> + * the service.
> + */
> + if (!l && (l = find_service_with_uuid(device->services, uuid_str)))
> service_accept(l->data);
>
> + device_probe_gatt_profile(device, attr);
> +
> store_device_info(device);
>
> btd_gatt_client_service_added(device->client_dbus, attr);
> @@ -2750,24 +2700,26 @@ static void gatt_service_removed(struct gatt_db_attribute *attr,
> prim = l->data;
> device->primaries = g_slist_delete_link(device->primaries, l);
>
> - /*
> - * If this happend since the db was cleared for a non-bonded device,
> - * then don't remove the btd_service just yet. We do this so that we can
> - * avoid re-probing the profile if the same GATT service is found on the
> - * device on re-connection. However, if the device is marked as
> - * temporary, then we remove it anyway.
> - */
> - if (device->client || device->temporary == TRUE)
> - device_remove_gatt_profile(device, attr);
> + /* Get the UUID entry to be removed below */
> + l = g_slist_find_custom(device->uuids, prim->uuid, bt_uuid_strcmp);
>
> /*
> - * Remove the corresponding UUIDs entry, only if this is the last
> - * service with this UUID.
> + * Remove the corresponding UUIDs entry and the profile, only if this
> + * is the last service with this UUID.
> */
> - l = g_slist_find_custom(device->uuids, prim->uuid, bt_uuid_strcmp);
> -
> if (!g_slist_find_custom(device->primaries, prim->uuid,
> prim_uuid_cmp)) {
> + /*
> + * If this happened since the db was cleared for a non-bonded
> + * device, then don't remove the btd_service just yet. We do
> + * this so that we can avoid re-probing the profile if the same
> + * GATT service is found on the device on re-connection.
> + * However, if the device is marked as temporary, then we remove
> + * it anyway.
> + */
> + if (device->client || device->temporary == TRUE)
> + device_remove_gatt_profile(device, attr);
> +
> g_free(l->data);
> device->uuids = g_slist_delete_link(device->uuids, l);
> g_dbus_emit_property_changed(dbus_conn, device->path,
> --
> 2.2.0.rc0.207.ga3a616c
>
> --
> 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
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH BlueZ v2 10/14] profiles/gap: Fix probe/accept behavior.
2015-01-08 5:48 [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
` (8 preceding siblings ...)
2015-01-08 5:48 ` [PATCH BlueZ v2 09/14] core: device: Fix GATT profile probing Arman Uguray
@ 2015-01-08 5:48 ` Arman Uguray
2015-01-08 5:48 ` [PATCH BlueZ v2 11/14] core: service: Remove GATT handle logic Arman Uguray
` (4 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Arman Uguray @ 2015-01-08 5:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch fixes the GAP profile so that it assumes that there will be
one btd_service instance per-device rather than per-service-per-device.
---
profiles/gap/gas.c | 124 +++++++++++++++++++++++++++++------------------------
1 file changed, 68 insertions(+), 56 deletions(-)
diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
index 01b6ec2..0014fc8 100644
--- a/profiles/gap/gas.c
+++ b/profiles/gap/gas.c
@@ -43,21 +43,25 @@
#include "src/service.h"
#include "src/log.h"
+#define GAP_UUID16 0x1800
+
/* Generic Attribute/Access Service */
struct gas {
struct btd_device *device;
struct gatt_db *db;
+ unsigned int db_id;
struct bt_gatt_client *client;
- uint16_t start_handle, end_handle;
+ struct gatt_db_attribute *attr;
};
static GSList *devices;
static void gas_free(struct gas *gas)
{
- btd_device_unref(gas->device);
+ gatt_db_unregister(gas->db, gas->db_id);
gatt_db_unref(gas->db);
bt_gatt_client_unref(gas->client);
+ btd_device_unref(gas->device);
g_free(gas);
}
@@ -183,48 +187,31 @@ static void handle_characteristic(struct gatt_db_attribute *attr,
static void handle_gap_service(struct gas *gas)
{
- struct gatt_db_attribute *attr;
-
- attr = gatt_db_get_attribute(gas->db, gas->start_handle);
- if (!attr) {
- error("Service with handle 0x%04x not found in db",
- gas->start_handle);
- return;
- }
-
- gatt_db_service_foreach_char(attr, handle_characteristic, gas);
+ gatt_db_service_foreach_char(gas->attr, handle_characteristic, gas);
}
static int gap_driver_probe(struct btd_service *service)
{
struct btd_device *device = btd_service_get_device(service);
struct gas *gas;
- uint16_t start_handle, end_handle;
GSList *l;
char addr[18];
- if (!btd_service_get_gatt_handles(service, &start_handle, &end_handle))
- return -1;
-
ba2str(device_get_address(device), addr);
- DBG("GAP profile probe (%s): start: 0x%04x, end 0x%04x", addr,
- start_handle, end_handle);
+ DBG("GAP profile probe (%s)", addr);
- /*
- * There can't be more than one instance of the GAP service on the same
- * device.
- */
+ /* Ignore, if we were probed for this device already */
l = g_slist_find_custom(devices, device, cmp_device);
if (l) {
- error("More than one GAP service exists on device");
+ error("Profile probed twice for the same device!");
return -1;
}
gas = g_new0(struct gas, 1);
+ if (!gas)
+ return -1;
gas->device = btd_device_ref(device);
- gas->start_handle = start_handle;
- gas->end_handle = end_handle;
devices = g_slist_append(devices, gas);
return 0;
@@ -234,19 +221,11 @@ static void gap_driver_remove(struct btd_service *service)
{
struct btd_device *device = btd_service_get_device(service);
struct gas *gas;
- uint16_t start_handle, end_handle;
GSList *l;
char addr[18];
- if (!btd_service_get_gatt_handles(service, &start_handle,
- &end_handle)) {
- error("Removed service is not a GATT service");
- return;
- }
-
ba2str(device_get_address(device), addr);
- DBG("GAP profile remove (%s): start: 0x%04x, end 0x%04x", addr,
- start_handle, end_handle);
+ DBG("GAP profile remove (%s)", addr);
l = g_slist_find_custom(devices, device, cmp_device);
if (!l) {
@@ -256,14 +235,55 @@ static void gap_driver_remove(struct btd_service *service)
gas = l->data;
- if (gas->start_handle != start_handle ||
- gas->end_handle != end_handle) {
- error("Removed unknown GAP service");
+ devices = g_slist_remove(devices, gas);
+ gas_free(gas);
+}
+
+static void foreach_gap_service(struct gatt_db_attribute *attr, void *user_data)
+{
+ struct gas *gas = user_data;
+
+ if (gas->attr) {
+ error("More than one GAP service exists for this device");
return;
}
- devices = g_slist_remove(devices, gas);
- gas_free(gas);
+ gas->attr = attr;
+ handle_gap_service(gas);
+}
+
+static void service_added(struct gatt_db_attribute *attr, void *user_data)
+{
+ struct gas *gas = user_data;
+ bt_uuid_t uuid, gap_uuid;
+
+ gatt_db_attribute_get_service_uuid(attr, &uuid);
+ bt_uuid16_create(&gap_uuid, GAP_UUID16);
+
+ if (bt_uuid_cmp(&uuid, &gap_uuid))
+ return;
+
+ if (gas->attr) {
+ error("More than one GAP service added to device");
+ return;
+ }
+
+ DBG("GAP service added");
+
+ gas->attr = attr;
+ handle_gap_service(gas);
+}
+
+static void service_removed(struct gatt_db_attribute *attr, void *user_data)
+{
+ struct gas *gas = user_data;
+
+ if (gas->attr != attr)
+ return;
+
+ DBG("GAP service removed");
+
+ gas->attr = NULL;
}
static int gap_driver_accept(struct btd_service *service)
@@ -272,19 +292,12 @@ static int gap_driver_accept(struct btd_service *service)
struct gatt_db *db = btd_device_get_gatt_db(device);
struct bt_gatt_client *client = btd_device_get_gatt_client(device);
struct gas *gas;
- uint16_t start_handle, end_handle;
GSList *l;
char addr[18];
-
- if (!btd_service_get_gatt_handles(service, &start_handle,
- &end_handle)) {
- error("Service is not a GATT service");
- return -1;
- }
+ bt_uuid_t gap_uuid;
ba2str(device_get_address(device), addr);
- DBG("GAP profile accept (%s): start: 0x%04x, end 0x%04x", addr,
- start_handle, end_handle);
+ DBG("GAP profile accept (%s)", addr);
l = g_slist_find_custom(devices, device, cmp_device);
if (!l) {
@@ -294,21 +307,20 @@ static int gap_driver_accept(struct btd_service *service)
gas = l->data;
- if (gas->start_handle != start_handle ||
- gas->end_handle != end_handle) {
- error("Accepting unknown GAP service");
- return -1;
- }
-
/* Clean-up any old client/db and acquire the new ones */
+ gas->attr = NULL;
+ gatt_db_unregister(gas->db, gas->db_id);
gatt_db_unref(gas->db);
bt_gatt_client_unref(gas->client);
gas->db = gatt_db_ref(db);
gas->client = bt_gatt_client_ref(client);
+ gas->db_id = gatt_db_register(db, service_added, service_removed, gas,
+ NULL);
- /* Handle the service */
- handle_gap_service(gas);
+ /* Handle the GAP services */
+ bt_uuid16_create(&gap_uuid, GAP_UUID16);
+ gatt_db_foreach_service(db, &gap_uuid, foreach_gap_service, gas);
return 0;
}
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH BlueZ v2 11/14] core: service: Remove GATT handle logic
2015-01-08 5:48 [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
` (9 preceding siblings ...)
2015-01-08 5:48 ` [PATCH BlueZ v2 10/14] profiles/gap: Fix probe/accept behavior Arman Uguray
@ 2015-01-08 5:48 ` Arman Uguray
2015-01-08 5:48 ` [PATCH BlueZ v2 12/14] shared/gatt-db: Fix crash in gatt_db_find_by_type Arman Uguray
` (3 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Arman Uguray @ 2015-01-08 5:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch reverts the recent addition of GATT service handles to
btd_service.
---
src/service.c | 38 --------------------------------------
src/service.h | 7 -------
2 files changed, 45 deletions(-)
diff --git a/src/service.c b/src/service.c
index 2ea2b7a..b66b1c9 100644
--- a/src/service.c
+++ b/src/service.c
@@ -53,8 +53,6 @@ struct btd_service {
void *user_data;
btd_service_state_t state;
int err;
- uint16_t start_handle;
- uint16_t end_handle;
};
struct service_state_callback {
@@ -151,26 +149,6 @@ struct btd_service *service_create(struct btd_device *device,
return service;
}
-struct btd_service *service_create_gatt(struct btd_device *device,
- struct btd_profile *profile,
- uint16_t start_handle,
- uint16_t end_handle)
-{
- struct btd_service *service;
-
- if (!start_handle || !end_handle || start_handle > end_handle)
- return NULL;
-
- service = service_create(device, profile);
- if (!service)
- return NULL;
-
- service->start_handle = start_handle;
- service->end_handle = end_handle;
-
- return service;
-}
-
int service_probe(struct btd_service *service)
{
char addr[18];
@@ -322,22 +300,6 @@ int btd_service_get_error(const struct btd_service *service)
return service->err;
}
-bool btd_service_get_gatt_handles(const struct btd_service *service,
- uint16_t *start_handle,
- uint16_t *end_handle)
-{
- if (!service || !service->start_handle || !service->end_handle)
- return false;
-
- if (start_handle)
- *start_handle = service->start_handle;
-
- if (end_handle)
- *end_handle = service->end_handle;
-
- return true;
-}
-
unsigned int btd_service_add_state_cb(btd_service_state_cb cb, void *user_data)
{
struct service_state_callback *state_cb;
diff --git a/src/service.h b/src/service.h
index 3a0db6e..c1f97f6 100644
--- a/src/service.h
+++ b/src/service.h
@@ -44,10 +44,6 @@ void btd_service_unref(struct btd_service *service);
/* Service management functions used by the core */
struct btd_service *service_create(struct btd_device *device,
struct btd_profile *profile);
-struct btd_service *service_create_gatt(struct btd_device *device,
- struct btd_profile *profile,
- uint16_t start_handle,
- uint16_t end_handle);
int service_probe(struct btd_service *service);
void service_remove(struct btd_service *service);
@@ -63,9 +59,6 @@ struct btd_device *btd_service_get_device(const struct btd_service *service);
struct btd_profile *btd_service_get_profile(const struct btd_service *service);
btd_service_state_t btd_service_get_state(const struct btd_service *service);
int btd_service_get_error(const struct btd_service *service);
-bool btd_service_get_gatt_handles(const struct btd_service *service,
- uint16_t *start_handle,
- uint16_t *end_handle);
unsigned int btd_service_add_state_cb(btd_service_state_cb cb,
void *user_data);
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH BlueZ v2 12/14] shared/gatt-db: Fix crash in gatt_db_find_by_type
2015-01-08 5:48 [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
` (10 preceding siblings ...)
2015-01-08 5:48 ` [PATCH BlueZ v2 11/14] core: service: Remove GATT handle logic Arman Uguray
@ 2015-01-08 5:48 ` Arman Uguray
2015-01-08 5:48 ` [PATCH BlueZ v2 13/14] shared/gatt-db: Add "claimed" field to services Arman Uguray
` (2 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Arman Uguray @ 2015-01-08 5:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
Fixed a crash due to an invalid access in the find_by_type foreach
callback by correctly initializing the data fields to 0. The crash
happened because the same callback is used for find_by_type and
find_by_type_value and however find_by_type didn't correctly set the
value pointer to NULL.
---
src/shared/gatt-db.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index 9a9cadc..157d859 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -880,6 +880,8 @@ void gatt_db_find_by_type(struct gatt_db *db, uint16_t start_handle,
{
struct find_by_type_value_data data;
+ memset(&data, 0, sizeof(data));
+
data.uuid = *type;
data.start_handle = start_handle;
data.end_handle = end_handle;
@@ -899,6 +901,8 @@ void gatt_db_find_by_type_value(struct gatt_db *db, uint16_t start_handle,
{
struct find_by_type_value_data data;
+ memset(&data, 0, sizeof(data));
+
data.uuid = *type;
data.start_handle = start_handle;
data.end_handle = end_handle;
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH BlueZ v2 13/14] shared/gatt-db: Add "claimed" field to services
2015-01-08 5:48 [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
` (11 preceding siblings ...)
2015-01-08 5:48 ` [PATCH BlueZ v2 12/14] shared/gatt-db: Fix crash in gatt_db_find_by_type Arman Uguray
@ 2015-01-08 5:48 ` Arman Uguray
2015-01-08 5:48 ` [PATCH BlueZ v2 14/14] core: gatt: Use "claimed" instead of "active" Arman Uguray
2015-01-12 21:37 ` [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
14 siblings, 0 replies; 18+ messages in thread
From: Arman Uguray @ 2015-01-08 5:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
Added the ability to mark a service as "claimed". This is distinct from
"active", which denotes whether this service should be accessed at all
and is tied to the service added/removed events.
---
src/shared/gatt-db.c | 20 ++++++++++++++++++++
src/shared/gatt-db.h | 4 ++++
2 files changed, 24 insertions(+)
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index 157d859..e082992 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -102,6 +102,7 @@ struct gatt_db_attribute {
struct gatt_db_service {
struct gatt_db *db;
bool active;
+ bool claimed;
uint16_t num_handles;
struct gatt_db_attribute **attributes;
};
@@ -786,6 +787,25 @@ bool gatt_db_service_get_active(struct gatt_db_attribute *attrib)
return attrib->service->active;
}
+bool gatt_db_service_set_claimed(struct gatt_db_attribute *attrib,
+ bool claimed)
+{
+ if (!attrib)
+ return false;
+
+ attrib->service->claimed = claimed;
+
+ return true;
+}
+
+bool gatt_db_service_get_claimed(struct gatt_db_attribute *attrib)
+{
+ if (!attrib)
+ return false;
+
+ return attrib->service->claimed;
+}
+
void gatt_db_read_by_group_type(struct gatt_db *db, uint16_t start_handle,
uint16_t end_handle,
const bt_uuid_t type,
diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
index 1f4005e..34ba28f 100644
--- a/src/shared/gatt-db.h
+++ b/src/shared/gatt-db.h
@@ -83,6 +83,10 @@ gatt_db_service_add_included(struct gatt_db_attribute *attrib,
bool gatt_db_service_set_active(struct gatt_db_attribute *attrib, bool active);
bool gatt_db_service_get_active(struct gatt_db_attribute *attrib);
+bool gatt_db_service_set_claimed(struct gatt_db_attribute *attrib,
+ bool claimed);
+bool gatt_db_service_get_claimed(struct gatt_db_attribute *attrib);
+
typedef void (*gatt_db_attribute_cb_t)(struct gatt_db_attribute *attrib,
void *user_data);
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH BlueZ v2 14/14] core: gatt: Use "claimed" instead of "active"
2015-01-08 5:48 [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
` (12 preceding siblings ...)
2015-01-08 5:48 ` [PATCH BlueZ v2 13/14] shared/gatt-db: Add "claimed" field to services Arman Uguray
@ 2015-01-08 5:48 ` Arman Uguray
2015-01-12 21:37 ` [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
14 siblings, 0 replies; 18+ messages in thread
From: Arman Uguray @ 2015-01-08 5:48 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
Using the "active" state of a gatt-db service to mark a service as
claimed by a profile somewhat abuses the API and has the additional
side-effect of sending a service_removed event. This causes errors in
the way btd_device manages profile/service lifetime. To fix this, this
patch uses the new "claimed" state to mark a service as claimed.
Also included is a minor fix to btd_gatt_client_* APIs that early return
if the client is not yet ready.
---
src/device.c | 7 +++++--
src/gatt-client.c | 9 ++++++---
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/src/device.c b/src/device.c
index a9dc22d..9e40467 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2478,7 +2478,7 @@ static void dev_probe_gatt(struct btd_profile *p, void *user_data)
}
/* Mark service as claimed */
- gatt_db_service_set_active(data->cur_attr, false);
+ gatt_db_service_set_claimed(data->cur_attr, true);
data->dev->services = g_slist_append(data->dev->services, service);
}
@@ -2504,8 +2504,11 @@ static void dev_probe_gatt_profile(struct gatt_db_attribute *attr,
g_strdup(data->cur_uuid));
/* Don't probe the profiles if a matching service already exists. */
- if (find_service_with_uuid(data->dev->services, data->cur_uuid))
+ if (find_service_with_uuid(data->dev->services, data->cur_uuid)) {
+ /* Mark the service as claimed by the existing profile. */
+ gatt_db_service_set_claimed(data->cur_attr, true);
return;
+ }
btd_profile_foreach(dev_probe_gatt, data);
diff --git a/src/gatt-client.c b/src/gatt-client.c
index d792670..3e23fbd 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -52,6 +52,7 @@
struct btd_gatt_client {
struct btd_device *device;
+ bool ready;
char devaddr[18];
struct gatt_db *db;
struct bt_gatt_client *gatt;
@@ -1369,7 +1370,7 @@ static void export_service(struct gatt_db_attribute *attr, void *user_data)
struct btd_gatt_client *client = user_data;
struct service *service;
- if (!gatt_db_service_get_active(attr))
+ if (gatt_db_service_get_claimed(attr))
return;
service = service_create(attr, client);
@@ -1457,13 +1458,15 @@ void btd_gatt_client_ready(struct btd_gatt_client *client)
bt_gatt_client_unref(client->gatt);
client->gatt = bt_gatt_client_ref(gatt);
+ client->ready = true;
+
create_services(client);
}
void btd_gatt_client_service_added(struct btd_gatt_client *client,
struct gatt_db_attribute *attrib)
{
- if (!client)
+ if (!client || !attrib || !client->ready)
return;
export_service(attrib, client);
@@ -1482,7 +1485,7 @@ void btd_gatt_client_service_removed(struct btd_gatt_client *client,
{
uint16_t start_handle, end_handle;
- if (!client || !attrib)
+ if (!client || !attrib || !client->ready)
return;
gatt_db_attribute_get_service_handles(attrib, &start_handle,
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client
2015-01-08 5:48 [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
` (13 preceding siblings ...)
2015-01-08 5:48 ` [PATCH BlueZ v2 14/14] core: gatt: Use "claimed" instead of "active" Arman Uguray
@ 2015-01-12 21:37 ` Arman Uguray
2015-01-12 22:58 ` Luiz Augusto von Dentz
14 siblings, 1 reply; 18+ messages in thread
From: Arman Uguray @ 2015-01-12 21:37 UTC (permalink / raw)
To: BlueZ development
On Wed, Jan 7, 2015 at 9:48 PM, Arman Uguray <armansito@chromium.org> wrote:
> *v2: This set addresses comments by Luiz and includes several bug fixes:
> - Make shared/gatt-client read/write procedures cancelable. Have exported
> characteristics and descriptors keep track of their operations and cancel
> them when free'd.
> - Fix bug in attribute value caching by truncating the value before setting
> it.
> - Expose extended properties in the GattCharacteristic1.Flags property.
> - Hande Service Changed events.
> - Fix bug in profile probing that created a btd_service for each GATT service
> as opposed to once per UUID (mostly due to a misunderstanding).
> - Fix crash introduced by recent gatt_db_find_by_type refactor.
> - Fix incorrect behavior caused by using gatt_db_service_set_active to mark
> services as claimed.
>
> *v1: Picking up remaining patches from before the holidays:
> - Rebased on top of Luiz's modifications.
> - Fixed small bugs that appeared after the merge.
> - Addressed some of the initial comments.
> - Added StartNotify/StopNotify. I left these methods as they are.
> I'm planning to address the issue with potentially missed
> notifications in a GattProfile1 API.
>
> Arman Uguray (14):
> core: gatt: Expose charac. extended properties.
> shared/gatt-client: Make read/write cancelable
> shared/gatt-client: Make long-write cancelable
> core: gatt: Cancel pending reads/writes
> shared/gatt-db: Add gatt_db_attribute_reset
> core: gatt: Reset value in db when caching
> core: gatt: Issue long write for reliable-write
> core: gatt: Handle Service Changed.
> core: device: Fix GATT profile probing
> profiles/gap: Fix probe/accept behavior.
> core: service: Remove GATT handle logic
> shared/gatt-db: Fix crash in gatt_db_find_by_type
> shared/gatt-db: Add "claimed" field to services
> core: gatt: Use "claimed" instead of "active"
>
> profiles/gap/gas.c | 124 +++++++------
> src/device.c | 183 +++++++------------
> src/gatt-client.c | 359 ++++++++++++++++++++++++++-----------
> src/service.c | 38 ----
> src/service.h | 7 -
> src/shared/gatt-client.c | 458 ++++++++++++++++++++++++++++++++++-------------
> src/shared/gatt-client.h | 16 +-
> src/shared/gatt-db.c | 39 ++++
> src/shared/gatt-db.h | 6 +
> 9 files changed, 781 insertions(+), 449 deletions(-)
>
> --
> 2.2.0.rc0.207.ga3a616c
>
ping
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client
2015-01-12 21:37 ` [PATCH BlueZ v2 00/14] Implmenet doc/gatt-api.txt for client Arman Uguray
@ 2015-01-12 22:58 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 18+ messages in thread
From: Luiz Augusto von Dentz @ 2015-01-12 22:58 UTC (permalink / raw)
To: Arman Uguray; +Cc: BlueZ development
Hi Arman,
On Mon, Jan 12, 2015 at 7:37 PM, Arman Uguray <armansito@chromium.org> wrote:
> On Wed, Jan 7, 2015 at 9:48 PM, Arman Uguray <armansito@chromium.org> wrote:
>> *v2: This set addresses comments by Luiz and includes several bug fixes:
>> - Make shared/gatt-client read/write procedures cancelable. Have exported
>> characteristics and descriptors keep track of their operations and cancel
>> them when free'd.
>> - Fix bug in attribute value caching by truncating the value before setting
>> it.
>> - Expose extended properties in the GattCharacteristic1.Flags property.
>> - Hande Service Changed events.
>> - Fix bug in profile probing that created a btd_service for each GATT service
>> as opposed to once per UUID (mostly due to a misunderstanding).
>> - Fix crash introduced by recent gatt_db_find_by_type refactor.
>> - Fix incorrect behavior caused by using gatt_db_service_set_active to mark
>> services as claimed.
>>
>> *v1: Picking up remaining patches from before the holidays:
>> - Rebased on top of Luiz's modifications.
>> - Fixed small bugs that appeared after the merge.
>> - Addressed some of the initial comments.
>> - Added StartNotify/StopNotify. I left these methods as they are.
>> I'm planning to address the issue with potentially missed
>> notifications in a GattProfile1 API.
>>
>> Arman Uguray (14):
>> core: gatt: Expose charac. extended properties.
>> shared/gatt-client: Make read/write cancelable
>> shared/gatt-client: Make long-write cancelable
>> core: gatt: Cancel pending reads/writes
>> shared/gatt-db: Add gatt_db_attribute_reset
>> core: gatt: Reset value in db when caching
>> core: gatt: Issue long write for reliable-write
>> core: gatt: Handle Service Changed.
>> core: device: Fix GATT profile probing
>> profiles/gap: Fix probe/accept behavior.
>> core: service: Remove GATT handle logic
>> shared/gatt-db: Fix crash in gatt_db_find_by_type
>> shared/gatt-db: Add "claimed" field to services
>> core: gatt: Use "claimed" instead of "active"
>>
>> profiles/gap/gas.c | 124 +++++++------
>> src/device.c | 183 +++++++------------
>> src/gatt-client.c | 359 ++++++++++++++++++++++++++-----------
>> src/service.c | 38 ----
>> src/service.h | 7 -
>> src/shared/gatt-client.c | 458 ++++++++++++++++++++++++++++++++++-------------
>> src/shared/gatt-client.h | 16 +-
>> src/shared/gatt-db.c | 39 ++++
>> src/shared/gatt-db.h | 6 +
>> 9 files changed, 781 insertions(+), 449 deletions(-)
>>
>> --
>> 2.2.0.rc0.207.ga3a616c
>>
>
> ping
> --
Ive applied patches 1-7, please rebase and check the comments
regarding patch 8/14.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 18+ messages in thread