[PATCH 00/18] Making GATT a first class citizen

[PATCH 00/18] Making GATT a first class citizen

[Vinicius Costa Gomes]

Hi,

These are the first steps in the direction of making GATT a first
class citizen in Bluez.

For this, the first step is to move the primary service discovery
procedure to the core bluetoothd. For that we needed to move a lot of
code around, and add a few features, for example, we now are able to
store the remote device type and restore it from storage, it is useful
when deciding which medium should be used for service discovery.

While doing this, I noticed something that could cause some confusion:
in the Attribute API, both the Device Service hierarchy
([prefix]/{hci0}/{device0}/{service0}) and the Device Characteristic
([prefix]/{hci0}/{device0}/{service0}/{characteristic0}) have the same
Interface name (org.bluez.Characteristic). Is that on purpose?


Cheers,
--

Sheldon Demario (1):
  Move primary service storage to device.c

Vinicius Costa Gomes (17):
  Fix attrib plugin deregistration
  Fix memory leaks in the attrib-server
  Add a way to store the remote device type
  Add support for storing the device type
  Add support for creating devices from stored types
  Add support for creating devices from stored primary services
  Add a way to retrieve ATT primary services
  Remove GATT information when the device is removed
  Add the btd_ prefix to device_add_service
  Remove duplicated code for discovering GATT services
  Clean up the primary service DBus registration
  Add support for making LE connections to GATT client
  Add a Discover method to the GATT Client
  Remove GetCharacteristics DBus method
  Add support for adding services to the Services property
  Add GetProperties method the Service Interface
  Add a "services" command to test-device

 attrib/att.c        |    5 +
 attrib/att.h        |    7 +
 attrib/client.c     |  449 ++++++++++-----------------------------------------
 attrib/gattrib.c    |    1 +
 input/device.c      |    4 +-
 plugins/hciops.c    |    2 +-
 serial/port.c       |    2 +
 src/adapter.c       |  105 ++++++++++++
 src/attrib-server.c |    6 +
 src/device.c        |   82 +++++++++-
 src/device.h        |    6 +-
 src/glib-helper.c   |   23 ++-
 src/storage.c       |   40 +++++
 src/storage.h       |    3 +
 test/test-device    |   13 ++
 15 files changed, 366 insertions(+), 382 deletions(-)

--
1.7.3.4

[PATCH 01/18] Fix attrib plugin deregistration

[Vinicius Costa Gomes]

As the comparison method used for find what to de-register was
wrong, it was causing the btd_device reference that the attrib
plugin was keeping never to be dropped.
---
 attrib/client.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 8e96af4..69e4fb8 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -167,7 +167,7 @@ static int gatt_dev_cmp(gconstpointer a, gconstpointer b)
 	const struct gatt_service *gatt = a;
 	const struct btd_device *dev = b;
 
-	return gatt->dev == dev;
+	return gatt->dev != dev;
 }
 
 static int characteristic_handle_cmp(gconstpointer a, gconstpointer b)
-- 
1.7.3.4

Re: [PATCH 01/18] Fix attrib plugin deregistration

[Johan Hedberg]

Hi Vinicius,

On Tue, Dec 21, 2010, Vinicius Costa Gomes wrote:
> As the comparison method used for find what to de-register was
> wrong, it was causing the btd_device reference that the attrib
> plugin was keeping never to be dropped.
> ---
>  attrib/client.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Pushed upstream. Thanks.

Johan

[PATCH 02/18] Fix memory leaks in the attrib-server

[Vinicius Costa Gomes]

---
 src/attrib-server.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 18759e7..cbc01ee 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -724,6 +724,7 @@ static void connect_event(GIOChannel *io, GError *err, void *user_data)
 
 	channel->attrib = g_attrib_new(io);
 	channel->mtu = ATT_DEFAULT_MTU;
+	g_io_channel_unref(io);
 
 	channel->id = g_attrib_register(channel->attrib, GATTRIB_ALL_EVENTS,
 				channel_handler, channel, NULL);
@@ -803,6 +804,11 @@ failed:
 	g_io_channel_unref(l2cap_io);
 	l2cap_io = NULL;
 
+	if (le_io) {
+		g_io_channel_unref(le_io);
+		le_io = NULL;
+	}
+
 	return -1;
 }
 
-- 
1.7.3.4

Re: [PATCH 02/18] Fix memory leaks in the attrib-server

[Johan Hedberg]

Hi Vinicius,

On Tue, Dec 21, 2010, Vinicius Costa Gomes wrote:
> ---
>  src/attrib-server.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)

Pushed upstream, though isn't the following bit kind of unnecessary. I
don't see how the code could end up there when le_io isn't NULL:

> @@ -803,6 +804,11 @@ failed:
>  	g_io_channel_unref(l2cap_io);
>  	l2cap_io = NULL;
>  
> +	if (le_io) {
> +		g_io_channel_unref(le_io);
> +		le_io = NULL;
> +	}
> +

Johan

[PATCH 03/18] Move primary service storage to device.c

[Vinicius Costa Gomes]

From: Sheldon Demario <sheldon.demario@openbossa.org>

Discover All Primary Services has beed moved to device.c in order
to follow a similar approach of BR/EDR service records.
---
 attrib/att.c      |    5 +++++
 attrib/att.h      |    7 +++++++
 attrib/client.c   |   41 -----------------------------------------
 attrib/gattrib.c  |    1 +
 src/device.c      |   44 +++++++++++++++++++++++++++++++++++++++++++-
 src/glib-helper.c |   24 ++++++++++++++++--------
 6 files changed, 72 insertions(+), 50 deletions(-)

diff --git a/attrib/att.c b/attrib/att.c
index f8dbc02..6c2d799 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -78,6 +78,11 @@ const char *att_ecode2str(uint8_t status)
 	}
 }
 
+void att_primary_free(struct att_primary *prim)
+{
+	g_free(prim);
+}
+
 void att_data_list_free(struct att_data_list *list)
 {
 	int i;
diff --git a/attrib/att.h b/attrib/att.h
index 0b8612e..17124d7 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -137,6 +137,12 @@ struct att_range {
 	uint16_t end;
 };
 
+struct att_primary {
+	char uuid[MAX_LEN_UUID_STR + 1];
+	uint16_t start;
+	uint16_t end;
+};
+
 /* These functions do byte conversion */
 static inline uint8_t att_get_u8(const void *ptr)
 {
@@ -172,6 +178,7 @@ static inline void att_put_u32(uint16_t src, void *dst)
 }
 
 void att_data_list_free(struct att_data_list *list);
+void att_primary_free(struct att_primary *prim);
 
 const char *att_ecode2str(uint8_t status);
 uint16_t enc_read_by_grp_req(uint16_t start, uint16_t end, uuid_t *uuid,
diff --git a/attrib/client.c b/attrib/client.c
index 69e4fb8..00d0bbc 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -1086,35 +1086,6 @@ static void load_attribute_data(char *key, char *value, void *data)
 		chr->format = attr_data_from_string(value + MAX_LEN_UUID_STR);
 }
 
-static char *primary_list_to_string(GSList *primary_list)
-{
-	GString *services;
-	GSList *l;
-
-	services = g_string_new(NULL);
-
-	for (l = primary_list; l; l = l->next) {
-		struct primary *primary = l->data;
-		uuid_t *uuid128;
-		char service[64];
-		char uuidstr[MAX_LEN_UUID_STR];
-
-		memset(service, 0, sizeof(service));
-
-		uuid128 = sdp_uuid_to_uuid128(&primary->uuid);
-		sdp_uuid2strn(uuid128, uuidstr, MAX_LEN_UUID_STR);
-
-		bt_free(uuid128);
-
-		snprintf(service, sizeof(service), "%04X#%04X#%s ",
-				primary->start, primary->end, uuidstr);
-
-		services = g_string_append(services, service);
-	}
-
-	return g_string_free(services, FALSE);
-}
-
 static GSList *string_to_primary_list(struct gatt_service *gatt,
 							const char *str)
 {
@@ -1158,17 +1129,6 @@ static GSList *string_to_primary_list(struct gatt_service *gatt,
 	return l;
 }
 
-static void store_primary_services(struct gatt_service *gatt)
-{
-       char *services;
-
-       services = primary_list_to_string(gatt->primary);
-
-       write_device_services(&gatt->sba, &gatt->dba, services);
-
-       g_free(services);
-}
-
 static gboolean load_primary_services(struct gatt_service *gatt)
 {
 	GSList *primary_list;
@@ -1225,7 +1185,6 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 		if (gatt->primary == NULL)
 			goto done;
 
-		store_primary_services(gatt);
 		register_primary(gatt);
 
 		g_slist_foreach(gatt->primary, discover_all_char, gatt);
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index eace01b..9268001 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -30,6 +30,7 @@
 
 #include <bluetooth/bluetooth.h>
 #include <bluetooth/sdp.h>
+#include <bluetooth/sdp_lib.h>
 
 #include "att.h"
 #include "btio.h"
diff --git a/src/device.c b/src/device.c
index c306e43..4bf9b52 100644
--- a/src/device.c
+++ b/src/device.c
@@ -45,6 +45,7 @@
 #include "log.h"
 #include "textfile.h"
 
+#include "att.h"
 #include "hcid.h"
 #include "adapter.h"
 #include "device.h"
@@ -1518,10 +1519,36 @@ static void init_browse(struct browse_req *req, gboolean reverse)
 						l->data);
 }
 
+static char *primary_list_to_string(GSList *primary_list)
+{
+	GString *services;
+	GSList *l;
+
+	services = g_string_new(NULL);
+
+	for (l = primary_list; l; l = l->next) {
+		struct att_primary *primary = l->data;
+		char service[64];
+
+		memset(service, 0, sizeof(service));
+
+		snprintf(service, sizeof(service), "%04X#%04X#%s ",
+				primary->start, primary->end, primary->uuid);
+
+		services = g_string_append(services, service);
+	}
+
+	return g_string_free(services, FALSE);
+}
+
 static void primary_cb(GSList *services, int err, gpointer user_data)
 {
 	struct browse_req *req = user_data;
 	struct btd_device *device = req->device;
+	struct btd_adapter *adapter = device->adapter;
+	GSList *l, *uuids = NULL;
+	bdaddr_t dba, sba;
+	char *str;
 
 	if (err) {
 		DBusMessage *reply;
@@ -1532,10 +1559,25 @@ static void primary_cb(GSList *services, int err, gpointer user_data)
 
 	services_changed(device);
 	device_set_temporary(device, FALSE);
-	device_probe_drivers(device, services);
+
+	for (l = services; l; l = l->next) {
+		struct att_primary *prim = l->data;
+		uuids = g_slist_append(uuids, prim->uuid);
+	}
+
+	device_probe_drivers(device, uuids);
+	g_slist_free(uuids);
 
 	create_device_reply(device, req);
 
+	str = primary_list_to_string(services);
+
+	adapter_get_address(adapter, &sba);
+	device_get_address(device, &dba);
+
+	write_device_services(&sba, &dba, str);
+	g_free(str);
+
 done:
 	device->browse = NULL;
 	browse_request_free(req);
diff --git a/src/glib-helper.c b/src/glib-helper.c
index bf39a83..4bd6a50 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -55,7 +55,7 @@ struct gattrib_context {
 	bt_primary_t cb;
 	bt_destroy_t destroy;
 	gpointer user_data;
-	GSList *uuids;
+	GSList *primaries;
 };
 
 static GSList *gattrib_list = NULL;
@@ -75,8 +75,8 @@ static void gattrib_context_free(struct gattrib_context *ctxt)
 	if (ctxt->destroy)
 		ctxt->destroy(ctxt->user_data);
 
-	g_slist_foreach(ctxt->uuids, (GFunc) g_free, NULL);
-	g_slist_free(ctxt->uuids);
+	g_slist_foreach(ctxt->primaries, (GFunc) att_primary_free, NULL);
+	g_slist_free(ctxt->primaries);
 	g_attrib_unref(ctxt->attrib);
 	if (ctxt->io) {
 		g_io_channel_unref(ctxt->io);
@@ -439,7 +439,7 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 	struct gattrib_context *ctxt = user_data;
 	struct att_data_list *list;
 	unsigned int i, err;
-	uint16_t end;
+	uint16_t start, end;
 
 	if (status == ATT_ECODE_ATTR_NOT_FOUND) {
 		err = 0;
@@ -459,9 +459,10 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 
 	for (i = 0, end = 0; i < list->num; i++) {
 		const uint8_t *data = list->data[i];
-		char *prim;
+		struct att_primary *primary;
 		uuid_t u128, u16;
 
+		start = att_get_u16(&data[0]);
 		end = att_get_u16(&data[2]);
 
 		if (list->len == 6) {
@@ -475,8 +476,15 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 			/* Skipping invalid data */
 			continue;
 
-		prim = bt_uuid2string(&u128);
-		ctxt->uuids = g_slist_append(ctxt->uuids, prim);
+		primary = g_try_new0(struct att_primary, 1);
+		if (!primary) {
+			err = -ENOMEM;
+			goto done;
+		}
+		primary->start = start;
+		primary->end = end;
+		sdp_uuid2strn(&u128, primary->uuid, sizeof(primary->uuid));
+		ctxt->primaries = g_slist_append(ctxt->primaries, primary);
 	}
 
 	att_data_list_free(list);
@@ -489,7 +497,7 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 	}
 
 done:
-	ctxt->cb(ctxt->uuids, err, ctxt->user_data);
+	ctxt->cb(ctxt->primaries, err, ctxt->user_data);
 	gattrib_context_free(ctxt);
 }
 
-- 
1.7.3.4

Re: [PATCH 03/18] Move primary service storage to device.c

[Johan Hedberg]

Hi,

On Tue, Dec 21, 2010, Vinicius Costa Gomes wrote:
> +void att_primary_free(struct att_primary *prim)
> +{
> +	g_free(prim);
> +}

<snip>

> +	g_slist_foreach(ctxt->primaries, (GFunc) att_primary_free, NULL);

If you're gonna do typecasting to GFunc (something which btw should be
avoided whenever possible) you might as well pass g_free directly and
not create any dedicated function for it.

Johan

[PATCH v2 3/18] Move primary service storage to device.c

[Sheldon Demario]

Discover All Primary Services has beed moved to device.c in order
to follow a similar approach of BR/EDR service records.
---
 attrib/att.h      |    6 ++++++
 attrib/client.c   |   41 -----------------------------------------
 attrib/gattrib.c  |    1 +
 src/device.c      |   44 +++++++++++++++++++++++++++++++++++++++++++-
 src/glib-helper.c |   24 ++++++++++++++++--------
 5 files changed, 66 insertions(+), 50 deletions(-)

diff --git a/attrib/att.h b/attrib/att.h
index 0b8612e..08feeec 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -137,6 +137,12 @@ struct att_range {
 	uint16_t end;
 };
 
+struct att_primary {
+	char uuid[MAX_LEN_UUID_STR + 1];
+	uint16_t start;
+	uint16_t end;
+};
+
 /* These functions do byte conversion */
 static inline uint8_t att_get_u8(const void *ptr)
 {
diff --git a/attrib/client.c b/attrib/client.c
index 69e4fb8..00d0bbc 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -1086,35 +1086,6 @@ static void load_attribute_data(char *key, char *value, void *data)
 		chr->format = attr_data_from_string(value + MAX_LEN_UUID_STR);
 }
 
-static char *primary_list_to_string(GSList *primary_list)
-{
-	GString *services;
-	GSList *l;
-
-	services = g_string_new(NULL);
-
-	for (l = primary_list; l; l = l->next) {
-		struct primary *primary = l->data;
-		uuid_t *uuid128;
-		char service[64];
-		char uuidstr[MAX_LEN_UUID_STR];
-
-		memset(service, 0, sizeof(service));
-
-		uuid128 = sdp_uuid_to_uuid128(&primary->uuid);
-		sdp_uuid2strn(uuid128, uuidstr, MAX_LEN_UUID_STR);
-
-		bt_free(uuid128);
-
-		snprintf(service, sizeof(service), "%04X#%04X#%s ",
-				primary->start, primary->end, uuidstr);
-
-		services = g_string_append(services, service);
-	}
-
-	return g_string_free(services, FALSE);
-}
-
 static GSList *string_to_primary_list(struct gatt_service *gatt,
 							const char *str)
 {
@@ -1158,17 +1129,6 @@ static GSList *string_to_primary_list(struct gatt_service *gatt,
 	return l;
 }
 
-static void store_primary_services(struct gatt_service *gatt)
-{
-       char *services;
-
-       services = primary_list_to_string(gatt->primary);
-
-       write_device_services(&gatt->sba, &gatt->dba, services);
-
-       g_free(services);
-}
-
 static gboolean load_primary_services(struct gatt_service *gatt)
 {
 	GSList *primary_list;
@@ -1225,7 +1185,6 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 		if (gatt->primary == NULL)
 			goto done;
 
-		store_primary_services(gatt);
 		register_primary(gatt);
 
 		g_slist_foreach(gatt->primary, discover_all_char, gatt);
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index eace01b..9268001 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -30,6 +30,7 @@
 
 #include <bluetooth/bluetooth.h>
 #include <bluetooth/sdp.h>
+#include <bluetooth/sdp_lib.h>
 
 #include "att.h"
 #include "btio.h"
diff --git a/src/device.c b/src/device.c
index c306e43..4bf9b52 100644
--- a/src/device.c
+++ b/src/device.c
@@ -45,6 +45,7 @@
 #include "log.h"
 #include "textfile.h"
 
+#include "att.h"
 #include "hcid.h"
 #include "adapter.h"
 #include "device.h"
@@ -1518,10 +1519,36 @@ static void init_browse(struct browse_req *req, gboolean reverse)
 						l->data);
 }
 
+static char *primary_list_to_string(GSList *primary_list)
+{
+	GString *services;
+	GSList *l;
+
+	services = g_string_new(NULL);
+
+	for (l = primary_list; l; l = l->next) {
+		struct att_primary *primary = l->data;
+		char service[64];
+
+		memset(service, 0, sizeof(service));
+
+		snprintf(service, sizeof(service), "%04X#%04X#%s ",
+				primary->start, primary->end, primary->uuid);
+
+		services = g_string_append(services, service);
+	}
+
+	return g_string_free(services, FALSE);
+}
+
 static void primary_cb(GSList *services, int err, gpointer user_data)
 {
 	struct browse_req *req = user_data;
 	struct btd_device *device = req->device;
+	struct btd_adapter *adapter = device->adapter;
+	GSList *l, *uuids = NULL;
+	bdaddr_t dba, sba;
+	char *str;
 
 	if (err) {
 		DBusMessage *reply;
@@ -1532,10 +1559,25 @@ static void primary_cb(GSList *services, int err, gpointer user_data)
 
 	services_changed(device);
 	device_set_temporary(device, FALSE);
-	device_probe_drivers(device, services);
+
+	for (l = services; l; l = l->next) {
+		struct att_primary *prim = l->data;
+		uuids = g_slist_append(uuids, prim->uuid);
+	}
+
+	device_probe_drivers(device, uuids);
+	g_slist_free(uuids);
 
 	create_device_reply(device, req);
 
+	str = primary_list_to_string(services);
+
+	adapter_get_address(adapter, &sba);
+	device_get_address(device, &dba);
+
+	write_device_services(&sba, &dba, str);
+	g_free(str);
+
 done:
 	device->browse = NULL;
 	browse_request_free(req);
diff --git a/src/glib-helper.c b/src/glib-helper.c
index bf39a83..60248cf 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -55,7 +55,7 @@ struct gattrib_context {
 	bt_primary_t cb;
 	bt_destroy_t destroy;
 	gpointer user_data;
-	GSList *uuids;
+	GSList *primaries;
 };
 
 static GSList *gattrib_list = NULL;
@@ -75,8 +75,8 @@ static void gattrib_context_free(struct gattrib_context *ctxt)
 	if (ctxt->destroy)
 		ctxt->destroy(ctxt->user_data);
 
-	g_slist_foreach(ctxt->uuids, (GFunc) g_free, NULL);
-	g_slist_free(ctxt->uuids);
+	g_slist_foreach(ctxt->primaries, (GFunc) g_free, NULL);
+	g_slist_free(ctxt->primaries);
 	g_attrib_unref(ctxt->attrib);
 	if (ctxt->io) {
 		g_io_channel_unref(ctxt->io);
@@ -439,7 +439,7 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 	struct gattrib_context *ctxt = user_data;
 	struct att_data_list *list;
 	unsigned int i, err;
-	uint16_t end;
+	uint16_t start, end;
 
 	if (status == ATT_ECODE_ATTR_NOT_FOUND) {
 		err = 0;
@@ -459,9 +459,10 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 
 	for (i = 0, end = 0; i < list->num; i++) {
 		const uint8_t *data = list->data[i];
-		char *prim;
+		struct att_primary *primary;
 		uuid_t u128, u16;
 
+		start = att_get_u16(&data[0]);
 		end = att_get_u16(&data[2]);
 
 		if (list->len == 6) {
@@ -475,8 +476,15 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 			/* Skipping invalid data */
 			continue;
 
-		prim = bt_uuid2string(&u128);
-		ctxt->uuids = g_slist_append(ctxt->uuids, prim);
+		primary = g_try_new0(struct att_primary, 1);
+		if (!primary) {
+			err = -ENOMEM;
+			goto done;
+		}
+		primary->start = start;
+		primary->end = end;
+		sdp_uuid2strn(&u128, primary->uuid, sizeof(primary->uuid));
+		ctxt->primaries = g_slist_append(ctxt->primaries, primary);
 	}
 
 	att_data_list_free(list);
@@ -489,7 +497,7 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
 	}
 
 done:
-	ctxt->cb(ctxt->uuids, err, ctxt->user_data);
+	ctxt->cb(ctxt->primaries, err, ctxt->user_data);
 	gattrib_context_free(ctxt);
 }
 
-- 
1.7.3.2

Re: [PATCH v2 3/18] Move primary service storage to device.c

[Johan Hedberg]

Hi Sheldon,

On Wed, Dec 22, 2010, Sheldon Demario wrote:
> Discover All Primary Services has beed moved to device.c in order
> to follow a similar approach of BR/EDR service records.
> ---
>  attrib/att.h      |    6 ++++++
>  attrib/client.c   |   41 -----------------------------------------
>  attrib/gattrib.c  |    1 +
>  src/device.c      |   44 +++++++++++++++++++++++++++++++++++++++++++-
>  src/glib-helper.c |   24 ++++++++++++++++--------
>  5 files changed, 66 insertions(+), 50 deletions(-)

Pushed upstream. Thanks.

Johan

[PATCH 04/18] Add a way to store the remote device type

[Vinicius Costa Gomes]

Because we need to know the device type (LE, Basic Rate or Dual Mode)
to be able to fully restore the device from storage, we have to store
and load this information to permanent storage.

Note: due to "device_type_t" usage in storage.h, some header includes
needed to be reordered in files which include storage.h.
---
 input/device.c   |    4 ++--
 plugins/hciops.c |    2 +-
 serial/port.c    |    2 ++
 src/storage.c    |   40 ++++++++++++++++++++++++++++++++++++++++
 src/storage.h    |    3 +++
 5 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/input/device.c b/input/device.c
index d735028..8a17966 100644
--- a/input/device.c
+++ b/input/device.c
@@ -46,11 +46,11 @@
 #include "textfile.h"
 #include "uinput.h"
 
-#include "../src/storage.h"
 #include "../src/adapter.h"
+#include "../src/device.h"
+#include "../src/storage.h"
 #include "../src/manager.h"
 #include "../src/dbus-common.h"
-#include "../src/device.h"
 
 #include "device.h"
 #include "error.h"
diff --git a/plugins/hciops.c b/plugins/hciops.c
index 131df1a..43e3d93 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -41,11 +41,11 @@
 #include "hcid.h"
 #include "sdpd.h"
 #include "adapter.h"
+#include "device.h"
 #include "plugin.h"
 #include "log.h"
 #include "storage.h"
 #include "event.h"
-#include "device.h"
 #include "manager.h"
 
 static int child_pipe[2] = { -1, -1 };
diff --git a/serial/port.c b/serial/port.c
index 233e317..7d56faa 100644
--- a/serial/port.c
+++ b/serial/port.c
@@ -53,6 +53,8 @@
 
 #include "error.h"
 #include "manager.h"
+#include "adapter.h"
+#include "device.h"
 #include "storage.h"
 #include "port.h"
 
diff --git a/src/storage.c b/src/storage.c
index 06b36f1..fb6e401 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -45,6 +45,8 @@
 #include <bluetooth/sdp_lib.h>
 
 #include "textfile.h"
+#include "adapter.h"
+#include "device.h"
 #include "glib-helper.h"
 #include "storage.h"
 
@@ -1391,3 +1393,41 @@ int read_device_attributes(const bdaddr_t *sba, textfile_cb func, void *data)
 
 	return textfile_foreach(filename, func, data);
 }
+
+int write_device_type(const bdaddr_t *sba, const bdaddr_t *dba,
+						device_type_t type)
+{
+	char filename[PATH_MAX + 1], addr[18], chars[3];
+
+	create_filename(filename, PATH_MAX, sba, "types");
+
+	create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+	ba2str(dba, addr);
+
+	snprintf(chars, sizeof(chars), "%2.2X", type);
+
+	return textfile_put(filename, addr, chars);
+}
+
+device_type_t read_device_type(const bdaddr_t *sba, const bdaddr_t *dba)
+{
+	char filename[PATH_MAX + 1], addr[18], *chars;
+	device_type_t type;
+
+	create_filename(filename, PATH_MAX, sba, "types");
+
+	create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+	ba2str(dba, addr);
+
+	chars = textfile_caseget(filename, addr);
+	if (chars == NULL)
+		return DEVICE_TYPE_UNKNOWN;
+
+	type = strtol(chars, NULL, 16);
+
+	free(chars);
+
+	return type;
+}
diff --git a/src/storage.h b/src/storage.h
index c7e342c..f36cefb 100644
--- a/src/storage.h
+++ b/src/storage.h
@@ -91,6 +91,9 @@ char *read_device_characteristics(const bdaddr_t *sba, const bdaddr_t *dba,
 int write_device_attribute(const bdaddr_t *sba, const bdaddr_t *dba,
                                         uint16_t handle, const char *chars);
 int read_device_attributes(const bdaddr_t *sba, textfile_cb func, void *data);
+int write_device_type(const bdaddr_t *sba, const bdaddr_t *dba,
+						device_type_t type);
+device_type_t read_device_type(const bdaddr_t *sba, const bdaddr_t *dba);
 
 #define PNP_UUID		"00001200-0000-1000-8000-00805f9b34fb"
 
-- 
1.7.3.4

Re: [PATCH 04/18] Add a way to store the remote device type

[Johan Hedberg]

Hi Vinicius,

On Tue, Dec 21, 2010, Vinicius Costa Gomes wrote:
> Because we need to know the device type (LE, Basic Rate or Dual Mode)
> to be able to fully restore the device from storage, we have to store
> and load this information to permanent storage.
> 
> Note: due to "device_type_t" usage in storage.h, some header includes
> needed to be reordered in files which include storage.h.
> ---
>  input/device.c   |    4 ++--
>  plugins/hciops.c |    2 +-
>  serial/port.c    |    2 ++
>  src/storage.c    |   40 ++++++++++++++++++++++++++++++++++++++++
>  src/storage.h    |    3 +++
>  5 files changed, 48 insertions(+), 3 deletions(-)

Pushed upstream. Thanks.

Johan

[PATCH 05/18] Add support for storing the device type

[Vinicius Costa Gomes]

When the service discovery (SDP or GATT) is finished, write the device
type so it can be retrieved from storage when needed.
---
 src/device.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/src/device.c b/src/device.c
index 4bf9b52..ca2c5bc 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1463,8 +1463,16 @@ send_reply:
 	}
 
 cleanup:
-	if (!device->temporary)
+	if (!device->temporary) {
+		bdaddr_t sba, dba;
+
+		adapter_get_address(device->adapter, &sba);
+		device_get_address(device, &dba);
+
 		store_profiles(device);
+		write_device_type(&sba, &dba, device->type);
+	}
+
 	device->browse = NULL;
 	browse_request_free(req);
 }
@@ -1575,6 +1583,7 @@ static void primary_cb(GSList *services, int err, gpointer user_data)
 	adapter_get_address(adapter, &sba);
 	device_get_address(device, &dba);
 
+	write_device_type(&sba, &dba, device->type);
 	write_device_services(&sba, &dba, str);
 	g_free(str);
 
-- 
1.7.3.4

Re: [PATCH 05/18] Add support for storing the device type

[Johan Hedberg]

Hi Vinicius,

On Tue, Dec 21, 2010, Vinicius Costa Gomes wrote:
> When the service discovery (SDP or GATT) is finished, write the device
> type so it can be retrieved from storage when needed.
> ---
>  src/device.c |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)

This one doesn't apply, probably because I didn't apply 3/18. Please
resend a fixed 3/18 and then I'll proceed with processing this patch
set.

Johan

[PATCH 06/18] Add support for creating devices from stored types

[Vinicius Costa Gomes]

This adds a way to restore devices from their types.
---
 src/adapter.c |   28 ++++++++++++++++++++++++++++
 src/device.c  |    8 ++++++++
 src/device.h  |    1 +
 3 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index ea81722..9c57bea 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1906,6 +1906,31 @@ static void create_stored_device_from_blocked(char *key, char *value,
 	}
 }
 
+static void create_stored_device_from_types(char *key, char *value,
+							void *user_data)
+{
+	GSList *l;
+	struct btd_adapter *adapter = user_data;
+	struct btd_device *device;
+	uint8_t type;
+
+	type = strtol(value, NULL, 16);
+
+	l = g_slist_find_custom(adapter->devices,
+				key, (GCompareFunc) device_address_cmp);
+	if (l) {
+		device = l->data;
+		device_set_type(device, type);
+		return;
+	}
+
+	device = device_create(connection, adapter, key, type);
+	if (device) {
+		device_set_temporary(device, FALSE);
+		adapter->devices = g_slist_append(adapter->devices, device);
+	}
+}
+
 static void load_devices(struct btd_adapter *adapter)
 {
 	char filename[PATH_MAX + 1];
@@ -1933,6 +1958,9 @@ static void load_devices(struct btd_adapter *adapter)
 
 	create_name(filename, PATH_MAX, STORAGEDIR, srcaddr, "blocked");
 	textfile_foreach(filename, create_stored_device_from_blocked, adapter);
+
+	create_name(filename, PATH_MAX, STORAGEDIR, srcaddr, "types");
+	textfile_foreach(filename, create_stored_device_from_types, adapter);
 }
 
 int btd_adapter_block_address(struct btd_adapter *adapter, bdaddr_t *bdaddr)
diff --git a/src/device.c b/src/device.c
index ca2c5bc..d67f804 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1767,6 +1767,14 @@ uint8_t device_get_auth(struct btd_device *device)
 	return device->auth;
 }
 
+void device_set_type(struct btd_device *device, device_type_t type)
+{
+	if (!device)
+		return;
+
+	device->type = type;
+}
+
 static gboolean start_discovery(gpointer user_data)
 {
 	struct btd_device *device = user_data;
diff --git a/src/device.h b/src/device.h
index 74b968c..7820636 100644
--- a/src/device.h
+++ b/src/device.h
@@ -66,6 +66,7 @@ gboolean device_is_trusted(struct btd_device *device);
 void device_set_paired(struct btd_device *device, gboolean paired);
 void device_set_temporary(struct btd_device *device, gboolean temporary);
 void device_set_cap(struct btd_device *device, uint8_t cap);
+void device_set_type(struct btd_device *device, device_type_t type);
 uint8_t device_get_cap(struct btd_device *device);
 void device_set_auth(struct btd_device *device, uint8_t auth);
 uint8_t device_get_auth(struct btd_device *device);
-- 
1.7.3.4

[PATCH 07/18] Add support for creating devices from stored primary services

[Vinicius Costa Gomes]

>From what we can retrieve from storage we are able to create the devices
and probe the device drivers.
---
 src/adapter.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 77 insertions(+), 0 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 9c57bea..3d5fafc 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -55,6 +55,7 @@
 #include "glib-helper.h"
 #include "agent.h"
 #include "storage.h"
+#include "att.h"
 
 /* Flags Descriptions */
 #define EIR_LIM_DISC                0x01 /* LE Limited Discoverable Mode */
@@ -1931,6 +1932,78 @@ static void create_stored_device_from_types(char *key, char *value,
 	}
 }
 
+static GSList *string_to_primary_list(char *str)
+{
+	GSList *l = NULL;
+	char **services;
+	int i;
+
+	if (str == NULL)
+		return NULL;
+
+	services = g_strsplit(str, " ", 0);
+	if (services == NULL)
+		return NULL;
+
+	for (i = 0; services[i]; i++) {
+		struct att_primary *prim;
+		int ret;
+
+		prim = g_new0(struct att_primary, 1);
+
+		ret = sscanf(services[i], "%04hX#%04hX#%s", &prim->start,
+							&prim->end, prim->uuid);
+
+		if (ret < 3) {
+			att_primary_free(prim);
+			continue;
+		}
+
+		l = g_slist_append(l, prim);
+	}
+
+	g_strfreev(services);
+
+	return l;
+}
+
+static void create_stored_device_from_primary(char *key, char *value,
+							void *user_data)
+{
+	struct btd_adapter *adapter = user_data;
+	struct btd_device *device;
+	GSList *services, *uuids, *l;
+
+	l = g_slist_find_custom(adapter->devices,
+				key, (GCompareFunc) device_address_cmp);
+	if (l)
+		device = l->data;
+	else {
+		device = device_create(connection, adapter, key, DEVICE_TYPE_BREDR);
+		if (!device)
+			return;
+
+		device_set_temporary(device, FALSE);
+		adapter->devices = g_slist_append(adapter->devices, device);
+	}
+
+	services = string_to_primary_list(value);
+	if (services == NULL)
+		return;
+
+	for (l = services, uuids = NULL; l; l = l->next) {
+		struct att_primary *prim = l->data;
+		uuids = g_slist_append(uuids, prim->uuid);
+	}
+
+	device_probe_drivers(device, uuids);
+
+	g_slist_free(uuids);
+
+	g_slist_foreach(services, (GFunc) att_primary_free, NULL);
+	g_slist_free(services);
+}
+
 static void load_devices(struct btd_adapter *adapter)
 {
 	char filename[PATH_MAX + 1];
@@ -1944,6 +2017,10 @@ static void load_devices(struct btd_adapter *adapter)
 	textfile_foreach(filename, create_stored_device_from_profiles,
 								adapter);
 
+	create_name(filename, PATH_MAX, STORAGEDIR, srcaddr, "primary");
+	textfile_foreach(filename, create_stored_device_from_primary,
+								adapter);
+
 	create_name(filename, PATH_MAX, STORAGEDIR, srcaddr, "linkkeys");
 	textfile_foreach(filename, create_stored_device_from_linkkeys, &keys);
 
-- 
1.7.3.4

Re: [PATCH 07/18] Add support for creating devices from stored primary services

[Johan Hedberg]

Hi Vinicius,

On Tue, Dec 21, 2010, Vinicius Costa Gomes wrote:
> From what we can retrieve from storage we are able to create the devices
> and probe the device drivers.
> ---
>  src/adapter.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 77 insertions(+), 0 deletions(-)

Patches 5 and 6 have been pushed upstream, but this one doesn't compile:

src/adapter.c: In function 'string_to_primary_list':
src/adapter.c:1958: error: implicit declaration of function 'att_primary_free'
src/adapter.c: In function 'create_stored_device_from_primary':
src/adapter.c:2003: error: 'att_primary_free' undeclared (first use in this function)
src/adapter.c:2003: error: (Each undeclared identifier is reported only once
src/adapter.c:2003: error: for each function it appears in.)
make[1]: *** [src/adapter.o] Error 1

Please fix and resubmit.

Johan

[PATCH] Add support for creating devices from stored primary services

[Vinicius Costa Gomes]

>From what we can retrieve from storage we are able to create the devices
and probe the device drivers.
---
 src/adapter.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 77 insertions(+), 0 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 9c57bea..77a04f0 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -55,6 +55,7 @@
 #include "glib-helper.h"
 #include "agent.h"
 #include "storage.h"
+#include "att.h"
 
 /* Flags Descriptions */
 #define EIR_LIM_DISC                0x01 /* LE Limited Discoverable Mode */
@@ -1931,6 +1932,78 @@ static void create_stored_device_from_types(char *key, char *value,
 	}
 }
 
+static GSList *string_to_primary_list(char *str)
+{
+	GSList *l = NULL;
+	char **services;
+	int i;
+
+	if (str == NULL)
+		return NULL;
+
+	services = g_strsplit(str, " ", 0);
+	if (services == NULL)
+		return NULL;
+
+	for (i = 0; services[i]; i++) {
+		struct att_primary *prim;
+		int ret;
+
+		prim = g_new0(struct att_primary, 1);
+
+		ret = sscanf(services[i], "%04hX#%04hX#%s", &prim->start,
+							&prim->end, prim->uuid);
+
+		if (ret < 3) {
+			g_free(prim);
+			continue;
+		}
+
+		l = g_slist_append(l, prim);
+	}
+
+	g_strfreev(services);
+
+	return l;
+}
+
+static void create_stored_device_from_primary(char *key, char *value,
+							void *user_data)
+{
+	struct btd_adapter *adapter = user_data;
+	struct btd_device *device;
+	GSList *services, *uuids, *l;
+
+	l = g_slist_find_custom(adapter->devices,
+				key, (GCompareFunc) device_address_cmp);
+	if (l)
+		device = l->data;
+	else {
+		device = device_create(connection, adapter, key, DEVICE_TYPE_BREDR);
+		if (!device)
+			return;
+
+		device_set_temporary(device, FALSE);
+		adapter->devices = g_slist_append(adapter->devices, device);
+	}
+
+	services = string_to_primary_list(value);
+	if (services == NULL)
+		return;
+
+	for (l = services, uuids = NULL; l; l = l->next) {
+		struct att_primary *prim = l->data;
+		uuids = g_slist_append(uuids, prim->uuid);
+	}
+
+	device_probe_drivers(device, uuids);
+
+	g_slist_free(uuids);
+
+	g_slist_foreach(services, (GFunc) g_free, NULL);
+	g_slist_free(services);
+}
+
 static void load_devices(struct btd_adapter *adapter)
 {
 	char filename[PATH_MAX + 1];
@@ -1944,6 +2017,10 @@ static void load_devices(struct btd_adapter *adapter)
 	textfile_foreach(filename, create_stored_device_from_profiles,
 								adapter);
 
+	create_name(filename, PATH_MAX, STORAGEDIR, srcaddr, "primary");
+	textfile_foreach(filename, create_stored_device_from_primary,
+								adapter);
+
 	create_name(filename, PATH_MAX, STORAGEDIR, srcaddr, "linkkeys");
 	textfile_foreach(filename, create_stored_device_from_linkkeys, &keys);
 
-- 
1.7.3.4

Re: [PATCH] Add support for creating devices from stored primary services

[Johan Hedberg]

Hi Vinicius,

On Wed, Dec 22, 2010, Vinicius Costa Gomes wrote:
> From what we can retrieve from storage we are able to create the devices
> and probe the device drivers.
> ---
>  src/adapter.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 77 insertions(+), 0 deletions(-)

Pushed upstream. Thanks.

Johan

[PATCH 08/18] Add a way to retrieve ATT primary services

[Vinicius Costa Gomes]

As the primary services were discovered by the core bluetoothd, we need
a way to export that information.

The service discovery uses the same primary list as the device, there's no
need to free that list when the discovery finishes. That list will be removed
when the device is free'd.
---
 src/adapter.c     |    6 +++---
 src/device.c      |   15 +++++++++++++++
 src/device.h      |    3 +++
 src/glib-helper.c |    1 -
 4 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 3d5fafc..e48f1fd 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1994,14 +1994,14 @@ static void create_stored_device_from_primary(char *key, char *value,
 	for (l = services, uuids = NULL; l; l = l->next) {
 		struct att_primary *prim = l->data;
 		uuids = g_slist_append(uuids, prim->uuid);
+
+		device_add_primary(device, prim);
 	}
 
 	device_probe_drivers(device, uuids);
 
-	g_slist_free(uuids);
-
-	g_slist_foreach(services, (GFunc) att_primary_free, NULL);
 	g_slist_free(services);
+	g_slist_free(uuids);
 }
 
 static void load_devices(struct btd_adapter *adapter)
diff --git a/src/device.c b/src/device.c
index d67f804..627db8f 100644
--- a/src/device.c
+++ b/src/device.c
@@ -115,6 +115,7 @@ struct btd_device {
 	struct btd_adapter	*adapter;
 	GSList		*uuids;
 	GSList		*services;		/* Primary services path */
+	GSList		*primaries;		/* List of primary services */
 	GSList		*drivers;		/* List of driver_data */
 	GSList		*watches;		/* List of disconnect_data */
 	gboolean	temporary;
@@ -209,6 +210,9 @@ static void device_free(gpointer user_data)
 	g_slist_foreach(device->uuids, (GFunc) g_free, NULL);
 	g_slist_free(device->uuids);
 
+	g_slist_foreach(device->primaries, (GFunc) att_primary_free, NULL);
+	g_slist_free(device->primaries);
+
 	if (device->tmp_records)
 		sdp_list_free(device->tmp_records,
 					(sdp_free_func_t) sdp_record_free);
@@ -1571,6 +1575,7 @@ static void primary_cb(GSList *services, int err, gpointer user_data)
 	for (l = services; l; l = l->next) {
 		struct att_primary *prim = l->data;
 		uuids = g_slist_append(uuids, prim->uuid);
+		device_add_primary(device, prim);
 	}
 
 	device_probe_drivers(device, uuids);
@@ -2383,6 +2388,16 @@ void device_add_service(struct btd_device *device, const char *path)
 	device->services = g_slist_append(device->services, g_strdup(path));
 }
 
+void device_add_primary(struct btd_device *device, struct att_primary *prim)
+{
+	device->primaries = g_slist_append(device->primaries, prim);
+}
+
+GSList *btd_device_get_primaries(struct btd_device *device)
+{
+	return device->primaries;
+}
+
 void btd_device_add_uuid(struct btd_device *device, const char *uuid)
 {
 	GSList *uuid_list;
diff --git a/src/device.h b/src/device.h
index 7820636..0bd6fff 100644
--- a/src/device.h
+++ b/src/device.h
@@ -25,6 +25,7 @@
 #define DEVICE_INTERFACE	"org.bluez.Device"
 
 struct btd_device;
+struct att_primary;
 
 typedef enum {
 	AUTH_TYPE_PINCODE,
@@ -53,7 +54,9 @@ int device_browse(struct btd_device *device, DBusConnection *conn,
 void device_probe_drivers(struct btd_device *device, GSList *profiles);
 const sdp_record_t *btd_device_get_record(struct btd_device *device,
 						const char *uuid);
+GSList *btd_device_get_primaries(struct btd_device *device);
 void device_add_service(struct btd_device *device, const char *path);
+void device_add_primary(struct btd_device *device, struct att_primary *prim);
 void btd_device_add_uuid(struct btd_device *device, const char *uuid);
 struct btd_adapter *device_get_adapter(struct btd_device *device);
 void device_get_address(struct btd_device *device, bdaddr_t *bdaddr);
diff --git a/src/glib-helper.c b/src/glib-helper.c
index 4bd6a50..648dd62 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -75,7 +75,6 @@ static void gattrib_context_free(struct gattrib_context *ctxt)
 	if (ctxt->destroy)
 		ctxt->destroy(ctxt->user_data);
 
-	g_slist_foreach(ctxt->primaries, (GFunc) att_primary_free, NULL);
 	g_slist_free(ctxt->primaries);
 	g_attrib_unref(ctxt->attrib);
 	if (ctxt->io) {
-- 
1.7.3.4

Re: [PATCH 08/18] Add a way to retrieve ATT primary services

[Johan Hedberg]

Hi Vinicius,

On Tue, Dec 21, 2010, Vinicius Costa Gomes wrote:
> As the primary services were discovered by the core bluetoothd, we need
> a way to export that information.
> 
> The service discovery uses the same primary list as the device, there's no
> need to free that list when the discovery finishes. That list will be removed
> when the device is free'd.
> ---
>  src/adapter.c     |    6 +++---
>  src/device.c      |   15 +++++++++++++++
>  src/device.h      |    3 +++
>  src/glib-helper.c |    1 -
>  4 files changed, 21 insertions(+), 4 deletions(-)

I tried to continue applying the patches starting with this one, but it
doesn't apply cleanly anymore probably due to me skipping patch 7 with
its compilation errors. So I'll once again stop processing the patch set
here.

Johan

[PATCH] Add a way to retrieve ATT primary services

[Vinicius Costa Gomes]

As the primary services were discovered by the core bluetoothd, we need
a way to export that information.

The service discovery uses the same primary list as the device, there's no
need to free that list when the discovery finishes. That list will be removed
when the device is free'd.
---
 src/adapter.c     |    6 +++---
 src/device.c      |   15 +++++++++++++++
 src/device.h      |    3 +++
 src/glib-helper.c |    1 -
 4 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 77a04f0..ff1f0a4 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1994,14 +1994,14 @@ static void create_stored_device_from_primary(char *key, char *value,
 	for (l = services, uuids = NULL; l; l = l->next) {
 		struct att_primary *prim = l->data;
 		uuids = g_slist_append(uuids, prim->uuid);
+
+		device_add_primary(device, prim);
 	}
 
 	device_probe_drivers(device, uuids);
 
-	g_slist_free(uuids);
-
-	g_slist_foreach(services, (GFunc) g_free, NULL);
 	g_slist_free(services);
+	g_slist_free(uuids);
 }
 
 static void load_devices(struct btd_adapter *adapter)
diff --git a/src/device.c b/src/device.c
index d67f804..ae399f3 100644
--- a/src/device.c
+++ b/src/device.c
@@ -115,6 +115,7 @@ struct btd_device {
 	struct btd_adapter	*adapter;
 	GSList		*uuids;
 	GSList		*services;		/* Primary services path */
+	GSList		*primaries;		/* List of primary services */
 	GSList		*drivers;		/* List of driver_data */
 	GSList		*watches;		/* List of disconnect_data */
 	gboolean	temporary;
@@ -209,6 +210,9 @@ static void device_free(gpointer user_data)
 	g_slist_foreach(device->uuids, (GFunc) g_free, NULL);
 	g_slist_free(device->uuids);
 
+	g_slist_foreach(device->primaries, (GFunc) g_free, NULL);
+	g_slist_free(device->primaries);
+
 	if (device->tmp_records)
 		sdp_list_free(device->tmp_records,
 					(sdp_free_func_t) sdp_record_free);
@@ -1571,6 +1575,7 @@ static void primary_cb(GSList *services, int err, gpointer user_data)
 	for (l = services; l; l = l->next) {
 		struct att_primary *prim = l->data;
 		uuids = g_slist_append(uuids, prim->uuid);
+		device_add_primary(device, prim);
 	}
 
 	device_probe_drivers(device, uuids);
@@ -2383,6 +2388,16 @@ void device_add_service(struct btd_device *device, const char *path)
 	device->services = g_slist_append(device->services, g_strdup(path));
 }
 
+void device_add_primary(struct btd_device *device, struct att_primary *prim)
+{
+	device->primaries = g_slist_append(device->primaries, prim);
+}
+
+GSList *btd_device_get_primaries(struct btd_device *device)
+{
+	return device->primaries;
+}
+
 void btd_device_add_uuid(struct btd_device *device, const char *uuid)
 {
 	GSList *uuid_list;
diff --git a/src/device.h b/src/device.h
index 7820636..0bd6fff 100644
--- a/src/device.h
+++ b/src/device.h
@@ -25,6 +25,7 @@
 #define DEVICE_INTERFACE	"org.bluez.Device"
 
 struct btd_device;
+struct att_primary;
 
 typedef enum {
 	AUTH_TYPE_PINCODE,
@@ -53,7 +54,9 @@ int device_browse(struct btd_device *device, DBusConnection *conn,
 void device_probe_drivers(struct btd_device *device, GSList *profiles);
 const sdp_record_t *btd_device_get_record(struct btd_device *device,
 						const char *uuid);
+GSList *btd_device_get_primaries(struct btd_device *device);
 void device_add_service(struct btd_device *device, const char *path);
+void device_add_primary(struct btd_device *device, struct att_primary *prim);
 void btd_device_add_uuid(struct btd_device *device, const char *uuid);
 struct btd_adapter *device_get_adapter(struct btd_device *device);
 void device_get_address(struct btd_device *device, bdaddr_t *bdaddr);
diff --git a/src/glib-helper.c b/src/glib-helper.c
index 60248cf..648dd62 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -75,7 +75,6 @@ static void gattrib_context_free(struct gattrib_context *ctxt)
 	if (ctxt->destroy)
 		ctxt->destroy(ctxt->user_data);
 
-	g_slist_foreach(ctxt->primaries, (GFunc) g_free, NULL);
 	g_slist_free(ctxt->primaries);
 	g_attrib_unref(ctxt->attrib);
 	if (ctxt->io) {
-- 
1.7.3.4

Re: [PATCH] Add a way to retrieve ATT primary services

[Johan Hedberg]

Hi Vinicius,

On Wed, Dec 22, 2010, Vinicius Costa Gomes wrote:
> As the primary services were discovered by the core bluetoothd, we need
> a way to export that information.
> 
> The service discovery uses the same primary list as the device, there's no
> need to free that list when the discovery finishes. That list will be removed
> when the device is free'd.
> ---
>  src/adapter.c     |    6 +++---
>  src/device.c      |   15 +++++++++++++++
>  src/device.h      |    3 +++
>  src/glib-helper.c |    1 -
>  4 files changed, 21 insertions(+), 4 deletions(-)

This one has also been pushed upstream. Thanks.

Johan

[PATCH 09/18] Remove GATT information when the device is removed

[Vinicius Costa Gomes]

This adds a way to remove the information about the device type and its
primary services when the device is going to be removed from the permanent
storage.
---
 src/device.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/device.c b/src/device.c
index 627db8f..f123288 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1024,6 +1024,8 @@ static void device_remove_stored(struct btd_device *device)
 		device_remove_bonding(device);
 	delete_entry(&src, "profiles", addr);
 	delete_entry(&src, "trusts", addr);
+	delete_entry(&src, "types", addr);
+	delete_entry(&src, "primary", addr);
 	delete_all_records(&src, &device->bdaddr);
 	delete_device_service(&src, &device->bdaddr);
 
-- 
1.7.3.4

[PATCH 10/18] Add the btd_ prefix to device_add_service

[Vinicius Costa Gomes]

This is needed to keep consistency, as device_add_service would be used
from outside the core bluetoothd.
---
 attrib/client.c |    2 +-
 src/device.c    |    2 +-
 src/device.h    |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 00d0bbc..dbfe5d3 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -606,7 +606,7 @@ static void register_primary(struct gatt_service *gatt)
 				NULL, NULL, prim, NULL);
 		DBG("Registered: %s", prim->path);
 
-		device_add_service(gatt->dev, prim->path);
+		btd_device_add_service(gatt->dev, prim->path);
 	}
 }
 
diff --git a/src/device.c b/src/device.c
index f123288..9d98bf8 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2382,7 +2382,7 @@ void device_set_renewed_key(struct btd_device *device, gboolean renewed)
 	device->renewed_key = renewed;
 }
 
-void device_add_service(struct btd_device *device, const char *path)
+void btd_device_add_service(struct btd_device *device, const char *path)
 {
 	if (g_slist_find_custom(device->services, path, (GCompareFunc) strcmp))
 		return;
diff --git a/src/device.h b/src/device.h
index 0bd6fff..86721bf 100644
--- a/src/device.h
+++ b/src/device.h
@@ -55,7 +55,7 @@ void device_probe_drivers(struct btd_device *device, GSList *profiles);
 const sdp_record_t *btd_device_get_record(struct btd_device *device,
 						const char *uuid);
 GSList *btd_device_get_primaries(struct btd_device *device);
-void device_add_service(struct btd_device *device, const char *path);
+void btd_device_add_service(struct btd_device *device, const char *path);
 void device_add_primary(struct btd_device *device, struct att_primary *prim);
 void btd_device_add_uuid(struct btd_device *device, const char *uuid);
 struct btd_adapter *device_get_adapter(struct btd_device *device);
-- 
1.7.3.4

[PATCH 11/18] Remove duplicated code for discovering GATT services

[Vinicius Costa Gomes]

Most of this functionality was moved to src/device.c. Since that move
this code doesn't work. Some ugly hack were added to avoid breaking
the build.
---
 attrib/client.c |  101 ++----------------------------------------------------
 1 files changed, 4 insertions(+), 97 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index dbfe5d3..9fcaaf8 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -60,7 +60,6 @@ struct gatt_service {
 	GSList *primary;
 	GAttrib *attrib;
 	int psm;
-	guint atid;
 	gboolean listen;
 };
 
@@ -309,9 +308,6 @@ static void events_handler(const uint8_t *pdu, uint16_t len,
 	}
 }
 
-static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
-							gpointer user_data);
-
 static void attrib_destroy(gpointer user_data)
 {
 	struct gatt_service *gatt = user_data;
@@ -330,7 +326,6 @@ static void attrib_disconnect(gpointer user_data)
 static void connect_cb(GIOChannel *chan, GError *gerr, gpointer user_data)
 {
 	struct gatt_service *gatt = user_data;
-	guint atid;
 
 	if (gerr) {
 		error("%s", gerr->message);
@@ -351,13 +346,6 @@ static void connect_cb(GIOChannel *chan, GError *gerr, gpointer user_data)
 		return;
 	}
 
-	atid = gatt_discover_primary(gatt->attrib, 0x0001, 0xffff, NULL,
-							primary_cb, gatt);
-	if (atid == 0)
-		goto fail;
-
-	gatt->atid = atid;
-
 	return;
 fail:
 	g_attrib_unref(gatt->attrib);
@@ -1173,91 +1161,6 @@ static void discover_all_char(gpointer data, gpointer user_data)
 						char_discovered_cb, qchr);
 }
 
-static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
-							gpointer user_data)
-{
-	struct gatt_service *gatt = user_data;
-	struct att_data_list *list;
-	unsigned int i;
-	uint16_t end, start;
-
-	if (status == ATT_ECODE_ATTR_NOT_FOUND) {
-		if (gatt->primary == NULL)
-			goto done;
-
-		register_primary(gatt);
-
-		g_slist_foreach(gatt->primary, discover_all_char, gatt);
-		goto done;
-	}
-
-	if (status != 0) {
-		error("Discover all primary services failed: %s",
-						att_ecode2str(status));
-		goto done;
-	}
-
-	list = dec_read_by_grp_resp(pdu, plen);
-	if (list == NULL) {
-		error("Protocol error");
-		goto done;
-	}
-
-	DBG("Read by Group Type Response received");
-
-	for (i = 0, end = 0; i < list->num; i++) {
-		struct primary *prim;
-		uint8_t *info = list->data[i];
-
-		/* Each element contains: attribute handle, end group handle
-		 * and attribute value */
-		start = att_get_u16(info);
-		end = att_get_u16(&info[2]);
-
-		prim = g_new0(struct primary, 1);
-		prim->gatt = gatt;
-		prim->start = start;
-		prim->end = end;
-
-		if (list->len == 6) {
-			sdp_uuid16_create(&prim->uuid,
-					att_get_u16(&info[4]));
-
-		} else if (list->len == 20) {
-			/* FIXME: endianness */
-			sdp_uuid128_create(&prim->uuid, &info[4]);
-		} else {
-			DBG("ATT: Invalid Length field");
-			g_free(prim);
-			att_data_list_free(list);
-			goto done;
-		}
-
-		prim->path = g_strdup_printf("%s/service%04x", gatt->path,
-								prim->start);
-
-		gatt->primary = g_slist_append(gatt->primary, prim);
-	}
-
-	att_data_list_free(list);
-
-	if (end == 0) {
-		DBG("ATT: Invalid PDU format");
-		goto done;
-	}
-
-	/*
-	 * Discover all primary services sub-procedure shall send another
-	 * Read by Group Type Request until Error Response is received and
-	 * the Error Code is set to Attribute Not Found.
-	 */
-	gatt->attrib = g_attrib_ref(gatt->attrib);
-	gatt->atid = gatt_discover_primary(gatt->attrib, end + 1, 0xffff, NULL,
-							primary_cb, gatt);
-done:
-	g_attrib_unref(gatt->attrib);
-}
-
 int attrib_client_register(struct btd_device *device, int psm)
 {
 	struct btd_adapter *adapter = device_get_adapter(device);
@@ -1279,6 +1182,10 @@ int attrib_client_register(struct btd_device *device, int psm)
 	if (load_primary_services(gatt))
 		DBG("Primary services loaded");
 
+	/* FIXME: just to avoid breaking the build */
+	if (FALSE)
+		discover_all_char(NULL, NULL);
+
 	gatt_services = g_slist_append(gatt_services, gatt);
 
 	return 0;
-- 
1.7.3.4

[PATCH 12/18] Clean up the primary service DBus registration

[Vinicius Costa Gomes]

Now, that much of this funcionality was moved to the core, we can go
straight to the registration of DBus interfaces.
---
 attrib/client.c |  223 ++++++++-----------------------------------------------
 1 files changed, 32 insertions(+), 191 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 9fcaaf8..0eadf1e 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -73,10 +73,8 @@ struct format {
 
 struct primary {
 	struct gatt_service *gatt;
+	struct att_primary *att;
 	char *path;
-	uuid_t uuid;
-	uint16_t start;
-	uint16_t end;
 	GSList *chars;
 	GSList *watchers;
 };
@@ -583,21 +581,6 @@ static GDBusMethodTable char_methods[] = {
 	{ }
 };
 
-static void register_primary(struct gatt_service *gatt)
-{
-	GSList *l;
-
-	for (l = gatt->primary; l; l = l->next) {
-		struct primary *prim = l->data;
-		g_dbus_register_interface(connection, prim->path,
-				CHAR_INTERFACE, prim_methods,
-				NULL, NULL, prim, NULL);
-		DBG("Registered: %s", prim->path);
-
-		btd_device_add_service(gatt->dev, prim->path);
-	}
-}
-
 static char *characteristic_list_to_string(GSList *chars)
 {
 	GString *characteristics;
@@ -631,10 +614,11 @@ static void store_characteristics(struct gatt_service *gatt,
 							struct primary *prim)
 {
 	char *characteristics;
+	struct att_primary *att = prim->att;
 
 	characteristics = characteristic_list_to_string(prim->chars);
 
-	write_device_characteristics(&gatt->sba, &gatt->dba, prim->start,
+	write_device_characteristics(&gatt->sba, &gatt->dba, att->start,
 							characteristics);
 
 	g_free(characteristics);
@@ -698,6 +682,7 @@ static GSList *string_to_characteristic_list(struct primary *prim,
 static void load_characteristics(gpointer data, gpointer user_data)
 {
 	struct primary *prim = data;
+	struct att_primary *att = prim->att;
 	struct gatt_service *gatt = user_data;
 	GSList *chrs_list;
 	char *str;
@@ -707,7 +692,7 @@ static void load_characteristics(gpointer data, gpointer user_data)
 		return;
 	}
 
-	str = read_device_characteristics(&gatt->sba, &gatt->dba, prim->start);
+	str = read_device_characteristics(&gatt->sba, &gatt->dba, att->start);
 	if (str == NULL)
 		return;
 
@@ -905,6 +890,7 @@ static void char_discovered_cb(guint8 status, const guint8 *pdu, guint16 plen,
 {
 	struct query_data *current = user_data;
 	struct primary *prim = current->prim;
+	struct att_primary *att = prim->att;
 	struct gatt_service *gatt = prim->gatt;
 	struct att_data_list *list;
 	uint16_t last, *previous_end = NULL;
@@ -953,15 +939,15 @@ static void char_discovered_cb(guint8 status, const guint8 *pdu, guint16 plen,
 	}
 
 	if (previous_end)
-		*previous_end = prim->end;
+		*previous_end = att->end;
 
 	att_data_list_free(list);
 
-	if (last >= prim->end)
+	if (last >= att->end)
 		goto done;
 
 	/* Fetch remaining characteristics for the CURRENT primary service */
-	gatt_discover_char(gatt->attrib, last + 1, prim->end,
+	gatt_discover_char(gatt->attrib, last + 1, att->end,
 						char_discovered_cb, current);
 
 	return;
@@ -977,188 +963,43 @@ fail:
 	g_free(current);
 }
 
-static void *attr_data_from_string(const char *str)
-{
-	uint8_t *data;
-	int size, i;
-	char tmp[3];
-
-	size = strlen(str) / 2;
-	data = g_try_malloc0(size);
-	if (data == NULL)
-		return NULL;
-
-	tmp[2] = '\0';
-	for (i = 0; i < size; i++) {
-		memcpy(tmp, str + (i * 2), 2);
-		data[i] = (uint8_t) strtol(tmp, NULL, 16);
-	}
-
-	return data;
-}
-
-static int find_primary(gconstpointer a, gconstpointer b)
-{
-	const struct primary *primary = a;
-	uint16_t handle = GPOINTER_TO_UINT(b);
-
-	if (handle < primary->start)
-		return -1;
-
-	if (handle > primary->end)
-		return -1;
-
-	return 0;
-}
-
-static int find_characteristic(gconstpointer a, gconstpointer b)
+static void discover_all_char(gpointer data, gpointer user_data)
 {
-	const struct characteristic *chr = a;
-	uint16_t handle = GPOINTER_TO_UINT(b);
-
-	if (handle < chr->handle)
-		return -1;
+	struct query_data *qchr;
+	struct gatt_service *gatt = user_data;
+	struct primary *prim = data;
+	struct att_primary *att = prim->att;
 
-	if (handle > chr->end)
-		return -1;
+	qchr = g_new0(struct query_data, 1);
+	qchr->prim = prim;
 
-	return 0;
+	gatt->attrib = g_attrib_ref(gatt->attrib);
+	gatt_discover_char(gatt->attrib, att->start, att->end,
+						char_discovered_cb, qchr);
 }
 
-static void load_attribute_data(char *key, char *value, void *data)
+static void register_primaries(struct gatt_service *gatt, GSList *primaries)
 {
-	struct gatt_service *gatt = data;
-	struct characteristic *chr;
-	struct primary *primary;
-	char addr[18], dst[18];
-	uint16_t handle;
-	uuid_t uuid;
 	GSList *l;
-	guint h;
-
-	if (sscanf(key, "%17s#%04hX", addr, &handle) < 2)
-		return;
-
-	ba2str(&gatt->dba, dst);
-
-	if (strcmp(addr, dst) != 0)
-		return;
-
-	h = handle;
-
-	l = g_slist_find_custom(gatt->primary, GUINT_TO_POINTER(h),
-								find_primary);
-	if (!l)
-		return;
-
-	primary = l->data;
-
-	l = g_slist_find_custom(primary->chars, GUINT_TO_POINTER(h),
-							find_characteristic);
-	if (!l)
-		return;
-
-	chr = l->data;
-
-	/* value[] contains "<UUID>#<data>", but bt_string2uuid() expects a
-	 * string containing only the UUID. To avoid creating a new buffer,
-	 * "truncate" the string in place before calling bt_string2uuid(). */
-	value[MAX_LEN_UUID_STR - 1] = '\0';
-	if (bt_string2uuid(&uuid, value) < 0)
-		return;
-
-	/* Fill the characteristic field according to the attribute type. */
-	if (uuid_desc16_cmp(&uuid, GATT_CHARAC_USER_DESC_UUID) == 0)
-		chr->desc = attr_data_from_string(value + MAX_LEN_UUID_STR);
-	else if (uuid_desc16_cmp(&uuid, GATT_CHARAC_FMT_UUID) == 0)
-		chr->format = attr_data_from_string(value + MAX_LEN_UUID_STR);
-}
-
-static GSList *string_to_primary_list(struct gatt_service *gatt,
-							const char *str)
-{
-	GSList *l = NULL;
-	char **services;
-	int i;
-
-	if (str == NULL)
-		return NULL;
 
-	services = g_strsplit(str, " ", 0);
-	if (services == NULL)
-		return NULL;
-
-	for (i = 0; services[i]; i++) {
+	for (l = primaries; l; l = l->next) {
+		struct att_primary *att = l->data;
 		struct primary *prim;
-		char uuidstr[MAX_LEN_UUID_STR + 1];
-		int ret;
 
 		prim = g_new0(struct primary, 1);
+		prim->att = att;
 		prim->gatt = gatt;
-
-		ret = sscanf(services[i], "%04hX#%04hX#%s", &prim->start,
-							&prim->end, uuidstr);
-
-		if (ret < 3) {
-			g_free(prim);
-			continue;
-		}
-
 		prim->path = g_strdup_printf("%s/service%04x", gatt->path,
-								prim->start);
-
-		bt_string2uuid(&prim->uuid, uuidstr);
-
-		l = g_slist_append(l, prim);
-	}
+								att->start);
 
-	g_strfreev(services);
-
-	return l;
-}
-
-static gboolean load_primary_services(struct gatt_service *gatt)
-{
-	GSList *primary_list;
-	char *str;
+		g_dbus_register_interface(connection, prim->path,
+				CHAR_INTERFACE, prim_methods,
+				NULL, NULL, prim, NULL);
+		DBG("Registered: %s", prim->path);
 
-	if (gatt->primary) {
-		DBG("Services already loaded");
-		return FALSE;
+		gatt->primary = g_slist_append(gatt->primary, prim);
+		load_characteristics(prim, gatt);
 	}
-
-	str = read_device_services(&gatt->sba, &gatt->dba);
-	if (str == NULL)
-		return FALSE;
-
-	primary_list = string_to_primary_list(gatt, str);
-
-	free(str);
-
-	if (primary_list == NULL)
-		return FALSE;
-
-	gatt->primary = primary_list;
-	register_primary(gatt);
-
-	g_slist_foreach(gatt->primary, load_characteristics, gatt);
-	read_device_attributes(&gatt->sba, load_attribute_data, gatt);
-
-	return TRUE;
-}
-
-static void discover_all_char(gpointer data, gpointer user_data)
-{
-	struct query_data *qchr;
-	struct gatt_service *gatt = user_data;
-	struct primary *prim = data;
-
-	qchr = g_new0(struct query_data, 1);
-	qchr->prim = prim;
-
-	gatt->attrib = g_attrib_ref(gatt->attrib);
-	gatt_discover_char(gatt->attrib, prim->start, prim->end,
-						char_discovered_cb, qchr);
 }
 
 int attrib_client_register(struct btd_device *device, int psm)
@@ -1166,6 +1007,7 @@ int attrib_client_register(struct btd_device *device, int psm)
 	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);
@@ -1179,8 +1021,7 @@ int attrib_client_register(struct btd_device *device, int psm)
 	bacpy(&gatt->dba, &dba);
 	gatt->psm = psm;
 
-	if (load_primary_services(gatt))
-		DBG("Primary services loaded");
+	register_primaries(gatt, primaries);
 
 	/* FIXME: just to avoid breaking the build */
 	if (FALSE)
-- 
1.7.3.4

[PATCH 13/18] Add support for making LE connections to GATT client

[Vinicius Costa Gomes]

Now GATT client should be able to make LE connections. The information
used to determine if we should make a LE connection is the psm stored
in the gatt_service structure.
---
 attrib/client.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 0eadf1e..21ce439 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -409,7 +409,15 @@ static int l2cap_connect(struct gatt_service *gatt, GError **gerr,
 	 * Configuration it is necessary to poll the server from time
 	 * to time checking for modifications.
 	 */
-	io = bt_io_connect(BT_IO_L2CAP, connect_cb, gatt, NULL, gerr,
+	if (gatt->psm < 0)
+		io = bt_io_connect(BT_IO_L2CAP, connect_cb, gatt, NULL, gerr,
+			BT_IO_OPT_SOURCE_BDADDR, &gatt->sba,
+			BT_IO_OPT_DEST_BDADDR, &gatt->dba,
+			BT_IO_OPT_CID, GATT_CID,
+			BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+			BT_IO_OPT_INVALID);
+	else
+		io = bt_io_connect(BT_IO_L2CAP, connect_cb, gatt, NULL, gerr,
 			BT_IO_OPT_SOURCE_BDADDR, &gatt->sba,
 			BT_IO_OPT_DEST_BDADDR, &gatt->dba,
 			BT_IO_OPT_PSM, gatt->psm,
-- 
1.7.3.4

Re: [PATCH 13/18] Add support for making LE connections to GATT client

[Brian Gix]

Hi Vinicius,

On Tue, 2010-12-21 at 19:26 -0200, Vinicius Costa Gomes wrote:
> Now GATT client should be able to make LE connections. The information
> used to determine if we should make a LE connection is the psm stored
> in the gatt_service structure.

Most of your patches listed here are at a higher level than what I am
currently looking at, so I am just very happy that they are getting
done.

So all of my comments of your patch list are here.

> ---
>  attrib/client.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/attrib/client.c b/attrib/client.c
> index 0eadf1e..21ce439 100644
> --- a/attrib/client.c
> +++ b/attrib/client.c
> @@ -409,7 +409,15 @@ static int l2cap_connect(struct gatt_service *gatt, GError **gerr,
>  	 * Configuration it is necessary to poll the server from time
>  	 * to time checking for modifications.
>  	 */
> -	io = bt_io_connect(BT_IO_L2CAP, connect_cb, gatt, NULL, gerr,
> +	if (gatt->psm < 0)
> +		io = bt_io_connect(BT_IO_L2CAP, connect_cb, gatt, NULL, gerr,
> +			BT_IO_OPT_SOURCE_BDADDR, &gatt->sba,
> +			BT_IO_OPT_DEST_BDADDR, &gatt->dba,
> +			BT_IO_OPT_CID, GATT_CID,
> +			BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
> +			BT_IO_OPT_INVALID);

As I have mentioned previously, I am interested in assuring support of
LE Privacy.  This is something we saw for the first time at the
Barcelona UPF, which caused a few teams problems.

While I understand that for now you just want to get Public Addressing
working, I think we may want to include a framework which allows for
Ransom and private addressing in the future, even if for now they are
just place-holders.

Towards that end, any interface like bt_io_connect which is specifying
the Source and Destination address in this manner, should also include
the address type, which at a minimum will need to be stored with the
Destination BDADDR.  Source address type is less of a concern, as the
lower kernel layers can probably derive it.  But I would suggest a
connect parameter of:
BT_IO_OPT_DEST_BDADDR_TYPE
with possible values of
0x00 - Public
0x01 - Random

And a gatt structure member of something like "uint8_t dbat".


> +	else
> +		io = bt_io_connect(BT_IO_L2CAP, connect_cb, gatt, NULL, gerr,
>  			BT_IO_OPT_SOURCE_BDADDR, &gatt->sba,
>  			BT_IO_OPT_DEST_BDADDR, &gatt->dba,
>  			BT_IO_OPT_PSM, gatt->psm,

Regards,

-- 
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[PATCH 14/18] Add a Discover method to the GATT Client

[Vinicius Costa Gomes]

This methods allows users to actively start the discovery of characteristics
presents in a service.
---
 attrib/client.c |   40 +++++++++++++++++++++++-----------------
 1 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 21ce439..8dc5c79 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -497,15 +497,6 @@ static DBusMessage *unregister_watcher(DBusConnection *conn,
 	return dbus_message_new_method_return(msg);
 }
 
-static GDBusMethodTable prim_methods[] = {
-	{ "GetCharacteristics",	"",	"a{oa{sv}}", get_characteristics},
-	{ "RegisterCharacteristicsWatcher",	"o", "",
-						register_watcher	},
-	{ "UnregisterCharacteristicsWatcher",	"o", "",
-						unregister_watcher	},
-	{ }
-};
-
 static DBusMessage *set_value(DBusConnection *conn, DBusMessage *msg,
 			DBusMessageIter *iter, struct characteristic *chr)
 {
@@ -971,21 +962,40 @@ fail:
 	g_free(current);
 }
 
-static void discover_all_char(gpointer data, gpointer user_data)
+static DBusMessage *discover_char(DBusConnection *conn, DBusMessage *msg,
+								void *data)
 {
-	struct query_data *qchr;
-	struct gatt_service *gatt = user_data;
 	struct primary *prim = data;
 	struct att_primary *att = prim->att;
+	struct gatt_service *gatt = prim->gatt;
+	struct query_data *qchr;
+	GError *gerr = NULL;
+
+	if (l2cap_connect(prim->gatt, &gerr, FALSE) < 0) {
+		DBusMessage *reply = btd_error_failed(msg, gerr->message);
+		g_error_free(gerr);
+		return reply;
+	}
 
 	qchr = g_new0(struct query_data, 1);
 	qchr->prim = prim;
 
-	gatt->attrib = g_attrib_ref(gatt->attrib);
 	gatt_discover_char(gatt->attrib, att->start, att->end,
 						char_discovered_cb, qchr);
+
+	return dbus_message_new_method_return(msg);
 }
 
+static GDBusMethodTable prim_methods[] = {
+	{ "GetCharacteristics",	"",	"a{oa{sv}}", get_characteristics},
+	{ "Discover",		"",	"",		discover_char	},
+	{ "RegisterCharacteristicsWatcher",	"o", "",
+						register_watcher	},
+	{ "UnregisterCharacteristicsWatcher",	"o", "",
+						unregister_watcher	},
+	{ }
+};
+
 static void register_primaries(struct gatt_service *gatt, GSList *primaries)
 {
 	GSList *l;
@@ -1031,10 +1041,6 @@ int attrib_client_register(struct btd_device *device, int psm)
 
 	register_primaries(gatt, primaries);
 
-	/* FIXME: just to avoid breaking the build */
-	if (FALSE)
-		discover_all_char(NULL, NULL);
-
 	gatt_services = g_slist_append(gatt_services, gatt);
 
 	return 0;
-- 
1.7.3.4

[PATCH 15/18] Remove GetCharacteristics DBus method

[Vinicius Costa Gomes]

This method was not documented in the API, and it has the same
functionality as getting the Characteristics property.
---
 attrib/client.c |   46 ----------------------------------------------
 1 files changed, 0 insertions(+), 46 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 8dc5c79..8b5aea8 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -349,51 +349,6 @@ fail:
 	g_attrib_unref(gatt->attrib);
 }
 
-static DBusMessage *get_characteristics(DBusConnection *conn,
-						DBusMessage *msg, void *data)
-{
-	struct primary *prim = data;
-	DBusMessage *reply;
-	DBusMessageIter iter, array;
-	GSList *l;
-
-	reply = dbus_message_new_method_return(msg);
-	if (reply == NULL)
-		return NULL;
-
-	dbus_message_iter_init_append(reply, &iter);
-
-	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
-			DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
-			DBUS_TYPE_OBJECT_PATH_AS_STRING
-			DBUS_TYPE_ARRAY_AS_STRING
-			DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
-			DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
-			DBUS_DICT_ENTRY_END_CHAR_AS_STRING
-			DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &array);
-
-	for (l = prim->chars; l; l = l->next) {
-		struct characteristic *chr = l->data;
-		DBusMessageIter sub;
-
-		DBG("path %s", chr->path);
-
-		dbus_message_iter_open_container(&array, DBUS_TYPE_DICT_ENTRY,
-								NULL, &sub);
-
-		dbus_message_iter_append_basic(&sub, DBUS_TYPE_OBJECT_PATH,
-								&chr->path);
-
-		append_char_dict(&sub, chr);
-
-		dbus_message_iter_close_container(&array, &sub);
-	}
-
-	dbus_message_iter_close_container(&iter, &array);
-
-	return reply;
-}
-
 static int l2cap_connect(struct gatt_service *gatt, GError **gerr,
 								gboolean listen)
 {
@@ -987,7 +942,6 @@ static DBusMessage *discover_char(DBusConnection *conn, DBusMessage *msg,
 }
 
 static GDBusMethodTable prim_methods[] = {
-	{ "GetCharacteristics",	"",	"a{oa{sv}}", get_characteristics},
 	{ "Discover",		"",	"",		discover_char	},
 	{ "RegisterCharacteristicsWatcher",	"o", "",
 						register_watcher	},
-- 
1.7.3.4

[PATCH 16/18] Add support for adding services to the Services property

[Vinicius Costa Gomes]

We need to fill the devices property as each service path gets
registered in the DBus system bus.
---
 attrib/client.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 8b5aea8..e717eaa 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -970,6 +970,7 @@ static void register_primaries(struct gatt_service *gatt, GSList *primaries)
 		DBG("Registered: %s", prim->path);
 
 		gatt->primary = g_slist_append(gatt->primary, prim);
+		btd_device_add_service(gatt->dev, prim->path);
 		load_characteristics(prim, gatt);
 	}
 }
-- 
1.7.3.4

[PATCH 17/18] Add GetProperties method the Service Interface

[Vinicius Costa Gomes]

For now this interface just includes the path that each characteristic
is registered at.
---
 attrib/client.c |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index e717eaa..10bbf7d 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -941,12 +941,51 @@ static DBusMessage *discover_char(DBusConnection *conn, DBusMessage *msg,
 	return dbus_message_new_method_return(msg);
 }
 
+static DBusMessage *prim_get_properties(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	struct primary *prim = data;
+	DBusMessage *reply;
+	DBusMessageIter iter;
+	DBusMessageIter dict;
+	GSList *l;
+	char **chars;
+	int i;
+
+	reply = dbus_message_new_method_return(msg);
+	if (!reply)
+		return NULL;
+
+	dbus_message_iter_init_append(reply, &iter);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+			DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+			DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
+			DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
+
+	chars = g_new0(char *, g_slist_length(prim->chars) + 1);
+
+	for (i = 0, l = prim->chars; l; l = l->next, i++) {
+		struct characteristic *chr = l->data;
+		chars[i] = chr->path;
+	}
+
+	dict_append_array(&dict, "Characteristics", DBUS_TYPE_OBJECT_PATH,
+								&chars, i);
+	g_free(chars);
+
+	dbus_message_iter_close_container(&iter, &dict);
+
+	return reply;
+}
+
 static GDBusMethodTable prim_methods[] = {
 	{ "Discover",		"",	"",		discover_char	},
 	{ "RegisterCharacteristicsWatcher",	"o", "",
 						register_watcher	},
 	{ "UnregisterCharacteristicsWatcher",	"o", "",
 						unregister_watcher	},
+	{ "GetProperties",	"",	"a{sv}",prim_get_properties	},
 	{ }
 };
 
-- 
1.7.3.4

[PATCH 18/18] Add a "services" command to test-device

[Vinicius Costa Gomes]

This command adds a way to retrieve the Services property that each
device has.
---
 test/test-device |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/test/test-device b/test/test-device
index 06d7c46..154af19 100755
--- a/test/test-device
+++ b/test/test-device
@@ -34,6 +34,7 @@ if (len(args) < 1):
 	print "Usage: %s <command>" % (sys.argv[0])
 	print ""
 	print "  list"
+	print "  services <address>"
 	print "  create <address>"
 	print "  remove <address|path>"
 	print "  disconnect <address>"
@@ -190,5 +191,17 @@ if (args[0] == "blocked"):
 			device.SetProperty("Blocked", value)
 	sys.exit(0)
 
+if (args[0] == "services"):
+	if (len(args) < 2):
+		print "Need address parameter"
+	else:
+		path = adapter.FindDevice(args[1])
+		device = dbus.Interface(bus.get_object("org.bluez", path),
+							"org.bluez.Device")
+		properties = device.GetProperties()
+		for path in properties["Services"]:
+			print path
+	sys.exit(0)
+
 print "Unknown command"
 sys.exit(1)
-- 
1.7.3.4

Re: [PATCH 18/18] Add a "services" command to test-device

[Johan Hedberg]

Hi Vinicius,

On Tue, Dec 21, 2010, Vinicius Costa Gomes wrote:
> This command adds a way to retrieve the Services property that each
> device has.
> ---
>  test/test-device |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)

I've finished pushing all the patches in this set to the upstream tree.
Please double check that I didn't miss anything.

Johan

Re: [PATCH 00/18] Making GATT a first class citizen

[Brian Gix]

Hi Vinicius,

On Tue, 2010-12-21 at 19:26 -0200, Vinicius Costa Gomes wrote:
> Hi,
> 
> These are the first steps in the direction of making GATT a first
> class citizen in Bluez.

Is there a simple text file anywhere yet, that details the up-to-date
Object-Path/Interface/Method/Properties/etc DBus API layout of the LE
additions?

Thanks,

-- 
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [PATCH 00/18] Making GATT a first class citizen

[Vinicius Gomes]

Hi Brian,

On Tue, Dec 21, 2010 at 8:06 PM, Brian Gix <bgix@codeaurora.org> wrote:
> Hi Vinicius,
>
> On Tue, 2010-12-21 at 19:26 -0200, Vinicius Costa Gomes wrote:
>> Hi,
>>
>> These are the first steps in the direction of making GATT a first
>> class citizen in Bluez.
>
> Is there a simple text file anywhere yet, that details the up-to-date
> Object-Path/Interface/Method/Properties/etc DBus API layout of the LE
> additions?
>

Yes, doc/attribute-api.txt should contain the documentation of the DBus API.

> Thanks,
>
> --
> Brian Gix
> bgix@codeaurora.org
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Vinicius