[PATCH v2 0/6] Attrib client move

[PATCH v2 0/6] Attrib client move

[Vinicius Costa Gomes]

Hi,

This patch series moves the Attrib client to the core, this move is
more related to the build system than with code being moved.

Things left to be done (that will be done in later patches):
 - Remove the attrib plugin;
 - Use SDP information to register primary services;

--
Cheers,


Vinicius Costa Gomes (6):
  Move Attrib client to the core
  Register Attrib interface when loading device from storage
  Register Attrib interface after Primary Service discovery
  Remove _init and _exit methods from Attrib client
  Add support for re-using the attrib channel
  Fix disconnecting when primary service discovery is done

 Makefile.am      |    4 +-
 attrib/client.c  |   49 +++++++++++++++---------------------------
 attrib/client.h  |    6 ++--
 attrib/main.c    |   14 +-----------
 attrib/manager.c |   62 ++---------------------------------------------------
 attrib/manager.h |    2 +-
 src/adapter.c    |    5 ++++
 src/device.c     |   20 +++++++++++------
 8 files changed, 46 insertions(+), 116 deletions(-)

--
1.7.4.1

[PATCH v2 1/6] Move Attrib client to the core

[Vinicius Costa Gomes]

---
 Makefile.am      |    4 +-
 attrib/main.c    |   14 +-----------
 attrib/manager.c |   62 ++---------------------------------------------------
 attrib/manager.h |    2 +-
 4 files changed, 7 insertions(+), 75 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index a23cc60..77b9b7d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -89,7 +89,8 @@ endif
 endif
 
 attrib_sources = attrib/att.h attrib/att.c attrib/gatt.h attrib/gatt.c \
-		attrib/gattrib.h attrib/gattrib.c
+		attrib/gattrib.h attrib/gattrib.c attrib/client.h \
+		attrib/client.c
 
 gdbus_sources = gdbus/gdbus.h gdbus/mainloop.c gdbus/watch.c \
 					gdbus/object.c gdbus/polkit.c
@@ -204,7 +205,6 @@ endif
 builtin_modules += attrib
 builtin_sources += attrib/main.c \
 		attrib/manager.h attrib/manager.c \
-		attrib/client.h attrib/client.c \
 		attrib/example.h attrib/example.c
 endif
 
diff --git a/attrib/main.c b/attrib/main.c
index 6c946be..91ddab1 100644
--- a/attrib/main.c
+++ b/attrib/main.c
@@ -28,32 +28,20 @@
 
 #include <errno.h>
 
-#include <gdbus.h>
-
 #include "plugin.h"
 #include "manager.h"
 
-static DBusConnection *connection;
-
 static int attrib_init(void)
 {
-	connection = dbus_bus_get(DBUS_BUS_SYSTEM, NULL);
-	if (connection == NULL)
+	if (attrib_manager_init() < 0)
 		return -EIO;
 
-	if (attrib_manager_init(connection) < 0) {
-		dbus_connection_unref(connection);
-		return -EIO;
-	}
-
 	return 0;
 }
 
 static void attrib_exit(void)
 {
 	attrib_manager_exit();
-
-	dbus_connection_unref(connection);
 }
 
 BLUETOOTH_PLUGIN_DEFINE(attrib, VERSION,
diff --git a/attrib/manager.c b/attrib/manager.c
index a5a7de4..7c05720 100644
--- a/attrib/manager.c
+++ b/attrib/manager.c
@@ -26,66 +26,16 @@
 #include <config.h>
 #endif
 
-#include <bluetooth/bluetooth.h>
-#include <bluetooth/sdp.h>
-#include <bluetooth/sdp_lib.h>
+#include <stdint.h>
+#include <glib.h>
 
-#include "../src/adapter.h"
-#include "../src/device.h"
 #include "hcid.h"
 
 #include "manager.h"
-#include "client.h"
 #include "example.h"
 
-#define GATT_UUID	"00001801-0000-1000-8000-00805f9b34fb"
-
-static DBusConnection *connection;
-
-static int client_probe(struct btd_device *device, GSList *uuids)
+int attrib_manager_init(void)
 {
-	const sdp_record_t *rec;
-	int psm = -1;
-
-	rec = btd_device_get_record(device, GATT_UUID);
-	if (rec) {
-		sdp_list_t *list;
-		if (sdp_get_access_protos(rec, &list) < 0)
-			return -1;
-
-		psm = sdp_get_proto_port(list, L2CAP_UUID);
-
-		sdp_list_foreach(list, (sdp_list_func_t) sdp_list_free, NULL);
-		sdp_list_free(list, NULL);
-
-		if (psm < 0)
-			return -1;
-	}
-
-	return attrib_client_register(device, psm);
-}
-
-static void client_remove(struct btd_device *device)
-{
-	attrib_client_unregister(device);
-}
-
-static struct btd_device_driver client_driver = {
-	.name = "gatt-client",
-	.uuids = BTD_UUIDS(GATT_UUID),
-	.probe = client_probe,
-	.remove = client_remove,
-};
-
-int attrib_manager_init(DBusConnection *conn)
-{
-	connection = dbus_connection_ref(conn);
-
-	attrib_client_init(connection);
-
-	btd_register_device_driver(&client_driver);
-
-
 	if (main_opts.attrib_server)
 		return server_example_init();
 
@@ -94,12 +44,6 @@ int attrib_manager_init(DBusConnection *conn)
 
 void attrib_manager_exit(void)
 {
-	btd_unregister_device_driver(&client_driver);
-
 	if (main_opts.attrib_server)
 		server_example_exit();
-
-	attrib_client_exit();
-
-	dbus_connection_unref(connection);
 }
diff --git a/attrib/manager.h b/attrib/manager.h
index fabf342..19dc539 100644
--- a/attrib/manager.h
+++ b/attrib/manager.h
@@ -22,5 +22,5 @@
  *
  */
 
-int attrib_manager_init(DBusConnection *conn);
+int attrib_manager_init(void);
 void attrib_manager_exit(void);
-- 
1.7.4.1

[PATCH v2 2/6] Register Attrib interface when loading device from storage

[Vinicius Costa Gomes]

Now that the GATT client functionality is being moved to the
core we need to register the interface when the device is created.
---
 attrib/client.c |    3 +--
 attrib/client.h |    2 +-
 src/adapter.c   |    4 ++++
 src/device.c    |    3 +++
 4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 54bdc79..590f33b 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -1044,12 +1044,11 @@ static void register_primaries(struct gatt_service *gatt, GSList *primaries)
 	}
 }
 
-int attrib_client_register(struct btd_device *device, int psm)
+int attrib_client_register(struct btd_device *device, int psm, GSList *primaries)
 {
 	struct btd_adapter *adapter = device_get_adapter(device);
 	const char *path = device_get_path(device);
 	struct gatt_service *gatt;
-	GSList *primaries = btd_device_get_primaries(device);
 	bdaddr_t sba, dba;
 
 	adapter_get_address(adapter, &sba);
diff --git a/attrib/client.h b/attrib/client.h
index 50e2b5f..2bee84c 100644
--- a/attrib/client.h
+++ b/attrib/client.h
@@ -24,5 +24,5 @@
 
 int attrib_client_init(DBusConnection *conn);
 void attrib_client_exit(void);
-int attrib_client_register(struct btd_device *device, int psm);
+int attrib_client_register(struct btd_device *device, int psm, GSList *primaries);
 void attrib_client_unregister(struct btd_device *device);
diff --git a/src/adapter.c b/src/adapter.c
index 5894146..dad427f 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -58,6 +58,7 @@
 #include "storage.h"
 #include "attrib-server.h"
 #include "att.h"
+#include "attrib/client.h"
 
 /* Flags Descriptions */
 #define EIR_LIM_DISC                0x01 /* LE Limited Discoverable Mode */
@@ -2180,6 +2181,9 @@ static void create_stored_device_from_primary(char *key, char *value,
 		device_add_primary(device, prim);
 	}
 
+	/* FIXME: Need the correct psm */
+	attrib_client_register(device, -1, services);
+
 	device_probe_drivers(device, uuids);
 
 	g_slist_free(services);
diff --git a/src/device.c b/src/device.c
index 771a908..9a3202f 100644
--- a/src/device.c
+++ b/src/device.c
@@ -60,6 +60,7 @@
 #include "sdp-xml.h"
 #include "storage.h"
 #include "btio.h"
+#include "attrib/client.h"
 
 #define DISCONNECT_TIMER	2
 #define DISCOVERY_TIMER		2
@@ -1030,6 +1031,8 @@ void device_remove(struct btd_device *device, gboolean remove_stored)
 	g_slist_free(device->drivers);
 	device->drivers = NULL;
 
+	attrib_client_unregister(device);
+
 	btd_device_unref(device);
 }
 
-- 
1.7.4.1

[PATCH v2 3/6] Register Attrib interface after Primary Service discovery

[Vinicius Costa Gomes]

---
 src/device.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/src/device.c b/src/device.c
index 9a3202f..1d7094b 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1546,6 +1546,10 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 	}
 
 	device_probe_drivers(device, uuids);
+
+	/* FIXME: Need the correct psm */
+	attrib_client_register(device, -1, services);
+
 	g_slist_free(uuids);
 
 	create_device_reply(device, req);
-- 
1.7.4.1

[PATCH v2 4/6] Remove _init and _exit methods from Attrib client

[Vinicius Costa Gomes]

Now that Attrib client is being moved to the core, there's no
need to have explicit _init and _exit methods.

This also means that we need to pass the DBusConnection explicitly
to the registration method.
---
 attrib/client.c |   44 ++++++++++++++------------------------------
 attrib/client.h |    5 ++---
 src/adapter.c   |    2 +-
 src/device.c    |    2 +-
 4 files changed, 18 insertions(+), 35 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 590f33b..acd35f9 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -52,6 +52,7 @@
 
 struct gatt_service {
 	struct btd_device *dev;
+	DBusConnection *conn;
 	bdaddr_t sba;
 	bdaddr_t dba;
 	char *path;
@@ -108,8 +109,6 @@ struct watcher {
 
 static GSList *gatt_services = NULL;
 
-static DBusConnection *connection;
-
 static void characteristic_free(void *user_data)
 {
 	struct characteristic *chr = user_data;
@@ -138,7 +137,7 @@ static void primary_free(void *user_data)
 
 	for (l = prim->watchers; l; l = l->next) {
 		struct watcher *watcher = l->data;
-		g_dbus_remove_watch(connection, watcher->id);
+		g_dbus_remove_watch(prim->gatt->conn, watcher->id);
 	}
 
 	g_slist_foreach(prim->chars, (GFunc) characteristic_free, NULL);
@@ -156,6 +155,7 @@ static void gatt_service_free(void *user_data)
 	g_attrib_unref(gatt->attrib);
 	g_free(gatt->path);
 	btd_device_unref(gatt->dev);
+	dbus_connection_unref(gatt->conn);
 	g_free(gatt);
 }
 
@@ -249,6 +249,7 @@ static void update_watchers(gpointer data, gpointer user_data)
 {
 	struct watcher *w = data;
 	struct characteristic *chr = user_data;
+	DBusConnection *conn = w->prim->gatt->conn;
 	DBusMessage *msg;
 
 	msg = dbus_message_new_method_call(w->name, w->path,
@@ -261,7 +262,7 @@ static void update_watchers(gpointer data, gpointer user_data)
 			&chr->value, chr->vlen, DBUS_TYPE_INVALID);
 
 	dbus_message_set_no_reply(msg, TRUE);
-	g_dbus_send_message(connection, msg);
+	g_dbus_send_message(conn, msg);
 }
 
 static void events_handler(const uint8_t *pdu, uint16_t len,
@@ -337,7 +338,7 @@ static void connect_cb(GIOChannel *chan, GError *gerr, gpointer user_data)
 		if (gatt->msg) {
 			DBusMessage *reply = btd_error_failed(gatt->msg,
 							gerr->message);
-			g_dbus_send_message(connection, reply);
+			g_dbus_send_message(gatt->conn, reply);
 		}
 
 		error("%s", gerr->message);
@@ -592,7 +593,7 @@ static void register_characteristics(struct primary *prim)
 
 	for (lc = prim->chars; lc; lc = lc->next) {
 		struct characteristic *chr = lc->data;
-		g_dbus_register_interface(connection, chr->path,
+		g_dbus_register_interface(prim->gatt->conn, chr->path,
 				CHAR_INTERFACE, char_methods,
 				NULL, NULL, chr, NULL);
 		DBG("Registered: %s", chr->path);
@@ -936,7 +937,7 @@ static void char_discovered_cb(GSList *characteristics, guint8 status,
 	g_slist_foreach(prim->chars, update_all_chars, prim);
 
 fail:
-	g_dbus_send_message(connection, reply);
+	g_dbus_send_message(gatt->conn, reply);
 	g_attrib_unref(gatt->attrib);
 	g_free(current);
 }
@@ -1033,7 +1034,7 @@ static void register_primaries(struct gatt_service *gatt, GSList *primaries)
 		prim->path = g_strdup_printf("%s/service%04x", gatt->path,
 								att->start);
 
-		g_dbus_register_interface(connection, prim->path,
+		g_dbus_register_interface(gatt->conn, prim->path,
 				CHAR_INTERFACE, prim_methods,
 				NULL, NULL, prim, NULL);
 		DBG("Registered: %s", prim->path);
@@ -1044,7 +1045,8 @@ static void register_primaries(struct gatt_service *gatt, GSList *primaries)
 	}
 }
 
-int attrib_client_register(struct btd_device *device, int psm, GSList *primaries)
+int attrib_client_register(DBusConnection *connection,
+		struct btd_device *device, int psm, GSList *primaries)
 {
 	struct btd_adapter *adapter = device_get_adapter(device);
 	const char *path = device_get_path(device);
@@ -1056,6 +1058,7 @@ int attrib_client_register(struct btd_device *device, int psm, GSList *primaries
 
 	gatt = g_new0(struct gatt_service, 1);
 	gatt->dev = btd_device_ref(device);
+	gatt->conn = dbus_connection_ref(connection);
 	gatt->listen = FALSE;
 	gatt->path = g_strdup(path);
 	bacpy(&gatt->sba, &sba);
@@ -1085,31 +1088,12 @@ void attrib_client_unregister(struct btd_device *device)
 		struct primary *prim = lp->data;
 		for (lc = prim->chars; lc; lc = lc->next) {
 			struct characteristic *chr = lc->data;
-			g_dbus_unregister_interface(connection, chr->path,
+			g_dbus_unregister_interface(gatt->conn, chr->path,
 								CHAR_INTERFACE);
 		}
-		g_dbus_unregister_interface(connection, prim->path,
+		g_dbus_unregister_interface(gatt->conn, prim->path,
 								CHAR_INTERFACE);
 	}
 
 	gatt_service_free(gatt);
 }
-
-int attrib_client_init(DBusConnection *conn)
-{
-
-	connection = dbus_connection_ref(conn);
-
-	/*
-	 * FIXME: if the adapter supports BLE start scanning. Temporary
-	 * solution, this approach doesn't allow to control scanning based
-	 * on the discoverable property.
-	 */
-
-	return 0;
-}
-
-void attrib_client_exit(void)
-{
-	dbus_connection_unref(connection);
-}
diff --git a/attrib/client.h b/attrib/client.h
index 2bee84c..650b0c1 100644
--- a/attrib/client.h
+++ b/attrib/client.h
@@ -22,7 +22,6 @@
  *
  */
 
-int attrib_client_init(DBusConnection *conn);
-void attrib_client_exit(void);
-int attrib_client_register(struct btd_device *device, int psm, GSList *primaries);
+int attrib_client_register(DBusConnection *connection,
+		struct btd_device *device, int psm, GSList *primaries);
 void attrib_client_unregister(struct btd_device *device);
diff --git a/src/adapter.c b/src/adapter.c
index dad427f..da13f69 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2182,7 +2182,7 @@ static void create_stored_device_from_primary(char *key, char *value,
 	}
 
 	/* FIXME: Need the correct psm */
-	attrib_client_register(device, -1, services);
+	attrib_client_register(connection, device, -1, services);
 
 	device_probe_drivers(device, uuids);
 
diff --git a/src/device.c b/src/device.c
index 1d7094b..511c9d0 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1548,7 +1548,7 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 	device_probe_drivers(device, uuids);
 
 	/* FIXME: Need the correct psm */
-	attrib_client_register(device, -1, services);
+	attrib_client_register(req->conn, device, -1, services);
 
 	g_slist_free(uuids);
 
-- 
1.7.4.1

[PATCH v2 5/6] Add support for re-using the attrib channel

[Vinicius Costa Gomes]

In some cases, when the device is already connected there's no need
to create another GAttrib instance.

This will allow the Attrib client to use the connection already
estabilished, this will be very useful when we support more
LE profiles.
---
 attrib/client.c |    6 +++++-
 attrib/client.h |    3 ++-
 src/adapter.c   |    3 ++-
 src/device.c    |    2 +-
 4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index acd35f9..28e5704 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -1046,7 +1046,8 @@ static void register_primaries(struct gatt_service *gatt, GSList *primaries)
 }
 
 int attrib_client_register(DBusConnection *connection,
-		struct btd_device *device, int psm, GSList *primaries)
+					struct btd_device *device, int psm,
+					GAttrib *attrib, GSList *primaries)
 {
 	struct btd_adapter *adapter = device_get_adapter(device);
 	const char *path = device_get_path(device);
@@ -1065,6 +1066,9 @@ int attrib_client_register(DBusConnection *connection,
 	bacpy(&gatt->dba, &dba);
 	gatt->psm = psm;
 
+	if (attrib)
+		gatt->attrib = g_attrib_ref(attrib);
+
 	register_primaries(gatt, primaries);
 
 	gatt_services = g_slist_append(gatt_services, gatt);
diff --git a/attrib/client.h b/attrib/client.h
index 650b0c1..b4a4ecc 100644
--- a/attrib/client.h
+++ b/attrib/client.h
@@ -23,5 +23,6 @@
  */
 
 int attrib_client_register(DBusConnection *connection,
-		struct btd_device *device, int psm, GSList *primaries);
+					struct btd_device *device, int psm,
+					GAttrib *attrib, GSList *primaries);
 void attrib_client_unregister(struct btd_device *device);
diff --git a/src/adapter.c b/src/adapter.c
index da13f69..bb9d9e2 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -58,6 +58,7 @@
 #include "storage.h"
 #include "attrib-server.h"
 #include "att.h"
+#include "gattrib.h"
 #include "attrib/client.h"
 
 /* Flags Descriptions */
@@ -2182,7 +2183,7 @@ static void create_stored_device_from_primary(char *key, char *value,
 	}
 
 	/* FIXME: Need the correct psm */
-	attrib_client_register(connection, device, -1, services);
+	attrib_client_register(connection, device, -1, NULL, services);
 
 	device_probe_drivers(device, uuids);
 
diff --git a/src/device.c b/src/device.c
index 511c9d0..fa5764e 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1548,7 +1548,7 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
 	device_probe_drivers(device, uuids);
 
 	/* FIXME: Need the correct psm */
-	attrib_client_register(req->conn, device, -1, services);
+	attrib_client_register(req->conn, device, -1, req->attrib, services);
 
 	g_slist_free(uuids);
 
-- 
1.7.4.1

[PATCH v2 6/6] Fix disconnecting when primary service discovery is done

[Vinicius Costa Gomes]

We shouldn't disconnect when the primary service discovery is done.
Most LE devices leave advertising mode as soon as the connection is
established, so shouldn't assume that we are able to reconnect to
them again.

For that we rely on the fact that GAttrib will close the connection
when its last reference is dropped.
---
 src/device.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/device.c b/src/device.c
index fa5764e..d567952 100644
--- a/src/device.c
+++ b/src/device.c
@@ -94,7 +94,6 @@ struct browse_req {
 	DBusConnection *conn;
 	DBusMessage *msg;
 	GAttrib *attrib;
-	GIOChannel *io;
 	struct btd_device *device;
 	GSList *match_uuids;
 	GSList *profiles_added;
@@ -166,11 +165,8 @@ static void browse_request_free(struct browse_req *req)
 	if (req->records)
 		sdp_list_free(req->records, (sdp_free_func_t) sdp_record_free);
 
-	if (req->io) {
+	if (req->attrib)
 		g_attrib_unref(req->attrib);
-		g_io_channel_unref(req->io);
-		g_io_channel_shutdown(req->io, FALSE, NULL);
-	}
 
 	g_free(req);
 }
@@ -1588,6 +1584,8 @@ static void gatt_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 	}
 
 	req->attrib = g_attrib_new(io);
+	g_io_channel_unref(io);
+
 	gatt_discover_primary(req->attrib, NULL, primary_cb, req);
 }
 
@@ -1597,6 +1595,7 @@ 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)
@@ -1609,14 +1608,14 @@ 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,
+	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, GATT_CID,
 				BT_IO_OPT_SEC_LEVEL, sec_level,
 				BT_IO_OPT_INVALID);
 
-	if (req->io == NULL ) {
+	if (io == NULL ) {
 		browse_request_free(req);
 		return -EIO;
 	}
-- 
1.7.4.1

Re: [PATCH v2 0/6] Attrib client move

[Johan Hedberg]

Hi Vinicius,

On Mon, Mar 28, 2011, Vinicius Costa Gomes wrote:
> Hi,
> 
> This patch series moves the Attrib client to the core, this move is
> more related to the build system than with code being moved.
> 
> Things left to be done (that will be done in later patches):
>  - Remove the attrib plugin;
>  - Use SDP information to register primary services;
> 
> --
> Cheers,
> 
> 
> Vinicius Costa Gomes (6):
>   Move Attrib client to the core
>   Register Attrib interface when loading device from storage
>   Register Attrib interface after Primary Service discovery
>   Remove _init and _exit methods from Attrib client
>   Add support for re-using the attrib channel
>   Fix disconnecting when primary service discovery is done
> 
>  Makefile.am      |    4 +-
>  attrib/client.c  |   49 +++++++++++++++---------------------------
>  attrib/client.h  |    6 ++--
>  attrib/main.c    |   14 +-----------
>  attrib/manager.c |   62 ++---------------------------------------------------
>  attrib/manager.h |    2 +-
>  src/adapter.c    |    5 ++++
>  src/device.c     |   20 +++++++++++------
>  8 files changed, 46 insertions(+), 116 deletions(-)

All patches in this set have been pushed upstream. Thanks.

Johan