[PATCH BlueZ 0/2] Add support for find included services

[PATCH BlueZ 0/2] Add support for find included services

[Jefferson Delfes]

This patch series introduces support for find included services.
The interactive mode of gatttool receives a new command "included" that
uses gatt_find_included to find these services in given range.

Jefferson Delfes (2):
  GATT: Add support for find included services
  gatttool: Add "included" command

 attrib/gatt.c        |  173 ++++++++++++++++++++++++++++++++++++++++++++++++++
 attrib/gatt.h        |    9 +++
 attrib/interactive.c |   54 ++++++++++++++++
 3 files changed, 236 insertions(+)

-- 
1.7.9.5

[PATCH BlueZ 1/2] GATT: Add support for find included services

[Jefferson Delfes]

Some services like HID over LE can reference another service using
included services.

See Vol 3, Part G, section 2.6.3 of Core specification for more
details.
---
 attrib/gatt.c |  173 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 attrib/gatt.h |    9 +++
 2 files changed, 182 insertions(+)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index 1c3ff78..3802200 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -47,6 +47,22 @@ struct discover_primary {
 	void *user_data;
 };
 
+struct find_included {
+	GAttrib *attrib;
+	uint16_t end;
+	GSList *includes;
+	gatt_cb_t cb;
+	uint16_t n_uuid_missing;
+	gboolean final;
+	unsigned int err;
+	void *user_data;
+};
+
+struct find_include_data {
+	struct find_included *find_incl;
+	struct gatt_included *gatt_incl;
+};
+
 struct discover_char {
 	GAttrib *attrib;
 	bt_uuid_t *uuid;
@@ -63,6 +79,13 @@ static void discover_primary_free(struct discover_primary *dp)
 	g_free(dp);
 }
 
+static void find_included_free(struct find_included *fi)
+{
+	g_slist_free_full(fi->includes, g_free);
+	g_attrib_unref(fi->attrib);
+	g_free(fi);
+}
+
 static void discover_char_free(struct discover_char *dc)
 {
 	g_slist_free_full(dc->characteristics, g_free);
@@ -107,6 +130,16 @@ static guint16 encode_discover_primary(uint16_t start, uint16_t end,
 	return plen;
 }
 
+static size_t encode_find_included(uint16_t start, uint16_t end,
+					uint8_t *pdu, size_t len)
+{
+	bt_uuid_t uuid;
+	
+	bt_uuid16_create(&uuid, GATT_INCLUDE_UUID);
+
+	return enc_read_by_type_req(start, end, &uuid, pdu, len);
+}
+
 static void primary_by_uuid_cb(guint8 status, const guint8 *ipdu,
 					guint16 iplen, gpointer user_data)
 
@@ -219,6 +252,124 @@ done:
 	discover_primary_free(dp);
 }
 
+static void find_included_helper(uint8_t status, const uint8_t *pdu,
+					uint16_t len, gpointer user_data)
+{
+	struct find_include_data *fid = user_data;
+	struct find_included *fi = fid->find_incl;
+	int buflen;
+	uint8_t *buf = g_attrib_get_buffer(fi->attrib, &buflen);
+
+	fi->n_uuid_missing--;
+
+	if (status) {
+		fi->err = status;
+		goto done;
+	}
+
+	if (dec_read_resp(pdu, len, buf, &buflen) != len) {
+		fi->err = ATT_ECODE_IO;
+		goto done;
+	}
+
+	if (buflen >= 16) {
+		bt_uuid_t uuid = att_get_uuid128(buf);
+		bt_uuid_to_string(&uuid, fid->gatt_incl->uuid,
+					sizeof(fid->gatt_incl->uuid));
+	}
+
+done:
+	if (fi->final && fi->n_uuid_missing == 0) {
+		fi->cb(fi->includes, fi->err, fi->user_data);
+		find_included_free(fi);
+	}
+
+	g_free(fid);
+}
+
+static void included_cb(uint8_t status, const uint8_t *pdu, uint16_t len,
+							gpointer user_data)
+{
+	struct find_included *fi = user_data;
+	struct att_data_list *list;
+	unsigned int i;
+	uint16_t last_handle = fi->end;
+
+	if (status) {
+		if (status != ATT_ECODE_ATTR_NOT_FOUND)
+			fi->err = status;
+		fi->final = TRUE;
+		goto done;
+	}
+
+	list = dec_read_by_type_resp(pdu, len);
+	if (list == NULL) {
+		fi->err = ATT_ECODE_IO;
+		fi->final = TRUE;
+		goto done;
+	}
+
+	for (i = 0; i < list->num; i++) {
+		const uint8_t *data = list->data[i];
+		struct gatt_included *include;
+		
+		/* Skipping invalid data */
+		if (list->len != 6 && list->len != 8)
+			continue;
+
+		include = g_new0(struct gatt_included, 1);
+		include->handle = att_get_u16(&data[0]);
+		include->range.start = att_get_u16(&data[2]);
+		include->range.end = att_get_u16(&data[4]);
+		fi->includes = g_slist_append(fi->includes, include);
+
+		if (list->len == 8) {
+			bt_uuid_t uuid128;
+			bt_uuid_t uuid16 = att_get_uuid16(&data[6]);
+			bt_uuid_to_uuid128(&uuid16, &uuid128);
+			bt_uuid_to_string(&uuid128, include->uuid,
+						sizeof(include->uuid));
+		} else {
+			uint8_t *buf;
+			int buflen;
+			uint16_t plen;
+			struct find_include_data *fid =
+					g_new0(struct find_include_data, 1);
+			fid->find_incl = fi;
+			fid->gatt_incl = include;
+
+			/* 128-bit UUID, we need to "resolve" it */
+			buf = g_attrib_get_buffer(fi->attrib, &buflen);
+			plen = enc_read_req(include->range.start, buf, buflen);
+			fi->n_uuid_missing++;
+			g_attrib_send(fi->attrib, 0, buf[0], buf,
+						plen, find_included_helper,
+						fid, NULL);
+		}
+
+		last_handle = include->handle;
+	}
+
+	att_data_list_free(list);
+
+	if (last_handle != fi->end) {
+		int buflen = 0;
+		uint8_t *buf = g_attrib_get_buffer(fi->attrib, &buflen);
+		size_t oplen = encode_find_included(last_handle + 1,
+							fi->end, buf, buflen);
+
+		g_attrib_send(fi->attrib, 0, buf[0], buf, oplen,
+					included_cb, fi, NULL);
+	} else
+		fi->final = TRUE;
+
+done:
+	if (fi->final && fi->n_uuid_missing == 0) {
+		fi->cb(fi->includes, fi->err, fi->user_data);
+		find_included_free(fi);
+	}
+}
+
 guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
 							gpointer user_data)
 {
@@ -249,6 +400,28 @@ guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
 	return g_attrib_send(attrib, 0, buf[0], buf, plen, cb, dp, NULL);
 }
 
+unsigned int gatt_find_included(GAttrib *attrib, uint16_t start, uint16_t end,
+				gatt_cb_t func, gpointer user_data)
+{
+	struct find_included *fi;
+	int buflen;
+	uint8_t *buf = g_attrib_get_buffer(attrib, &buflen);
+	size_t plen;
+
+	plen = encode_find_included(start, end, buf, buflen);
+	if (plen == 0)
+		return 0;
+
+	fi = g_new0(struct find_included, 1);
+	fi->attrib = g_attrib_ref(attrib);
+	fi->end = end;
+	fi->cb = func;
+	fi->user_data = user_data;
+
+	return g_attrib_send(attrib, 0, buf[0], buf, plen, included_cb,
+					fi, NULL);
+}
+
 static void char_discovered_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
 							gpointer user_data)
 {
diff --git a/attrib/gatt.h b/attrib/gatt.h
index 1732270..b420711 100644
--- a/attrib/gatt.h
+++ b/attrib/gatt.h
@@ -30,6 +30,12 @@ struct gatt_primary {
 	struct att_range range;
 };
 
+struct gatt_included {
+	char uuid[MAX_LEN_UUID_STR + 1];
+	uint16_t handle;
+	struct att_range range;
+};
+
 struct gatt_char {
 	char uuid[MAX_LEN_UUID_STR + 1];
 	uint16_t handle;
@@ -40,6 +46,9 @@ struct gatt_char {
 guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
 							gpointer user_data);
 
+unsigned int gatt_find_included(GAttrib *attrib, uint16_t start, uint16_t end,
+					gatt_cb_t func, gpointer user_data);
+
 guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
 					bt_uuid_t *uuid, gatt_cb_t func,
 					gpointer user_data);
-- 
1.7.9.5

Re: [PATCH BlueZ 1/2] GATT: Add support for find included services

[Johan Hedberg]

Hi Jefferson,

On Wed, Apr 04, 2012, Jefferson Delfes wrote:
> Some services like HID over LE can reference another service using
> included services.
> 
> See Vol 3, Part G, section 2.6.3 of Core specification for more
> details.
> ---
>  attrib/gatt.c |  173 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  attrib/gatt.h |    9 +++
>  2 files changed, 182 insertions(+)
> 
> diff --git a/attrib/gatt.c b/attrib/gatt.c
> index 1c3ff78..3802200 100644
> --- a/attrib/gatt.c
> +++ b/attrib/gatt.c
> @@ -47,6 +47,22 @@ struct discover_primary {
>  	void *user_data;
>  };
>  
> +struct find_included {
> +	GAttrib *attrib;
> +	uint16_t end;
> +	GSList *includes;
> +	gatt_cb_t cb;
> +	uint16_t n_uuid_missing;
> +	gboolean final;
> +	unsigned int err;
> +	void *user_data;
> +};

Could you try to make this struct a bit more readable, e.g. use tabs to
align the variable names and group related things together (like cb and
user_data). I think uuids_missing or missing_uuids would be better than
n_uuid_missing.

> +static size_t encode_find_included(uint16_t start, uint16_t end,
> +					uint8_t *pdu, size_t len)
> +{
> +	bt_uuid_t uuid;
> +	
> +	bt_uuid16_create(&uuid, GATT_INCLUDE_UUID);

The empty line above is not actually empty but adds an unnecessary tab.
Please fix.

> +static void included_cb(uint8_t status, const uint8_t *pdu, uint16_t len,
> +							gpointer user_data)
> +{
> +	struct find_included *fi = user_data;
> +	struct att_data_list *list;
> +	unsigned int i;
> +	uint16_t last_handle = fi->end;
> +
> +	if (status) {
> +		if (status != ATT_ECODE_ATTR_NOT_FOUND)
> +			fi->err = status;
> +		fi->final = TRUE;
> +		goto done;
> +	}
> +
> +	list = dec_read_by_type_resp(pdu, len);
> +	if (list == NULL) {
> +		fi->err = ATT_ECODE_IO;
> +		fi->final = TRUE;
> +		goto done;
> +	}
> +
> +	for (i = 0; i < list->num; i++) {
> +		const uint8_t *data = list->data[i];
> +		struct gatt_included *include;
> +		
> +		/* Skipping invalid data */

The above empty line is not actually empty but adds two unnecessary
tabs. Please fix.

> +		if (list->len != 6 && list->len != 8)
> +			continue;

The value of list->len doesn't change anywhere inside this for loop so
it doesn't make sense to keep checking for it at every iteration.
Instead it seems like the right place to check for it is before entering
the for loop.

> +
> +		include = g_new0(struct gatt_included, 1);
> +		include->handle = att_get_u16(&data[0]);
> +		include->range.start = att_get_u16(&data[2]);
> +		include->range.end = att_get_u16(&data[4]);
> +		fi->includes = g_slist_append(fi->includes, include);
> +
> +		if (list->len == 8) {
> +			bt_uuid_t uuid128;
> +			bt_uuid_t uuid16 = att_get_uuid16(&data[6]);
> +			bt_uuid_to_uuid128(&uuid16, &uuid128);
> +			bt_uuid_to_string(&uuid128, include->uuid,
> +						sizeof(include->uuid));
> +		} else {
> +			uint8_t *buf;
> +			int buflen;
> +			uint16_t plen;
> +			struct find_include_data *fid =
> +					g_new0(struct find_include_data, 1);
> +			fid->find_incl = fi;
> +			fid->gatt_incl = include;
> +
> +			/* 128-bit UUID, we need to "resolve" it */
> +			buf = g_attrib_get_buffer(fi->attrib, &buflen);
> +			plen = enc_read_req(include->range.start, buf, buflen);
> +			fi->n_uuid_missing++;
> +			g_attrib_send(fi->attrib, 0, buf[0], buf,
> +						plen, find_included_helper,
> +						fid, NULL);
> +		}
> +
> +		last_handle = include->handle;
> +	}
> +
> +	att_data_list_free(list);
> +
> +	if (last_handle != fi->end) {
> +		int buflen = 0;
> +		uint8_t *buf = g_attrib_get_buffer(fi->attrib, &buflen);
> +		size_t oplen = encode_find_included(last_handle + 1,
> +							fi->end, buf, buflen);
> +
> +		g_attrib_send(fi->attrib, 0, buf[0], buf, oplen,
> +					included_cb, fi, NULL);
> +	} else
> +		fi->final = TRUE;
> +
> +done:
> +	if (fi->final && fi->n_uuid_missing == 0) {
> +		fi->cb(fi->includes, fi->err, fi->user_data);
> +		find_included_free(fi);
> +	}
> +}

This whole function is too long. See if you can refactor it somehow.

> +unsigned int gatt_find_included(GAttrib *attrib, uint16_t start, uint16_t end,
> +				gatt_cb_t func, gpointer user_data)
> +{
> +	struct find_included *fi;
> +	int buflen;
> +	uint8_t *buf = g_attrib_get_buffer(attrib, &buflen);
> +	size_t plen;
> +
> +	plen = encode_find_included(start, end, buf, buflen);
> +	if (plen == 0)
> +		return 0;
> +
> +	fi = g_new0(struct find_included, 1);
> +	fi->attrib = g_attrib_ref(attrib);
> +	fi->end = end;
> +	fi->cb = func;
> +	fi->user_data = user_data;
> +
> +	return g_attrib_send(attrib, 0, buf[0], buf, plen, included_cb,
> +					fi, NULL);

Looks like the above line is not appropriately indented. Please indent
as much as possible while staying under 80 chars.

Johan

[PATCH BlueZ 2/2] gatttool: Add "included" command

[Jefferson Delfes]

New command to find included services in interactive mode.
---
 attrib/interactive.c |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/attrib/interactive.c b/attrib/interactive.c
index 0064ba2..a7757ad 100644
--- a/attrib/interactive.c
+++ b/attrib/interactive.c
@@ -206,6 +206,28 @@ static void primary_by_uuid_cb(GSList *ranges, guint8 status,
 	rl_forced_update_display();
 }
 
+static void included_cb(GSList *includes, guint8 status, gpointer user_data)
+{
+	GSList *l;
+
+	if (status) {
+		printf("Find included services failed: %s\n",
+						att_ecode2str(status));
+		return;
+	}
+
+	printf("\n");
+	for (l = includes; l; l = l->next) {
+		struct gatt_included *incl = l->data;
+		printf("handle: 0x%04x, start handle: 0x%04x, "
+				"end handle: 0x%04x uuid: %s\n",
+				incl->handle, incl->range.start,
+				incl->range.end, incl->uuid);
+	}
+
+	rl_forced_update_display();
+}
+
 static void char_cb(GSList *characteristics, guint8 status, gpointer user_data)
 {
 	GSList *l;
@@ -415,6 +437,36 @@ static int strtohandle(const char *src)
 	return dst;
 }
 
+static void cmd_included(int argcp, char **argvp)
+{
+	int start = 0x0001;
+	int end = 0xffff;
+
+	if (conn_state != STATE_CONNECTED) {
+		printf("Command failed: disconnected\n");
+		return;
+	}
+
+	if (argcp > 1) {
+		start = strtohandle(argvp[1]);
+		if (start < 0) {
+			printf("Invalid start handle: %s\n", argvp[1]);
+			return;
+		}
+		end = start;
+	}
+
+	if (argcp > 2) {
+		end = strtohandle(argvp[2]);
+		if (end < 0) {
+			printf("Invalid end handle: %s\n", argvp[2]);
+			return;
+		}
+	}
+
+	gatt_find_included(attrib, start, end, included_cb, NULL);
+}
+
 static void cmd_char(int argcp, char **argvp)
 {
 	int start = 0x0001;
@@ -741,6 +793,8 @@ static struct {
 		"Disconnect from a remote device" },
 	{ "primary",		cmd_primary,	"[UUID]",
 		"Primary Service Discovery" },
+	{ "included",		cmd_included,	"[start hnd [end hnd]]",
+		"Find Included Services" },
 	{ "characteristics",	cmd_char,	"[start hnd [end hnd [UUID]]]",
 		"Characteristics Discovery" },
 	{ "char-desc",		cmd_char_desc,	"[start hnd] [end hnd]",
-- 
1.7.9.5

[PATCHv2 BlueZ 0/2] Add support for find included services

[Jefferson Delfes]

This patch series introduces support for find included services.
The interactive mode of gatttool receives a new command "included" that
uses gatt_find_included to find these services in given range.

Jefferson Delfes (2):
  GATT: Add support for find included services
  gatttool: Add "included" command

 attrib/gatt.c        |  198 ++++++++++++++++++++++++++++++++++++++++++++++++++
 attrib/gatt.h        |    9 +++
 attrib/interactive.c |   54 ++++++++++++++
 3 files changed, 261 insertions(+)

-- 
1.7.10

[PATCHv2 BlueZ 1/2] GATT: Add support for find included services

[Jefferson Delfes]

Some services like HID over LE can reference another service using
included services.

See Vol 3, Part G, section 2.6.3 of Core specification for more
details.
---
 attrib/gatt.c |  198 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 attrib/gatt.h |    9 +++
 2 files changed, 207 insertions(+)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index 1c3ff78..8abbc39 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -47,6 +47,22 @@ struct discover_primary {
 	void *user_data;
 };
 
+struct find_included {
+	GAttrib		*attrib;
+	uint16_t	end;
+	GSList		*includes;
+	gatt_cb_t	cb;
+	void		*user_data;
+	uint16_t	missing_uuids;	/* number of missing 128-bit UUID */
+	gboolean	final;		/* final data flag */
+	unsigned int	err;
+};
+
+struct find_include_data {
+	struct find_included *find_incl;
+	struct gatt_included *gatt_incl;
+};
+
 struct discover_char {
 	GAttrib *attrib;
 	bt_uuid_t *uuid;
@@ -63,6 +79,13 @@ static void discover_primary_free(struct discover_primary *dp)
 	g_free(dp);
 }
 
+static void find_included_free(struct find_included *fi)
+{
+	g_slist_free_full(fi->includes, g_free);
+	g_attrib_unref(fi->attrib);
+	g_free(fi);
+}
+
 static void discover_char_free(struct discover_char *dc)
 {
 	g_slist_free_full(dc->characteristics, g_free);
@@ -107,6 +130,16 @@ static guint16 encode_discover_primary(uint16_t start, uint16_t end,
 	return plen;
 }
 
+static size_t encode_find_included(uint16_t start, uint16_t end, uint8_t *pdu,
+								size_t len)
+{
+	bt_uuid_t uuid;
+
+	bt_uuid16_create(&uuid, GATT_INCLUDE_UUID);
+
+	return enc_read_by_type_req(start, end, &uuid, pdu, len);
+}
+
 static void primary_by_uuid_cb(guint8 status, const guint8 *ipdu,
 					guint16 iplen, gpointer user_data)
 
@@ -219,6 +252,149 @@ done:
 	discover_primary_free(dp);
 }
 
+static void find_included_helper(uint8_t status, const uint8_t *pdu,
+					uint16_t len, gpointer user_data)
+{
+	struct find_include_data *fid = user_data;
+	struct find_included *fi = fid->find_incl;
+	int buflen;
+	uint8_t *buf = g_attrib_get_buffer(fi->attrib, &buflen);
+
+	fi->missing_uuids--;
+
+	if (status) {
+		fi->err = status;
+		goto done;
+	}
+
+	if (dec_read_resp(pdu, len, buf, &buflen) != len) {
+		fi->err = ATT_ECODE_IO;
+		goto done;
+	}
+
+	if (buflen >= 16) {
+		bt_uuid_t uuid = att_get_uuid128(buf);
+		bt_uuid_to_string(&uuid, fid->gatt_incl->uuid,
+						sizeof(fid->gatt_incl->uuid));
+	}
+
+done:
+	if (fi->final && fi->missing_uuids == 0) {
+		fi->cb(fi->includes, fi->err, fi->user_data);
+		find_included_free(fi);
+	}
+
+	g_free(fid);
+}
+
+static void parse_included_uuid16(const struct att_data_list *list,
+						struct find_included *fi,
+						uint16_t *last_handle)
+{
+	unsigned int i;
+
+	for (i = 0; i < list->num; i++) {
+		struct gatt_included *include;
+		const uint8_t *data = list->data[i];
+		bt_uuid_t uuid128, uuid16;
+
+		include = g_new0(struct gatt_included, 1);
+		include->handle = att_get_u16(&data[0]);
+		include->range.start = att_get_u16(&data[2]);
+		include->range.end = att_get_u16(&data[4]);
+		fi->includes = g_slist_append(fi->includes, include);
+
+		uuid16 = att_get_uuid16(&data[6]);
+		bt_uuid_to_uuid128(&uuid16, &uuid128);
+		bt_uuid_to_string(&uuid128, include->uuid,
+							sizeof(include->uuid));
+
+		*last_handle = include->handle;
+	}
+}
+
+static void parse_included_uuid128(const struct att_data_list *list,
+						struct find_included *fi,
+						uint16_t *last_handle)
+{
+	unsigned int i;
+
+	for (i = 0; i < list->num; i++) {
+		const uint8_t *data = list->data[i];
+		struct gatt_included *include;
+		struct find_include_data *fid;
+		uint8_t *buf;
+		int buflen;
+		uint16_t plen;
+
+		include = g_new0(struct gatt_included, 1);
+		include->handle = att_get_u16(&data[0]);
+		include->range.start = att_get_u16(&data[2]);
+		include->range.end = att_get_u16(&data[4]);
+		fi->includes = g_slist_append(fi->includes, include);
+
+		fid = g_new0(struct find_include_data, 1);
+		fid->find_incl = fi;
+		fid->gatt_incl = include;
+
+		/* 128-bit UUID, we need to "resolve" it */
+		buf = g_attrib_get_buffer(fi->attrib, &buflen);
+		plen = enc_read_req(include->range.start, buf, buflen);
+		fi->missing_uuids++;
+		g_attrib_send(fi->attrib, 0, buf[0], buf, plen,
+					find_included_helper, fid, NULL);
+
+		*last_handle = include->handle;
+	}
+}
+
+static void included_cb(uint8_t status, const uint8_t *pdu, uint16_t len,
+							gpointer user_data)
+{
+	struct find_included *fi = user_data;
+	struct att_data_list *list;
+	uint16_t last_handle = fi->end;
+
+	if (status) {
+		if (status != ATT_ECODE_ATTR_NOT_FOUND)
+			fi->err = status;
+		fi->final = TRUE;
+		goto done;
+	}
+
+	list = dec_read_by_type_resp(pdu, len);
+	if (list == NULL) {
+		fi->err = ATT_ECODE_IO;
+		fi->final = TRUE;
+		goto done;
+	}
+
+	/* parse valid data only */
+	if (list->len == 6)
+		parse_included_uuid128(list, fi, &last_handle);
+	else if (list->len == 8)
+		parse_included_uuid16(list, fi, &last_handle);
+
+	att_data_list_free(list);
+
+	if (last_handle != fi->end) {
+		int buflen = 0;
+		uint8_t *buf = g_attrib_get_buffer(fi->attrib, &buflen);
+		size_t oplen = encode_find_included(last_handle + 1, fi->end,
+								buf, buflen);
+
+		g_attrib_send(fi->attrib, 0, buf[0], buf, oplen, included_cb,
+								fi, NULL);
+	} else
+		fi->final = TRUE;
+
+done:
+	if (fi->final && fi->missing_uuids == 0) {
+		fi->cb(fi->includes, fi->err, fi->user_data);
+		find_included_free(fi);
+	}
+}
+
 guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
 							gpointer user_data)
 {
@@ -249,6 +425,28 @@ guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
 	return g_attrib_send(attrib, 0, buf[0], buf, plen, cb, dp, NULL);
 }
 
+unsigned int gatt_find_included(GAttrib *attrib, uint16_t start, uint16_t end,
+					gatt_cb_t func, gpointer user_data)
+{
+	struct find_included *fi;
+	int buflen;
+	uint8_t *buf = g_attrib_get_buffer(attrib, &buflen);
+	size_t plen;
+
+	plen = encode_find_included(start, end, buf, buflen);
+	if (plen == 0)
+		return 0;
+
+	fi = g_new0(struct find_included, 1);
+	fi->attrib = g_attrib_ref(attrib);
+	fi->end = end;
+	fi->cb = func;
+	fi->user_data = user_data;
+
+	return g_attrib_send(attrib, 0, buf[0], buf, plen, included_cb, fi,
+									NULL);
+}
+
 static void char_discovered_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
 							gpointer user_data)
 {
diff --git a/attrib/gatt.h b/attrib/gatt.h
index 1732270..b420711 100644
--- a/attrib/gatt.h
+++ b/attrib/gatt.h
@@ -30,6 +30,12 @@ struct gatt_primary {
 	struct att_range range;
 };
 
+struct gatt_included {
+	char uuid[MAX_LEN_UUID_STR + 1];
+	uint16_t handle;
+	struct att_range range;
+};
+
 struct gatt_char {
 	char uuid[MAX_LEN_UUID_STR + 1];
 	uint16_t handle;
@@ -40,6 +46,9 @@ struct gatt_char {
 guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
 							gpointer user_data);
 
+unsigned int gatt_find_included(GAttrib *attrib, uint16_t start, uint16_t end,
+					gatt_cb_t func, gpointer user_data);
+
 guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
 					bt_uuid_t *uuid, gatt_cb_t func,
 					gpointer user_data);
-- 
1.7.10

Re: [PATCHv2 BlueZ 1/2] GATT: Add support for find included services

[Johan Hedberg]

Hi Jefferson,

On Fri, Apr 13, 2012, Jefferson Delfes wrote:
> --- a/attrib/gatt.c
> +++ b/attrib/gatt.c
> @@ -47,6 +47,22 @@ struct discover_primary {
>  	void *user_data;
>  };
>  
> +struct find_included {
> +	GAttrib		*attrib;
> +	uint16_t	end;
> +	GSList		*includes;
> +	gatt_cb_t	cb;
> +	void		*user_data;
> +	uint16_t	missing_uuids;	/* number of missing 128-bit UUID */
> +	gboolean	final;		/* final data flag */
> +	unsigned int	err;
> +};
> +
> +struct find_include_data {
> +	struct find_included *find_incl;
> +	struct gatt_included *gatt_incl;
> +};

The reason I didn't apply this immediately and instead put it aside (and
then forgot about it) is that I'm having trouble figuring out these data
structures, their purposes and their relation to each other. The first
thing that starts off the confusion is that you've named than based on
an action instead of something that could be considered an object.

So please try to come up with better naming for them, and if the new
names do not in themselves already explain most of the purpose then at
least provide proper code comments to clarify this.

Another thing that threw me off is the "fid" variable naming. The
existing and most common use of "id" is a shorthand for "identifier". So
what's "fid"? A file identifier? Please come up with better names for
these things. In many cases the context is quite clear and if you can't
come up with anything better simply "data" could be good enough.

Johan

Re: [PATCHv2 BlueZ 1/2] GATT: Add support for find included services

[Jefferson Delfes]

Hi Johan.

On Thu, Apr 26, 2012 at 10:31 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Jefferson,
>
> On Fri, Apr 13, 2012, Jefferson Delfes wrote:
>> --- a/attrib/gatt.c
>> +++ b/attrib/gatt.c
>> @@ -47,6 +47,22 @@ struct discover_primary {
>>       void *user_data;
>>  };
>>
>> +struct find_included {
>> +     GAttrib         *attrib;
>> +     uint16_t        end;
>> +     GSList          *includes;
>> +     gatt_cb_t       cb;
>> +     void            *user_data;
>> +     uint16_t        missing_uuids;  /* number of missing 128-bit UUID */
>> +     gboolean        final;          /* final data flag */
>> +     unsigned int    err;
>> +};
>> +
>> +struct find_include_data {
>> +     struct find_included *find_incl;
>> +     struct gatt_included *gatt_incl;
>> +};
>
> The reason I didn't apply this immediately and instead put it aside (and
> then forgot about it) is that I'm having trouble figuring out these data
> structures, their purposes and their relation to each other. The first
> thing that starts off the confusion is that you've named than based on
> an action instead of something that could be considered an object.
>
> So please try to come up with better naming for them, and if the new
> names do not in themselves already explain most of the purpose then at
> least provide proper code comments to clarify this.

Hmm, ok. I will think up a better naming for them.

> Another thing that threw me off is the "fid" variable naming. The
> existing and most common use of "id" is a shorthand for "identifier". So
> what's "fid"? A file identifier? Please come up with better names for
> these things. In many cases the context is quite clear and if you can't
> come up with anything better simply "data" could be good enough.

Yeah, it seems ambiguous.

> Johan

Thanks for your feedback.

[PATCHv2 BlueZ 2/2] gatttool: Add "included" command

[Jefferson Delfes]

New command to find included services in interactive mode.
---
 attrib/interactive.c |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/attrib/interactive.c b/attrib/interactive.c
index 0064ba2..cb48455 100644
--- a/attrib/interactive.c
+++ b/attrib/interactive.c
@@ -206,6 +206,28 @@ static void primary_by_uuid_cb(GSList *ranges, guint8 status,
 	rl_forced_update_display();
 }
 
+static void included_cb(GSList *includes, guint8 status, gpointer user_data)
+{
+	GSList *l;
+
+	if (status) {
+		printf("Find included services failed: %s\n",
+							att_ecode2str(status));
+		return;
+	}
+
+	printf("\n");
+	for (l = includes; l; l = l->next) {
+		struct gatt_included *incl = l->data;
+		printf("handle: 0x%04x, start handle: 0x%04x, "
+						"end handle: 0x%04x uuid: %s\n",
+						incl->handle, incl->range.start,
+						incl->range.end, incl->uuid);
+	}
+
+	rl_forced_update_display();
+}
+
 static void char_cb(GSList *characteristics, guint8 status, gpointer user_data)
 {
 	GSList *l;
@@ -415,6 +437,36 @@ static int strtohandle(const char *src)
 	return dst;
 }
 
+static void cmd_included(int argcp, char **argvp)
+{
+	int start = 0x0001;
+	int end = 0xffff;
+
+	if (conn_state != STATE_CONNECTED) {
+		printf("Command failed: disconnected\n");
+		return;
+	}
+
+	if (argcp > 1) {
+		start = strtohandle(argvp[1]);
+		if (start < 0) {
+			printf("Invalid start handle: %s\n", argvp[1]);
+			return;
+		}
+		end = start;
+	}
+
+	if (argcp > 2) {
+		end = strtohandle(argvp[2]);
+		if (end < 0) {
+			printf("Invalid end handle: %s\n", argvp[2]);
+			return;
+		}
+	}
+
+	gatt_find_included(attrib, start, end, included_cb, NULL);
+}
+
 static void cmd_char(int argcp, char **argvp)
 {
 	int start = 0x0001;
@@ -741,6 +793,8 @@ static struct {
 		"Disconnect from a remote device" },
 	{ "primary",		cmd_primary,	"[UUID]",
 		"Primary Service Discovery" },
+	{ "included",		cmd_included,	"[start hnd [end hnd]]",
+		"Find Included Services" },
 	{ "characteristics",	cmd_char,	"[start hnd [end hnd [UUID]]]",
 		"Characteristics Discovery" },
 	{ "char-desc",		cmd_char_desc,	"[start hnd] [end hnd]",
-- 
1.7.10

Re: [PATCHv2 BlueZ 0/2] Add support for find included services

[Jefferson Delfes]

On Fri, Apr 13, 2012 at 5:01 PM, Jefferson Delfes
<jefferson.delfes@openbossa.org> wrote:
> This patch series introduces support for find included services.
> The interactive mode of gatttool receives a new command "included" that
> uses gatt_find_included to find these services in given range.
>
> Jefferson Delfes (2):
>  GATT: Add support for find included services
>  gatttool: Add "included" command
>
>  attrib/gatt.c        |  198 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  attrib/gatt.h        |    9 +++
>  attrib/interactive.c |   54 ++++++++++++++
>  3 files changed, 261 insertions(+)
>
> --
> 1.7.10
>

ping

[PATCH v3 BlueZ 0/2] Add support for find included services

[Jefferson Delfes]

This patch series introduces support for find included services.
The interactive mode of gatttool receives a new command "included" that
uses gatt_find_included to find these services in given range.

After some refactors, that version looks a lot easier to read and understand.
It was tested with PTS v4.5.0.10.

Jefferson Delfes (2):
  GATT: Add support for find included services
  gatttool: Add "included" command

 attrib/gatt.c        | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++
 attrib/gatt.h        |   9 +++
 attrib/interactive.c |  60 ++++++++++++++++
 3 files changed, 264 insertions(+)

-- 
1.7.11.3

[PATCH v3 BlueZ 1/2] GATT: Add support for find included services

[Jefferson Delfes]

Some services like HID over LE can reference another service using
included services.

See Vol 3, Part G, section 2.6.3 of Core specification for more
details.
---
 attrib/gatt.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 attrib/gatt.h |   9 +++
 2 files changed, 204 insertions(+)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index 6f9a11d..c182088 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -45,6 +45,15 @@ struct discover_primary {
 	void *user_data;
 };
 
+struct find_included_data {
+	GAttrib		*attrib;
+	uint16_t	end_handle;
+	GSList		*includes;
+	GSList		*current_include;
+	gatt_cb_t	cb;
+	void		*user_data;
+};
+
 struct discover_char {
 	GAttrib *attrib;
 	bt_uuid_t *uuid;
@@ -61,6 +70,13 @@ static void discover_primary_free(struct discover_primary *dp)
 	g_free(dp);
 }
 
+static void find_included_data_free(struct find_included_data *data)
+{
+	g_slist_free_full(data->includes, g_free);
+	g_attrib_unref(data->attrib);
+	g_free(data);
+}
+
 static void discover_char_free(struct discover_char *dc)
 {
 	g_slist_free_full(dc->characteristics, g_free);
@@ -247,6 +263,185 @@ guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
 	return g_attrib_send(attrib, 0, buf[0], buf, plen, cb, dp, NULL);
 }
 
+static void find_included_cb(uint8_t status, const uint8_t *pdu, uint16_t len,
+							gpointer user_data);
+
+static guint call_find_included(struct find_included_data *data,
+							uint16_t start_handle)
+{
+	bt_uuid_t uuid;
+	int buflen;
+	uint8_t *buf = g_attrib_get_buffer(data->attrib, &buflen);
+	guint16 oplen;
+
+	bt_uuid16_create(&uuid, GATT_INCLUDE_UUID);
+	oplen = enc_read_by_type_req(start_handle, data->end_handle, &uuid, buf,
+									buflen);
+
+	return g_attrib_send(data->attrib, 0, buf[0], buf, oplen,
+						find_included_cb, data, NULL);
+}
+
+static void find_included_uuid_cb(uint8_t status, const uint8_t *pdu,
+					uint16_t len, gpointer user_data);
+
+static guint call_find_included_uuid(struct find_included_data *data,
+								uint16_t handle)
+{
+	int buflen;
+	uint8_t *buf = g_attrib_get_buffer(data->attrib, &buflen);
+	guint16 oplen = enc_read_req(handle, buf, buflen);
+
+	return g_attrib_send(data->attrib, 0, buf[0], buf, oplen,
+					find_included_uuid_cb, data, NULL);
+}
+
+static void find_included_uuid_cb(uint8_t status, const uint8_t *pdu,
+					uint16_t len, gpointer user_data)
+{
+	struct find_included_data *data = user_data;
+	struct gatt_included *incl;
+	bt_uuid_t uuid;
+	unsigned int err = status;
+	int buflen;
+	uint8_t *buf;
+
+	if (err)
+		goto done;
+
+	buf = g_attrib_get_buffer(data->attrib, &buflen);
+	if (dec_read_resp(pdu, len, buf, buflen) != 16) {
+		err = ATT_ECODE_IO;
+		goto done;
+	}
+
+	incl = g_slist_nth_data(data->current_include, 0);
+	uuid = att_get_uuid128(buf);
+	bt_uuid_to_string(&uuid, incl->uuid, sizeof(incl->uuid));
+
+	data->current_include = g_slist_next(data->current_include);
+	if (data->current_include) {
+		/* resolve next 128-Bit UUID */
+		incl = g_slist_nth_data(data->current_include, 0);
+		call_find_included_uuid(data, incl->range.start);
+		return;
+	}
+
+	if (incl->handle != data->end_handle) {
+		call_find_included(data, incl->handle + 1);
+		return;
+	}
+
+done:
+	data->cb(data->includes, err, data->user_data);
+	find_included_data_free(data);
+}
+
+static struct gatt_included* fill_new_included(struct att_data_list *list,
+							unsigned int pos)
+{
+	struct gatt_included *incl = g_new0(struct gatt_included, 1);
+	uint8_t *d = list->data[pos];
+
+	incl->handle = att_get_u16(&d[0]);
+	incl->range.start = att_get_u16(&d[2]);
+	incl->range.end = att_get_u16(&d[4]);
+
+	if (list->len == 8) {
+		bt_uuid_t uuid128;
+		bt_uuid_t uuid16 = att_get_uuid16(&d[6]);
+
+		bt_uuid_to_uuid128(&uuid16, &uuid128);
+		bt_uuid_to_string(&uuid128, incl->uuid, sizeof(incl->uuid));
+	}
+
+	return incl;
+}
+
+static void parse_included_uuid(struct att_data_list *list,
+						struct find_included_data *data)
+{
+	struct gatt_included *incl = NULL;
+	GSList *includes = NULL;
+	unsigned int i;
+
+	for (i = list->num; i > 0; i--) {
+		incl = fill_new_included(list, i - 1);
+		includes = g_slist_prepend(includes, incl);
+	}
+
+	data->includes = g_slist_concat(data->includes, includes);
+
+	/* for 128-bit UUID, we need to resolve it */
+	if (list->len == 6) {
+		data->current_include = includes;
+		call_find_included_uuid(data, incl->range.start);
+	}
+}
+
+static void find_included_cb(uint8_t status, const uint8_t *pdu, uint16_t len,
+							gpointer user_data)
+{
+	struct find_included_data *data = user_data;
+	uint16_t last_handle = data->end_handle;
+	unsigned int err = status;
+	struct att_data_list *list;
+
+	if (err == ATT_ECODE_ATTR_NOT_FOUND)
+		err = 0;
+
+	if (status)
+		goto done;
+
+	list = dec_read_by_type_resp(pdu, len);
+	if (list == NULL) {
+		err = ATT_ECODE_IO;
+		goto done;
+	}
+
+	if (list->num == 0) {
+		att_data_list_free(list);
+		goto done;
+	}
+
+	/* parse valid data only */
+	if (list->len == 6) {
+		parse_included_uuid(list, data);
+		att_data_list_free(list);
+		return;
+	} else if (list->len == 8) {
+		struct gatt_included *incl;
+		parse_included_uuid(list, data);
+		incl = g_slist_nth_data(g_slist_last(data->includes), 0);
+		last_handle = incl->handle;
+	}
+
+	att_data_list_free(list);
+
+	if (last_handle != data->end_handle) {
+		call_find_included(data, last_handle + 1);
+		return;
+	}
+
+done:
+	data->cb(data->includes, err, data->user_data);
+	find_included_data_free(data);
+}
+
+unsigned int gatt_find_included(GAttrib *attrib, uint16_t start, uint16_t end,
+					gatt_cb_t func, gpointer user_data)
+{
+	struct find_included_data *data;
+
+	data = g_new0(struct find_included_data, 1);
+	data->attrib = g_attrib_ref(attrib);
+	data->end_handle = end;
+	data->cb = func;
+	data->user_data = user_data;
+
+	return call_find_included(data, start);
+}
+
 static void char_discovered_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
 							gpointer user_data)
 {
diff --git a/attrib/gatt.h b/attrib/gatt.h
index c7e79ab..3bda1b2 100644
--- a/attrib/gatt.h
+++ b/attrib/gatt.h
@@ -59,6 +59,12 @@ struct gatt_primary {
 	struct att_range range;
 };
 
+struct gatt_included {
+	char uuid[MAX_LEN_UUID_STR + 1];
+	uint16_t handle;
+	struct att_range range;
+};
+
 struct gatt_char {
 	char uuid[MAX_LEN_UUID_STR + 1];
 	uint16_t handle;
@@ -69,6 +75,9 @@ struct gatt_char {
 guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
 							gpointer user_data);
 
+unsigned int gatt_find_included(GAttrib *attrib, uint16_t start, uint16_t end,
+					gatt_cb_t func, gpointer user_data);
+
 guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
 					bt_uuid_t *uuid, gatt_cb_t func,
 					gpointer user_data);
-- 
1.7.11.3

[PATCH v3 BlueZ 2/2] gatttool: Add "included" command

[Jefferson Delfes]

New command to find included services in interactive mode.
---
 attrib/interactive.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/attrib/interactive.c b/attrib/interactive.c
index 3657798..2bb44ee 100644
--- a/attrib/interactive.c
+++ b/attrib/interactive.c
@@ -209,6 +209,34 @@ static void primary_by_uuid_cb(GSList *ranges, guint8 status,
 	rl_forced_update_display();
 }
 
+static void included_cb(GSList *includes, guint8 status, gpointer user_data)
+{
+	GSList *l;
+
+	if (status) {
+		printf("Find included services failed: %s\n",
+							att_ecode2str(status));
+		goto done;
+	}
+
+	if (includes == NULL) {
+		printf("No included services found for this range\n");
+		goto done;
+	}
+
+	printf("\n");
+	for (l = includes; l; l = l->next) {
+		struct gatt_included *incl = l->data;
+		printf("handle: 0x%04x, start handle: 0x%04x, "
+						"end handle: 0x%04x uuid: %s\n",
+						incl->handle, incl->range.start,
+						incl->range.end, incl->uuid);
+	}
+
+done:
+	rl_forced_update_display();
+}
+
 static void char_cb(GSList *characteristics, guint8 status, gpointer user_data)
 {
 	GSList *l;
@@ -426,6 +454,36 @@ static int strtohandle(const char *src)
 	return dst;
 }
 
+static void cmd_included(int argcp, char **argvp)
+{
+	int start = 0x0001;
+	int end = 0xffff;
+
+	if (conn_state != STATE_CONNECTED) {
+		printf("Command failed: disconnected\n");
+		return;
+	}
+
+	if (argcp > 1) {
+		start = strtohandle(argvp[1]);
+		if (start < 0) {
+			printf("Invalid start handle: %s\n", argvp[1]);
+			return;
+		}
+		end = start;
+	}
+
+	if (argcp > 2) {
+		end = strtohandle(argvp[2]);
+		if (end < 0) {
+			printf("Invalid end handle: %s\n", argvp[2]);
+			return;
+		}
+	}
+
+	gatt_find_included(attrib, start, end, included_cb, NULL);
+}
+
 static void cmd_char(int argcp, char **argvp)
 {
 	int start = 0x0001;
@@ -752,6 +810,8 @@ static struct {
 		"Disconnect from a remote device" },
 	{ "primary",		cmd_primary,	"[UUID]",
 		"Primary Service Discovery" },
+	{ "included",		cmd_included,	"[start hnd [end hnd]]",
+		"Find Included Services" },
 	{ "characteristics",	cmd_char,	"[start hnd [end hnd [UUID]]]",
 		"Characteristics Discovery" },
 	{ "char-desc",		cmd_char_desc,	"[start hnd] [end hnd]",
-- 
1.7.11.3