[PATCH BlueZ 0/8] Generic GATT client fixes

[PATCH BlueZ 0/8] Generic GATT client fixes

[Anderson Lizardo]

Hi,

These patches contains fixes (and two refactorings) found during our latest
round of tests.

Best Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia (INdT)
Manaus - Brazil

[PATCH BlueZ 1/8] attrib: Centralize ATTIO connection management

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

On Generic Attribute, register only one ATTIO connection callback to
centralize the connection management.

Also make sure the connection reference is properly dropped only if
there is no pending operation.
---
 attrib/client.c |   40 ++++++++++++++++++++++++----------------
 1 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 60cff01..412ff19 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -62,7 +62,6 @@ struct format {
 
 struct query {
 	DBusMessage *msg;
-	guint attioid;
 	GSList *list;
 };
 
@@ -141,6 +140,22 @@ static void gatt_service_free(struct gatt_service *gatt)
 	g_free(gatt);
 }
 
+static void remove_attio(struct gatt_service *gatt)
+{
+	if (gatt->offline_chars || gatt->watchers || gatt->query)
+		return;
+
+	if (gatt->attioid) {
+		btd_device_remove_attio_callback(gatt->dev, gatt->attioid);
+		gatt->attioid = 0;
+	}
+
+	if (gatt->attrib) {
+		g_attrib_unref(gatt->attrib);
+		gatt->attrib = NULL;
+	}
+}
+
 static void gatt_get_address(struct gatt_service *gatt,
 				bdaddr_t *sba, bdaddr_t *dba)
 {
@@ -298,11 +313,7 @@ static void offline_char_written(gpointer user_data)
 
 	gatt->offline_chars = g_slist_remove(gatt->offline_chars, chr);
 
-	if (gatt->offline_chars || gatt->watchers)
-		return;
-
-	btd_device_remove_attio_callback(gatt->dev, gatt->attioid);
-	gatt->attioid = 0;
+	remove_attio(gatt);
 }
 
 static void offline_char_write(gpointer data, gpointer user_data)
@@ -394,10 +405,7 @@ static DBusMessage *unregister_watcher(DBusConnection *conn,
 	gatt->watchers = g_slist_remove(gatt->watchers, watcher);
 	watcher_free(watcher);
 
-	if (gatt->watchers == NULL && gatt->attioid) {
-		btd_device_remove_attio_callback(gatt->dev, gatt->attioid);
-		gatt->attioid = 0;
-	}
+	remove_attio(gatt);
 
 	return dbus_message_new_method_return(msg);
 }
@@ -632,10 +640,10 @@ static void query_list_remove(struct gatt_service *gatt, struct query_data *data
 	if (query->list != NULL)
 		return;
 
-	btd_device_remove_attio_callback(gatt->dev, query->attioid);
 	g_free(query);
-
 	gatt->query = NULL;
+
+	remove_attio(gatt);
 }
 
 static void update_char_desc(guint8 status, const guint8 *pdu, guint16 len,
@@ -928,7 +936,8 @@ static DBusMessage *discover_char(DBusConnection *conn, DBusMessage *msg,
 	qchr->gatt = gatt;
 
 	query->msg = dbus_message_ref(msg);
-	query->attioid = btd_device_add_attio_callback(gatt->dev,
+
+	gatt->attioid = btd_device_add_attio_callback(gatt->dev,
 							send_discover,
 							cancel_discover,
 							qchr);
@@ -1046,9 +1055,6 @@ static void primary_unregister(struct gatt_service *gatt)
 {
 	GSList *l;
 
-	if (gatt->attioid)
-		btd_device_remove_attio_callback(gatt->dev, gatt->attioid);
-
 	for (l = gatt->chars; l; l = l->next) {
 		struct characteristic *chr = l->data;
 		g_dbus_unregister_interface(gatt->conn, chr->path,
@@ -1056,6 +1062,8 @@ static void primary_unregister(struct gatt_service *gatt)
 	}
 
 	g_dbus_unregister_interface(gatt->conn, gatt->path, CHAR_INTERFACE);
+
+	remove_attio(gatt);
 }
 
 static int path_cmp(gconstpointer data, gconstpointer user_data)
-- 
1.7.5.4

[PATCH BlueZ 2/8] attrib: Discover Characteristics if already connected

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

On Generic Attribute, Discover All Characteristics of a Service can be
started immediately if already connected, without registering a ATTIO
connection callback.
---
 attrib/client.c |   47 ++++++++++++++++++++++-------------------------
 1 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 412ff19..73d4d95 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -325,6 +325,9 @@ static void offline_char_write(gpointer data, gpointer user_data)
 						offline_char_written, chr);
 }
 
+static void char_discovered_cb(GSList *characteristics, guint8 status,
+							gpointer user_data);
+
 static void attio_connected(GAttrib *attrib, gpointer user_data)
 {
 	struct gatt_service *gatt = user_data;
@@ -337,6 +340,15 @@ static void attio_connected(GAttrib *attrib, gpointer user_data)
 					events_handler, gatt, NULL);
 
 	g_slist_foreach(gatt->offline_chars, offline_char_write, attrib);
+
+	if (gatt->query) {
+		struct att_primary *prim = gatt->prim;
+		struct query_data *qchr;
+
+		qchr = g_slist_nth_data(gatt->query->list, 0);
+		gatt_discover_char(gatt->attrib, prim->start, prim->end, NULL,
+						char_discovered_cb, qchr);
+	}
 }
 
 static void attio_disconnected(gpointer user_data)
@@ -899,27 +911,6 @@ fail:
 	g_free(current);
 }
 
-static void send_discover(GAttrib *attrib, gpointer user_data)
-{
-	struct query_data *qchr = user_data;
-	struct gatt_service *gatt = qchr->gatt;
-	struct att_primary *prim = gatt->prim;
-
-	gatt->attrib = g_attrib_ref(attrib);
-
-	gatt_discover_char(gatt->attrib, prim->start, prim->end, NULL,
-						char_discovered_cb, qchr);
-}
-
-static void cancel_discover(gpointer user_data)
-{
-	struct query_data *qchr = user_data;
-	struct gatt_service *gatt = qchr->gatt;
-
-	g_attrib_unref(gatt->attrib);
-	gatt->attrib = NULL;
-}
-
 static DBusMessage *discover_char(DBusConnection *conn, DBusMessage *msg,
 								void *data)
 {
@@ -937,10 +928,16 @@ static DBusMessage *discover_char(DBusConnection *conn, DBusMessage *msg,
 
 	query->msg = dbus_message_ref(msg);
 
-	gatt->attioid = btd_device_add_attio_callback(gatt->dev,
-							send_discover,
-							cancel_discover,
-							qchr);
+	if (gatt->attioid == 0) {
+		gatt->attioid = btd_device_add_attio_callback(gatt->dev,
+							attio_connected,
+							attio_disconnected,
+							gatt);
+	} else if (gatt->attrib) {
+		struct att_primary *prim = gatt->prim;
+		gatt_discover_char(gatt->attrib, prim->start, prim->end, NULL,
+						char_discovered_cb, qchr);
+	}
 
 	gatt->query = query;
 
-- 
1.7.5.4

[PATCH BlueZ 3/8] attrib: Move Characteristic Discovery reply to a new function

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

Minor cleanup patch moving the creation of the reply for Discover
All Characteristics call to a new local function in the Generic
Attribute.
---
 attrib/client.c |   44 +++++++++++++++++++++++++++-----------------
 1 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 73d4d95..d19158c 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -834,11 +834,35 @@ static void update_all_chars(gpointer data, gpointer user_data)
 	gatt_read_char(gatt->attrib, chr->handle, 0, update_char_value, qvalue);
 }
 
+static DBusMessage *create_discover_char_reply(DBusMessage *msg, GSList *chars)
+{
+	DBusMessage *reply;
+	DBusMessageIter iter, array_iter;
+	GSList *l;
+
+	reply = dbus_message_new_method_return(msg);
+
+	dbus_message_iter_init_append(reply, &iter);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+				DBUS_TYPE_OBJECT_PATH_AS_STRING, &array_iter);
+
+	for (l = chars; l; l = l->next) {
+		struct characteristic *chr = l->data;
+
+		dbus_message_iter_append_basic(&array_iter,
+					DBUS_TYPE_OBJECT_PATH, &chr->path);
+	}
+
+	dbus_message_iter_close_container(&iter, &array_iter);
+
+	return reply;
+}
+
 static void char_discovered_cb(GSList *characteristics, guint8 status,
 							gpointer user_data)
 {
 	DBusMessage *reply;
-	DBusMessageIter iter, array_iter;
 	struct query_data *current = user_data;
 	struct gatt_service *gatt = current->gatt;
 	struct att_primary *prim = gatt->prim;
@@ -887,24 +911,10 @@ static void char_discovered_cb(GSList *characteristics, guint8 status,
 
 	g_slist_foreach(gatt->chars, register_characteristic, gatt->path);
 
-	reply = dbus_message_new_method_return(gatt->query->msg);
-
-	dbus_message_iter_init_append(reply, &iter);
-
-	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
-				DBUS_TYPE_OBJECT_PATH_AS_STRING, &array_iter);
-
-	for (l = gatt->chars; l; l = l->next) {
-		struct characteristic *chr = l->data;
-
-		dbus_message_iter_append_basic(&array_iter,
-					DBUS_TYPE_OBJECT_PATH, &chr->path);
-	}
-
-	dbus_message_iter_close_container(&iter, &array_iter);
-
 	g_slist_foreach(gatt->chars, update_all_chars, gatt);
 
+	reply = create_discover_char_reply(gatt->query->msg, gatt->chars);
+
 fail:
 	g_dbus_send_message(gatt->conn, reply);
 	query_list_remove(gatt, current);
-- 
1.7.5.4

[PATCH BlueZ 4/8] attrib: Register only new characteristics found

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

This avoids duplicating registration for previously found
characteristics.
---
 attrib/client.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index d19158c..a4e7d4c 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -901,6 +901,7 @@ static void char_discovered_cb(GSList *characteristics, guint8 status,
 		previous_end = &chr->end;
 
 		gatt->chars = g_slist_append(gatt->chars, chr);
+		register_characteristic(chr, gatt->path);
 	}
 
 	if (previous_end)
@@ -909,8 +910,6 @@ static void char_discovered_cb(GSList *characteristics, guint8 status,
 	gatt_get_address(gatt, &sba, &dba);
 	store_characteristics(&sba, &dba, prim->start, gatt->chars);
 
-	g_slist_foreach(gatt->chars, register_characteristic, gatt->path);
-
 	g_slist_foreach(gatt->chars, update_all_chars, gatt);
 
 	reply = create_discover_char_reply(gatt->query->msg, gatt->chars);
-- 
1.7.5.4

[PATCH BlueZ 5/8] attrib: Fix missing Discovery reply when disconnected

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

This patch fixes missing reply for DiscoverCharacteristics method on
Generic Attribute. If link is disconnected reply message is not sent.
---
 attrib/client.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index a4e7d4c..bbb7de3 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -355,6 +355,20 @@ static void attio_disconnected(gpointer user_data)
 {
 	struct gatt_service *gatt = user_data;
 
+	if (gatt->query) {
+		if (gatt->query->msg) {
+			DBusMessage *reply;
+
+			reply = btd_error_failed(gatt->query->msg,
+						strerror(ECONNRESET));
+			g_dbus_send_message(gatt->conn, reply);
+			dbus_message_unref(gatt->query->msg);
+		}
+
+		g_slist_free_full(gatt->query->list, g_free);
+		gatt->query = NULL;
+	}
+
 	if (gatt->attrib) {
 		g_attrib_unref(gatt->attrib);
 		gatt->attrib = NULL;
-- 
1.7.5.4

[PATCH BlueZ 6/8] attrib: Fix missing D-Bus message unref on DiscoverCharacteristics

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

This patch fixes a missing message unref when DiscoverCharacteristics on
Generic Attribute finishes.
---
 attrib/client.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index bbb7de3..ac1b354 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -929,6 +929,9 @@ static void char_discovered_cb(GSList *characteristics, guint8 status,
 	reply = create_discover_char_reply(gatt->query->msg, gatt->chars);
 
 fail:
+	dbus_message_unref(gatt->query->msg);
+	gatt->query->msg = NULL;
+
 	g_dbus_send_message(gatt->conn, reply);
 	query_list_remove(gatt, current);
 	g_free(current);
-- 
1.7.5.4

[PATCH BlueZ 7/8] attrib: Attempt to remove attio callback on watcher exit

[Anderson Lizardo]

When registering a watcher, it is attempted to add an attio callback.
Therefore, when the watcher is unregistered or exits, this attio should
be removed if there are no other users.

This is already done when unregistering a watcher, it just missed when
it exits without unregistering.
---
 attrib/client.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index ac1b354..52f50b4 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -227,6 +227,7 @@ static void watcher_exit(DBusConnection *conn, void *user_data)
 	DBG("%s watcher %s exited", gatt->path, watcher->name);
 
 	gatt->watchers = g_slist_remove(gatt->watchers, watcher);
+	remove_attio(gatt);
 }
 
 static int characteristic_set_value(struct characteristic *chr,
-- 
1.7.5.4

[PATCH BlueZ 8/8] gattrib: Protect GAttrib when there is a pending write

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

---
 attrib/gattrib.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 8a1e97b..c3a70b9 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -293,14 +293,18 @@ static void destroy_sender(gpointer data)
 	struct _GAttrib *attrib = data;
 
 	attrib->write_watch = 0;
+	g_attrib_unref(attrib);
 }
 
 static void wake_up_sender(struct _GAttrib *attrib)
 {
-	if (attrib->write_watch == 0)
-		attrib->write_watch = g_io_add_watch_full(attrib->io,
-			G_PRIORITY_DEFAULT, G_IO_OUT, can_write_data,
-			attrib, destroy_sender);
+	if (attrib->write_watch != 0)
+		return;
+
+	attrib = g_attrib_ref(attrib);
+	attrib->write_watch = g_io_add_watch_full(attrib->io,
+				G_PRIORITY_DEFAULT, G_IO_OUT,
+				can_write_data, attrib, destroy_sender);
 }
 
 static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
-- 
1.7.5.4

[PATCH v2 BlueZ 0/8] Generic GATT client fixes

[Anderson Lizardo]

Hi,

These patches contain fixes (and two refactorings) found during our latest
round of tests.

Change since v1: rebased against latest upstream.



Anderson Lizardo (1):
  attrib: Attempt to remove attio callback on watcher exit

Claudio Takahasi (7):
  attrib: Centralize ATTIO connection management
  attrib: Discover Characteristics if already connected
  attrib: Move Characteristic Discovery reply to a new function
  attrib: Register only new characteristics found
  attrib: Fix missing Discovery reply when disconnected
  attrib: Fix missing D-Bus message unref on DiscoverCharacteristics
  gattrib: Protect GAttrib when there is a pending write

 attrib/client.c  |  152 +++++++++++++++++++++++++++++++++---------------------
 attrib/gattrib.c |   12 +++--
 2 files changed, 101 insertions(+), 63 deletions(-)

-- 
1.7.5.4

[PATCH v2 BlueZ 1/8] attrib: Centralize ATTIO connection management

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

On Generic Attribute, register only one ATTIO connection callback to
centralize the connection management.

Also make sure the connection reference is properly dropped only if
there is no pending operation.
---
 attrib/client.c |   40 ++++++++++++++++++++++++----------------
 1 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index c0e00c1..143c06b 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -62,7 +62,6 @@ struct format {
 
 struct query {
 	DBusMessage *msg;
-	guint attioid;
 	GSList *list;
 };
 
@@ -141,6 +140,22 @@ static void gatt_service_free(struct gatt_service *gatt)
 	g_free(gatt);
 }
 
+static void remove_attio(struct gatt_service *gatt)
+{
+	if (gatt->offline_chars || gatt->watchers || gatt->query)
+		return;
+
+	if (gatt->attioid) {
+		btd_device_remove_attio_callback(gatt->dev, gatt->attioid);
+		gatt->attioid = 0;
+	}
+
+	if (gatt->attrib) {
+		g_attrib_unref(gatt->attrib);
+		gatt->attrib = NULL;
+	}
+}
+
 static void gatt_get_address(struct gatt_service *gatt,
 				bdaddr_t *sba, bdaddr_t *dba)
 {
@@ -298,11 +313,7 @@ static void offline_char_written(gpointer user_data)
 
 	gatt->offline_chars = g_slist_remove(gatt->offline_chars, chr);
 
-	if (gatt->offline_chars || gatt->watchers)
-		return;
-
-	btd_device_remove_attio_callback(gatt->dev, gatt->attioid);
-	gatt->attioid = 0;
+	remove_attio(gatt);
 }
 
 static void offline_char_write(gpointer data, gpointer user_data)
@@ -394,10 +405,7 @@ static DBusMessage *unregister_watcher(DBusConnection *conn,
 	gatt->watchers = g_slist_remove(gatt->watchers, watcher);
 	watcher_free(watcher);
 
-	if (gatt->watchers == NULL && gatt->attioid) {
-		btd_device_remove_attio_callback(gatt->dev, gatt->attioid);
-		gatt->attioid = 0;
-	}
+	remove_attio(gatt);
 
 	return dbus_message_new_method_return(msg);
 }
@@ -632,10 +640,10 @@ static void query_list_remove(struct gatt_service *gatt, struct query_data *data
 	if (query->list != NULL)
 		return;
 
-	btd_device_remove_attio_callback(gatt->dev, query->attioid);
 	g_free(query);
-
 	gatt->query = NULL;
+
+	remove_attio(gatt);
 }
 
 static void update_char_desc(guint8 status, const guint8 *pdu, guint16 len,
@@ -928,7 +936,8 @@ static DBusMessage *discover_char(DBusConnection *conn, DBusMessage *msg,
 	qchr->gatt = gatt;
 
 	query->msg = dbus_message_ref(msg);
-	query->attioid = btd_device_add_attio_callback(gatt->dev,
+
+	gatt->attioid = btd_device_add_attio_callback(gatt->dev,
 							send_discover,
 							cancel_discover,
 							qchr);
@@ -1046,9 +1055,6 @@ static void primary_unregister(struct gatt_service *gatt)
 {
 	GSList *l;
 
-	if (gatt->attioid)
-		btd_device_remove_attio_callback(gatt->dev, gatt->attioid);
-
 	for (l = gatt->chars; l; l = l->next) {
 		struct characteristic *chr = l->data;
 		g_dbus_unregister_interface(gatt->conn, chr->path,
@@ -1056,6 +1062,8 @@ static void primary_unregister(struct gatt_service *gatt)
 	}
 
 	g_dbus_unregister_interface(gatt->conn, gatt->path, CHAR_INTERFACE);
+
+	remove_attio(gatt);
 }
 
 static int path_cmp(gconstpointer data, gconstpointer user_data)
-- 
1.7.5.4

[PATCH v2 BlueZ 2/8] attrib: Discover Characteristics if already connected

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

On Generic Attribute, Discover All Characteristics of a Service can be
started immediately if already connected, without registering a ATTIO
connection callback.
---
 attrib/client.c |   49 ++++++++++++++++++++++++-------------------------
 1 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 143c06b..2ad3956 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -325,6 +325,9 @@ static void offline_char_write(gpointer data, gpointer user_data)
 						offline_char_written, chr);
 }
 
+static void char_discovered_cb(GSList *characteristics, guint8 status,
+							gpointer user_data);
+
 static void attio_connected(GAttrib *attrib, gpointer user_data)
 {
 	struct gatt_service *gatt = user_data;
@@ -337,6 +340,16 @@ static void attio_connected(GAttrib *attrib, gpointer user_data)
 					events_handler, gatt, NULL);
 
 	g_slist_foreach(gatt->offline_chars, offline_char_write, attrib);
+
+	if (gatt->query) {
+		struct gatt_primary *prim = gatt->prim;
+		struct query_data *qchr;
+
+		qchr = g_slist_nth_data(gatt->query->list, 0);
+		gatt_discover_char(gatt->attrib, prim->range.start,
+						prim->range.end, NULL,
+						char_discovered_cb, qchr);
+	}
 }
 
 static void attio_disconnected(gpointer user_data)
@@ -899,27 +912,6 @@ fail:
 	g_free(current);
 }
 
-static void send_discover(GAttrib *attrib, gpointer user_data)
-{
-	struct query_data *qchr = user_data;
-	struct gatt_service *gatt = qchr->gatt;
-	struct gatt_primary *prim = gatt->prim;
-
-	gatt->attrib = g_attrib_ref(attrib);
-
-	gatt_discover_char(gatt->attrib, prim->range.start, prim->range.end, NULL,
-						char_discovered_cb, qchr);
-}
-
-static void cancel_discover(gpointer user_data)
-{
-	struct query_data *qchr = user_data;
-	struct gatt_service *gatt = qchr->gatt;
-
-	g_attrib_unref(gatt->attrib);
-	gatt->attrib = NULL;
-}
-
 static DBusMessage *discover_char(DBusConnection *conn, DBusMessage *msg,
 								void *data)
 {
@@ -937,10 +929,17 @@ static DBusMessage *discover_char(DBusConnection *conn, DBusMessage *msg,
 
 	query->msg = dbus_message_ref(msg);
 
-	gatt->attioid = btd_device_add_attio_callback(gatt->dev,
-							send_discover,
-							cancel_discover,
-							qchr);
+	if (gatt->attioid == 0) {
+		gatt->attioid = btd_device_add_attio_callback(gatt->dev,
+							attio_connected,
+							attio_disconnected,
+							gatt);
+	} else if (gatt->attrib) {
+		struct gatt_primary *prim = gatt->prim;
+		gatt_discover_char(gatt->attrib, prim->range.start,
+						prim->range.end, NULL,
+						char_discovered_cb, qchr);
+	}
 
 	gatt->query = query;
 
-- 
1.7.5.4

[PATCH v2 BlueZ 3/8] attrib: Move Characteristic Discovery reply to a new function

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

Minor cleanup patch moving the creation of the reply for Discover
All Characteristics call to a new local function in the Generic
Attribute.
---
 attrib/client.c |   44 +++++++++++++++++++++++++++-----------------
 1 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 2ad3956..651eade 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -835,11 +835,35 @@ static void update_all_chars(gpointer data, gpointer user_data)
 	gatt_read_char(gatt->attrib, chr->handle, 0, update_char_value, qvalue);
 }
 
+static DBusMessage *create_discover_char_reply(DBusMessage *msg, GSList *chars)
+{
+	DBusMessage *reply;
+	DBusMessageIter iter, array_iter;
+	GSList *l;
+
+	reply = dbus_message_new_method_return(msg);
+
+	dbus_message_iter_init_append(reply, &iter);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+				DBUS_TYPE_OBJECT_PATH_AS_STRING, &array_iter);
+
+	for (l = chars; l; l = l->next) {
+		struct characteristic *chr = l->data;
+
+		dbus_message_iter_append_basic(&array_iter,
+					DBUS_TYPE_OBJECT_PATH, &chr->path);
+	}
+
+	dbus_message_iter_close_container(&iter, &array_iter);
+
+	return reply;
+}
+
 static void char_discovered_cb(GSList *characteristics, guint8 status,
 							gpointer user_data)
 {
 	DBusMessage *reply;
-	DBusMessageIter iter, array_iter;
 	struct query_data *current = user_data;
 	struct gatt_service *gatt = current->gatt;
 	struct gatt_primary *prim = gatt->prim;
@@ -888,24 +912,10 @@ static void char_discovered_cb(GSList *characteristics, guint8 status,
 
 	g_slist_foreach(gatt->chars, register_characteristic, gatt->path);
 
-	reply = dbus_message_new_method_return(gatt->query->msg);
-
-	dbus_message_iter_init_append(reply, &iter);
-
-	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
-				DBUS_TYPE_OBJECT_PATH_AS_STRING, &array_iter);
-
-	for (l = gatt->chars; l; l = l->next) {
-		struct characteristic *chr = l->data;
-
-		dbus_message_iter_append_basic(&array_iter,
-					DBUS_TYPE_OBJECT_PATH, &chr->path);
-	}
-
-	dbus_message_iter_close_container(&iter, &array_iter);
-
 	g_slist_foreach(gatt->chars, update_all_chars, gatt);
 
+	reply = create_discover_char_reply(gatt->query->msg, gatt->chars);
+
 fail:
 	g_dbus_send_message(gatt->conn, reply);
 	query_list_remove(gatt, current);
-- 
1.7.5.4

[PATCH v2 BlueZ 4/8] attrib: Register only new characteristics found

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

This avoids duplicating registration for previously found
characteristics.
---
 attrib/client.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 651eade..ce78ed6 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -902,6 +902,7 @@ static void char_discovered_cb(GSList *characteristics, guint8 status,
 		previous_end = &chr->end;
 
 		gatt->chars = g_slist_append(gatt->chars, chr);
+		register_characteristic(chr, gatt->path);
 	}
 
 	if (previous_end)
@@ -910,8 +911,6 @@ static void char_discovered_cb(GSList *characteristics, guint8 status,
 	gatt_get_address(gatt, &sba, &dba);
 	store_characteristics(&sba, &dba, prim->range.start, gatt->chars);
 
-	g_slist_foreach(gatt->chars, register_characteristic, gatt->path);
-
 	g_slist_foreach(gatt->chars, update_all_chars, gatt);
 
 	reply = create_discover_char_reply(gatt->query->msg, gatt->chars);
-- 
1.7.5.4

[PATCH v2 BlueZ 5/8] attrib: Fix missing Discovery reply when disconnected

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

This patch fixes missing reply for DiscoverCharacteristics method on
Generic Attribute. If link is disconnected reply message is not sent.
---
 attrib/client.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index ce78ed6..57cd028 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -356,6 +356,20 @@ static void attio_disconnected(gpointer user_data)
 {
 	struct gatt_service *gatt = user_data;
 
+	if (gatt->query) {
+		if (gatt->query->msg) {
+			DBusMessage *reply;
+
+			reply = btd_error_failed(gatt->query->msg,
+						strerror(ECONNRESET));
+			g_dbus_send_message(gatt->conn, reply);
+			dbus_message_unref(gatt->query->msg);
+		}
+
+		g_slist_free_full(gatt->query->list, g_free);
+		gatt->query = NULL;
+	}
+
 	if (gatt->attrib) {
 		g_attrib_unref(gatt->attrib);
 		gatt->attrib = NULL;
-- 
1.7.5.4

Re: [PATCH v2 BlueZ 5/8] attrib: Fix missing Discovery reply when disconnected

[Johan Hedberg]

Hi,

On Tue, Mar 27, 2012, Anderson Lizardo wrote:
> From: Claudio Takahasi <claudio.takahasi@openbossa.org>
> 
> This patch fixes missing reply for DiscoverCharacteristics method on
> Generic Attribute. If link is disconnected reply message is not sent.
> ---
>  attrib/client.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)

I've applied the first four patches but there was something in this one
that bothered me:

> diff --git a/attrib/client.c b/attrib/client.c
> index ce78ed6..57cd028 100644
> --- a/attrib/client.c
> +++ b/attrib/client.c
> @@ -356,6 +356,20 @@ static void attio_disconnected(gpointer user_data)
>  {
>  	struct gatt_service *gatt = user_data;
>  
> +	if (gatt->query) {
> +		if (gatt->query->msg) {
> +			DBusMessage *reply;
> +
> +			reply = btd_error_failed(gatt->query->msg,
> +						strerror(ECONNRESET));

What's the point of picking a POSIX error and using its glibc string
translation here? Why don't you provide your own string instead which
can be much more understandable for someone who reads the logs, e.g.
"Attribute IO got disconnected" or whatever makes more sense.

Johan

[PATCH v2 BlueZ 6/8] attrib: Fix missing D-Bus message unref on DiscoverCharacteristics

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

This patch fixes a missing message unref when DiscoverCharacteristics on
Generic Attribute finishes.
---
 attrib/client.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 57cd028..609f8b9 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -930,6 +930,9 @@ static void char_discovered_cb(GSList *characteristics, guint8 status,
 	reply = create_discover_char_reply(gatt->query->msg, gatt->chars);
 
 fail:
+	dbus_message_unref(gatt->query->msg);
+	gatt->query->msg = NULL;
+
 	g_dbus_send_message(gatt->conn, reply);
 	query_list_remove(gatt, current);
 	g_free(current);
-- 
1.7.5.4

[PATCH v2 BlueZ 7/8] attrib: Attempt to remove attio callback on watcher exit

[Anderson Lizardo]

When registering a watcher, it is attempted to add an attio callback.
Therefore, when the watcher is unregistered or exits, this attio should
be removed if there are no other users.

This is already done when unregistering a watcher, it just missed when
it exits without unregistering.
---
 attrib/client.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 609f8b9..d5b2bc9 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -227,6 +227,7 @@ static void watcher_exit(DBusConnection *conn, void *user_data)
 	DBG("%s watcher %s exited", gatt->path, watcher->name);
 
 	gatt->watchers = g_slist_remove(gatt->watchers, watcher);
+	remove_attio(gatt);
 }
 
 static int characteristic_set_value(struct characteristic *chr,
-- 
1.7.5.4

[PATCH v2 BlueZ 8/8] gattrib: Protect GAttrib when there is a pending write

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

---
 attrib/gattrib.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 8a1e97b..c3a70b9 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -293,14 +293,18 @@ static void destroy_sender(gpointer data)
 	struct _GAttrib *attrib = data;
 
 	attrib->write_watch = 0;
+	g_attrib_unref(attrib);
 }
 
 static void wake_up_sender(struct _GAttrib *attrib)
 {
-	if (attrib->write_watch == 0)
-		attrib->write_watch = g_io_add_watch_full(attrib->io,
-			G_PRIORITY_DEFAULT, G_IO_OUT, can_write_data,
-			attrib, destroy_sender);
+	if (attrib->write_watch != 0)
+		return;
+
+	attrib = g_attrib_ref(attrib);
+	attrib->write_watch = g_io_add_watch_full(attrib->io,
+				G_PRIORITY_DEFAULT, G_IO_OUT,
+				can_write_data, attrib, destroy_sender);
 }
 
 static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
-- 
1.7.5.4

Re: [PATCH v2 BlueZ 8/8] gattrib: Protect GAttrib when there is a pending write

[Johan Hedberg]

Hi,

On Tue, Mar 27, 2012, Anderson Lizardo wrote:
> From: Claudio Takahasi <claudio.takahasi@openbossa.org>
> 
> ---
>  attrib/gattrib.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)

Patches 6 and 7 have also been applied but there was one minor thing
with this one:

> +	if (attrib->write_watch != 0)
> +		return;

Could you please try to be consistent with the rest of the code base and
use > 0 for watch id tests.

So I'll be awaiting v3's of patch 5 and 8.

Johan

Re: [PATCH v2 BlueZ 8/8] gattrib: Protect GAttrib when there is a pending write

[Anderson Lizardo]

Hi Johan,

On Fri, Mar 30, 2012 at 6:25 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> So I'll be awaiting v3's of patch 5 and 8.

Just sent fixed versions. I also did a small refactor to patch 5 to
remove the nested if(). Hope that makes the code clearer.

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

[PATCH v3 BlueZ 0/2] Generic GATT client fixes

[Anderson Lizardo]

Hi,

These are the remaining patches, with fixes from Johan comments.

Claudio Takahasi (2):
  attrib: Fix missing Discovery reply when disconnected
  gattrib: Protect GAttrib when there is a pending write

 attrib/client.c  |   14 ++++++++++++++
 attrib/gattrib.c |   12 ++++++++----
 2 files changed, 22 insertions(+), 4 deletions(-)

-- 
1.7.5.4

[PATCH v3 BlueZ 1/2] attrib: Fix missing Discovery reply when disconnected

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

This patch fixes missing reply for DiscoverCharacteristics method on
Generic Attribute. If link is disconnected reply message is not sent.
---
 attrib/client.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 9efcec9..f1c26c4 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -357,6 +357,20 @@ static void attio_disconnected(gpointer user_data)
 {
 	struct gatt_service *gatt = user_data;
 
+	if (gatt->query && gatt->query->msg) {
+		DBusMessage *reply;
+
+		reply = btd_error_failed(gatt->query->msg,
+					"ATT IO channel was disconnected");
+		g_dbus_send_message(gatt->conn, reply);
+		dbus_message_unref(gatt->query->msg);
+	}
+
+	if (gatt->query) {
+		g_slist_free_full(gatt->query->list, g_free);
+		gatt->query = NULL;
+	}
+
 	if (gatt->attrib) {
 		g_attrib_unref(gatt->attrib);
 		gatt->attrib = NULL;
-- 
1.7.5.4

[PATCH v3 BlueZ 2/2] gattrib: Protect GAttrib when there is a pending write

[Anderson Lizardo]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

---
 attrib/gattrib.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 8a1e97b..769be36 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -293,14 +293,18 @@ static void destroy_sender(gpointer data)
 	struct _GAttrib *attrib = data;
 
 	attrib->write_watch = 0;
+	g_attrib_unref(attrib);
 }
 
 static void wake_up_sender(struct _GAttrib *attrib)
 {
-	if (attrib->write_watch == 0)
-		attrib->write_watch = g_io_add_watch_full(attrib->io,
-			G_PRIORITY_DEFAULT, G_IO_OUT, can_write_data,
-			attrib, destroy_sender);
+	if (attrib->write_watch > 0)
+		return;
+
+	attrib = g_attrib_ref(attrib);
+	attrib->write_watch = g_io_add_watch_full(attrib->io,
+				G_PRIORITY_DEFAULT, G_IO_OUT,
+				can_write_data, attrib, destroy_sender);
 }
 
 static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
-- 
1.7.5.4

Re: [PATCH v3 BlueZ 0/2] Generic GATT client fixes

[Johan Hedberg]

Hi,

On Mon, Apr 02, 2012, Anderson Lizardo wrote:
> These are the remaining patches, with fixes from Johan comments.
> 
> Claudio Takahasi (2):
>   attrib: Fix missing Discovery reply when disconnected
>   gattrib: Protect GAttrib when there is a pending write
> 
>  attrib/client.c  |   14 ++++++++++++++
>  attrib/gattrib.c |   12 ++++++++----
>  2 files changed, 22 insertions(+), 4 deletions(-)

Both patches have been applied. Thanks.

Johan