All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ 1/2] core/gatt-client: Allow clients to call ReadValue in parallel
@ 2016-06-23 11:02 Luiz Augusto von Dentz
  2016-06-23 11:02 ` [PATCH BlueZ 2/2] core/gatt-client: Always use bt_gatt_client_read_long_value Luiz Augusto von Dentz
  2016-06-27 14:14 ` [PATCH BlueZ 1/2] core/gatt-client: Allow clients to call ReadValue in parallel Luiz Augusto von Dentz
  0 siblings, 2 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2016-06-23 11:02 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This queues ReadValue messages if the offset is the same so all the
client interested in reading the attribute value are replied when the
operation is completed.
---
 src/gatt-client.c | 318 ++++++++++++++++++++++++++++--------------------------
 1 file changed, 163 insertions(+), 155 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index 6fc0d19..c2a888f 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -74,6 +74,17 @@ struct service {
 	struct queue *chrcs;
 };
 
+typedef bool (*async_dbus_op_complete_t)(void *data);
+
+struct async_dbus_op {
+	int ref_count;
+	unsigned int id;
+	struct queue *msgs;
+	void *data;
+	uint16_t offset;
+	async_dbus_op_complete_t complete;
+};
+
 struct characteristic {
 	struct service *service;
 	struct gatt_db_attribute *attr;
@@ -85,8 +96,8 @@ struct characteristic {
 	bt_uuid_t uuid;
 	char *path;
 
-	unsigned int read_id;
-	unsigned int write_id;
+	struct async_dbus_op *read_op;
+	struct async_dbus_op *write_op;
 
 	struct queue *descs;
 
@@ -101,8 +112,8 @@ struct descriptor {
 	bt_uuid_t uuid;
 	char *path;
 
-	unsigned int read_id;
-	unsigned int write_id;
+	struct async_dbus_op *read_op;
+	struct async_dbus_op *write_op;
 };
 
 static bool uuid_cmp(const bt_uuid_t *uuid, uint16_t u16)
@@ -205,22 +216,11 @@ static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
 	return 0;
 }
 
-typedef bool (*async_dbus_op_complete_t)(void *data);
-
-struct async_dbus_op {
-	int ref_count;
-	DBusMessage *msg;
-	void *data;
-	uint16_t offset;
-	async_dbus_op_complete_t complete;
-};
-
 static void async_dbus_op_free(void *data)
 {
 	struct async_dbus_op *op = data;
 
-	if (op->msg)
-		dbus_message_unref(op->msg);
+	queue_destroy(op->msgs, (void *)dbus_message_unref);
 
 	free(op);
 }
@@ -296,27 +296,44 @@ static void write_descriptor_cb(struct gatt_db_attribute *attr, int err,
 					GATT_DESCRIPTOR_IFACE, "Value");
 }
 
-static void read_op_cb(struct gatt_db_attribute *attrib, int err,
-				const uint8_t *value, size_t length,
-				void *user_data)
+static void async_dbus_op_reply(struct async_dbus_op *op, int err,
+				const uint8_t *value, size_t length)
 {
-	struct async_dbus_op *op = user_data;
+	const struct queue_entry *entry;
 	DBusMessage *reply;
 
-	if (err) {
-		error("Failed to read attribute");
-		return;
-	}
+	op->id = 0;
 
-	reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
-	if (!reply) {
-		error("Failed to allocate D-Bus message reply");
-		return;
+	for (entry = queue_get_entries(op->msgs); entry; entry = entry->next) {
+		DBusMessage *msg = entry->data;
+
+		if (err) {
+			reply = err > 0 ? create_gatt_dbus_error(msg, err) :
+				btd_error_failed(msg, strerror(err));
+			goto send_reply;
+		}
+
+		reply = g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+		if (!reply) {
+			error("Failed to allocate D-Bus message reply");
+			return;
+		}
+
+		if (value)
+			message_append_byte_array(reply, value, length);
+
+send_reply:
+		g_dbus_send_message(btd_get_dbus_connection(), reply);
 	}
+}
 
-	message_append_byte_array(reply, value, length);
+static void read_op_cb(struct gatt_db_attribute *attrib, int err,
+				const uint8_t *value, size_t length,
+				void *user_data)
+{
+	struct async_dbus_op *op = user_data;
 
-	g_dbus_send_message(btd_get_dbus_connection(), reply);
+	async_dbus_op_reply(op, err, value, length);
 }
 
 static void desc_read_cb(bool success, uint8_t att_ecode,
@@ -326,7 +343,6 @@ static void desc_read_cb(bool success, uint8_t att_ecode,
 	struct async_dbus_op *op = user_data;
 	struct descriptor *desc = op->data;
 	struct service *service = desc->chrc->service;
-	DBusMessage *reply;
 
 	if (!success)
 		goto fail;
@@ -337,6 +353,7 @@ static void desc_read_cb(bool success, uint8_t att_ecode,
 	if (!gatt_db_attribute_write(desc->attr, op->offset, value, length, 0,
 					NULL, write_descriptor_cb, desc)) {
 		error("Failed to store attribute");
+		att_ecode = BT_ATT_ERROR_UNLIKELY;
 		goto fail;
 	}
 
@@ -347,32 +364,30 @@ 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;
-		desc->read_id = bt_gatt_client_read_long_value(
-							service->client->gatt,
+		op->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);
-		if (desc->read_id)
+		if (op->id)
 			return;
 	}
 
 	/* Read the stored data from db */
 	if (!gatt_db_attribute_read(desc->attr, 0, 0, NULL, read_op_cb, op)) {
 		error("Failed to read database");
+		att_ecode = BT_ATT_ERROR_UNLIKELY;
 		goto fail;
 	}
 
-	desc->read_id = 0;
+	desc->read_op = NULL;
 
 	return;
 
 fail:
-	reply = create_gatt_dbus_error(op->msg, att_ecode);
-	desc->read_id = 0;
-	g_dbus_send_message(btd_get_dbus_connection(), reply);
-	return;
+	async_dbus_op_reply(op, att_ecode, NULL, 0);
+	desc->read_op = NULL;
 }
 
 static int parse_options(DBusMessageIter *iter, uint16_t *offset)
@@ -408,19 +423,45 @@ static int parse_options(DBusMessageIter *iter, uint16_t *offset)
 	return 0;
 }
 
-static unsigned int read_value(struct bt_gatt_client *gatt, uint16_t handle,
-				bt_gatt_client_read_callback_t callback,
-				struct async_dbus_op *op)
+static struct async_dbus_op *async_dbus_op_new(DBusMessage *msg, void *data)
+{
+	struct async_dbus_op *op;
+
+	op = new0(struct async_dbus_op, 1);
+	op->msgs = queue_new();
+	queue_push_tail(op->msgs, dbus_message_ref(msg));
+	op->data = data;
+
+	return op;
+}
+
+static struct async_dbus_op *read_value(struct bt_gatt_client *gatt,
+					DBusMessage *msg, uint16_t handle,
+					uint16_t offset,
+					bt_gatt_client_read_callback_t callback,
+					void *data)
 {
+	struct async_dbus_op *op;
+
+	op = async_dbus_op_new(msg, data);
+	op->offset = offset;
+
 	if (op->offset)
-		return bt_gatt_client_read_long_value(gatt, handle, op->offset,
+		op->id = bt_gatt_client_read_long_value(gatt, handle, offset,
 							callback,
 							async_dbus_op_ref(op),
 							async_dbus_op_unref);
 	else
-		return bt_gatt_client_read_value(gatt, handle, callback,
+		op->id = bt_gatt_client_read_value(gatt, handle, callback,
 							async_dbus_op_ref(op),
 							async_dbus_op_unref);
+
+	if (op->id)
+		return op;
+
+	async_dbus_op_free(op);
+
+	return NULL;
 }
 
 static DBusMessage *descriptor_read_value(DBusConnection *conn,
@@ -429,63 +470,51 @@ static DBusMessage *descriptor_read_value(DBusConnection *conn,
 	struct descriptor *desc = user_data;
 	struct bt_gatt_client *gatt = desc->chrc->service->client->gatt;
 	DBusMessageIter iter;
-	struct async_dbus_op *op;
 	uint16_t offset = 0;
 
 	if (!gatt)
 		return btd_error_failed(msg, "Not connected");
 
-	if (desc->read_id)
-		return btd_error_in_progress(msg);
-
 	dbus_message_iter_init(msg, &iter);
 
 	if (parse_options(&iter, &offset))
 		return btd_error_invalid_args(msg);
 
-	op = new0(struct async_dbus_op, 1);
-	op->msg = dbus_message_ref(msg);
-	op->data = desc;
-	op->offset = offset;
-
-	desc->read_id = read_value(gatt, desc->handle, desc_read_cb, op);
-	if (desc->read_id)
+	if (desc->read_op) {
+		if (desc->read_op->offset != offset)
+			return btd_error_in_progress(msg);
+		queue_push_tail(desc->read_op->msgs, dbus_message_ref(msg));
 		return NULL;
+	}
 
-	async_dbus_op_free(op);
+	desc->read_op = read_value(gatt, msg, desc->handle, offset,
+							desc_read_cb, desc);
+	if (!desc->read_op)
+		return btd_error_failed(msg, "Failed to send read request");
 
-	return btd_error_failed(msg, "Failed to send read request");
+	return NULL;
 }
 
 static void write_result_cb(bool success, bool reliable_error,
 					uint8_t att_ecode, void *user_data)
 {
 	struct async_dbus_op *op = user_data;
-	DBusMessage *reply;
+	int err = 0;
 
 	if (op->complete && !op->complete(op->data)) {
-		reply = btd_error_failed(op->msg, "Operation failed");
+		err = -EFAULT;
 		goto done;
 	}
 
 	if (!success) {
 		if (reliable_error)
-			reply = btd_error_failed(op->msg,
-						"Reliable write failed");
+			err = -EFAULT;
 		else
-			reply = create_gatt_dbus_error(op->msg, att_ecode);
-
-		goto done;
-	}
-
-	reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
-	if (!reply) {
-		error("Failed to allocate D-Bus message reply");
-		return;
+			err = att_ecode;
 	}
 
 done:
-	g_dbus_send_message(btd_get_dbus_connection(), reply);
+	async_dbus_op_reply(op, err, NULL, 0);
 }
 
 static void write_cb(bool success, uint8_t att_ecode, void *user_data)
@@ -493,7 +522,7 @@ static void write_cb(bool success, uint8_t att_ecode, void *user_data)
 	write_result_cb(success, false, att_ecode, user_data);
 }
 
-static unsigned int start_long_write(DBusMessage *msg, uint16_t handle,
+static struct async_dbus_op *start_long_write(DBusMessage *msg, uint16_t handle,
 					struct bt_gatt_client *gatt,
 					bool reliable, const uint8_t *value,
 					size_t value_len, uint16_t offset,
@@ -501,53 +530,52 @@ static unsigned int start_long_write(DBusMessage *msg, uint16_t handle,
 					async_dbus_op_complete_t complete)
 {
 	struct async_dbus_op *op;
-	unsigned int id;
 
-	op = new0(struct async_dbus_op, 1);
-	op->msg = dbus_message_ref(msg);
-	op->data = data;
+	op = async_dbus_op_new(msg, data);
 	op->complete = complete;
 	op->offset = offset;
 
-	id = bt_gatt_client_write_long_value(gatt, reliable, handle, offset,
+	op->id = bt_gatt_client_write_long_value(gatt, reliable, handle, offset,
 							value, value_len,
 							write_result_cb, op,
 							async_dbus_op_free);
 
-	if (!id)
+	if (!op->id) {
 		async_dbus_op_free(op);
+		return NULL;
+	}
 
-	return id;
+	return op;
 }
 
-static unsigned int start_write_request(DBusMessage *msg, uint16_t handle,
+static struct async_dbus_op *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);
-	op->msg = dbus_message_ref(msg);
-	op->data = data;
+	op = async_dbus_op_new(msg, data);
 	op->complete = complete;
 
-	id = bt_gatt_client_write_value(gatt, handle, value, value_len,
+	op->id = bt_gatt_client_write_value(gatt, handle, value, value_len,
 							write_cb, op,
 							async_dbus_op_free);
-	if (!id)
+	if (!op->id) {
 		async_dbus_op_free(op);
+		return NULL;
+	}
 
-	return id;
+	return op;
 }
 
 static bool desc_write_complete(void *data)
 {
 	struct descriptor *desc = data;
 
-	desc->write_id = 0;
+	desc->write_op = NULL;
 
 	/*
 	 * The descriptor might have been unregistered during the read. Return
@@ -569,7 +597,7 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
 	if (!gatt)
 		return btd_error_failed(msg, "Not connected");
 
-	if (desc->write_id)
+	if (desc->write_op)
 		return btd_error_in_progress(msg);
 
 	dbus_message_iter_init(msg, &iter);
@@ -593,17 +621,17 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
 	 * write.
 	 */
 	if (value_len <= bt_gatt_client_get_mtu(gatt) - 3 && !offset)
-		desc->write_id = start_write_request(msg, desc->handle,
+		desc->write_op = start_write_request(msg, desc->handle,
 							gatt, value,
 							value_len, desc,
 							desc_write_complete);
 	else
-		desc->write_id = start_long_write(msg, desc->handle, gatt,
+		desc->write_op = start_long_write(msg, desc->handle, gatt,
 							false, value,
 							value_len, offset, desc,
 							desc_write_complete);
 
-	if (!desc->write_id)
+	if (!desc->write_op)
 		return btd_error_failed(msg, "Failed to initiate write");
 
 	return NULL;
@@ -681,11 +709,11 @@ static void unregister_descriptor(void *data)
 
 	DBG("Removing GATT descriptor: %s", desc->path);
 
-	if (desc->read_id)
-		bt_gatt_client_cancel(gatt, desc->read_id);
+	if (desc->read_op)
+		bt_gatt_client_cancel(gatt, desc->read_op->id);
 
-	if (desc->write_id)
-		bt_gatt_client_cancel(gatt, desc->write_id);
+	if (desc->write_op)
+		bt_gatt_client_cancel(gatt, desc->write_op->id);
 
 	desc->chrc = NULL;
 
@@ -832,7 +860,6 @@ static void chrc_read_cb(bool success, uint8_t att_ecode, const uint8_t *value,
 	struct async_dbus_op *op = user_data;
 	struct characteristic *chrc = op->data;
 	struct service *service = chrc->service;
-	DBusMessage *reply;
 
 	if (!success)
 		goto fail;
@@ -843,6 +870,7 @@ static void chrc_read_cb(bool success, uint8_t att_ecode, const uint8_t *value,
 	if (!gatt_db_attribute_write(chrc->attr, op->offset, value, length, 0,
 					NULL, write_characteristic_cb, chrc)) {
 		error("Failed to store attribute");
+		att_ecode = BT_ATT_ERROR_UNLIKELY;
 		goto fail;
 	}
 
@@ -853,31 +881,30 @@ 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;
-		chrc->read_id = bt_gatt_client_read_long_value(
-							service->client->gatt,
+		op->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);
-		if (chrc->read_id)
+		if (op->id)
 			return;
 	}
 
-	chrc->read_id = 0;
-
 	/* Read the stored data from db */
 	if (!gatt_db_attribute_read(chrc->attr, 0, 0, NULL, read_op_cb, op)) {
 		error("Failed to read database");
+		att_ecode = BT_ATT_ERROR_UNLIKELY;
 		goto fail;
 	}
 
+	chrc->read_op = NULL;
+
 	return;
 
 fail:
-	reply = create_gatt_dbus_error(op->msg, att_ecode);
-	chrc->read_id = 0;
-	g_dbus_send_message(btd_get_dbus_connection(), reply);
+	async_dbus_op_reply(op, att_ecode, NULL, 0);
+	chrc->read_op = NULL;
 }
 
 static DBusMessage *characteristic_read_value(DBusConnection *conn,
@@ -886,39 +913,36 @@ static DBusMessage *characteristic_read_value(DBusConnection *conn,
 	struct characteristic *chrc = user_data;
 	struct bt_gatt_client *gatt = chrc->service->client->gatt;
 	DBusMessageIter iter;
-	struct async_dbus_op *op;
 	uint16_t offset = 0;
 
 	if (!gatt)
 		return btd_error_failed(msg, "Not connected");
 
-	if (chrc->read_id)
-		return btd_error_in_progress(msg);
-
 	dbus_message_iter_init(msg, &iter);
 
 	if (parse_options(&iter, &offset))
 		return btd_error_invalid_args(msg);
 
-	op = new0(struct async_dbus_op, 1);
-	op->msg = dbus_message_ref(msg);
-	op->data = chrc;
-	op->offset = offset;
-
-	chrc->read_id = read_value(gatt, chrc->value_handle, chrc_read_cb, op);
-	if (chrc->read_id)
+	if (chrc->read_op) {
+		if (chrc->read_op->offset != offset)
+			return btd_error_in_progress(msg);
+		queue_push_tail(chrc->read_op->msgs, dbus_message_ref(msg));
 		return NULL;
+	}
 
-	async_dbus_op_free(op);
+	chrc->read_op = read_value(gatt, msg, chrc->value_handle, offset,
+							chrc_read_cb, chrc);
+	if (!chrc->read_op)
+		return btd_error_failed(msg, "Failed to send read request");
 
-	return btd_error_failed(msg, "Failed to send read request");
+	return NULL;
 }
 
 static bool chrc_write_complete(void *data)
 {
 	struct characteristic *chrc = data;
 
-	chrc->write_id = 0;
+	chrc->write_op = NULL;
 
 	/*
 	 * The characteristic might have been unregistered during the read.
@@ -941,7 +965,7 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
 	if (!gatt)
 		return btd_error_failed(msg, "Not connected");
 
-	if (chrc->write_id)
+	if (chrc->write_op)
 		return btd_error_in_progress(msg);
 
 	dbus_message_iter_init(msg, &iter);
@@ -965,10 +989,10 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
 	 */
 	if ((chrc->ext_props & BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE)) {
 		supported = true;
-		chrc->write_id = start_long_write(msg, chrc->value_handle, gatt,
+		chrc->write_op = start_long_write(msg, chrc->value_handle, gatt,
 						true, value, value_len, offset,
 						chrc, chrc_write_complete);
-		if (chrc->write_id)
+		if (chrc->write_op)
 			return NULL;
 	}
 
@@ -981,17 +1005,17 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
 			return btd_error_failed(msg, "No ATT transport");
 
 		if (value_len <= mtu - 3 && !offset)
-			chrc->write_id = start_write_request(msg,
+			chrc->write_op = start_write_request(msg,
 						chrc->value_handle,
 						gatt, value, value_len,
 						chrc, chrc_write_complete);
 		else
-			chrc->write_id = start_long_write(msg,
+			chrc->write_op = start_long_write(msg,
 						chrc->value_handle, gatt,
 						false, value, value_len, offset,
 						chrc, chrc_write_complete);
 
-		if (chrc->write_id)
+		if (chrc->write_op)
 			return NULL;
 	}
 
@@ -1140,26 +1164,17 @@ static void notify_cb(uint16_t value_handle, const uint8_t *value,
 						write_characteristic_cb, chrc);
 }
 
-static DBusMessage *create_notify_reply(struct async_dbus_op *op,
-						bool success, uint8_t att_ecode)
+static void create_notify_reply(struct async_dbus_op *op, bool success,
+							uint8_t att_ecode)
 {
-	DBusMessage *reply = NULL;
-
-	if (!op->msg)
-		return NULL;
+	int err;
 
 	if (success)
-		reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
-	else if (att_ecode)
-		reply = create_gatt_dbus_error(op->msg, att_ecode);
+		err = 0;
 	else
-		reply = btd_error_failed(op->msg,
-						"Characteristic not available");
+		err = att_ecode ? att_ecode : -ENOENT;
 
-	if (!reply)
-		error("Failed to construct D-Bus message reply");
-
-	return reply;
+	async_dbus_op_reply(op, err, NULL, 0);
 }
 
 static void register_notify_cb(uint16_t att_ecode, void *user_data)
@@ -1167,16 +1182,15 @@ static void register_notify_cb(uint16_t att_ecode, void *user_data)
 	struct async_dbus_op *op = user_data;
 	struct notify_client *client = op->data;
 	struct characteristic *chrc = client->chrc;
-	DBusMessage *reply;
 
 	if (att_ecode) {
 		queue_remove(chrc->notify_clients, client);
 		queue_remove(chrc->service->client->all_notify_clients, client);
 		notify_client_free(client);
 
-		reply = create_notify_reply(op, false, att_ecode);
+		create_notify_reply(op, false, att_ecode);
 
-		goto done;
+		return;
 	}
 
 	if (!chrc->notifying) {
@@ -1186,11 +1200,7 @@ static void register_notify_cb(uint16_t att_ecode, void *user_data)
 					"Notifying");
 	}
 
-	reply = create_notify_reply(op, true, 0);
-
-done:
-	if (reply)
-		g_dbus_send_message(btd_get_dbus_connection(), reply);
+	create_notify_reply(op, true, 0);
 }
 
 static DBusMessage *characteristic_start_notify(DBusConnection *conn,
@@ -1240,9 +1250,7 @@ static DBusMessage *characteristic_start_notify(DBusConnection *conn,
 		goto fail;
 	}
 
-	op = new0(struct async_dbus_op, 1);
-	op->data = client;
-	op->msg = dbus_message_ref(msg);
+	op = async_dbus_op_new(msg, client);
 
 	client->notify_id = bt_gatt_client_register_notify(gatt,
 						chrc->value_handle,
@@ -1395,11 +1403,11 @@ static void unregister_characteristic(void *data)
 
 	DBG("Removing GATT characteristic: %s", chrc->path);
 
-	if (chrc->read_id)
-		bt_gatt_client_cancel(gatt, chrc->read_id);
+	if (chrc->read_op)
+		bt_gatt_client_cancel(gatt, chrc->read_op->id);
 
-	if (chrc->write_id)
-		bt_gatt_client_cancel(gatt, chrc->write_id);
+	if (chrc->write_op)
+		bt_gatt_client_cancel(gatt, chrc->write_op->id);
 
 	queue_remove_all(chrc->notify_clients, NULL, NULL, remove_client);
 	queue_remove_all(chrc->descs, NULL, NULL, unregister_descriptor);
-- 
2.5.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH BlueZ 2/2] core/gatt-client: Always use bt_gatt_client_read_long_value
  2016-06-23 11:02 [PATCH BlueZ 1/2] core/gatt-client: Allow clients to call ReadValue in parallel Luiz Augusto von Dentz
@ 2016-06-23 11:02 ` Luiz Augusto von Dentz
  2016-06-27 14:14 ` [PATCH BlueZ 1/2] core/gatt-client: Allow clients to call ReadValue in parallel Luiz Augusto von Dentz
  1 sibling, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2016-06-23 11:02 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

bt_gatt_client_read_long_value always reads the full value so the code
don't have to check if there is more data to be read.
---
 src/gatt-client.c | 49 +++----------------------------------------------
 1 file changed, 3 insertions(+), 46 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index c2a888f..7abb306 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -342,7 +342,6 @@ static void desc_read_cb(bool success, uint8_t att_ecode,
 {
 	struct async_dbus_op *op = user_data;
 	struct descriptor *desc = op->data;
-	struct service *service = desc->chrc->service;
 
 	if (!success)
 		goto fail;
@@ -357,23 +356,6 @@ static void desc_read_cb(bool success, uint8_t att_ecode,
 		goto fail;
 	}
 
-	/*
-	 * If the value length is exactly MTU-1, then we may not have read the
-	 * entire value. Perform a long read to obtain the rest, otherwise,
-	 * we're done.
-	 */
-	if (length == bt_gatt_client_get_mtu(service->client->gatt) - 1) {
-		op->offset += length;
-		op->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);
-		if (op->id)
-			return;
-	}
-
 	/* Read the stored data from db */
 	if (!gatt_db_attribute_read(desc->attr, 0, 0, NULL, read_op_cb, op)) {
 		error("Failed to read database");
@@ -446,16 +428,9 @@ static struct async_dbus_op *read_value(struct bt_gatt_client *gatt,
 	op = async_dbus_op_new(msg, data);
 	op->offset = offset;
 
-	if (op->offset)
-		op->id = bt_gatt_client_read_long_value(gatt, handle, offset,
-							callback,
-							async_dbus_op_ref(op),
-							async_dbus_op_unref);
-	else
-		op->id = bt_gatt_client_read_value(gatt, handle, callback,
-							async_dbus_op_ref(op),
-							async_dbus_op_unref);
-
+	op->id = bt_gatt_client_read_long_value(gatt, handle, offset, callback,
+						async_dbus_op_ref(op),
+						async_dbus_op_unref);
 	if (op->id)
 		return op;
 
@@ -859,7 +834,6 @@ static void chrc_read_cb(bool success, uint8_t att_ecode, const uint8_t *value,
 {
 	struct async_dbus_op *op = user_data;
 	struct characteristic *chrc = op->data;
-	struct service *service = chrc->service;
 
 	if (!success)
 		goto fail;
@@ -874,23 +848,6 @@ static void chrc_read_cb(bool success, uint8_t att_ecode, const uint8_t *value,
 		goto fail;
 	}
 
-	/*
-	 * If the value length is exactly MTU-1, then we may not have read the
-	 * entire value. Perform a long read to obtain the rest, otherwise,
-	 * we're done.
-	 */
-	if (length == bt_gatt_client_get_mtu(service->client->gatt) - 1) {
-		op->offset += length;
-		op->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);
-		if (op->id)
-			return;
-	}
-
 	/* Read the stored data from db */
 	if (!gatt_db_attribute_read(chrc->attr, 0, 0, NULL, read_op_cb, op)) {
 		error("Failed to read database");
-- 
2.5.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH BlueZ 1/2] core/gatt-client: Allow clients to call ReadValue in parallel
  2016-06-23 11:02 [PATCH BlueZ 1/2] core/gatt-client: Allow clients to call ReadValue in parallel Luiz Augusto von Dentz
  2016-06-23 11:02 ` [PATCH BlueZ 2/2] core/gatt-client: Always use bt_gatt_client_read_long_value Luiz Augusto von Dentz
@ 2016-06-27 14:14 ` Luiz Augusto von Dentz
  1 sibling, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2016-06-27 14:14 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

On Thu, Jun 23, 2016 at 2:02 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This queues ReadValue messages if the offset is the same so all the
> client interested in reading the attribute value are replied when the
> operation is completed.
> ---
>  src/gatt-client.c | 318 ++++++++++++++++++++++++++++--------------------------
>  1 file changed, 163 insertions(+), 155 deletions(-)
>
> diff --git a/src/gatt-client.c b/src/gatt-client.c
> index 6fc0d19..c2a888f 100644
> --- a/src/gatt-client.c
> +++ b/src/gatt-client.c
> @@ -74,6 +74,17 @@ struct service {
>         struct queue *chrcs;
>  };
>
> +typedef bool (*async_dbus_op_complete_t)(void *data);
> +
> +struct async_dbus_op {
> +       int ref_count;
> +       unsigned int id;
> +       struct queue *msgs;
> +       void *data;
> +       uint16_t offset;
> +       async_dbus_op_complete_t complete;
> +};
> +
>  struct characteristic {
>         struct service *service;
>         struct gatt_db_attribute *attr;
> @@ -85,8 +96,8 @@ struct characteristic {
>         bt_uuid_t uuid;
>         char *path;
>
> -       unsigned int read_id;
> -       unsigned int write_id;
> +       struct async_dbus_op *read_op;
> +       struct async_dbus_op *write_op;
>
>         struct queue *descs;
>
> @@ -101,8 +112,8 @@ struct descriptor {
>         bt_uuid_t uuid;
>         char *path;
>
> -       unsigned int read_id;
> -       unsigned int write_id;
> +       struct async_dbus_op *read_op;
> +       struct async_dbus_op *write_op;
>  };
>
>  static bool uuid_cmp(const bt_uuid_t *uuid, uint16_t u16)
> @@ -205,22 +216,11 @@ static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
>         return 0;
>  }
>
> -typedef bool (*async_dbus_op_complete_t)(void *data);
> -
> -struct async_dbus_op {
> -       int ref_count;
> -       DBusMessage *msg;
> -       void *data;
> -       uint16_t offset;
> -       async_dbus_op_complete_t complete;
> -};
> -
>  static void async_dbus_op_free(void *data)
>  {
>         struct async_dbus_op *op = data;
>
> -       if (op->msg)
> -               dbus_message_unref(op->msg);
> +       queue_destroy(op->msgs, (void *)dbus_message_unref);
>
>         free(op);
>  }
> @@ -296,27 +296,44 @@ static void write_descriptor_cb(struct gatt_db_attribute *attr, int err,
>                                         GATT_DESCRIPTOR_IFACE, "Value");
>  }
>
> -static void read_op_cb(struct gatt_db_attribute *attrib, int err,
> -                               const uint8_t *value, size_t length,
> -                               void *user_data)
> +static void async_dbus_op_reply(struct async_dbus_op *op, int err,
> +                               const uint8_t *value, size_t length)
>  {
> -       struct async_dbus_op *op = user_data;
> +       const struct queue_entry *entry;
>         DBusMessage *reply;
>
> -       if (err) {
> -               error("Failed to read attribute");
> -               return;
> -       }
> +       op->id = 0;
>
> -       reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
> -       if (!reply) {
> -               error("Failed to allocate D-Bus message reply");
> -               return;
> +       for (entry = queue_get_entries(op->msgs); entry; entry = entry->next) {
> +               DBusMessage *msg = entry->data;
> +
> +               if (err) {
> +                       reply = err > 0 ? create_gatt_dbus_error(msg, err) :
> +                               btd_error_failed(msg, strerror(err));
> +                       goto send_reply;
> +               }
> +
> +               reply = g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
> +               if (!reply) {
> +                       error("Failed to allocate D-Bus message reply");
> +                       return;
> +               }
> +
> +               if (value)
> +                       message_append_byte_array(reply, value, length);
> +
> +send_reply:
> +               g_dbus_send_message(btd_get_dbus_connection(), reply);
>         }
> +}
>
> -       message_append_byte_array(reply, value, length);
> +static void read_op_cb(struct gatt_db_attribute *attrib, int err,
> +                               const uint8_t *value, size_t length,
> +                               void *user_data)
> +{
> +       struct async_dbus_op *op = user_data;
>
> -       g_dbus_send_message(btd_get_dbus_connection(), reply);
> +       async_dbus_op_reply(op, err, value, length);
>  }
>
>  static void desc_read_cb(bool success, uint8_t att_ecode,
> @@ -326,7 +343,6 @@ static void desc_read_cb(bool success, uint8_t att_ecode,
>         struct async_dbus_op *op = user_data;
>         struct descriptor *desc = op->data;
>         struct service *service = desc->chrc->service;
> -       DBusMessage *reply;
>
>         if (!success)
>                 goto fail;
> @@ -337,6 +353,7 @@ static void desc_read_cb(bool success, uint8_t att_ecode,
>         if (!gatt_db_attribute_write(desc->attr, op->offset, value, length, 0,
>                                         NULL, write_descriptor_cb, desc)) {
>                 error("Failed to store attribute");
> +               att_ecode = BT_ATT_ERROR_UNLIKELY;
>                 goto fail;
>         }
>
> @@ -347,32 +364,30 @@ 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;
> -               desc->read_id = bt_gatt_client_read_long_value(
> -                                                       service->client->gatt,
> +               op->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);
> -               if (desc->read_id)
> +               if (op->id)
>                         return;
>         }
>
>         /* Read the stored data from db */
>         if (!gatt_db_attribute_read(desc->attr, 0, 0, NULL, read_op_cb, op)) {
>                 error("Failed to read database");
> +               att_ecode = BT_ATT_ERROR_UNLIKELY;
>                 goto fail;
>         }
>
> -       desc->read_id = 0;
> +       desc->read_op = NULL;
>
>         return;
>
>  fail:
> -       reply = create_gatt_dbus_error(op->msg, att_ecode);
> -       desc->read_id = 0;
> -       g_dbus_send_message(btd_get_dbus_connection(), reply);
> -       return;
> +       async_dbus_op_reply(op, att_ecode, NULL, 0);
> +       desc->read_op = NULL;
>  }
>
>  static int parse_options(DBusMessageIter *iter, uint16_t *offset)
> @@ -408,19 +423,45 @@ static int parse_options(DBusMessageIter *iter, uint16_t *offset)
>         return 0;
>  }
>
> -static unsigned int read_value(struct bt_gatt_client *gatt, uint16_t handle,
> -                               bt_gatt_client_read_callback_t callback,
> -                               struct async_dbus_op *op)
> +static struct async_dbus_op *async_dbus_op_new(DBusMessage *msg, void *data)
> +{
> +       struct async_dbus_op *op;
> +
> +       op = new0(struct async_dbus_op, 1);
> +       op->msgs = queue_new();
> +       queue_push_tail(op->msgs, dbus_message_ref(msg));
> +       op->data = data;
> +
> +       return op;
> +}
> +
> +static struct async_dbus_op *read_value(struct bt_gatt_client *gatt,
> +                                       DBusMessage *msg, uint16_t handle,
> +                                       uint16_t offset,
> +                                       bt_gatt_client_read_callback_t callback,
> +                                       void *data)
>  {
> +       struct async_dbus_op *op;
> +
> +       op = async_dbus_op_new(msg, data);
> +       op->offset = offset;
> +
>         if (op->offset)
> -               return bt_gatt_client_read_long_value(gatt, handle, op->offset,
> +               op->id = bt_gatt_client_read_long_value(gatt, handle, offset,
>                                                         callback,
>                                                         async_dbus_op_ref(op),
>                                                         async_dbus_op_unref);
>         else
> -               return bt_gatt_client_read_value(gatt, handle, callback,
> +               op->id = bt_gatt_client_read_value(gatt, handle, callback,
>                                                         async_dbus_op_ref(op),
>                                                         async_dbus_op_unref);
> +
> +       if (op->id)
> +               return op;
> +
> +       async_dbus_op_free(op);
> +
> +       return NULL;
>  }
>
>  static DBusMessage *descriptor_read_value(DBusConnection *conn,
> @@ -429,63 +470,51 @@ static DBusMessage *descriptor_read_value(DBusConnection *conn,
>         struct descriptor *desc = user_data;
>         struct bt_gatt_client *gatt = desc->chrc->service->client->gatt;
>         DBusMessageIter iter;
> -       struct async_dbus_op *op;
>         uint16_t offset = 0;
>
>         if (!gatt)
>                 return btd_error_failed(msg, "Not connected");
>
> -       if (desc->read_id)
> -               return btd_error_in_progress(msg);
> -
>         dbus_message_iter_init(msg, &iter);
>
>         if (parse_options(&iter, &offset))
>                 return btd_error_invalid_args(msg);
>
> -       op = new0(struct async_dbus_op, 1);
> -       op->msg = dbus_message_ref(msg);
> -       op->data = desc;
> -       op->offset = offset;
> -
> -       desc->read_id = read_value(gatt, desc->handle, desc_read_cb, op);
> -       if (desc->read_id)
> +       if (desc->read_op) {
> +               if (desc->read_op->offset != offset)
> +                       return btd_error_in_progress(msg);
> +               queue_push_tail(desc->read_op->msgs, dbus_message_ref(msg));
>                 return NULL;
> +       }
>
> -       async_dbus_op_free(op);
> +       desc->read_op = read_value(gatt, msg, desc->handle, offset,
> +                                                       desc_read_cb, desc);
> +       if (!desc->read_op)
> +               return btd_error_failed(msg, "Failed to send read request");
>
> -       return btd_error_failed(msg, "Failed to send read request");
> +       return NULL;
>  }
>
>  static void write_result_cb(bool success, bool reliable_error,
>                                         uint8_t att_ecode, void *user_data)
>  {
>         struct async_dbus_op *op = user_data;
> -       DBusMessage *reply;
> +       int err = 0;
>
>         if (op->complete && !op->complete(op->data)) {
> -               reply = btd_error_failed(op->msg, "Operation failed");
> +               err = -EFAULT;
>                 goto done;
>         }
>
>         if (!success) {
>                 if (reliable_error)
> -                       reply = btd_error_failed(op->msg,
> -                                               "Reliable write failed");
> +                       err = -EFAULT;
>                 else
> -                       reply = create_gatt_dbus_error(op->msg, att_ecode);
> -
> -               goto done;
> -       }
> -
> -       reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
> -       if (!reply) {
> -               error("Failed to allocate D-Bus message reply");
> -               return;
> +                       err = att_ecode;
>         }
>
>  done:
> -       g_dbus_send_message(btd_get_dbus_connection(), reply);
> +       async_dbus_op_reply(op, err, NULL, 0);
>  }
>
>  static void write_cb(bool success, uint8_t att_ecode, void *user_data)
> @@ -493,7 +522,7 @@ static void write_cb(bool success, uint8_t att_ecode, void *user_data)
>         write_result_cb(success, false, att_ecode, user_data);
>  }
>
> -static unsigned int start_long_write(DBusMessage *msg, uint16_t handle,
> +static struct async_dbus_op *start_long_write(DBusMessage *msg, uint16_t handle,
>                                         struct bt_gatt_client *gatt,
>                                         bool reliable, const uint8_t *value,
>                                         size_t value_len, uint16_t offset,
> @@ -501,53 +530,52 @@ static unsigned int start_long_write(DBusMessage *msg, uint16_t handle,
>                                         async_dbus_op_complete_t complete)
>  {
>         struct async_dbus_op *op;
> -       unsigned int id;
>
> -       op = new0(struct async_dbus_op, 1);
> -       op->msg = dbus_message_ref(msg);
> -       op->data = data;
> +       op = async_dbus_op_new(msg, data);
>         op->complete = complete;
>         op->offset = offset;
>
> -       id = bt_gatt_client_write_long_value(gatt, reliable, handle, offset,
> +       op->id = bt_gatt_client_write_long_value(gatt, reliable, handle, offset,
>                                                         value, value_len,
>                                                         write_result_cb, op,
>                                                         async_dbus_op_free);
>
> -       if (!id)
> +       if (!op->id) {
>                 async_dbus_op_free(op);
> +               return NULL;
> +       }
>
> -       return id;
> +       return op;
>  }
>
> -static unsigned int start_write_request(DBusMessage *msg, uint16_t handle,
> +static struct async_dbus_op *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);
> -       op->msg = dbus_message_ref(msg);
> -       op->data = data;
> +       op = async_dbus_op_new(msg, data);
>         op->complete = complete;
>
> -       id = bt_gatt_client_write_value(gatt, handle, value, value_len,
> +       op->id = bt_gatt_client_write_value(gatt, handle, value, value_len,
>                                                         write_cb, op,
>                                                         async_dbus_op_free);
> -       if (!id)
> +       if (!op->id) {
>                 async_dbus_op_free(op);
> +               return NULL;
> +       }
>
> -       return id;
> +       return op;
>  }
>
>  static bool desc_write_complete(void *data)
>  {
>         struct descriptor *desc = data;
>
> -       desc->write_id = 0;
> +       desc->write_op = NULL;
>
>         /*
>          * The descriptor might have been unregistered during the read. Return
> @@ -569,7 +597,7 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
>         if (!gatt)
>                 return btd_error_failed(msg, "Not connected");
>
> -       if (desc->write_id)
> +       if (desc->write_op)
>                 return btd_error_in_progress(msg);
>
>         dbus_message_iter_init(msg, &iter);
> @@ -593,17 +621,17 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
>          * write.
>          */
>         if (value_len <= bt_gatt_client_get_mtu(gatt) - 3 && !offset)
> -               desc->write_id = start_write_request(msg, desc->handle,
> +               desc->write_op = start_write_request(msg, desc->handle,
>                                                         gatt, value,
>                                                         value_len, desc,
>                                                         desc_write_complete);
>         else
> -               desc->write_id = start_long_write(msg, desc->handle, gatt,
> +               desc->write_op = start_long_write(msg, desc->handle, gatt,
>                                                         false, value,
>                                                         value_len, offset, desc,
>                                                         desc_write_complete);
>
> -       if (!desc->write_id)
> +       if (!desc->write_op)
>                 return btd_error_failed(msg, "Failed to initiate write");
>
>         return NULL;
> @@ -681,11 +709,11 @@ static void unregister_descriptor(void *data)
>
>         DBG("Removing GATT descriptor: %s", desc->path);
>
> -       if (desc->read_id)
> -               bt_gatt_client_cancel(gatt, desc->read_id);
> +       if (desc->read_op)
> +               bt_gatt_client_cancel(gatt, desc->read_op->id);
>
> -       if (desc->write_id)
> -               bt_gatt_client_cancel(gatt, desc->write_id);
> +       if (desc->write_op)
> +               bt_gatt_client_cancel(gatt, desc->write_op->id);
>
>         desc->chrc = NULL;
>
> @@ -832,7 +860,6 @@ static void chrc_read_cb(bool success, uint8_t att_ecode, const uint8_t *value,
>         struct async_dbus_op *op = user_data;
>         struct characteristic *chrc = op->data;
>         struct service *service = chrc->service;
> -       DBusMessage *reply;
>
>         if (!success)
>                 goto fail;
> @@ -843,6 +870,7 @@ static void chrc_read_cb(bool success, uint8_t att_ecode, const uint8_t *value,
>         if (!gatt_db_attribute_write(chrc->attr, op->offset, value, length, 0,
>                                         NULL, write_characteristic_cb, chrc)) {
>                 error("Failed to store attribute");
> +               att_ecode = BT_ATT_ERROR_UNLIKELY;
>                 goto fail;
>         }
>
> @@ -853,31 +881,30 @@ 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;
> -               chrc->read_id = bt_gatt_client_read_long_value(
> -                                                       service->client->gatt,
> +               op->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);
> -               if (chrc->read_id)
> +               if (op->id)
>                         return;
>         }
>
> -       chrc->read_id = 0;
> -
>         /* Read the stored data from db */
>         if (!gatt_db_attribute_read(chrc->attr, 0, 0, NULL, read_op_cb, op)) {
>                 error("Failed to read database");
> +               att_ecode = BT_ATT_ERROR_UNLIKELY;
>                 goto fail;
>         }
>
> +       chrc->read_op = NULL;
> +
>         return;
>
>  fail:
> -       reply = create_gatt_dbus_error(op->msg, att_ecode);
> -       chrc->read_id = 0;
> -       g_dbus_send_message(btd_get_dbus_connection(), reply);
> +       async_dbus_op_reply(op, att_ecode, NULL, 0);
> +       chrc->read_op = NULL;
>  }
>
>  static DBusMessage *characteristic_read_value(DBusConnection *conn,
> @@ -886,39 +913,36 @@ static DBusMessage *characteristic_read_value(DBusConnection *conn,
>         struct characteristic *chrc = user_data;
>         struct bt_gatt_client *gatt = chrc->service->client->gatt;
>         DBusMessageIter iter;
> -       struct async_dbus_op *op;
>         uint16_t offset = 0;
>
>         if (!gatt)
>                 return btd_error_failed(msg, "Not connected");
>
> -       if (chrc->read_id)
> -               return btd_error_in_progress(msg);
> -
>         dbus_message_iter_init(msg, &iter);
>
>         if (parse_options(&iter, &offset))
>                 return btd_error_invalid_args(msg);
>
> -       op = new0(struct async_dbus_op, 1);
> -       op->msg = dbus_message_ref(msg);
> -       op->data = chrc;
> -       op->offset = offset;
> -
> -       chrc->read_id = read_value(gatt, chrc->value_handle, chrc_read_cb, op);
> -       if (chrc->read_id)
> +       if (chrc->read_op) {
> +               if (chrc->read_op->offset != offset)
> +                       return btd_error_in_progress(msg);
> +               queue_push_tail(chrc->read_op->msgs, dbus_message_ref(msg));
>                 return NULL;
> +       }
>
> -       async_dbus_op_free(op);
> +       chrc->read_op = read_value(gatt, msg, chrc->value_handle, offset,
> +                                                       chrc_read_cb, chrc);
> +       if (!chrc->read_op)
> +               return btd_error_failed(msg, "Failed to send read request");
>
> -       return btd_error_failed(msg, "Failed to send read request");
> +       return NULL;
>  }
>
>  static bool chrc_write_complete(void *data)
>  {
>         struct characteristic *chrc = data;
>
> -       chrc->write_id = 0;
> +       chrc->write_op = NULL;
>
>         /*
>          * The characteristic might have been unregistered during the read.
> @@ -941,7 +965,7 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
>         if (!gatt)
>                 return btd_error_failed(msg, "Not connected");
>
> -       if (chrc->write_id)
> +       if (chrc->write_op)
>                 return btd_error_in_progress(msg);
>
>         dbus_message_iter_init(msg, &iter);
> @@ -965,10 +989,10 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
>          */
>         if ((chrc->ext_props & BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE)) {
>                 supported = true;
> -               chrc->write_id = start_long_write(msg, chrc->value_handle, gatt,
> +               chrc->write_op = start_long_write(msg, chrc->value_handle, gatt,
>                                                 true, value, value_len, offset,
>                                                 chrc, chrc_write_complete);
> -               if (chrc->write_id)
> +               if (chrc->write_op)
>                         return NULL;
>         }
>
> @@ -981,17 +1005,17 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
>                         return btd_error_failed(msg, "No ATT transport");
>
>                 if (value_len <= mtu - 3 && !offset)
> -                       chrc->write_id = start_write_request(msg,
> +                       chrc->write_op = start_write_request(msg,
>                                                 chrc->value_handle,
>                                                 gatt, value, value_len,
>                                                 chrc, chrc_write_complete);
>                 else
> -                       chrc->write_id = start_long_write(msg,
> +                       chrc->write_op = start_long_write(msg,
>                                                 chrc->value_handle, gatt,
>                                                 false, value, value_len, offset,
>                                                 chrc, chrc_write_complete);
>
> -               if (chrc->write_id)
> +               if (chrc->write_op)
>                         return NULL;
>         }
>
> @@ -1140,26 +1164,17 @@ static void notify_cb(uint16_t value_handle, const uint8_t *value,
>                                                 write_characteristic_cb, chrc);
>  }
>
> -static DBusMessage *create_notify_reply(struct async_dbus_op *op,
> -                                               bool success, uint8_t att_ecode)
> +static void create_notify_reply(struct async_dbus_op *op, bool success,
> +                                                       uint8_t att_ecode)
>  {
> -       DBusMessage *reply = NULL;
> -
> -       if (!op->msg)
> -               return NULL;
> +       int err;
>
>         if (success)
> -               reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
> -       else if (att_ecode)
> -               reply = create_gatt_dbus_error(op->msg, att_ecode);
> +               err = 0;
>         else
> -               reply = btd_error_failed(op->msg,
> -                                               "Characteristic not available");
> +               err = att_ecode ? att_ecode : -ENOENT;
>
> -       if (!reply)
> -               error("Failed to construct D-Bus message reply");
> -
> -       return reply;
> +       async_dbus_op_reply(op, err, NULL, 0);
>  }
>
>  static void register_notify_cb(uint16_t att_ecode, void *user_data)
> @@ -1167,16 +1182,15 @@ static void register_notify_cb(uint16_t att_ecode, void *user_data)
>         struct async_dbus_op *op = user_data;
>         struct notify_client *client = op->data;
>         struct characteristic *chrc = client->chrc;
> -       DBusMessage *reply;
>
>         if (att_ecode) {
>                 queue_remove(chrc->notify_clients, client);
>                 queue_remove(chrc->service->client->all_notify_clients, client);
>                 notify_client_free(client);
>
> -               reply = create_notify_reply(op, false, att_ecode);
> +               create_notify_reply(op, false, att_ecode);
>
> -               goto done;
> +               return;
>         }
>
>         if (!chrc->notifying) {
> @@ -1186,11 +1200,7 @@ static void register_notify_cb(uint16_t att_ecode, void *user_data)
>                                         "Notifying");
>         }
>
> -       reply = create_notify_reply(op, true, 0);
> -
> -done:
> -       if (reply)
> -               g_dbus_send_message(btd_get_dbus_connection(), reply);
> +       create_notify_reply(op, true, 0);
>  }
>
>  static DBusMessage *characteristic_start_notify(DBusConnection *conn,
> @@ -1240,9 +1250,7 @@ static DBusMessage *characteristic_start_notify(DBusConnection *conn,
>                 goto fail;
>         }
>
> -       op = new0(struct async_dbus_op, 1);
> -       op->data = client;
> -       op->msg = dbus_message_ref(msg);
> +       op = async_dbus_op_new(msg, client);
>
>         client->notify_id = bt_gatt_client_register_notify(gatt,
>                                                 chrc->value_handle,
> @@ -1395,11 +1403,11 @@ static void unregister_characteristic(void *data)
>
>         DBG("Removing GATT characteristic: %s", chrc->path);
>
> -       if (chrc->read_id)
> -               bt_gatt_client_cancel(gatt, chrc->read_id);
> +       if (chrc->read_op)
> +               bt_gatt_client_cancel(gatt, chrc->read_op->id);
>
> -       if (chrc->write_id)
> -               bt_gatt_client_cancel(gatt, chrc->write_id);
> +       if (chrc->write_op)
> +               bt_gatt_client_cancel(gatt, chrc->write_op->id);
>
>         queue_remove_all(chrc->notify_clients, NULL, NULL, remove_client);
>         queue_remove_all(chrc->descs, NULL, NULL, unregister_descriptor);
> --
> 2.5.5

Applied.


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-06-27 14:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-23 11:02 [PATCH BlueZ 1/2] core/gatt-client: Allow clients to call ReadValue in parallel Luiz Augusto von Dentz
2016-06-23 11:02 ` [PATCH BlueZ 2/2] core/gatt-client: Always use bt_gatt_client_read_long_value Luiz Augusto von Dentz
2016-06-27 14:14 ` [PATCH BlueZ 1/2] core/gatt-client: Allow clients to call ReadValue in parallel Luiz Augusto von Dentz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.