[RFC BlueZ 00/12] On demand connection management

[RFC BlueZ 00/12] On demand connection management

[Claudio Takahasi]

The following patches implement a mechanism to allow GATT based profiles
to request on demand connections. The ideia is to control connections based
on registered "connection" callbacks.

This patch series replaces partially the initial proposal "[RFC] Attribute
IO Driver". GATT based profiles can use "device" driver, we will extend later
if necessary or create a new "driver" to addresss LSTO, connection intervals
and other connection requirements for GATT based profiles.

Next step is to fix Generic Attribute API and Proximity to use this new
approach.

Claudio Takahasi (12):
  Add ATT connection callback registration
  Add a function to remove ATT connection callback
  Fix crash when create device is cancelled
  Notify the GAttrib instance if already connected
  Add ATT disconnect callback
  Add auto connection based on registered callbacks
  Enable ATT re-connection
  Drop GAttrib ref if callback list is empty
  Try to connect when registering the first callback
  Manage GAttrib refs based on registered callbacks
  Postpone calling connected callback
  Address remote initiated disconnection

 Makefile.am  |    2 +-
 src/attio.h  |   33 +++++++
 src/device.c |  265 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 276 insertions(+), 24 deletions(-)
 create mode 100644 src/attio.h

-- 
1.7.6

[RFC BlueZ 01/12] Add ATT connection callback registration

[Claudio Takahasi]

Function intends to be used by profiles to inform that a connection
is required and the callback that needs to be called when the it is
established. New header is required to avoid non GATT based profiles
including GAttrib header.
---
 Makefile.am  |    2 +-
 src/attio.h  |   29 +++++++++++++++++++++++++++++
 src/device.c |   30 +++++++++++++++++++++++++++++-
 3 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100644 src/attio.h

diff --git a/Makefile.am b/Makefile.am
index fab05eb..3b0dff9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -257,7 +257,7 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
 			src/error.h src/error.c \
 			src/manager.h src/manager.c \
 			src/adapter.h src/adapter.c \
-			src/device.h src/device.c \
+			src/device.h src/device.c src/attio.h \
 			src/dbus-common.c src/dbus-common.h \
 			src/event.h src/event.c \
 			src/oob.h src/oob.c src/eir.h src/eir.c
diff --git a/src/attio.h b/src/attio.h
new file mode 100644
index 0000000..fad8516
--- /dev/null
+++ b/src/attio.h
@@ -0,0 +1,29 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2011  Nokia Corporation
+ *  Copyright (C) 2011  Marcel Holtmann <marcel@holtmann.org>
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+typedef void (*attio_connect_cb) (GAttrib *attrib, gpointer user_data);
+
+guint btd_device_add_attio_callback(struct btd_device *device,
+						attio_connect_cb func,
+						gpointer user_data);
diff --git a/src/device.c b/src/device.c
index 82759a4..b0d9c30 100644
--- a/src/device.c
+++ b/src/device.c
@@ -49,12 +49,13 @@
 #include "att.h"
 #include "hcid.h"
 #include "adapter.h"
+#include "gattrib.h"
+#include "attio.h"
 #include "device.h"
 #include "dbus-common.h"
 #include "event.h"
 #include "error.h"
 #include "glib-helper.h"
-#include "gattrib.h"
 #include "gatt.h"
 #include "agent.h"
 #include "sdp-xml.h"
@@ -104,6 +105,12 @@ struct browse_req {
 	guint listener_id;
 };
 
+struct attio_data {
+	guint id;
+	attio_connect_cb func;
+	gpointer user_data;
+};
+
 struct btd_device {
 	bdaddr_t	bdaddr;
 	device_type_t	type;
@@ -124,6 +131,7 @@ struct btd_device {
 	struct bonding_req *bonding;
 	struct authentication_req *authr;	/* authentication request */
 	GSList		*disconnects;		/* disconnects message */
+	GSList		*attios;
 
 	gboolean	connected;
 
@@ -200,6 +208,7 @@ static void device_free(gpointer user_data)
 	g_slist_free_full(device->services, g_free);
 	g_slist_free_full(device->uuids, g_free);
 	g_slist_free_full(device->primaries, g_free);
+	g_slist_free_full(device->attios, g_free);
 
 	if (device->tmp_records)
 		sdp_list_free(device->tmp_records,
@@ -2441,3 +2450,22 @@ void device_set_class(struct btd_device *device, uint32_t value)
 	emit_property_changed(conn, device->path, DEVICE_INTERFACE, "Class",
 				DBUS_TYPE_UINT32, &value);
 }
+
+guint btd_device_add_attio_callback(struct btd_device *device,
+						attio_connect_cb func,
+						gpointer user_data)
+{
+	struct attio_data *attio;
+	static guint attio_id = 0;
+
+	DBG("%p registered ATT connection callback", device);
+
+	attio = g_new0(struct attio_data, 1);
+	attio->id = ++attio_id;
+	attio->func = func;
+	attio->user_data = user_data;
+
+	device->attios = g_slist_append(device->attios, attio);
+
+	return attio->id;
+}
-- 
1.7.6

[RFC BlueZ 02/12] Add a function to remove ATT connection callback

[Claudio Takahasi]

---
 src/attio.h  |    2 ++
 src/device.c |   27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/src/attio.h b/src/attio.h
index fad8516..7935dcd 100644
--- a/src/attio.h
+++ b/src/attio.h
@@ -27,3 +27,5 @@ typedef void (*attio_connect_cb) (GAttrib *attrib, gpointer user_data);
 guint btd_device_add_attio_callback(struct btd_device *device,
 						attio_connect_cb func,
 						gpointer user_data);
+
+gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id);
diff --git a/src/device.c b/src/device.c
index b0d9c30..f22a7fa 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2469,3 +2469,30 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 
 	return attio->id;
 }
+
+static int attio_id_cmp(gconstpointer a, gconstpointer b)
+{
+	const struct attio_data *attio = a;
+	guint id = GPOINTER_TO_UINT(b);
+
+	return attio->id - id;
+}
+
+gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
+{
+	struct attio_data *attio;
+	GSList *l;
+
+	l = g_slist_find_custom(device->attios, GUINT_TO_POINTER(id),
+								attio_id_cmp);
+	if (!l)
+		return FALSE;
+
+	attio = l->data;
+
+	device->attios = g_slist_remove(device->attios, attio);
+
+	g_free(attio);
+
+	return TRUE;
+}
-- 
1.7.6

[RFC BlueZ 03/12] Fix crash when create device is cancelled

[Claudio Takahasi]

If the owner leaves the bus or cancels the device creation process
the connection callback is called causing invalid memory access.
---
 src/device.c |   28 ++++++++++++++++++----------
 1 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/device.c b/src/device.c
index f22a7fa..b14e6e7 100644
--- a/src/device.c
+++ b/src/device.c
@@ -95,6 +95,7 @@ struct browse_req {
 	DBusConnection *conn;
 	DBusMessage *msg;
 	GAttrib *attrib;
+	GIOChannel *io;
 	struct btd_device *device;
 	GSList *match_uuids;
 	GSList *profiles_added;
@@ -154,10 +155,15 @@ static uint16_t uuid_list[] = {
 
 static GSList *device_drivers = NULL;
 
-static void browse_request_free(struct browse_req *req)
+static void browse_request_free(struct browse_req *req, gboolean shutdown)
 {
 	if (req->listener_id)
 		g_dbus_remove_watch(req->conn, req->listener_id);
+	if (req->io) {
+		if (shutdown)
+			g_io_channel_shutdown(req->io, FALSE, NULL);
+		g_io_channel_unref(req->io);
+	}
 	if (req->msg)
 		dbus_message_unref(req->msg);
 	if (req->conn)
@@ -189,7 +195,7 @@ static void browse_request_cancel(struct browse_req *req)
 	bt_cancel_discovery(&src, &device->bdaddr);
 
 	device->browse = NULL;
-	browse_request_free(req);
+	browse_request_free(req, TRUE);
 }
 
 static void device_free(gpointer user_data)
@@ -1493,7 +1499,7 @@ cleanup:
 	}
 
 	device->browse = NULL;
-	browse_request_free(req);
+	browse_request_free(req, FALSE);
 }
 
 static void browse_cb(sdp_list_t *recs, int err, gpointer user_data)
@@ -1588,11 +1594,13 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 	struct browse_req *req = user_data;
 	struct btd_device *device = req->device;
 	GSList *l, *uuids = NULL;
+	gboolean shutdown;
 
 	if (status) {
 		DBusMessage *reply;
 		reply = btd_error_failed(req->msg, att_ecode2str(status));
 		g_dbus_send_message(req->conn, reply);
+		shutdown = TRUE;
 		goto done;
 	}
 
@@ -1613,10 +1621,11 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 	create_device_reply(device, req);
 
 	store_services(device);
+	shutdown = FALSE;
 
 done:
 	device->browse = NULL;
-	browse_request_free(req);
+	browse_request_free(req, shutdown);
 }
 
 static void gatt_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
@@ -1633,7 +1642,7 @@ static void gatt_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 		g_dbus_send_message(req->conn, reply);
 
 		device->browse = NULL;
-		browse_request_free(req);
+		browse_request_free(req, TRUE);
 
 		return;
 	}
@@ -1650,7 +1659,6 @@ int device_browse_primary(struct btd_device *device, DBusConnection *conn,
 	struct btd_adapter *adapter = device->adapter;
 	struct browse_req *req;
 	BtIOSecLevel sec_level;
-	GIOChannel *io;
 	bdaddr_t src;
 
 	if (device->browse)
@@ -1663,15 +1671,15 @@ int device_browse_primary(struct btd_device *device, DBusConnection *conn,
 
 	sec_level = secure ? BT_IO_SEC_HIGH : BT_IO_SEC_LOW;
 
-	io = bt_io_connect(BT_IO_L2CAP, gatt_connect_cb, req, NULL, NULL,
+	req->io = bt_io_connect(BT_IO_L2CAP, gatt_connect_cb, req, NULL, NULL,
 				BT_IO_OPT_SOURCE_BDADDR, &src,
 				BT_IO_OPT_DEST_BDADDR, &device->bdaddr,
 				BT_IO_OPT_CID, ATT_CID,
 				BT_IO_OPT_SEC_LEVEL, sec_level,
 				BT_IO_OPT_INVALID);
 
-	if (io == NULL ) {
-		browse_request_free(req);
+	if (req->io == NULL) {
+		browse_request_free(req, FALSE);
 		return -EIO;
 	}
 
@@ -1724,7 +1732,7 @@ int device_browse_sdp(struct btd_device *device, DBusConnection *conn,
 
 	err = bt_search_service(&src, &device->bdaddr, &uuid, cb, req, NULL);
 	if (err < 0) {
-		browse_request_free(req);
+		browse_request_free(req, FALSE);
 		return err;
 	}
 
-- 
1.7.6

[RFC BlueZ 04/12] Notify the GAttrib instance if already connected

[Claudio Takahasi]

When registering the connect and disconnect callbacks for ATT, GAttrib
instance should be informed to the caller if the link is up.
---
 src/device.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/src/device.c b/src/device.c
index b14e6e7..eeeb25f 100644
--- a/src/device.c
+++ b/src/device.c
@@ -132,6 +132,7 @@ struct btd_device {
 	struct bonding_req *bonding;
 	struct authentication_req *authr;	/* authentication request */
 	GSList		*disconnects;		/* disconnects message */
+	GAttrib		*attrib;
 	GSList		*attios;
 
 	gboolean	connected;
@@ -2475,6 +2476,9 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 
 	device->attios = g_slist_append(device->attios, attio);
 
+	if (device->attrib && func)
+		func(device->attrib, user_data);
+
 	return attio->id;
 }
 
-- 
1.7.6

[RFC BlueZ 05/12] Add ATT disconnect callback

[Claudio Takahasi]

---
 src/attio.h  |    4 +++-
 src/device.c |   36 +++++++++++++++++++++++++++++++-----
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/src/attio.h b/src/attio.h
index 7935dcd..16e2873 100644
--- a/src/attio.h
+++ b/src/attio.h
@@ -23,9 +23,11 @@
  */
 
 typedef void (*attio_connect_cb) (GAttrib *attrib, gpointer user_data);
+typedef void (*attio_disconnect_cb) (gpointer user_data);
 
 guint btd_device_add_attio_callback(struct btd_device *device,
-						attio_connect_cb func,
+						attio_connect_cb cfunc,
+						attio_disconnect_cb dcfunc,
 						gpointer user_data);
 
 gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id);
diff --git a/src/device.c b/src/device.c
index eeeb25f..20248ed 100644
--- a/src/device.c
+++ b/src/device.c
@@ -108,7 +108,8 @@ struct browse_req {
 
 struct attio_data {
 	guint id;
-	attio_connect_cb func;
+	attio_connect_cb cfunc;
+	attio_disconnect_cb dcfunc;
 	gpointer user_data;
 };
 
@@ -1590,6 +1591,23 @@ static void store_services(struct btd_device *device)
 	g_free(str);
 }
 
+static void attio_disconnected(gpointer data, gpointer user_data)
+{
+	struct attio_data *attio = data;
+
+	if (attio->dcfunc)
+		attio->dcfunc(attio->user_data);
+}
+
+static void attrib_destroyed(gpointer user_data)
+{
+	struct btd_device *device = user_data;
+
+	device->attrib = NULL;
+
+	g_slist_foreach(device->attios, attio_disconnected, NULL);
+}
+
 static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 {
 	struct browse_req *req = user_data;
@@ -1619,6 +1637,12 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 
 	g_slist_free(uuids);
 
+	if (device->attios) {
+		device->attrib = g_attrib_ref(req->attrib);
+		g_attrib_set_destroy_function(device->attrib, attrib_destroyed,
+								device);
+	}
+
 	create_device_reply(device, req);
 
 	store_services(device);
@@ -2461,7 +2485,8 @@ void device_set_class(struct btd_device *device, uint32_t value)
 }
 
 guint btd_device_add_attio_callback(struct btd_device *device,
-						attio_connect_cb func,
+						attio_connect_cb cfunc,
+						attio_disconnect_cb dcfunc,
 						gpointer user_data)
 {
 	struct attio_data *attio;
@@ -2471,13 +2496,14 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 
 	attio = g_new0(struct attio_data, 1);
 	attio->id = ++attio_id;
-	attio->func = func;
+	attio->cfunc = cfunc;
+	attio->dcfunc = dcfunc;
 	attio->user_data = user_data;
 
 	device->attios = g_slist_append(device->attios, attio);
 
-	if (device->attrib && func)
-		func(device->attrib, user_data);
+	if (device->attrib && cfunc)
+		cfunc(device->attrib, user_data);
 
 	return attio->id;
 }
-- 
1.7.6

[RFC BlueZ 06/12] Add auto connection based on registered callbacks

[Claudio Takahasi]

This initial approach uses a hardcoded timer(45s). Passive scanning
or any other mechanism to avoid unnecessary connection attempt is not
supported at the moment. For LE, device discovery needs to be executed
to update the kernel advertising cache to allow re-connections.
---
 src/device.c |  106 +++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 86 insertions(+), 20 deletions(-)

diff --git a/src/device.c b/src/device.c
index 20248ed..7543a80 100644
--- a/src/device.c
+++ b/src/device.c
@@ -65,6 +65,7 @@
 
 #define DISCONNECT_TIMER	2
 #define DISCOVERY_TIMER		2
+#define AUTOCONNECT_INTERVAL	45
 
 /* When all services should trust a remote device */
 #define GLOBAL_TRUST "[all]"
@@ -94,7 +95,6 @@ struct authentication_req {
 struct browse_req {
 	DBusConnection *conn;
 	DBusMessage *msg;
-	GAttrib *attrib;
 	GIOChannel *io;
 	struct btd_device *device;
 	GSList *match_uuids;
@@ -135,6 +135,7 @@ struct btd_device {
 	GSList		*disconnects;		/* disconnects message */
 	GAttrib		*attrib;
 	GSList		*attios;
+	guint		attioid;
 
 	gboolean	connected;
 
@@ -177,9 +178,6 @@ static void browse_request_free(struct browse_req *req, gboolean shutdown)
 	if (req->records)
 		sdp_list_free(req->records, (sdp_free_func_t) sdp_record_free);
 
-	if (req->attrib)
-		g_attrib_unref(req->attrib);
-
 	g_free(req);
 }
 
@@ -218,6 +216,8 @@ static void device_free(gpointer user_data)
 	g_slist_free_full(device->primaries, g_free);
 	g_slist_free_full(device->attios, g_free);
 
+	g_attrib_unref(device->attrib);
+
 	if (device->tmp_records)
 		sdp_list_free(device->tmp_records,
 					(sdp_free_func_t) sdp_record_free);
@@ -228,6 +228,9 @@ static void device_free(gpointer user_data)
 	if (device->discov_timer)
 		g_source_remove(device->discov_timer);
 
+	if (device->attioid)
+		g_source_remove(device->attioid);
+
 	DBG("%p", device);
 
 	g_free(device->authr);
@@ -1591,6 +1594,15 @@ static void store_services(struct btd_device *device)
 	g_free(str);
 }
 
+static void attio_connected(gpointer data, gpointer user_data)
+{
+	struct attio_data *attio = data;
+	GAttrib *attrib = user_data;
+
+	if (attio->cfunc)
+		attio->cfunc(attrib, attio->user_data);
+}
+
 static void attio_disconnected(gpointer data, gpointer user_data)
 {
 	struct attio_data *attio = data;
@@ -1637,12 +1649,6 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 
 	g_slist_free(uuids);
 
-	if (device->attios) {
-		device->attrib = g_attrib_ref(req->attrib);
-		g_attrib_set_destroy_function(device->attrib, attrib_destroyed,
-								device);
-	}
-
 	create_device_reply(device, req);
 
 	store_services(device);
@@ -1653,29 +1659,78 @@ done:
 	browse_request_free(req, shutdown);
 }
 
-static void gatt_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
+static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 {
-	struct browse_req *req = user_data;
-	struct btd_device *device = req->device;
+	struct btd_device *device = user_data;
+	struct browse_req *req = device->browse;
 
 	if (gerr) {
 		DBusMessage *reply;
 
 		DBG("%s", gerr->message);
 
-		reply = btd_error_failed(req->msg, gerr->message);
-		g_dbus_send_message(req->conn, reply);
+		if (req) {
+			reply = btd_error_failed(req->msg, gerr->message);
+			g_dbus_send_message(req->conn, reply);
 
-		device->browse = NULL;
-		browse_request_free(req, TRUE);
+			device->browse = NULL;
+			browse_request_free(req, TRUE);
+		}
 
 		return;
 	}
 
-	req->attrib = g_attrib_new(io);
+	device->attrib = g_attrib_new(io);
+	g_attrib_set_destroy_function(device->attrib, attrib_destroyed, device);
+
+	if (req)
+		gatt_discover_primary(device->attrib, NULL, primary_cb, req);
+	else if (device->attios)
+		g_slist_foreach(device->attios, attio_connected,
+							device->attrib);
+
 	g_io_channel_unref(io);
+}
+
+static gboolean att_auto_connect(gpointer user_data)
+{
+	struct btd_device *device = user_data;
+	struct btd_adapter *adapter = device->adapter;
+	GIOChannel *io;
+	GError *gerr = NULL;
+	char addr[18];
+	bdaddr_t sba;
+
+	adapter_get_address(adapter, &sba);
+	ba2str(&device->bdaddr, addr);
+
+	DBG("Connection attempt to: %s", addr);
+
+	if (device->type != DEVICE_TYPE_LE) {
+		io = bt_io_connect(BT_IO_L2CAP, att_connect_cb,
+					device, NULL, &gerr,
+					BT_IO_OPT_SOURCE_BDADDR, &sba,
+					BT_IO_OPT_DEST_BDADDR, &device->bdaddr,
+					BT_IO_OPT_PSM, ATT_PSM,
+					BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+					BT_IO_OPT_INVALID);
+	} else {
+		io = bt_io_connect(BT_IO_L2CAP, att_connect_cb,
+					device, NULL, &gerr,
+					BT_IO_OPT_SOURCE_BDADDR, &sba,
+					BT_IO_OPT_DEST_BDADDR, &device->bdaddr,
+					BT_IO_OPT_CID, ATT_CID,
+					BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+					BT_IO_OPT_INVALID);
+	}
+
+	if (io == NULL) {
+		error("ATT bt_io_connect(%s): %s", addr, gerr->message);
+		g_error_free(gerr);
+		return TRUE;
+	}
 
-	gatt_discover_primary(req->attrib, NULL, primary_cb, req);
+	return TRUE;
 }
 
 int device_browse_primary(struct btd_device *device, DBusConnection *conn,
@@ -1696,7 +1751,8 @@ int device_browse_primary(struct btd_device *device, DBusConnection *conn,
 
 	sec_level = secure ? BT_IO_SEC_HIGH : BT_IO_SEC_LOW;
 
-	req->io = bt_io_connect(BT_IO_L2CAP, gatt_connect_cb, req, NULL, NULL,
+	req->io = bt_io_connect(BT_IO_L2CAP, att_connect_cb,
+				device, NULL, NULL,
 				BT_IO_OPT_SOURCE_BDADDR, &src,
 				BT_IO_OPT_DEST_BDADDR, &device->bdaddr,
 				BT_IO_OPT_CID, ATT_CID,
@@ -2505,6 +2561,11 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 	if (device->attrib && cfunc)
 		cfunc(device->attrib, user_data);
 
+	if (device->attioid == 0)
+		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
+							att_auto_connect,
+							device);
+
 	return attio->id;
 }
 
@@ -2532,5 +2593,10 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
 
 	g_free(attio);
 
+	if (device->attioid) {
+		g_source_remove(device->attioid);
+		device->attioid = 0;
+	}
+
 	return TRUE;
 }
-- 
1.7.6

[RFC BlueZ 07/12] Enable ATT re-connection

[Claudio Takahasi]

If the link is lost and there is at least one connection callback
registered the timer for ATT automatic connection shall be enabled.
---
 src/device.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/src/device.c b/src/device.c
index 7543a80..2185b77 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1611,6 +1611,8 @@ static void attio_disconnected(gpointer data, gpointer user_data)
 		attio->dcfunc(attio->user_data);
 }
 
+static gboolean att_auto_connect(gpointer user_data);
+
 static void attrib_destroyed(gpointer user_data)
 {
 	struct btd_device *device = user_data;
@@ -1618,6 +1620,11 @@ static void attrib_destroyed(gpointer user_data)
 	device->attrib = NULL;
 
 	g_slist_foreach(device->attios, attio_disconnected, NULL);
+
+	if (device->attioid == 0 && device->attios != NULL)
+		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
+							att_auto_connect,
+							device);
 }
 
 static void primary_cb(GSList *services, guint8 status, gpointer user_data)
@@ -1680,6 +1687,11 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 		return;
 	}
 
+	if (device->attioid) {
+		g_source_remove(device->attioid);
+		device->attioid = 0;
+	}
+
 	device->attrib = g_attrib_new(io);
 	g_attrib_set_destroy_function(device->attrib, attrib_destroyed, device);
 
@@ -2561,7 +2573,7 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 	if (device->attrib && cfunc)
 		cfunc(device->attrib, user_data);
 
-	if (device->attioid == 0)
+	if (device->attioid == 0 && device->attrib == NULL)
 		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
 							att_auto_connect,
 							device);
-- 
1.7.6

[RFC BlueZ 08/12] Drop GAttrib ref if callback list is empty

[Claudio Takahasi]

If all ATT connection callbacks are unregistered, GAttrib reference
can be dropped.
---
 src/device.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/src/device.c b/src/device.c
index 2185b77..3d092e2 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2605,10 +2605,15 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
 
 	g_free(attio);
 
+	if (device->attios != NULL)
+		return TRUE;
+
 	if (device->attioid) {
 		g_source_remove(device->attioid);
 		device->attioid = 0;
 	}
 
+	g_attrib_unref(device->attrib);
+
 	return TRUE;
 }
-- 
1.7.6

[RFC BlueZ 09/12] Try to connect when registering the first callback

[Claudio Takahasi]

If the device is disconnected and the first ATT connection callback is
registered, the first connection attempt can be triggered instead of
waiting the timer.
---
 src/device.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/src/device.c b/src/device.c
index 3d092e2..184252d 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2573,10 +2573,12 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 	if (device->attrib && cfunc)
 		cfunc(device->attrib, user_data);
 
-	if (device->attioid == 0 && device->attrib == NULL)
+	if (device->attioid == 0 && device->attrib == NULL) {
+		att_auto_connect(device);
 		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
 							att_auto_connect,
 							device);
+	}
 
 	return attio->id;
 }
-- 
1.7.6

[RFC BlueZ 10/12] Manage GAttrib refs based on registered callbacks

[Claudio Takahasi]

Connection should not be dropped if there is at least one ATT connection
callback registered.
---
 src/device.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/device.c b/src/device.c
index 184252d..fa0efc0 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1664,6 +1664,7 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 done:
 	device->browse = NULL;
 	browse_request_free(req, shutdown);
+	g_attrib_unref(device->attrib);
 }
 
 static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
@@ -1700,8 +1701,6 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 	else if (device->attios)
 		g_slist_foreach(device->attios, attio_connected,
 							device->attrib);
-
-	g_io_channel_unref(io);
 }
 
 static gboolean att_auto_connect(gpointer user_data)
@@ -1742,6 +1741,8 @@ static gboolean att_auto_connect(gpointer user_data)
 		return TRUE;
 	}
 
+	g_io_channel_unref(io);
+
 	return TRUE;
 }
 
@@ -2568,18 +2569,22 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 	attio->dcfunc = dcfunc;
 	attio->user_data = user_data;
 
-	device->attios = g_slist_append(device->attios, attio);
-
-	if (device->attrib && cfunc)
-		cfunc(device->attrib, user_data);
+	if (device->attrib) {
+		/* First element */
+		if (device->attios == NULL)
+			device->attrib = g_attrib_ref(device->attrib);
 
-	if (device->attioid == 0 && device->attrib == NULL) {
+		if (cfunc)
+			cfunc(device->attrib, user_data);
+	} else if (device->attioid == 0) {
 		att_auto_connect(device);
 		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
 							att_auto_connect,
 							device);
 	}
 
+	device->attios = g_slist_append(device->attios, attio);
+
 	return attio->id;
 }
 
-- 
1.7.6

[RFC BlueZ 11/12] Postpone calling connected callback

[Claudio Takahasi]

In the ATT callbacks registration function, the connection callback should
not be called in the same main loop iteraction to avoid accessing not
initialized data in the profiles during probing.
---
 src/device.c |   67 ++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/src/device.c b/src/device.c
index fa0efc0..52462be 100644
--- a/src/device.c
+++ b/src/device.c
@@ -134,7 +134,8 @@ struct btd_device {
 	struct authentication_req *authr;	/* authentication request */
 	GSList		*disconnects;		/* disconnects message */
 	GAttrib		*attrib;
-	GSList		*attios;
+	GSList		*attios_head;
+	GSList		*attios_tail;
 	guint		attioid;
 
 	gboolean	connected;
@@ -214,7 +215,8 @@ static void device_free(gpointer user_data)
 	g_slist_free_full(device->services, g_free);
 	g_slist_free_full(device->uuids, g_free);
 	g_slist_free_full(device->primaries, g_free);
-	g_slist_free_full(device->attios, g_free);
+	g_slist_free_full(device->attios_head, g_free);
+	g_slist_free_full(device->attios_tail, g_free);
 
 	g_attrib_unref(device->attrib);
 
@@ -1617,14 +1619,15 @@ static void attrib_destroyed(gpointer user_data)
 {
 	struct btd_device *device = user_data;
 
-	device->attrib = NULL;
-
-	g_slist_foreach(device->attios, attio_disconnected, NULL);
+	g_slist_foreach(device->attios_head, attio_disconnected, NULL);
 
-	if (device->attioid == 0 && device->attios != NULL)
+	if ((device->attios_head || device->attios_tail) &&
+						device->attioid == 0)
 		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
 							att_auto_connect,
 							device);
+
+	device->attrib = NULL;
 }
 
 static void primary_cb(GSList *services, guint8 status, gpointer user_data)
@@ -1698,8 +1701,8 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 
 	if (req)
 		gatt_discover_primary(device->attrib, NULL, primary_cb, req);
-	else if (device->attios)
-		g_slist_foreach(device->attios, attio_connected,
+	else if (device->attios_head)
+		g_slist_foreach(device->attios_head, attio_connected,
 							device->attrib);
 }
 
@@ -2553,6 +2556,21 @@ void device_set_class(struct btd_device *device, uint32_t value)
 				DBUS_TYPE_UINT32, &value);
 }
 
+static gboolean notify_attios(gpointer user_data)
+{
+	struct btd_device *device = user_data;
+
+	if (device->attrib == NULL)
+		return FALSE;
+
+	g_slist_foreach(device->attios_tail, attio_connected, device->attrib);
+	device->attios_head = g_slist_concat(device->attios_head,
+							device->attios_tail);
+	device->attios_tail = NULL;
+
+	return FALSE;
+}
+
 guint btd_device_add_attio_callback(struct btd_device *device,
 						attio_connect_cb cfunc,
 						attio_disconnect_cb dcfunc,
@@ -2571,20 +2589,22 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 
 	if (device->attrib) {
 		/* First element */
-		if (device->attios == NULL)
+		if (device->attios_head == NULL && device->attios_tail == NULL)
 			device->attrib = g_attrib_ref(device->attrib);
 
-		if (cfunc)
-			cfunc(device->attrib, user_data);
+		device->attios_tail = g_slist_append(device->attios_tail,
+									attio);
+		g_idle_add(notify_attios, device);
+
 	} else if (device->attioid == 0) {
 		att_auto_connect(device);
 		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
 							att_auto_connect,
 							device);
+		device->attios_head = g_slist_append(device->attios_head,
+									attio);
 	}
 
-	device->attios = g_slist_append(device->attios, attio);
-
 	return attio->id;
 }
 
@@ -2601,18 +2621,25 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
 	struct attio_data *attio;
 	GSList *l;
 
-	l = g_slist_find_custom(device->attios, GUINT_TO_POINTER(id),
+	l = g_slist_find_custom(device->attios_head, GUINT_TO_POINTER(id),
 								attio_id_cmp);
-	if (!l)
-		return FALSE;
-
-	attio = l->data;
+	if (l) {
+		attio = l->data;
+		device->attios_head = g_slist_remove(device->attios_head,
+									attio);
+	} else {
+		l = g_slist_find_custom(device->attios_tail,
+					GUINT_TO_POINTER(id), attio_id_cmp);
+		if (!l)
+			return FALSE;
 
-	device->attios = g_slist_remove(device->attios, attio);
+		attio = l->data;
+		device->attios_tail = g_slist_remove(device->attios_tail, attio);
+	}
 
 	g_free(attio);
 
-	if (device->attios != NULL)
+	if (device->attios_head != NULL || device->attios_tail != NULL)
 		return TRUE;
 
 	if (device->attioid) {
-- 
1.7.6

[RFC BlueZ 12/12] Address remote initiated disconnection

[Claudio Takahasi]

Local initiated disconnection will be triggered when the last GAttrib
reference is dropped. For remote initiated disconnection it is necessary
to track the socket HUP registering disconnection GAttrib callback.
---
 src/device.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/src/device.c b/src/device.c
index 52462be..90761d4 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1615,7 +1615,7 @@ static void attio_disconnected(gpointer data, gpointer user_data)
 
 static gboolean att_auto_connect(gpointer user_data);
 
-static void attrib_destroyed(gpointer user_data)
+static void attrib_disconnected(gpointer user_data)
 {
 	struct btd_device *device = user_data;
 
@@ -1627,6 +1627,13 @@ static void attrib_destroyed(gpointer user_data)
 							att_auto_connect,
 							device);
 
+	g_attrib_unref(device->attrib);
+}
+
+static void attrib_destroyed(gpointer user_data)
+{
+	struct btd_device *device = user_data;
+
 	device->attrib = NULL;
 }
 
@@ -1697,6 +1704,8 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 	}
 
 	device->attrib = g_attrib_new(io);
+	g_attrib_set_disconnect_function(device->attrib, attrib_disconnected,
+								device);
 	g_attrib_set_destroy_function(device->attrib, attrib_destroyed, device);
 
 	if (req)
-- 
1.7.6

[PATCH BlueZ 00/12] On demand connection management

[Claudio Takahasi]

The following patches implement a mechanism to allow GATT based profiles
to request on demand connections. The ideia is to control connections based
on registered "connection" callbacks.

This patch series replaces partially the initial proposal "[RFC] Attribute
IO Driver". GATT based profiles can use "device" driver, we will extend later
if necessary or create a new "driver" to addresss LSTO, connection intervals
and other connection requirements for GATT based profiles.

No changes from the RFC.

Links for curious developers:
[1] git://git.infradead.org/users/cktakahasi/bluez.git generic-api (using on demand connection)
[2] git://git.infradead.org/users/cktakahasi/bluez.git proximity-devel (under development)

Claudio Takahasi (12):
  Add ATT connection callback registration
  Add a function to remove ATT connection callback
  Fix crash when create device is cancelled
  Notify the GAttrib instance if already connected
  Add ATT disconnect callback
  Add auto connection based on registered callbacks
  Enable ATT re-connection
  Drop GAttrib ref if callback list is empty
  Try to connect when registering the first callback
  Manage GAttrib refs based on registered callbacks
  Postpone calling connected callback
  Address remote initiated disconnection

 Makefile.am  |    2 +-
 src/attio.h  |   33 +++++++
 src/device.c |  265 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 276 insertions(+), 24 deletions(-)
 create mode 100644 src/attio.h

-- 
1.7.6

[PATCH BlueZ 01/12] Add ATT connection callback registration

[Claudio Takahasi]

Function intends to be used by profiles to inform that a connection
is required and the callback that needs to be called when the it is
established. New header is required to avoid non GATT based profiles
including GAttrib header.
---
 Makefile.am  |    2 +-
 src/attio.h  |   29 +++++++++++++++++++++++++++++
 src/device.c |   30 +++++++++++++++++++++++++++++-
 3 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100644 src/attio.h

diff --git a/Makefile.am b/Makefile.am
index fab05eb..3b0dff9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -257,7 +257,7 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
 			src/error.h src/error.c \
 			src/manager.h src/manager.c \
 			src/adapter.h src/adapter.c \
-			src/device.h src/device.c \
+			src/device.h src/device.c src/attio.h \
 			src/dbus-common.c src/dbus-common.h \
 			src/event.h src/event.c \
 			src/oob.h src/oob.c src/eir.h src/eir.c
diff --git a/src/attio.h b/src/attio.h
new file mode 100644
index 0000000..fad8516
--- /dev/null
+++ b/src/attio.h
@@ -0,0 +1,29 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2011  Nokia Corporation
+ *  Copyright (C) 2011  Marcel Holtmann <marcel@holtmann.org>
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+typedef void (*attio_connect_cb) (GAttrib *attrib, gpointer user_data);
+
+guint btd_device_add_attio_callback(struct btd_device *device,
+						attio_connect_cb func,
+						gpointer user_data);
diff --git a/src/device.c b/src/device.c
index 82759a4..b0d9c30 100644
--- a/src/device.c
+++ b/src/device.c
@@ -49,12 +49,13 @@
 #include "att.h"
 #include "hcid.h"
 #include "adapter.h"
+#include "gattrib.h"
+#include "attio.h"
 #include "device.h"
 #include "dbus-common.h"
 #include "event.h"
 #include "error.h"
 #include "glib-helper.h"
-#include "gattrib.h"
 #include "gatt.h"
 #include "agent.h"
 #include "sdp-xml.h"
@@ -104,6 +105,12 @@ struct browse_req {
 	guint listener_id;
 };
 
+struct attio_data {
+	guint id;
+	attio_connect_cb func;
+	gpointer user_data;
+};
+
 struct btd_device {
 	bdaddr_t	bdaddr;
 	device_type_t	type;
@@ -124,6 +131,7 @@ struct btd_device {
 	struct bonding_req *bonding;
 	struct authentication_req *authr;	/* authentication request */
 	GSList		*disconnects;		/* disconnects message */
+	GSList		*attios;
 
 	gboolean	connected;
 
@@ -200,6 +208,7 @@ static void device_free(gpointer user_data)
 	g_slist_free_full(device->services, g_free);
 	g_slist_free_full(device->uuids, g_free);
 	g_slist_free_full(device->primaries, g_free);
+	g_slist_free_full(device->attios, g_free);
 
 	if (device->tmp_records)
 		sdp_list_free(device->tmp_records,
@@ -2441,3 +2450,22 @@ void device_set_class(struct btd_device *device, uint32_t value)
 	emit_property_changed(conn, device->path, DEVICE_INTERFACE, "Class",
 				DBUS_TYPE_UINT32, &value);
 }
+
+guint btd_device_add_attio_callback(struct btd_device *device,
+						attio_connect_cb func,
+						gpointer user_data)
+{
+	struct attio_data *attio;
+	static guint attio_id = 0;
+
+	DBG("%p registered ATT connection callback", device);
+
+	attio = g_new0(struct attio_data, 1);
+	attio->id = ++attio_id;
+	attio->func = func;
+	attio->user_data = user_data;
+
+	device->attios = g_slist_append(device->attios, attio);
+
+	return attio->id;
+}
-- 
1.7.6

[PATCH BlueZ 02/12] Add a function to remove ATT connection callback

[Claudio Takahasi]

---
 src/attio.h  |    2 ++
 src/device.c |   27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/src/attio.h b/src/attio.h
index fad8516..7935dcd 100644
--- a/src/attio.h
+++ b/src/attio.h
@@ -27,3 +27,5 @@ typedef void (*attio_connect_cb) (GAttrib *attrib, gpointer user_data);
 guint btd_device_add_attio_callback(struct btd_device *device,
 						attio_connect_cb func,
 						gpointer user_data);
+
+gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id);
diff --git a/src/device.c b/src/device.c
index b0d9c30..f22a7fa 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2469,3 +2469,30 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 
 	return attio->id;
 }
+
+static int attio_id_cmp(gconstpointer a, gconstpointer b)
+{
+	const struct attio_data *attio = a;
+	guint id = GPOINTER_TO_UINT(b);
+
+	return attio->id - id;
+}
+
+gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
+{
+	struct attio_data *attio;
+	GSList *l;
+
+	l = g_slist_find_custom(device->attios, GUINT_TO_POINTER(id),
+								attio_id_cmp);
+	if (!l)
+		return FALSE;
+
+	attio = l->data;
+
+	device->attios = g_slist_remove(device->attios, attio);
+
+	g_free(attio);
+
+	return TRUE;
+}
-- 
1.7.6

[PATCH BlueZ 03/12] Fix crash when create device is cancelled

[Claudio Takahasi]

If the owner leaves the bus or cancels the device creation process
the connection callback is called causing invalid memory access.
---
 src/device.c |   28 ++++++++++++++++++----------
 1 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/device.c b/src/device.c
index f22a7fa..b14e6e7 100644
--- a/src/device.c
+++ b/src/device.c
@@ -95,6 +95,7 @@ struct browse_req {
 	DBusConnection *conn;
 	DBusMessage *msg;
 	GAttrib *attrib;
+	GIOChannel *io;
 	struct btd_device *device;
 	GSList *match_uuids;
 	GSList *profiles_added;
@@ -154,10 +155,15 @@ static uint16_t uuid_list[] = {
 
 static GSList *device_drivers = NULL;
 
-static void browse_request_free(struct browse_req *req)
+static void browse_request_free(struct browse_req *req, gboolean shutdown)
 {
 	if (req->listener_id)
 		g_dbus_remove_watch(req->conn, req->listener_id);
+	if (req->io) {
+		if (shutdown)
+			g_io_channel_shutdown(req->io, FALSE, NULL);
+		g_io_channel_unref(req->io);
+	}
 	if (req->msg)
 		dbus_message_unref(req->msg);
 	if (req->conn)
@@ -189,7 +195,7 @@ static void browse_request_cancel(struct browse_req *req)
 	bt_cancel_discovery(&src, &device->bdaddr);
 
 	device->browse = NULL;
-	browse_request_free(req);
+	browse_request_free(req, TRUE);
 }
 
 static void device_free(gpointer user_data)
@@ -1493,7 +1499,7 @@ cleanup:
 	}
 
 	device->browse = NULL;
-	browse_request_free(req);
+	browse_request_free(req, FALSE);
 }
 
 static void browse_cb(sdp_list_t *recs, int err, gpointer user_data)
@@ -1588,11 +1594,13 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 	struct browse_req *req = user_data;
 	struct btd_device *device = req->device;
 	GSList *l, *uuids = NULL;
+	gboolean shutdown;
 
 	if (status) {
 		DBusMessage *reply;
 		reply = btd_error_failed(req->msg, att_ecode2str(status));
 		g_dbus_send_message(req->conn, reply);
+		shutdown = TRUE;
 		goto done;
 	}
 
@@ -1613,10 +1621,11 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 	create_device_reply(device, req);
 
 	store_services(device);
+	shutdown = FALSE;
 
 done:
 	device->browse = NULL;
-	browse_request_free(req);
+	browse_request_free(req, shutdown);
 }
 
 static void gatt_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
@@ -1633,7 +1642,7 @@ static void gatt_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 		g_dbus_send_message(req->conn, reply);
 
 		device->browse = NULL;
-		browse_request_free(req);
+		browse_request_free(req, TRUE);
 
 		return;
 	}
@@ -1650,7 +1659,6 @@ int device_browse_primary(struct btd_device *device, DBusConnection *conn,
 	struct btd_adapter *adapter = device->adapter;
 	struct browse_req *req;
 	BtIOSecLevel sec_level;
-	GIOChannel *io;
 	bdaddr_t src;
 
 	if (device->browse)
@@ -1663,15 +1671,15 @@ int device_browse_primary(struct btd_device *device, DBusConnection *conn,
 
 	sec_level = secure ? BT_IO_SEC_HIGH : BT_IO_SEC_LOW;
 
-	io = bt_io_connect(BT_IO_L2CAP, gatt_connect_cb, req, NULL, NULL,
+	req->io = bt_io_connect(BT_IO_L2CAP, gatt_connect_cb, req, NULL, NULL,
 				BT_IO_OPT_SOURCE_BDADDR, &src,
 				BT_IO_OPT_DEST_BDADDR, &device->bdaddr,
 				BT_IO_OPT_CID, ATT_CID,
 				BT_IO_OPT_SEC_LEVEL, sec_level,
 				BT_IO_OPT_INVALID);
 
-	if (io == NULL ) {
-		browse_request_free(req);
+	if (req->io == NULL) {
+		browse_request_free(req, FALSE);
 		return -EIO;
 	}
 
@@ -1724,7 +1732,7 @@ int device_browse_sdp(struct btd_device *device, DBusConnection *conn,
 
 	err = bt_search_service(&src, &device->bdaddr, &uuid, cb, req, NULL);
 	if (err < 0) {
-		browse_request_free(req);
+		browse_request_free(req, FALSE);
 		return err;
 	}
 
-- 
1.7.6

[PATCH BlueZ 04/12] Notify the GAttrib instance if already connected

[Claudio Takahasi]

When registering the connect and disconnect callbacks for ATT, GAttrib
instance should be informed to the caller if the link is up.
---
 src/device.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/src/device.c b/src/device.c
index b14e6e7..eeeb25f 100644
--- a/src/device.c
+++ b/src/device.c
@@ -132,6 +132,7 @@ struct btd_device {
 	struct bonding_req *bonding;
 	struct authentication_req *authr;	/* authentication request */
 	GSList		*disconnects;		/* disconnects message */
+	GAttrib		*attrib;
 	GSList		*attios;
 
 	gboolean	connected;
@@ -2475,6 +2476,9 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 
 	device->attios = g_slist_append(device->attios, attio);
 
+	if (device->attrib && func)
+		func(device->attrib, user_data);
+
 	return attio->id;
 }
 
-- 
1.7.6

[PATCH BlueZ 05/12] Add ATT disconnect callback

[Claudio Takahasi]

---
 src/attio.h  |    4 +++-
 src/device.c |   36 +++++++++++++++++++++++++++++++-----
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/src/attio.h b/src/attio.h
index 7935dcd..16e2873 100644
--- a/src/attio.h
+++ b/src/attio.h
@@ -23,9 +23,11 @@
  */
 
 typedef void (*attio_connect_cb) (GAttrib *attrib, gpointer user_data);
+typedef void (*attio_disconnect_cb) (gpointer user_data);
 
 guint btd_device_add_attio_callback(struct btd_device *device,
-						attio_connect_cb func,
+						attio_connect_cb cfunc,
+						attio_disconnect_cb dcfunc,
 						gpointer user_data);
 
 gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id);
diff --git a/src/device.c b/src/device.c
index eeeb25f..20248ed 100644
--- a/src/device.c
+++ b/src/device.c
@@ -108,7 +108,8 @@ struct browse_req {
 
 struct attio_data {
 	guint id;
-	attio_connect_cb func;
+	attio_connect_cb cfunc;
+	attio_disconnect_cb dcfunc;
 	gpointer user_data;
 };
 
@@ -1590,6 +1591,23 @@ static void store_services(struct btd_device *device)
 	g_free(str);
 }
 
+static void attio_disconnected(gpointer data, gpointer user_data)
+{
+	struct attio_data *attio = data;
+
+	if (attio->dcfunc)
+		attio->dcfunc(attio->user_data);
+}
+
+static void attrib_destroyed(gpointer user_data)
+{
+	struct btd_device *device = user_data;
+
+	device->attrib = NULL;
+
+	g_slist_foreach(device->attios, attio_disconnected, NULL);
+}
+
 static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 {
 	struct browse_req *req = user_data;
@@ -1619,6 +1637,12 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 
 	g_slist_free(uuids);
 
+	if (device->attios) {
+		device->attrib = g_attrib_ref(req->attrib);
+		g_attrib_set_destroy_function(device->attrib, attrib_destroyed,
+								device);
+	}
+
 	create_device_reply(device, req);
 
 	store_services(device);
@@ -2461,7 +2485,8 @@ void device_set_class(struct btd_device *device, uint32_t value)
 }
 
 guint btd_device_add_attio_callback(struct btd_device *device,
-						attio_connect_cb func,
+						attio_connect_cb cfunc,
+						attio_disconnect_cb dcfunc,
 						gpointer user_data)
 {
 	struct attio_data *attio;
@@ -2471,13 +2496,14 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 
 	attio = g_new0(struct attio_data, 1);
 	attio->id = ++attio_id;
-	attio->func = func;
+	attio->cfunc = cfunc;
+	attio->dcfunc = dcfunc;
 	attio->user_data = user_data;
 
 	device->attios = g_slist_append(device->attios, attio);
 
-	if (device->attrib && func)
-		func(device->attrib, user_data);
+	if (device->attrib && cfunc)
+		cfunc(device->attrib, user_data);
 
 	return attio->id;
 }
-- 
1.7.6

[PATCH BlueZ 06/12] Add auto connection based on registered callbacks

[Claudio Takahasi]

This initial approach uses a hardcoded timer(45s). Passive scanning
or any other mechanism to avoid unnecessary connection attempt is not
supported at the moment. For LE, device discovery needs to be executed
to update the kernel advertising cache to allow re-connections.
---
 src/device.c |  106 +++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 86 insertions(+), 20 deletions(-)

diff --git a/src/device.c b/src/device.c
index 20248ed..7543a80 100644
--- a/src/device.c
+++ b/src/device.c
@@ -65,6 +65,7 @@
 
 #define DISCONNECT_TIMER	2
 #define DISCOVERY_TIMER		2
+#define AUTOCONNECT_INTERVAL	45
 
 /* When all services should trust a remote device */
 #define GLOBAL_TRUST "[all]"
@@ -94,7 +95,6 @@ struct authentication_req {
 struct browse_req {
 	DBusConnection *conn;
 	DBusMessage *msg;
-	GAttrib *attrib;
 	GIOChannel *io;
 	struct btd_device *device;
 	GSList *match_uuids;
@@ -135,6 +135,7 @@ struct btd_device {
 	GSList		*disconnects;		/* disconnects message */
 	GAttrib		*attrib;
 	GSList		*attios;
+	guint		attioid;
 
 	gboolean	connected;
 
@@ -177,9 +178,6 @@ static void browse_request_free(struct browse_req *req, gboolean shutdown)
 	if (req->records)
 		sdp_list_free(req->records, (sdp_free_func_t) sdp_record_free);
 
-	if (req->attrib)
-		g_attrib_unref(req->attrib);
-
 	g_free(req);
 }
 
@@ -218,6 +216,8 @@ static void device_free(gpointer user_data)
 	g_slist_free_full(device->primaries, g_free);
 	g_slist_free_full(device->attios, g_free);
 
+	g_attrib_unref(device->attrib);
+
 	if (device->tmp_records)
 		sdp_list_free(device->tmp_records,
 					(sdp_free_func_t) sdp_record_free);
@@ -228,6 +228,9 @@ static void device_free(gpointer user_data)
 	if (device->discov_timer)
 		g_source_remove(device->discov_timer);
 
+	if (device->attioid)
+		g_source_remove(device->attioid);
+
 	DBG("%p", device);
 
 	g_free(device->authr);
@@ -1591,6 +1594,15 @@ static void store_services(struct btd_device *device)
 	g_free(str);
 }
 
+static void attio_connected(gpointer data, gpointer user_data)
+{
+	struct attio_data *attio = data;
+	GAttrib *attrib = user_data;
+
+	if (attio->cfunc)
+		attio->cfunc(attrib, attio->user_data);
+}
+
 static void attio_disconnected(gpointer data, gpointer user_data)
 {
 	struct attio_data *attio = data;
@@ -1637,12 +1649,6 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 
 	g_slist_free(uuids);
 
-	if (device->attios) {
-		device->attrib = g_attrib_ref(req->attrib);
-		g_attrib_set_destroy_function(device->attrib, attrib_destroyed,
-								device);
-	}
-
 	create_device_reply(device, req);
 
 	store_services(device);
@@ -1653,29 +1659,78 @@ done:
 	browse_request_free(req, shutdown);
 }
 
-static void gatt_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
+static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 {
-	struct browse_req *req = user_data;
-	struct btd_device *device = req->device;
+	struct btd_device *device = user_data;
+	struct browse_req *req = device->browse;
 
 	if (gerr) {
 		DBusMessage *reply;
 
 		DBG("%s", gerr->message);
 
-		reply = btd_error_failed(req->msg, gerr->message);
-		g_dbus_send_message(req->conn, reply);
+		if (req) {
+			reply = btd_error_failed(req->msg, gerr->message);
+			g_dbus_send_message(req->conn, reply);
 
-		device->browse = NULL;
-		browse_request_free(req, TRUE);
+			device->browse = NULL;
+			browse_request_free(req, TRUE);
+		}
 
 		return;
 	}
 
-	req->attrib = g_attrib_new(io);
+	device->attrib = g_attrib_new(io);
+	g_attrib_set_destroy_function(device->attrib, attrib_destroyed, device);
+
+	if (req)
+		gatt_discover_primary(device->attrib, NULL, primary_cb, req);
+	else if (device->attios)
+		g_slist_foreach(device->attios, attio_connected,
+							device->attrib);
+
 	g_io_channel_unref(io);
+}
+
+static gboolean att_auto_connect(gpointer user_data)
+{
+	struct btd_device *device = user_data;
+	struct btd_adapter *adapter = device->adapter;
+	GIOChannel *io;
+	GError *gerr = NULL;
+	char addr[18];
+	bdaddr_t sba;
+
+	adapter_get_address(adapter, &sba);
+	ba2str(&device->bdaddr, addr);
+
+	DBG("Connection attempt to: %s", addr);
+
+	if (device->type != DEVICE_TYPE_LE) {
+		io = bt_io_connect(BT_IO_L2CAP, att_connect_cb,
+					device, NULL, &gerr,
+					BT_IO_OPT_SOURCE_BDADDR, &sba,
+					BT_IO_OPT_DEST_BDADDR, &device->bdaddr,
+					BT_IO_OPT_PSM, ATT_PSM,
+					BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+					BT_IO_OPT_INVALID);
+	} else {
+		io = bt_io_connect(BT_IO_L2CAP, att_connect_cb,
+					device, NULL, &gerr,
+					BT_IO_OPT_SOURCE_BDADDR, &sba,
+					BT_IO_OPT_DEST_BDADDR, &device->bdaddr,
+					BT_IO_OPT_CID, ATT_CID,
+					BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+					BT_IO_OPT_INVALID);
+	}
+
+	if (io == NULL) {
+		error("ATT bt_io_connect(%s): %s", addr, gerr->message);
+		g_error_free(gerr);
+		return TRUE;
+	}
 
-	gatt_discover_primary(req->attrib, NULL, primary_cb, req);
+	return TRUE;
 }
 
 int device_browse_primary(struct btd_device *device, DBusConnection *conn,
@@ -1696,7 +1751,8 @@ int device_browse_primary(struct btd_device *device, DBusConnection *conn,
 
 	sec_level = secure ? BT_IO_SEC_HIGH : BT_IO_SEC_LOW;
 
-	req->io = bt_io_connect(BT_IO_L2CAP, gatt_connect_cb, req, NULL, NULL,
+	req->io = bt_io_connect(BT_IO_L2CAP, att_connect_cb,
+				device, NULL, NULL,
 				BT_IO_OPT_SOURCE_BDADDR, &src,
 				BT_IO_OPT_DEST_BDADDR, &device->bdaddr,
 				BT_IO_OPT_CID, ATT_CID,
@@ -2505,6 +2561,11 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 	if (device->attrib && cfunc)
 		cfunc(device->attrib, user_data);
 
+	if (device->attioid == 0)
+		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
+							att_auto_connect,
+							device);
+
 	return attio->id;
 }
 
@@ -2532,5 +2593,10 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
 
 	g_free(attio);
 
+	if (device->attioid) {
+		g_source_remove(device->attioid);
+		device->attioid = 0;
+	}
+
 	return TRUE;
 }
-- 
1.7.6

[PATCH BlueZ 07/12] Enable ATT re-connection

[Claudio Takahasi]

If the link is lost and there is at least one connection callback
registered the timer for ATT automatic connection shall be enabled.
---
 src/device.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/src/device.c b/src/device.c
index 7543a80..2185b77 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1611,6 +1611,8 @@ static void attio_disconnected(gpointer data, gpointer user_data)
 		attio->dcfunc(attio->user_data);
 }
 
+static gboolean att_auto_connect(gpointer user_data);
+
 static void attrib_destroyed(gpointer user_data)
 {
 	struct btd_device *device = user_data;
@@ -1618,6 +1620,11 @@ static void attrib_destroyed(gpointer user_data)
 	device->attrib = NULL;
 
 	g_slist_foreach(device->attios, attio_disconnected, NULL);
+
+	if (device->attioid == 0 && device->attios != NULL)
+		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
+							att_auto_connect,
+							device);
 }
 
 static void primary_cb(GSList *services, guint8 status, gpointer user_data)
@@ -1680,6 +1687,11 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 		return;
 	}
 
+	if (device->attioid) {
+		g_source_remove(device->attioid);
+		device->attioid = 0;
+	}
+
 	device->attrib = g_attrib_new(io);
 	g_attrib_set_destroy_function(device->attrib, attrib_destroyed, device);
 
@@ -2561,7 +2573,7 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 	if (device->attrib && cfunc)
 		cfunc(device->attrib, user_data);
 
-	if (device->attioid == 0)
+	if (device->attioid == 0 && device->attrib == NULL)
 		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
 							att_auto_connect,
 							device);
-- 
1.7.6

[PATCH BlueZ 08/12] Drop GAttrib ref if callback list is empty

[Claudio Takahasi]

If all ATT connection callbacks are unregistered, GAttrib reference
can be dropped.
---
 src/device.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/src/device.c b/src/device.c
index 2185b77..3d092e2 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2605,10 +2605,15 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
 
 	g_free(attio);
 
+	if (device->attios != NULL)
+		return TRUE;
+
 	if (device->attioid) {
 		g_source_remove(device->attioid);
 		device->attioid = 0;
 	}
 
+	g_attrib_unref(device->attrib);
+
 	return TRUE;
 }
-- 
1.7.6

[PATCH BlueZ 09/12] Try to connect when registering the first callback

[Claudio Takahasi]

If the device is disconnected and the first ATT connection callback is
registered, the first connection attempt can be triggered instead of
waiting the timer.
---
 src/device.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/src/device.c b/src/device.c
index 3d092e2..184252d 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2573,10 +2573,12 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 	if (device->attrib && cfunc)
 		cfunc(device->attrib, user_data);
 
-	if (device->attioid == 0 && device->attrib == NULL)
+	if (device->attioid == 0 && device->attrib == NULL) {
+		att_auto_connect(device);
 		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
 							att_auto_connect,
 							device);
+	}
 
 	return attio->id;
 }
-- 
1.7.6

[PATCH BlueZ 10/12] Manage GAttrib refs based on registered callbacks

[Claudio Takahasi]

Connection should not be dropped if there is at least one ATT connection
callback registered.
---
 src/device.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/device.c b/src/device.c
index 184252d..fa0efc0 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1664,6 +1664,7 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 done:
 	device->browse = NULL;
 	browse_request_free(req, shutdown);
+	g_attrib_unref(device->attrib);
 }
 
 static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
@@ -1700,8 +1701,6 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 	else if (device->attios)
 		g_slist_foreach(device->attios, attio_connected,
 							device->attrib);
-
-	g_io_channel_unref(io);
 }
 
 static gboolean att_auto_connect(gpointer user_data)
@@ -1742,6 +1741,8 @@ static gboolean att_auto_connect(gpointer user_data)
 		return TRUE;
 	}
 
+	g_io_channel_unref(io);
+
 	return TRUE;
 }
 
@@ -2568,18 +2569,22 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 	attio->dcfunc = dcfunc;
 	attio->user_data = user_data;
 
-	device->attios = g_slist_append(device->attios, attio);
-
-	if (device->attrib && cfunc)
-		cfunc(device->attrib, user_data);
+	if (device->attrib) {
+		/* First element */
+		if (device->attios == NULL)
+			device->attrib = g_attrib_ref(device->attrib);
 
-	if (device->attioid == 0 && device->attrib == NULL) {
+		if (cfunc)
+			cfunc(device->attrib, user_data);
+	} else if (device->attioid == 0) {
 		att_auto_connect(device);
 		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
 							att_auto_connect,
 							device);
 	}
 
+	device->attios = g_slist_append(device->attios, attio);
+
 	return attio->id;
 }
 
-- 
1.7.6

[PATCH BlueZ 11/12] Postpone calling connected callback

[Claudio Takahasi]

In the ATT callbacks registration function, the connection callback should
not be called in the same main loop iteraction to avoid accessing not
initialized data in the profiles during probing.
---
 src/device.c |   67 ++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/src/device.c b/src/device.c
index fa0efc0..52462be 100644
--- a/src/device.c
+++ b/src/device.c
@@ -134,7 +134,8 @@ struct btd_device {
 	struct authentication_req *authr;	/* authentication request */
 	GSList		*disconnects;		/* disconnects message */
 	GAttrib		*attrib;
-	GSList		*attios;
+	GSList		*attios_head;
+	GSList		*attios_tail;
 	guint		attioid;
 
 	gboolean	connected;
@@ -214,7 +215,8 @@ static void device_free(gpointer user_data)
 	g_slist_free_full(device->services, g_free);
 	g_slist_free_full(device->uuids, g_free);
 	g_slist_free_full(device->primaries, g_free);
-	g_slist_free_full(device->attios, g_free);
+	g_slist_free_full(device->attios_head, g_free);
+	g_slist_free_full(device->attios_tail, g_free);
 
 	g_attrib_unref(device->attrib);
 
@@ -1617,14 +1619,15 @@ static void attrib_destroyed(gpointer user_data)
 {
 	struct btd_device *device = user_data;
 
-	device->attrib = NULL;
-
-	g_slist_foreach(device->attios, attio_disconnected, NULL);
+	g_slist_foreach(device->attios_head, attio_disconnected, NULL);
 
-	if (device->attioid == 0 && device->attios != NULL)
+	if ((device->attios_head || device->attios_tail) &&
+						device->attioid == 0)
 		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
 							att_auto_connect,
 							device);
+
+	device->attrib = NULL;
 }
 
 static void primary_cb(GSList *services, guint8 status, gpointer user_data)
@@ -1698,8 +1701,8 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 
 	if (req)
 		gatt_discover_primary(device->attrib, NULL, primary_cb, req);
-	else if (device->attios)
-		g_slist_foreach(device->attios, attio_connected,
+	else if (device->attios_head)
+		g_slist_foreach(device->attios_head, attio_connected,
 							device->attrib);
 }
 
@@ -2553,6 +2556,21 @@ void device_set_class(struct btd_device *device, uint32_t value)
 				DBUS_TYPE_UINT32, &value);
 }
 
+static gboolean notify_attios(gpointer user_data)
+{
+	struct btd_device *device = user_data;
+
+	if (device->attrib == NULL)
+		return FALSE;
+
+	g_slist_foreach(device->attios_tail, attio_connected, device->attrib);
+	device->attios_head = g_slist_concat(device->attios_head,
+							device->attios_tail);
+	device->attios_tail = NULL;
+
+	return FALSE;
+}
+
 guint btd_device_add_attio_callback(struct btd_device *device,
 						attio_connect_cb cfunc,
 						attio_disconnect_cb dcfunc,
@@ -2571,20 +2589,22 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 
 	if (device->attrib) {
 		/* First element */
-		if (device->attios == NULL)
+		if (device->attios_head == NULL && device->attios_tail == NULL)
 			device->attrib = g_attrib_ref(device->attrib);
 
-		if (cfunc)
-			cfunc(device->attrib, user_data);
+		device->attios_tail = g_slist_append(device->attios_tail,
+									attio);
+		g_idle_add(notify_attios, device);
+
 	} else if (device->attioid == 0) {
 		att_auto_connect(device);
 		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
 							att_auto_connect,
 							device);
+		device->attios_head = g_slist_append(device->attios_head,
+									attio);
 	}
 
-	device->attios = g_slist_append(device->attios, attio);
-
 	return attio->id;
 }
 
@@ -2601,18 +2621,25 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
 	struct attio_data *attio;
 	GSList *l;
 
-	l = g_slist_find_custom(device->attios, GUINT_TO_POINTER(id),
+	l = g_slist_find_custom(device->attios_head, GUINT_TO_POINTER(id),
 								attio_id_cmp);
-	if (!l)
-		return FALSE;
-
-	attio = l->data;
+	if (l) {
+		attio = l->data;
+		device->attios_head = g_slist_remove(device->attios_head,
+									attio);
+	} else {
+		l = g_slist_find_custom(device->attios_tail,
+					GUINT_TO_POINTER(id), attio_id_cmp);
+		if (!l)
+			return FALSE;
 
-	device->attios = g_slist_remove(device->attios, attio);
+		attio = l->data;
+		device->attios_tail = g_slist_remove(device->attios_tail, attio);
+	}
 
 	g_free(attio);
 
-	if (device->attios != NULL)
+	if (device->attios_head != NULL || device->attios_tail != NULL)
 		return TRUE;
 
 	if (device->attioid) {
-- 
1.7.6

[PATCH BlueZ 12/12] Address remote initiated disconnection

[Claudio Takahasi]

Local initiated disconnection will be triggered when the last GAttrib
reference is dropped. For remote initiated disconnection it is necessary
to track the socket HUP registering disconnection GAttrib callback.
---
 src/device.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/src/device.c b/src/device.c
index 52462be..90761d4 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1615,7 +1615,7 @@ static void attio_disconnected(gpointer data, gpointer user_data)
 
 static gboolean att_auto_connect(gpointer user_data);
 
-static void attrib_destroyed(gpointer user_data)
+static void attrib_disconnected(gpointer user_data)
 {
 	struct btd_device *device = user_data;
 
@@ -1627,6 +1627,13 @@ static void attrib_destroyed(gpointer user_data)
 							att_auto_connect,
 							device);
 
+	g_attrib_unref(device->attrib);
+}
+
+static void attrib_destroyed(gpointer user_data)
+{
+	struct btd_device *device = user_data;
+
 	device->attrib = NULL;
 }
 
@@ -1697,6 +1704,8 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 	}
 
 	device->attrib = g_attrib_new(io);
+	g_attrib_set_disconnect_function(device->attrib, attrib_disconnected,
+								device);
 	g_attrib_set_destroy_function(device->attrib, attrib_destroyed, device);
 
 	if (req)
-- 
1.7.6

Re: [PATCH BlueZ 08/12] Drop GAttrib ref if callback list is empty

[Johan Hedberg]

Hi Claudio,

On Wed, Jul 06, 2011, Claudio Takahasi wrote:
> If all ATT connection callbacks are unregistered, GAttrib reference
> can be dropped.
> ---
>  src/device.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)

Patches 1-7 have been applied, but this one looked strange:

> --- a/src/device.c
> +++ b/src/device.c
> @@ -2605,10 +2605,15 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
>  
>  	g_free(attio);
>  
> +	if (device->attios != NULL)
> +		return TRUE;
> +
>  	if (device->attioid) {
>  		g_source_remove(device->attioid);
>  		device->attioid = 0;
>  	}
>  
> +	g_attrib_unref(device->attrib);
> +
>  	return TRUE;
>  }

Looks like you're missing a device->attrib = NULL; after the unref.

Johan

Re: [PATCH BlueZ 08/12] Drop GAttrib ref if callback list is empty

[Anderson Lizardo]

Hi Johan,

On Thu, Jul 14, 2011 at 10:44 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> @@ -2605,10 +2605,15 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
>>
>>       g_free(attio);
>>
>> +     if (device->attios != NULL)
>> +             return TRUE;
>> +
>>       if (device->attioid) {
>>               g_source_remove(device->attioid);
>>               device->attioid = 0;
>>       }
>>
>> +     g_attrib_unref(device->attrib);
>> +
>>       return TRUE;
>>  }
>
> Looks like you're missing a device->attrib = NULL; after the unref.

I really don't see the point in setting a reference counted variable
to NULL. How would we know whether this is the last usage or not? In
this case we are just removing a callback. What if we registered two
callbacks?

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

Re: [PATCH BlueZ 08/12] Drop GAttrib ref if callback list is empty

[Johan Hedberg]

Hi Lizardo,

On Thu, Jul 14, 2011, Anderson Lizardo wrote:
> On Thu, Jul 14, 2011 at 10:44 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> >> @@ -2605,10 +2605,15 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
> >>
> >>       g_free(attio);
> >>
> >> +     if (device->attios != NULL)
> >> +             return TRUE;
> >> +
> >>       if (device->attioid) {
> >>               g_source_remove(device->attioid);
> >>               device->attioid = 0;
> >>       }
> >>
> >> +     g_attrib_unref(device->attrib);
> >> +
> >>       return TRUE;
> >>  }
> >
> > Looks like you're missing a device->attrib = NULL; after the unref.
> 
> I really don't see the point in setting a reference counted variable
> to NULL. How would we know whether this is the last usage or not?

You're not supposed to know. All you know is that device->attrib is one
reference to the GAttrib object. Calling unref is akin to saying "this
pointer is no longer valid" and since the btd_device object keeps living
after this function you should set the ->attrib pointer to NULL after
dropping this reference.

Johan

Re: [PATCH BlueZ 08/12] Drop GAttrib ref if callback list is empty

[Claudio Takahasi]

Hi Johan/Lizardo,

On Thu, Jul 14, 2011 at 12:17 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Lizardo,
>
> On Thu, Jul 14, 2011, Anderson Lizardo wrote:
>> On Thu, Jul 14, 2011 at 10:44 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> >> @@ -2605,10 +2605,15 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
>> >>
>> >>       g_free(attio);
>> >>
>> >> +     if (device->attios != NULL)
>> >> +             return TRUE;
>> >> +
>> >>       if (device->attioid) {
>> >>               g_source_remove(device->attioid);
>> >>               device->attioid = 0;
>> >>       }
>> >>
>> >> +     g_attrib_unref(device->attrib);
>> >> +
>> >>       return TRUE;
>> >>  }
>> >
>> > Looks like you're missing a device->attrib = NULL; after the unref.
>>
>> I really don't see the point in setting a reference counted variable
>> to NULL. How would we know whether this is the last usage or not?
>
> You're not supposed to know. All you know is that device->attrib is one
> reference to the GAttrib object. Calling unref is akin to saying "this
> pointer is no longer valid" and since the btd_device object keeps living
> after this function you should set the ->attrib pointer to NULL after
> dropping this reference.
>
> Johan
>

GAttrib destroy callback will set device->attrib to NULL. I gonna send
a new patch setting it to NULL explicitly and avoid misunderstandings.

Regards,
Claudio

[PATCH BlueZ 08/12] Drop GAttrib ref if callback list is empty

[Claudio Takahasi]

If all ATT connection callbacks are unregistered, GAttrib reference
can be dropped.
---
 src/device.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/src/device.c b/src/device.c
index 1f45211..571a287 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2603,10 +2603,18 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
 
 	g_free(attio);
 
+	if (device->attios != NULL)
+		return TRUE;
+
 	if (device->attioid) {
 		g_source_remove(device->attioid);
 		device->attioid = 0;
 	}
 
+	if (device->attrib) {
+		g_attrib_unref(device->attrib);
+		device->attrib = NULL;
+	}
+
 	return TRUE;
 }
-- 
1.7.6

Re: [PATCH BlueZ 10/12] Manage GAttrib refs based on registered callbacks

[Johan Hedberg]

Hi Claudio,

On Wed, Jul 06, 2011, Claudio Takahasi wrote:
> Connection should not be dropped if there is at least one ATT connection
> callback registered.
> ---
>  src/device.c |   19 ++++++++++++-------
>  1 files changed, 12 insertions(+), 7 deletions(-)

Patches 8 and 9 have now been pushed, but there's some reference
counting weirdness going on here again:

> --- a/src/device.c
> +++ b/src/device.c
> @@ -1664,6 +1664,7 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
>  done:
>  	device->browse = NULL;
>  	browse_request_free(req, shutdown);
> +	g_attrib_unref(device->attrib);
>  }

Where's the device->attrib = NULL?

> @@ -2568,18 +2569,22 @@ guint btd_device_add_attio_callback(struct btd_device *device,
>  	attio->dcfunc = dcfunc;
>  	attio->user_data = user_data;
>  
> -	device->attios = g_slist_append(device->attios, attio);
> -
> -	if (device->attrib && cfunc)
> -		cfunc(device->attrib, user_data);
> +	if (device->attrib) {
> +		/* First element */
> +		if (device->attios == NULL)
> +			device->attrib = g_attrib_ref(device->attrib);

Eh? It looks like attio is supposed to own this reference created by
g_attrib_ref() so you should really be assigning the return value to
attio->attrib. Or is it so that device->attrib is actually there only
for any elements in the device->attios list? If that's the case then
it's fine to have just create device->attrib when you add the first
element to the list and unref + set to NULL when you remove the last
element from the list.

Johan

Re: [PATCH BlueZ 10/12] Manage GAttrib refs based on registered callbacks

[Claudio Takahasi]

Hi Johan,

On Thu, Jul 14, 2011 at 2:12 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Claudio,
>
> On Wed, Jul 06, 2011, Claudio Takahasi wrote:
>> Connection should not be dropped if there is at least one ATT connection
>> callback registered.
>> ---
>>  src/device.c |   19 ++++++++++++-------
>>  1 files changed, 12 insertions(+), 7 deletions(-)
>
> Patches 8 and 9 have now been pushed, but there's some reference
> counting weirdness going on here again:
>
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -1664,6 +1664,7 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
>>  done:
>>       device->browse = NULL;
>>       browse_request_free(req, shutdown);
>> +     g_attrib_unref(device->attrib);
>>  }
>
> Where's the device->attrib = NULL?
>
>> @@ -2568,18 +2569,22 @@ guint btd_device_add_attio_callback(struct btd_device *device,
>>       attio->dcfunc = dcfunc;
>>       attio->user_data = user_data;
>>
>> -     device->attios = g_slist_append(device->attios, attio);
>> -
>> -     if (device->attrib && cfunc)
>> -             cfunc(device->attrib, user_data);
>> +     if (device->attrib) {
>> +             /* First element */
>> +             if (device->attios == NULL)
>> +                     device->attrib = g_attrib_ref(device->attrib);
>
> Eh? It looks like attio is supposed to own this reference created by
> g_attrib_ref() so you should really be assigning the return value to
> attio->attrib. Or is it so that device->attrib is actually there only
> for any elements in the device->attios list? If that's the case then
> it's fine to have just create device->attrib when you add the first
> element to the list and unref + set to NULL when you remove the last
> element from the list.
>
> Johan
>

Create attio->attrib is not a good approach, GAttrib is created after
the connection establishment and attio callbacks can be registered
anytime.
The patch that I will send will fix the reference counting using
device->attrib to hold the reference for attios. A new GAttrib pointer
will added in the "struct browse_req" and it will be used in the
discovery primary services. device->attrib reference will be
"released" if device->attios list is empty.

I realized that "shutdown" parameter used in browse_request_free
function can be removed if we use the list to control disconnections.
I gonna send this cleanup patch after you apply this patch series.


Regards,
Claudio.

[PATCH BlueZ v2 10/12] Manage GAttrib refs based on registered callbacks

[Claudio Takahasi]

Connection should not be dropped if there is at least one ATT connection
callback registered. After discovering primary services over the LE link,
GAttrib ref needs to be held to allow notifying an active connection
while registering connection callbacks(inside plugins probing).
---
 src/device.c |   40 ++++++++++++++++++++++++++++------------
 1 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/src/device.c b/src/device.c
index d624b46..002e424 100644
--- a/src/device.c
+++ b/src/device.c
@@ -96,6 +96,7 @@ struct browse_req {
 	DBusConnection *conn;
 	DBusMessage *msg;
 	GIOChannel *io;
+	GAttrib *attrib;
 	struct btd_device *device;
 	GSList *match_uuids;
 	GSList *profiles_added;
@@ -163,6 +164,8 @@ static void browse_request_free(struct browse_req *req, gboolean shutdown)
 {
 	if (req->listener_id)
 		g_dbus_remove_watch(req->conn, req->listener_id);
+	if (req->attrib)
+		g_attrib_unref(req->attrib);
 	if (req->io) {
 		if (shutdown)
 			g_io_channel_shutdown(req->io, FALSE, NULL);
@@ -1642,9 +1645,21 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 		uuids = g_slist_append(uuids, prim->uuid);
 	}
 
+	/*
+	 * Profiles may register attio callbacks in the probing callback.
+	 * GAttrib reference can be released if the registered callbacks
+	 * list is emtpy.
+	 */
+	device->attrib = g_attrib_ref(req->attrib);
+
 	device_register_services(req->conn, device, g_slist_copy(services), -1);
 	device_probe_drivers(device, uuids);
 
+	if (device->attios == NULL) {
+		g_attrib_unref(device->attrib);
+		device->attrib = NULL;
+	}
+
 	g_slist_free(uuids);
 
 	create_device_reply(device, req);
@@ -1683,16 +1698,16 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 		device->attioid = 0;
 	}
 
-	device->attrib = g_attrib_new(io);
-	g_attrib_set_destroy_function(device->attrib, attrib_destroyed, device);
-
-	if (req)
-		gatt_discover_primary(device->attrib, NULL, primary_cb, req);
-	else if (device->attios)
+	if (req) {
+		req->attrib = g_attrib_new(io);
+		gatt_discover_primary(req->attrib, NULL, primary_cb, req);
+	} else if (device->attios) {
+		device->attrib = g_attrib_new(io);
+		g_attrib_set_destroy_function(device->attrib, attrib_destroyed,
+								device);
 		g_slist_foreach(device->attios, attio_connected,
 							device->attrib);
-
-	g_io_channel_unref(io);
+	}
 }
 
 static gboolean att_auto_connect(gpointer user_data)
@@ -1733,6 +1748,8 @@ static gboolean att_auto_connect(gpointer user_data)
 		return TRUE;
 	}
 
+	g_io_channel_unref(io);
+
 	return TRUE;
 }
 
@@ -2566,18 +2583,17 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 	attio->dcfunc = dcfunc;
 	attio->user_data = user_data;
 
-	device->attios = g_slist_append(device->attios, attio);
-
 	if (device->attrib && cfunc)
 		cfunc(device->attrib, user_data);
-
-	if (device->attioid == 0 && device->attrib == NULL) {
+	else if (device->attioid == 0) {
 		att_auto_connect(device);
 		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
 							att_auto_connect,
 							device);
 	}
 
+	device->attios = g_slist_append(device->attios, attio);
+
 	return attio->id;
 }
 
-- 
1.7.6

[PATCH BlueZ v2 11/12] Postpone calling connected callback

[Claudio Takahasi]

In the ATT callbacks registration function, the connection callback should
not be called in the same main loop iteraction to avoid accessing not
initialized data in the profiles during probing.
---
 src/device.c |   54 +++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/src/device.c b/src/device.c
index 002e424..1287f52 100644
--- a/src/device.c
+++ b/src/device.c
@@ -136,6 +136,7 @@ struct btd_device {
 	GSList		*disconnects;		/* disconnects message */
 	GAttrib		*attrib;
 	GSList		*attios;
+	GSList		*attios_offline;
 	guint		attioid;
 
 	gboolean	connected;
@@ -219,6 +220,7 @@ static void device_free(gpointer user_data)
 	g_slist_free_full(device->uuids, g_free);
 	g_slist_free_full(device->primaries, g_free);
 	g_slist_free_full(device->attios, g_free);
+	g_slist_free_full(device->attios_offline, g_free);
 
 	g_attrib_unref(device->attrib);
 
@@ -1612,14 +1614,14 @@ static void attrib_destroyed(gpointer user_data)
 {
 	struct btd_device *device = user_data;
 
-	device->attrib = NULL;
-
 	g_slist_foreach(device->attios, attio_disconnected, NULL);
 
-	if (device->attioid == 0 && device->attios != NULL)
+	if ((device->attios || device->attios_offline) && device->attioid == 0)
 		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
 							att_auto_connect,
 							device);
+
+	device->attrib = NULL;
 }
 
 static void primary_cb(GSList *services, guint8 status, gpointer user_data)
@@ -1655,7 +1657,7 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 	device_register_services(req->conn, device, g_slist_copy(services), -1);
 	device_probe_drivers(device, uuids);
 
-	if (device->attios == NULL) {
+	if (device->attios == NULL && device->attios_offline == NULL) {
 		g_attrib_unref(device->attrib);
 		device->attrib = NULL;
 	}
@@ -2567,6 +2569,20 @@ void device_set_class(struct btd_device *device, uint32_t value)
 				DBUS_TYPE_UINT32, &value);
 }
 
+static gboolean notify_attios(gpointer user_data)
+{
+	struct btd_device *device = user_data;
+
+	if (device->attrib == NULL)
+		return FALSE;
+
+	g_slist_foreach(device->attios_offline, attio_connected, device->attrib);
+	device->attios = g_slist_concat(device->attios, device->attios_offline);
+	device->attios_offline = NULL;
+
+	return FALSE;
+}
+
 guint btd_device_add_attio_callback(struct btd_device *device,
 						attio_connect_cb cfunc,
 						attio_disconnect_cb dcfunc,
@@ -2583,17 +2599,18 @@ guint btd_device_add_attio_callback(struct btd_device *device,
 	attio->dcfunc = dcfunc;
 	attio->user_data = user_data;
 
-	if (device->attrib && cfunc)
-		cfunc(device->attrib, user_data);
-	else if (device->attioid == 0) {
+	if (device->attrib && cfunc) {
+		device->attios_offline = g_slist_append(device->attios_offline,
+									attio);
+		g_idle_add(notify_attios, device);
+	} else if (device->attioid == 0) {
 		att_auto_connect(device);
 		device->attioid = g_timeout_add_seconds(AUTOCONNECT_INTERVAL,
 							att_auto_connect,
 							device);
+		device->attios = g_slist_append(device->attios, attio);
 	}
 
-	device->attios = g_slist_append(device->attios, attio);
-
 	return attio->id;
 }
 
@@ -2612,16 +2629,23 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
 
 	l = g_slist_find_custom(device->attios, GUINT_TO_POINTER(id),
 								attio_id_cmp);
-	if (!l)
-		return FALSE;
-
-	attio = l->data;
+	if (l) {
+		attio = l->data;
+		device->attios = g_slist_remove(device->attios, attio);
+	} else {
+		l = g_slist_find_custom(device->attios_offline,
+					GUINT_TO_POINTER(id), attio_id_cmp);
+		if (!l)
+			return FALSE;
 
-	device->attios = g_slist_remove(device->attios, attio);
+		attio = l->data;
+		device->attios_offline = g_slist_remove(device->attios_offline,
+									attio);
+	}
 
 	g_free(attio);
 
-	if (device->attios != NULL)
+	if (device->attios != NULL || device->attios_offline != NULL)
 		return TRUE;
 
 	if (device->attioid) {
-- 
1.7.6

[PATCH BlueZ v2 12/12] Address remote initiated disconnection

[Claudio Takahasi]

Local initiated disconnection will be triggered when the last GAttrib
reference is dropped. For remote initiated disconnection it is necessary
to track the socket HUP registering disconnection GAttrib callback.
---
 src/device.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/device.c b/src/device.c
index 1287f52..6cc6310 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1610,7 +1610,7 @@ static void attio_disconnected(gpointer data, gpointer user_data)
 
 static gboolean att_auto_connect(gpointer user_data);
 
-static void attrib_destroyed(gpointer user_data)
+static void attrib_disconnected(gpointer user_data)
 {
 	struct btd_device *device = user_data;
 
@@ -1621,6 +1621,7 @@ static void attrib_destroyed(gpointer user_data)
 							att_auto_connect,
 							device);
 
+	g_attrib_unref(device->attrib);
 	device->attrib = NULL;
 }
 
@@ -1705,8 +1706,8 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 		gatt_discover_primary(req->attrib, NULL, primary_cb, req);
 	} else if (device->attios) {
 		device->attrib = g_attrib_new(io);
-		g_attrib_set_destroy_function(device->attrib, attrib_destroyed,
-								device);
+		g_attrib_set_disconnect_function(device->attrib,
+						attrib_disconnected, device);
 		g_slist_foreach(device->attios, attio_connected,
 							device->attrib);
 	}
-- 
1.7.6

Re: [PATCH BlueZ v2 12/12] Address remote initiated disconnection

[Johan Hedberg]

Hi Claudio,

On Wed, Jul 20, 2011, Claudio Takahasi wrote:
> Local initiated disconnection will be triggered when the last GAttrib
> reference is dropped. For remote initiated disconnection it is necessary
> to track the socket HUP registering disconnection GAttrib callback.
> ---
>  src/device.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)

These last three patches in this set have now also been pushed upstream.

Johan