core/gatt-client: Allow clients to call ReadValue in parallel

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.
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 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 @@
 	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 @@
 	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 @@
 	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 @@
 					GATT_DESCRIPTOR_IFACE, "Value");
 }
 
+static void async_dbus_op_reply(struct async_dbus_op *op, int err,
+				const uint8_t *value, size_t length)
+{
+	const struct queue_entry *entry;
+	DBusMessage *reply;
+
+	op->id = 0;
+
+	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);
+	}
+}
+
 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;
-	DBusMessage *reply;
 
-	if (err) {
-		error("Failed to read attribute");
-		return;
-	}
-
-	reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
-	if (!reply) {
-		error("Failed to allocate D-Bus message reply");
-		return;
-	}
-
-	message_append_byte_array(reply, value, length);
-
-	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 @@
 	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 @@
 	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 @@
 	 */
 	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 @@
 	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 @@
 	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 @@
 	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 @@
 					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 @@
 	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 @@
 	 * 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 @@
 
 	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 @@
 	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 @@
 	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 @@
 	 */
 	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 @@
 	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 @@
 	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 @@
 	 */
 	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 @@
 			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 @@
 						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 @@
 	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 @@
 					"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 @@
 		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 @@
 
 	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);