2012-01-25 13:12:33

by Santiago Carot

[permalink] [raw]
Subject: GATT improvements v3

The same set of patches but including all changes suggested.

[PATCH 1/8] gatt-service: Add support for 128-bit Bluetooth UUIDs
[PATCH 2/8] gatt-service: Move va_end just after processing the
[PATCH 3/8] gatt-service: Provide service uuid in
[PATCH 4/8] attrib-server: Allocate 16-bits UUIDS at the begining of
[PATCH 5/8] attrib-server: Set database uuids as a double linked
[PATCH 6/8] glib-compat: Add g_list_free_full to deal with issues in
[PATCH 7/8] attrib-server: Allocate 128-bits UUIDs using highest
[PATCH 8/8] gatt-example: Fix g_assert checks when an uint16_t value


2012-01-25 13:12:35

by Santiago Carot

[permalink] [raw]
Subject: [PATCH 2/8] gatt-service: Move va_end just after processing the argument list

---
attrib/gatt-service.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/attrib/gatt-service.c b/attrib/gatt-service.c
index 187ef5a..a5e6dcb 100644
--- a/attrib/gatt-service.c
+++ b/attrib/gatt-service.c
@@ -307,12 +307,14 @@ gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid,

va_start(args, opt1);
chrs = parse_opts(opt1, args);
+ va_end(args);
+
/* calculate how many attributes are necessary for this service */
for (l = chrs, size = 1; l != NULL; l = l->next) {
struct gatt_info *info = l->data;
size += info->num_attrs;
}
- va_end(args);
+
start_handle = attrib_db_find_avail(adapter, size);
if (start_handle == 0) {
error("Not enough free handles to register service");
--
1.7.8.4


2012-01-25 13:12:40

by Santiago Carot

[permalink] [raw]
Subject: [PATCH 7/8] attrib-server: Allocate 128-bits UUIDs using highest available handlers

128-uuids services are grouped at the end of the handlers database list.
This group grows from the highest handlers toward lowers handlers
until the whole range is used or the last 16 bit-uuid service is reached.
---
src/attrib-server.c | 42 +++++++++++++++++++++++++++++++++++++++++-
1 files changed, 41 insertions(+), 1 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 3f72ff8..46448c0 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -1318,7 +1318,47 @@ static uint16_t find_uuid16_avail(struct btd_adapter *adapter, uint16_t nitems)

static uint16_t find_uuid128_avail(struct btd_adapter *adapter, uint16_t nitems)
{
- /* TODO: Allocate 128 uuids at the end of the list */
+ uint16_t handle = 0, end = 0xffff;
+ struct gatt_server *server;
+ GList *dl;
+ GSList *l;
+
+ l = g_slist_find_custom(servers, adapter, adapter_cmp);
+ if (l == NULL)
+ return 0;
+
+ server = l->data;
+ if (server->database == NULL)
+ return 0xffff - nitems + 1;
+
+ for (dl = g_list_last(server->database); dl; dl = dl->prev) {
+ struct attribute *a = dl->data;
+
+ if (handle == 0)
+ handle = a->handle;
+
+ if (bt_uuid_cmp(&a->uuid, &prim_uuid) != 0 &&
+ bt_uuid_cmp(&a->uuid, &snd_uuid) != 0)
+ continue;
+
+ if (end - handle >= nitems)
+ return end - nitems + 1;
+
+ if (a->len == 2) {
+ /* 16 bit UUID service definition */
+ return 0;
+ }
+
+ if (a->handle == 0x0001)
+ return 0;
+
+ end = a->handle - 1;
+ handle = 0;
+ }
+
+ if (end - 0x0001 >= nitems)
+ return end - nitems + 1;
+
return 0;
}

--
1.7.8.4


2012-01-25 13:12:37

by Santiago Carot

[permalink] [raw]
Subject: [PATCH 4/8] attrib-server: Allocate 16-bits UUIDS at the begining of the list

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

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 93b7216..2b50fe4 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -1270,30 +1270,35 @@ void attrib_free_sdp(uint32_t sdp_handle)
remove_record_from_server(sdp_handle);
}

-uint16_t attrib_db_find_avail(struct btd_adapter *adapter, bt_uuid_t *svc_uuid,
- uint16_t nitems)
+static uint16_t find_uuid16_avail(struct btd_adapter *adapter, uint16_t nitems)
{
struct gatt_server *server;
uint16_t handle;
GSList *l;

- g_assert(nitems > 0);
-
l = g_slist_find_custom(servers, adapter, adapter_cmp);
if (l == NULL)
return 0;

server = l->data;
+ if (server->database == NULL)
+ return 0x0001;

- for (l = server->database, handle = 0; l; l = l->next) {
+ for (l = server->database, handle = 0x0001; l; l = l->next) {
struct attribute *a = l->data;

- if (handle && (bt_uuid_cmp(&a->uuid, &prim_uuid) == 0 ||
+ if ((bt_uuid_cmp(&a->uuid, &prim_uuid) == 0 ||
bt_uuid_cmp(&a->uuid, &snd_uuid) == 0) &&
a->handle - handle >= nitems)
/* Note: the range above excludes the current handle */
return handle;

+ if (a->len == 16 && (bt_uuid_cmp(&a->uuid, &prim_uuid) == 0 ||
+ bt_uuid_cmp(&a->uuid, &snd_uuid) == 0)) {
+ /* 128 bit UUID service definition */
+ return 0;
+ }
+
if (a->handle == 0xffff)
return 0;

@@ -1306,6 +1311,31 @@ uint16_t attrib_db_find_avail(struct btd_adapter *adapter, bt_uuid_t *svc_uuid,
return 0;
}

+static uint16_t find_uuid128_avail(struct btd_adapter *adapter, uint16_t nitems)
+{
+ /* TODO: Allocate 128 uuids at the end of the list */
+ return 0;
+}
+
+uint16_t attrib_db_find_avail(struct btd_adapter *adapter, bt_uuid_t *svc_uuid,
+ uint16_t nitems)
+{
+ g_assert(nitems > 0);
+
+ if (svc_uuid->type == BT_UUID16)
+ return find_uuid16_avail(adapter, nitems);
+ else if (svc_uuid->type == BT_UUID128)
+ return find_uuid128_avail(adapter, nitems);
+ else {
+ char uuidstr[MAX_LEN_UUID_STR];
+
+ bt_uuid_to_string(svc_uuid, uuidstr, MAX_LEN_UUID_STR);
+ error("Service uuid: %s is neither a 16-bit nor a 128-bit uuid",
+ uuidstr);
+ return 0;
+ }
+}
+
struct attribute *attrib_db_add(struct btd_adapter *adapter, uint16_t handle,
bt_uuid_t *uuid, int read_reqs, int write_reqs,
const uint8_t *value, int len)
--
1.7.8.4


2012-01-25 13:12:41

by Santiago Carot

[permalink] [raw]
Subject: [PATCH 8/8] gatt-example: Fix g_assert checks when an uint16_t value overflows

g_assert statements are buggy when the last handler available
overflows the uint16_t range. This check is currently used to
evaluate if the number of requested attributes for a sevice
match with the number of attributes added in the data base.
---
plugins/gatt-example.c | 21 ++++++++++-----------
1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/plugins/gatt-example.c b/plugins/gatt-example.c
index f12fbde..f026761 100644
--- a/plugins/gatt-example.c
+++ b/plugins/gatt-example.c
@@ -230,10 +230,10 @@ static void register_termometer_service(struct gatt_example_adapter *adapter,
bt_uuid16_create(&uuid, GATT_CHARAC_USER_DESC_UUID);
len = strlen(desc_out_hum);
strncpy((char *) atval, desc_out_hum, len);
- attrib_db_add(adapter->adapter, h++, &uuid, ATT_NONE, ATT_NOT_PERMITTED,
+ attrib_db_add(adapter->adapter, h, &uuid, ATT_NONE, ATT_NOT_PERMITTED,
atval, len);

- g_assert(h - start_handle == svc_size);
+ g_assert(h - start_handle + 1 == svc_size);

/* Add an SDP record for the above service */
sdp_handle = attrib_create_sdp(adapter->adapter, start_handle,
@@ -298,10 +298,10 @@ static void register_manuf1_service(struct gatt_example_adapter *adapter,
bt_uuid16_create(&uuid, MANUFACTURER_SERIAL_UUID);
len = strlen(serial1);
strncpy((char *) atval, serial1, len);
- attrib_db_add(adapter->adapter, h++, &uuid, ATT_NONE, ATT_NOT_PERMITTED,
+ attrib_db_add(adapter->adapter, h, &uuid, ATT_NONE, ATT_NOT_PERMITTED,
atval, len);

- g_assert(h - start_handle == svc_size);
+ g_assert(h - start_handle + 1 == svc_size);

range[0] = start_handle;
range[1] = start_handle + svc_size - 1;
@@ -362,10 +362,10 @@ static void register_manuf2_service(struct gatt_example_adapter *adapter,
bt_uuid16_create(&uuid, MANUFACTURER_SERIAL_UUID);
len = strlen(serial2);
strncpy((char *) atval, serial2, len);
- attrib_db_add(adapter->adapter, h++, &uuid, ATT_NONE, ATT_NOT_PERMITTED,
+ attrib_db_add(adapter->adapter, h, &uuid, ATT_NONE, ATT_NOT_PERMITTED,
atval, len);

- g_assert(h - start_handle == svc_size);
+ g_assert(h - start_handle + 1 == svc_size);

range[0] = start_handle;
range[1] = start_handle + svc_size - 1;
@@ -412,10 +412,10 @@ static void register_vendor_service(struct gatt_example_adapter *adapter,
atval[3] = 0x64;
atval[4] = 0x6F;
atval[5] = 0x72;
- attrib_db_add(adapter->adapter, h++, &uuid, ATT_NONE, ATT_NOT_PERMITTED,
+ attrib_db_add(adapter->adapter, h, &uuid, ATT_NONE, ATT_NOT_PERMITTED,
atval, 6);

- g_assert(h - start_handle == svc_size);
+ g_assert(h - start_handle + 1 == svc_size);

range[0] = start_handle;
range[1] = start_handle + svc_size - 1;
@@ -500,10 +500,9 @@ static void register_weight_service(struct gatt_example_adapter *adapter,
bt_uuid16_create(&uuid, GATT_CHARAC_USER_DESC_UUID);
len = strlen(desc_weight);
strncpy((char *) atval, desc_weight, len);
- attrib_db_add(adapter->adapter, h++, &uuid, ATT_NONE, ATT_NOT_PERMITTED,
+ attrib_db_add(adapter->adapter, h, &uuid, ATT_NONE, ATT_NOT_PERMITTED,
atval, len);
-
- g_assert(h - start_handle == svc_size);
+ g_assert(h - start_handle + 1 == svc_size);

/* Add an SDP record for the above service */
sdp_handle = attrib_create_sdp(adapter->adapter, start_handle,
--
1.7.8.4


2012-01-25 13:12:39

by Santiago Carot

[permalink] [raw]
Subject: [PATCH 6/8] glib-compat: Add g_list_free_full to deal with issues in old GLib versions

---
acinclude.m4 | 3 +++
src/glib-compat.h | 8 ++++++++
2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 57fc5e0..48a59a2 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -111,6 +111,9 @@ AC_DEFUN([AC_PATH_GLIB], [
AC_CHECK_LIB(glib-2.0, g_slist_free_full, dummy=yes,
AC_DEFINE(NEED_G_SLIST_FREE_FULL, 1,
[Define to 1 if you need g_slist_free_full() function.]))
+ AC_CHECK_LIB(glib-2.0, g_list_free_full, dummy=yes,
+ AC_DEFINE(NEED_G_LIST_FREE_FULL, 1,
+ [Define to 1 if you need g_list_free_full() function.]))
AC_SUBST(GLIB_CFLAGS)
AC_SUBST(GLIB_LIBS)
])
diff --git a/src/glib-compat.h b/src/glib-compat.h
index d50d5e9..b54d640 100644
--- a/src/glib-compat.h
+++ b/src/glib-compat.h
@@ -28,3 +28,11 @@ static inline void g_slist_free_full(GSList *list, GDestroyNotify free_func)
g_slist_free(list);
}
#endif
+
+#ifdef NEED_G_LIST_FREE_FULL
+static inline void g_list_free_full(GList *list, GDestroyNotify free_func)
+{
+ g_list_foreach(list, (GFunc) free_func, NULL);
+ g_list_free(list);
+}
+#endif
--
1.7.8.4


2012-01-25 13:12:38

by Santiago Carot

[permalink] [raw]
Subject: [PATCH 5/8] attrib-server: Set database uuids as a double linked list

16-bit UUIDs are allocated at the begining of the database list and
128-bit ones are grouped at the end. Replacing the list type with a double
linked one enables us to look for available handles in the 128-bit group
efficiently.
---
src/attrib-server.c | 75 ++++++++++++++++++++++++++++-----------------------
1 files changed, 41 insertions(+), 34 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 2b50fe4..3f72ff8 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -60,7 +60,7 @@ struct gatt_server {
GIOChannel *le_io;
uint32_t gatt_sdp_handle;
uint32_t gap_sdp_handle;
- GSList *database;
+ GList *database;
GSList *clients;
uint16_t name_handle;
uint16_t appearance_handle;
@@ -114,7 +114,7 @@ static void channel_free(struct gatt_channel *channel)

static void gatt_server_free(struct gatt_server *server)
{
- g_slist_free_full(server->database, attrib_free);
+ g_list_free_full(server->database, attrib_free);

if (server->l2cap_io != NULL) {
g_io_channel_unref(server->l2cap_io);
@@ -254,12 +254,12 @@ static struct attribute *find_primary_range(struct gatt_server *server,
{
struct attribute *attrib;
guint h = start;
- GSList *l;
+ GList *l;

if (end == NULL)
return NULL;

- l = g_slist_find_custom(server->database, GUINT_TO_POINTER(h),
+ l = g_list_find_custom(server->database, GUINT_TO_POINTER(h),
handle_cmp);
if (!l)
return NULL;
@@ -336,7 +336,7 @@ static struct attribute *attrib_db_add_new(struct gatt_server *server,

DBG("handle=0x%04x", handle);

- if (g_slist_find_custom(server->database, GUINT_TO_POINTER(h),
+ if (g_list_find_custom(server->database, GUINT_TO_POINTER(h),
handle_cmp))
return NULL;

@@ -348,7 +348,7 @@ static struct attribute *attrib_db_add_new(struct gatt_server *server,
a->read_reqs = read_reqs;
a->write_reqs = write_reqs;

- server->database = g_slist_insert_sorted(server->database, a,
+ server->database = g_list_insert_sorted(server->database, a,
attribute_cmp);

return a;
@@ -395,7 +395,8 @@ static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
struct att_data_list *adl;
struct attribute *a;
struct group_elem *cur, *old = NULL;
- GSList *l, *groups, *database;
+ GSList *l, *groups;
+ GList *dl, *database;
uint16_t length, last_handle, last_size = 0;
uint8_t status;
int i;
@@ -416,9 +417,9 @@ static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,

last_handle = end;
database = channel->server->database;
- for (l = database, groups = NULL, cur = NULL; l; l = l->next) {
+ for (dl = database, groups = NULL, cur = NULL; dl; dl = dl->next) {

- a = l->data;
+ a = dl->data;

if (a->handle < start)
continue;
@@ -472,7 +473,7 @@ static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
return enc_error_resp(ATT_OP_READ_BY_GROUP_REQ, start,
ATT_ECODE_ATTR_NOT_FOUND, pdu, len);

- if (l == NULL)
+ if (dl == NULL)
cur->end = a->handle;
else
cur->end = last_handle;
@@ -507,7 +508,8 @@ static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
uint8_t *pdu, int len)
{
struct att_data_list *adl;
- GSList *l, *types, *database;
+ GSList *l, *types;
+ GList *dl, *database;
struct attribute *a;
uint16_t num, length;
uint8_t status;
@@ -518,9 +520,9 @@ static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
ATT_ECODE_INVALID_HANDLE, pdu, len);

database = channel->server->database;
- for (l = database, length = 0, types = NULL; l; l = l->next) {
+ for (dl = database, length = 0, types = NULL; dl; dl = dl->next) {

- a = l->data;
+ a = dl->data;

if (a->handle < start)
continue;
@@ -589,7 +591,8 @@ static int find_info(struct gatt_channel *channel, uint16_t start, uint16_t end,
{
struct attribute *a;
struct att_data_list *adl;
- GSList *l, *info, *database;
+ GSList *l, *info;
+ GList *dl, *database;
uint8_t format, last_type = BT_UUID_UNSPEC;
uint16_t length, num;
int i;
@@ -599,8 +602,8 @@ static int find_info(struct gatt_channel *channel, uint16_t start, uint16_t end,
ATT_ECODE_INVALID_HANDLE, pdu, len);

database = channel->server->database;
- for (l = database, info = NULL, num = 0; l; l = l->next) {
- a = l->data;
+ for (dl = database, info = NULL, num = 0; dl; dl = dl->next) {
+ a = dl->data;

if (a->handle < start)
continue;
@@ -664,7 +667,8 @@ static int find_by_type(struct gatt_channel *channel, uint16_t start,
{
struct attribute *a;
struct att_range *range;
- GSList *l, *matches, *database;
+ GSList *matches;
+ GList *dl, *database;
int len;

if (start > end || start == 0x0000)
@@ -673,8 +677,8 @@ static int find_by_type(struct gatt_channel *channel, uint16_t start,

/* Searching first requested handle number */
database = channel->server->database;
- for (l = database, matches = NULL, range = NULL; l; l = l->next) {
- a = l->data;
+ for (dl = database, matches = NULL, range = NULL; dl; dl = dl->next) {
+ a = dl->data;

if (a->handle < start)
continue;
@@ -721,11 +725,11 @@ static uint16_t read_value(struct gatt_channel *channel, uint16_t handle,
{
struct attribute *a;
uint8_t status;
- GSList *l;
+ GList *l;
uint16_t cccval;
guint h = handle;

- l = g_slist_find_custom(channel->server->database,
+ l = g_list_find_custom(channel->server->database,
GUINT_TO_POINTER(h), handle_cmp);
if (!l)
return enc_error_resp(ATT_OP_READ_REQ, handle,
@@ -759,11 +763,11 @@ static uint16_t read_blob(struct gatt_channel *channel, uint16_t handle,
{
struct attribute *a;
uint8_t status;
- GSList *l;
+ GList *l;
uint16_t cccval;
guint h = handle;

- l = g_slist_find_custom(channel->server->database,
+ l = g_list_find_custom(channel->server->database,
GUINT_TO_POINTER(h), handle_cmp);
if (!l)
return enc_error_resp(ATT_OP_READ_BLOB_REQ, handle,
@@ -803,10 +807,10 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
{
struct attribute *a;
uint8_t status;
- GSList *l;
+ GList *l;
guint h = handle;

- l = g_slist_find_custom(channel->server->database,
+ l = g_list_find_custom(channel->server->database,
GUINT_TO_POINTER(h), handle_cmp);
if (!l)
return enc_error_resp(ATT_OP_WRITE_REQ, handle,
@@ -1275,6 +1279,7 @@ static uint16_t find_uuid16_avail(struct btd_adapter *adapter, uint16_t nitems)
struct gatt_server *server;
uint16_t handle;
GSList *l;
+ GList *dl;

l = g_slist_find_custom(servers, adapter, adapter_cmp);
if (l == NULL)
@@ -1284,8 +1289,8 @@ static uint16_t find_uuid16_avail(struct btd_adapter *adapter, uint16_t nitems)
if (server->database == NULL)
return 0x0001;

- for (l = server->database, handle = 0x0001; l; l = l->next) {
- struct attribute *a = l->data;
+ for (dl = server->database, handle = 0x0001; dl; dl = dl->next) {
+ struct attribute *a = dl->data;

if ((bt_uuid_cmp(&a->uuid, &prim_uuid) == 0 ||
bt_uuid_cmp(&a->uuid, &snd_uuid) == 0) &&
@@ -1357,6 +1362,7 @@ int attrib_db_update(struct btd_adapter *adapter, uint16_t handle,
struct gatt_server *server;
struct attribute *a;
GSList *l;
+ GList *dl;
guint h = handle;

l = g_slist_find_custom(servers, adapter, adapter_cmp);
@@ -1367,12 +1373,12 @@ int attrib_db_update(struct btd_adapter *adapter, uint16_t handle,

DBG("handle=0x%04x", handle);

- l = g_slist_find_custom(server->database, GUINT_TO_POINTER(h),
+ dl = g_list_find_custom(server->database, GUINT_TO_POINTER(h),
handle_cmp);
- if (!l)
+ if (dl == NULL)
return -ENOENT;

- a = l->data;
+ a = dl->data;

a->data = g_try_realloc(a->data, len);
if (a->data == NULL)
@@ -1395,6 +1401,7 @@ int attrib_db_del(struct btd_adapter *adapter, uint16_t handle)
struct gatt_server *server;
struct attribute *a;
GSList *l;
+ GList *dl;
guint h = handle;

l = g_slist_find_custom(servers, adapter, adapter_cmp);
@@ -1405,13 +1412,13 @@ int attrib_db_del(struct btd_adapter *adapter, uint16_t handle)

DBG("handle=0x%04x", handle);

- l = g_slist_find_custom(server->database, GUINT_TO_POINTER(h),
+ dl = g_list_find_custom(server->database, GUINT_TO_POINTER(h),
handle_cmp);
- if (!l)
+ if (dl == NULL)
return -ENOENT;

- a = l->data;
- server->database = g_slist_remove(server->database, a);
+ a = dl->data;
+ server->database = g_list_remove(server->database, a);
g_free(a->data);
g_free(a);

--
1.7.8.4


2012-01-25 13:12:36

by Santiago Carot

[permalink] [raw]
Subject: [PATCH 3/8] gatt-service: Provide service uuid in attrib_db_find_avail function

We need to provide the service uuid because of GATT server should group
16-bit uuid services together and 128-bit uuid services together,
(Bluetooth 4.0, Vol 3, Part G, 3.1).
---
attrib/gatt-service.c | 2 +-
plugins/gatt-example.c | 19 ++++++++++++-------
proximity/reporter.c | 9 ++++++---
src/attrib-server.c | 3 ++-
src/attrib-server.h | 3 ++-
5 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/attrib/gatt-service.c b/attrib/gatt-service.c
index a5e6dcb..4592a90 100644
--- a/attrib/gatt-service.c
+++ b/attrib/gatt-service.c
@@ -315,7 +315,7 @@ gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid,
size += info->num_attrs;
}

- start_handle = attrib_db_find_avail(adapter, size);
+ start_handle = attrib_db_find_avail(adapter, svc_uuid, size);
if (start_handle == 0) {
error("Not enough free handles to register service");
goto fail;
diff --git a/plugins/gatt-example.c b/plugins/gatt-example.c
index e791c53..f12fbde 100644
--- a/plugins/gatt-example.c
+++ b/plugins/gatt-example.c
@@ -132,7 +132,8 @@ static void register_termometer_service(struct gatt_example_adapter *adapter,
bt_uuid_t uuid;
int len;

- start_handle = attrib_db_find_avail(adapter->adapter, svc_size);
+ bt_uuid16_create(&uuid, THERM_HUMIDITY_SVC_UUID);
+ start_handle = attrib_db_find_avail(adapter->adapter, &uuid, svc_size);
if (start_handle == 0) {
error("Not enough free handles to register service");
return;
@@ -253,7 +254,8 @@ static void register_manuf1_service(struct gatt_example_adapter *adapter,
bt_uuid_t uuid;
int len;

- start_handle = attrib_db_find_avail(adapter->adapter, svc_size);
+ bt_uuid16_create(&uuid, MANUFACTURER_SVC_UUID);
+ start_handle = attrib_db_find_avail(adapter->adapter, &uuid, svc_size);
if (start_handle == 0) {
error("Not enough free handles to register service");
return;
@@ -316,7 +318,8 @@ static void register_manuf2_service(struct gatt_example_adapter *adapter,
bt_uuid_t uuid;
int len;

- start_handle = attrib_db_find_avail(adapter->adapter, svc_size);
+ bt_uuid16_create(&uuid, MANUFACTURER_SVC_UUID);
+ start_handle = attrib_db_find_avail(adapter->adapter, &uuid, svc_size);
if (start_handle == 0) {
error("Not enough free handles to register service");
return;
@@ -376,7 +379,8 @@ static void register_vendor_service(struct gatt_example_adapter *adapter,
uint8_t atval[256];
bt_uuid_t uuid;

- start_handle = attrib_db_find_avail(adapter->adapter, svc_size);
+ bt_uuid16_create(&uuid, VENDOR_SPECIFIC_SVC_UUID);
+ start_handle = attrib_db_find_avail(adapter->adapter, &uuid, svc_size);
if (start_handle == 0) {
error("Not enough free handles to register service");
return;
@@ -427,7 +431,7 @@ static void register_weight_service(struct gatt_example_adapter *adapter,
const uint128_t prim_weight_uuid_btorder = {
.data = { 0x4F, 0x0A, 0xC0, 0x96, 0x35, 0xD4, 0x49, 0x11,
0x96, 0x31, 0xDE, 0xA8, 0xDC, 0x74, 0xEE, 0xFE } };
- uint128_t char_weight_uuid;
+ uint128_t prim_weight_uuid, char_weight_uuid;
uint16_t start_handle, h;
const int svc_size = 6;
uint32_t sdp_handle;
@@ -436,8 +440,9 @@ static void register_weight_service(struct gatt_example_adapter *adapter,
int len;

btoh128(&char_weight_uuid_btorder, &char_weight_uuid);
-
- start_handle = attrib_db_find_avail(adapter->adapter, svc_size);
+ btoh128(&prim_weight_uuid_btorder, &prim_weight_uuid);
+ bt_uuid128_create(&uuid, prim_weight_uuid);
+ start_handle = attrib_db_find_avail(adapter->adapter, &uuid, svc_size);
if (start_handle == 0) {
error("Not enough free handles to register service");
return;
diff --git a/proximity/reporter.c b/proximity/reporter.c
index a139c7c..b29c75b 100644
--- a/proximity/reporter.c
+++ b/proximity/reporter.c
@@ -59,8 +59,9 @@ static void register_link_loss(void)
uint8_t atval[256];
bt_uuid_t uuid;

+ bt_uuid16_create(&uuid, LINK_LOSS_SVC_UUID);
/* FIXME: Provide the adapter in next function */
- start_handle = attrib_db_find_avail(NULL, svc_size);
+ start_handle = attrib_db_find_avail(NULL, &uuid, svc_size);
if (start_handle == 0) {
error("Not enough free handles to register service");
return;
@@ -100,8 +101,9 @@ static void register_tx_power(void)
uint8_t atval[256];
bt_uuid_t uuid;

+ bt_uuid16_create(&uuid, TX_POWER_SVC_UUID);
/* FIXME: Provide the adapter in next function */
- start_handle = attrib_db_find_avail(NULL, svc_size);
+ start_handle = attrib_db_find_avail(NULL, &uuid, svc_size);
if (start_handle == 0) {
error("Not enough free handles to register service");
return;
@@ -149,8 +151,9 @@ static void register_immediate_alert(void)
uint8_t atval[256];
bt_uuid_t uuid;

+ bt_uuid16_create(&uuid, IMMEDIATE_ALERT_SVC_UUID);
/* FIXME: Provide the adapter in next function */
- start_handle = attrib_db_find_avail(NULL, svc_size);
+ start_handle = attrib_db_find_avail(NULL, &uuid, svc_size);
if (start_handle == 0) {
error("Not enough free handles to register service");
return;
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 36a398f..93b7216 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -1270,7 +1270,8 @@ void attrib_free_sdp(uint32_t sdp_handle)
remove_record_from_server(sdp_handle);
}

-uint16_t attrib_db_find_avail(struct btd_adapter *adapter, uint16_t nitems)
+uint16_t attrib_db_find_avail(struct btd_adapter *adapter, bt_uuid_t *svc_uuid,
+ uint16_t nitems)
{
struct gatt_server *server;
uint16_t handle;
diff --git a/src/attrib-server.h b/src/attrib-server.h
index 2c6f428..fb4637c 100644
--- a/src/attrib-server.h
+++ b/src/attrib-server.h
@@ -22,7 +22,8 @@
*
*/

-uint16_t attrib_db_find_avail(struct btd_adapter *adapter, uint16_t nitems);
+uint16_t attrib_db_find_avail(struct btd_adapter *adapter, bt_uuid_t *svc_uuid,
+ uint16_t nitems);
struct attribute *attrib_db_add(struct btd_adapter *adapter, uint16_t handle,
bt_uuid_t *uuid, int read_reqs, int write_reqs,
const uint8_t *value, int len);
--
1.7.8.4


2012-01-25 13:12:34

by Santiago Carot

[permalink] [raw]
Subject: [PATCH 1/8] gatt-service: Add support for 128-bit Bluetooth UUIDs

UUID services in GATT should be either 16-bit or 128-bit. Current
GATT interface does not allow to provide 128-bit ones. This patch
enables plugins to register services using 128-bit UUIDs.
---
attrib/gatt-service.c | 57 ++++++++++++++++++++++++++++++++++-------------
attrib/gatt-service.h | 2 +-
plugins/gatt-example.c | 7 ++++-
time/server.c | 6 ++++-
4 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/attrib/gatt-service.c b/attrib/gatt-service.c
index bfefdee..187ef5a 100644
--- a/attrib/gatt-service.c
+++ b/attrib/gatt-service.c
@@ -116,6 +116,28 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
return l;
}

+static struct attribute *add_service_declaration(struct btd_adapter *adapter,
+ uint16_t handle, uint16_t svc, bt_uuid_t *uuid)
+{
+ bt_uuid_t bt_uuid;
+ uint8_t atval[16];
+ int len;
+
+ if (uuid->type == BT_UUID16) {
+ att_put_u16(uuid->value.u16, &atval[0]);
+ len = 2;
+ } else if (uuid->type == BT_UUID128) {
+ att_put_u128(uuid->value.u128, &atval[0]);
+ len = 16;
+ } else
+ return NULL;
+
+ bt_uuid16_create(&bt_uuid, svc);
+
+ return attrib_db_add(adapter, handle, &bt_uuid, ATT_NONE,
+ ATT_NOT_PERMITTED, atval, len);
+}
+
static int att_read_reqs(int authorization, int authentication, uint8_t props)
{
if (authorization == GATT_CHR_VALUE_READ ||
@@ -268,15 +290,21 @@ static void service_attr_del(struct btd_adapter *adapter, uint16_t start_handle,
}

gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid,
- uint16_t svc_uuid, gatt_option opt1, ...)
+ bt_uuid_t *svc_uuid, gatt_option opt1, ...)
{
+ char uuidstr[MAX_LEN_UUID_STR];
uint16_t start_handle, h;
unsigned int size;
- bt_uuid_t bt_uuid;
- uint8_t atval[2];
va_list args;
GSList *chrs, *l;

+ bt_uuid_to_string(svc_uuid, uuidstr, MAX_LEN_UUID_STR);
+
+ if (svc_uuid->type != BT_UUID16 && svc_uuid->type != BT_UUID128) {
+ error("Invalid service uuid: %s", uuidstr);
+ return FALSE;
+ }
+
va_start(args, opt1);
chrs = parse_opts(opt1, args);
/* calculate how many attributes are necessary for this service */
@@ -288,31 +316,24 @@ gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid,
start_handle = attrib_db_find_avail(adapter, size);
if (start_handle == 0) {
error("Not enough free handles to register service");
- g_slist_free_full(chrs, free_gatt_info);
- return FALSE;
+ goto fail;
}

- DBG("New service: handle 0x%04x, UUID 0x%04x, %d attributes",
- start_handle, svc_uuid, size);
+ DBG("New service: handle 0x%04x, UUID %s, %d attributes",
+ start_handle, uuidstr, size);

/* service declaration */
h = start_handle;
- bt_uuid16_create(&bt_uuid, uuid);
- att_put_u16(svc_uuid, &atval[0]);
- if (attrib_db_add(adapter, h++, &bt_uuid, ATT_NONE, ATT_NOT_PERMITTED,
- atval, sizeof(atval)) == NULL) {
- g_slist_free_full(chrs, free_gatt_info);
- return FALSE;
- }
+ if (add_service_declaration(adapter, h++, uuid, svc_uuid) == NULL)
+ goto fail;

for (l = chrs; l != NULL; l = l->next) {
struct gatt_info *info = l->data;

DBG("New characteristic: handle 0x%04x", h);
if (!add_characteristic(adapter, &h, info)) {
- g_slist_free_full(chrs, free_gatt_info);
service_attr_del(adapter, start_handle, h - 1);
- return FALSE;
+ goto fail;
}
}

@@ -321,4 +342,8 @@ gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid,
g_slist_free_full(chrs, free_gatt_info);

return TRUE;
+
+fail:
+ g_slist_free_full(chrs, free_gatt_info);
+ return FALSE;
}
diff --git a/attrib/gatt-service.h b/attrib/gatt-service.h
index 7af2d3e..b810e2e 100644
--- a/attrib/gatt-service.h
+++ b/attrib/gatt-service.h
@@ -48,4 +48,4 @@ typedef enum {
} attrib_event_t;

gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid,
- uint16_t svc_uuid, gatt_option opt1, ...);
+ bt_uuid_t *svc_uuid, gatt_option opt1, ...);
diff --git a/plugins/gatt-example.c b/plugins/gatt-example.c
index 701c1d7..e791c53 100644
--- a/plugins/gatt-example.c
+++ b/plugins/gatt-example.c
@@ -105,8 +105,11 @@ static uint8_t battery_state_read(struct attribute *a, gpointer user_data)

static gboolean register_battery_service(struct btd_adapter *adapter)
{
- return gatt_service_add(adapter, GATT_PRIM_SVC_UUID,
- BATTERY_STATE_SVC_UUID,
+ bt_uuid_t uuid;
+
+ bt_uuid16_create(&uuid, BATTERY_STATE_SVC_UUID);
+
+ return gatt_service_add(adapter, GATT_PRIM_SVC_UUID, &uuid,
/* battery state characteristic */
GATT_OPT_CHR_UUID, BATTERY_STATE_UUID,
GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
diff --git a/time/server.c b/time/server.c
index 0730fbb..42b12e2 100644
--- a/time/server.c
+++ b/time/server.c
@@ -115,9 +115,13 @@ static uint8_t local_time_info_read(struct attribute *a, gpointer user_data)

static void register_current_time_service(void)
{
+ bt_uuid_t uuid;
+
+ bt_uuid16_create(&uuid, CURRENT_TIME_SVC_UUID);
+
/* Current Time service */
/* FIXME: Provide the adapter in next function */
- gatt_service_add(NULL, GATT_PRIM_SVC_UUID, CURRENT_TIME_SVC_UUID,
+ gatt_service_add(NULL, GATT_PRIM_SVC_UUID, &uuid,
/* CT Time characteristic */
GATT_OPT_CHR_UUID, CT_TIME_CHR_UUID,
GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
--
1.7.8.4


2012-01-25 11:42:48

by Santiago Carot

[permalink] [raw]
Subject: Re: [PATCH 3/8] gatt-service: Provide service uuid in attrib_db_find_avail function

Hi Anderson,

2012/1/25 Anderson Lizardo <[email protected]>:
> Hi Santiago,
>
> On Wed, Jan 25, 2012 at 6:03 AM, Santiago Carot-Nemesio
> <[email protected]> wrote:
>> @@ -436,8 +440,8 @@ static void register_weight_service(struct gatt_example_adapter *adapter,
>> ? ? ? ?int len;
>>
>> ? ? ? ?btoh128(&char_weight_uuid_btorder, &char_weight_uuid);
>> -
>> - ? ? ? start_handle = attrib_db_find_avail(adapter->adapter, svc_size);
>> + ? ? ? bt_uuid128_create(&uuid, prim_weight_uuid_btorder);
>> + ? ? ? start_handle = attrib_db_find_avail(adapter->adapter, &uuid, svc_size);
>> ? ? ? ?if (start_handle == 0) {
>> ? ? ? ? ? ? ? ?error("Not enough free handles to register service");
>> ? ? ? ? ? ? ? ?return;
>
> This is still not right. bt_uuid128_create() should receive a
> uint128_t in *host* order. In other words, you need to do:
>
> uint128_t prim_weight_uuid;
> ...
> btoh128(&prim_weight_uuid_btorder, &prim_weight_uuid);
> bt_uuid128_create(&uuid, prim_weight_uuid);
> ...

Ok, I see, I'm going to send a new set fixing that.
Regards.

2012-01-25 11:01:46

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCH 3/8] gatt-service: Provide service uuid in attrib_db_find_avail function

Hi Santiago,

On Wed, Jan 25, 2012 at 6:03 AM, Santiago Carot-Nemesio
<[email protected]> wrote:
> @@ -436,8 +440,8 @@ static void register_weight_service(struct gatt_example_adapter *adapter,
> ? ? ? ?int len;
>
> ? ? ? ?btoh128(&char_weight_uuid_btorder, &char_weight_uuid);
> -
> - ? ? ? start_handle = attrib_db_find_avail(adapter->adapter, svc_size);
> + ? ? ? bt_uuid128_create(&uuid, prim_weight_uuid_btorder);
> + ? ? ? start_handle = attrib_db_find_avail(adapter->adapter, &uuid, svc_size);
> ? ? ? ?if (start_handle == 0) {
> ? ? ? ? ? ? ? ?error("Not enough free handles to register service");
> ? ? ? ? ? ? ? ?return;

This is still not right. bt_uuid128_create() should receive a
uint128_t in *host* order. In other words, you need to do:

uint128_t prim_weight_uuid;
...
btoh128(&prim_weight_uuid_btorder, &prim_weight_uuid);
bt_uuid128_create(&uuid, prim_weight_uuid);
...

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

2012-01-24 15:49:13

by Santiago Carot

[permalink] [raw]
Subject: Re: [PATCH 7/8] attrib-server: Allocate 128-bits UUIDs using highest available handlers

Hi Anderson,

2012/1/24 Anderson Lizardo <[email protected]>:
> Hi Santiago,
>
> On Tue, Jan 24, 2012 at 7:06 AM, Santiago Carot-Nemesio
> <[email protected]> wrote:
>> 128-uuids services are grouped at the end of the handlers database list.
>> This group grows up from the highest handlers toward lowers handlers
>> until the whole range is used or the last 16 bit-uuid service is reached.
>> ---
>> ?src/attrib-server.c | ? 47 ++++++++++++++++++++++++++++++++++++++++++++++-
>> ?1 files changed, 46 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/attrib-server.c b/src/attrib-server.c
>> index e52571c..f2bf5ef 100644
>> --- a/src/attrib-server.c
>> +++ b/src/attrib-server.c
>> @@ -1317,7 +1317,52 @@ static uint16_t find_uuid16_avail(struct btd_adapter *adapter, uint16_t nitems)
>>
>> ?static uint16_t find_uuid128_avail(struct btd_adapter *adapter, uint16_t nitems)
>> ?{
>> - ? ? ? /* TODO: Allocate 128 uuids at the end of the list */
>> + ? ? ? struct gatt_server *server;
>> + ? ? ? uint16_t handle = 0, end = 0xffff;
>> + ? ? ? gboolean pick = TRUE;
>> + ? ? ? GList *dl;
>> + ? ? ? GSList *l;
>> +
>> + ? ? ? l = g_slist_find_custom(servers, adapter, adapter_cmp);
>> + ? ? ? if (l == NULL)
>> + ? ? ? ? ? ? ? return 0;
>> +
>> + ? ? ? server = l->data;
>> + ? ? ? if (server->database == NULL)
>> + ? ? ? ? ? ? ? return 0xffff - nitems + 1;
>> +
>> + ? ? ? for (dl = g_list_last(server->database); dl; dl = dl->prev) {
>> + ? ? ? ? ? ? ? struct attribute *a = dl->data;
>> + ? ? ? ? ? ? ? if (pick) {
>> + ? ? ? ? ? ? ? ? ? ? ? handle = a->handle;
>> + ? ? ? ? ? ? ? ? ? ? ? pick = FALSE;
>> + ? ? ? ? ? ? ? }
>
> I suspect you can get this code a lot simpler without using this
> "pick" variable. Unfortunately, I don't have time to look at this
> further right now.
>
>> +
>> + ? ? ? ? ? ? ? if (bt_uuid_cmp(&a->uuid, &prim_uuid) != 0 &&
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? bt_uuid_cmp(&a->uuid, &snd_uuid) != 0)
>> + ? ? ? ? ? ? ? ? ? ? ? continue;
>
> At this point you know a->uuid is either primary or secondary. So I
> don't think it is necessary to check again below.
>
>> +
>> + ? ? ? ? ? ? ? if ((bt_uuid_cmp(&a->uuid, &prim_uuid) == 0 ||
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? bt_uuid_cmp(&a->uuid, &snd_uuid) == 0) &&
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? end - handle >= nitems)
>> + ? ? ? ? ? ? ? ? ? ? ? return end - nitems + 1;
>
> Same below. At this point, they are guaranteed to be either primary or
> secondary uuids. You only need to check for a->len.
>

Thanks for the feedback, I'll send a new set with the changes suggested.
Regards.

2012-01-24 13:48:05

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCH 7/8] attrib-server: Allocate 128-bits UUIDs using highest available handlers

Hi Santiago,

On Tue, Jan 24, 2012 at 7:06 AM, Santiago Carot-Nemesio
<[email protected]> wrote:
> 128-uuids services are grouped at the end of the handlers database list.
> This group grows up from the highest handlers toward lowers handlers
> until the whole range is used or the last 16 bit-uuid service is reached.
> ---
> ?src/attrib-server.c | ? 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> ?1 files changed, 46 insertions(+), 1 deletions(-)
>
> diff --git a/src/attrib-server.c b/src/attrib-server.c
> index e52571c..f2bf5ef 100644
> --- a/src/attrib-server.c
> +++ b/src/attrib-server.c
> @@ -1317,7 +1317,52 @@ static uint16_t find_uuid16_avail(struct btd_adapter *adapter, uint16_t nitems)
>
> ?static uint16_t find_uuid128_avail(struct btd_adapter *adapter, uint16_t nitems)
> ?{
> - ? ? ? /* TODO: Allocate 128 uuids at the end of the list */
> + ? ? ? struct gatt_server *server;
> + ? ? ? uint16_t handle = 0, end = 0xffff;
> + ? ? ? gboolean pick = TRUE;
> + ? ? ? GList *dl;
> + ? ? ? GSList *l;
> +
> + ? ? ? l = g_slist_find_custom(servers, adapter, adapter_cmp);
> + ? ? ? if (l == NULL)
> + ? ? ? ? ? ? ? return 0;
> +
> + ? ? ? server = l->data;
> + ? ? ? if (server->database == NULL)
> + ? ? ? ? ? ? ? return 0xffff - nitems + 1;
> +
> + ? ? ? for (dl = g_list_last(server->database); dl; dl = dl->prev) {
> + ? ? ? ? ? ? ? struct attribute *a = dl->data;
> + ? ? ? ? ? ? ? if (pick) {
> + ? ? ? ? ? ? ? ? ? ? ? handle = a->handle;
> + ? ? ? ? ? ? ? ? ? ? ? pick = FALSE;
> + ? ? ? ? ? ? ? }

I suspect you can get this code a lot simpler without using this
"pick" variable. Unfortunately, I don't have time to look at this
further right now.

> +
> + ? ? ? ? ? ? ? if (bt_uuid_cmp(&a->uuid, &prim_uuid) != 0 &&
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? bt_uuid_cmp(&a->uuid, &snd_uuid) != 0)
> + ? ? ? ? ? ? ? ? ? ? ? continue;

At this point you know a->uuid is either primary or secondary. So I
don't think it is necessary to check again below.

> +
> + ? ? ? ? ? ? ? if ((bt_uuid_cmp(&a->uuid, &prim_uuid) == 0 ||
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? bt_uuid_cmp(&a->uuid, &snd_uuid) == 0) &&
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? end - handle >= nitems)
> + ? ? ? ? ? ? ? ? ? ? ? return end - nitems + 1;

Same below. At this point, they are guaranteed to be either primary or
secondary uuids. You only need to check for a->len.

> +
> + ? ? ? ? ? ? ? if (a->len == 2 && (bt_uuid_cmp(&a->uuid, &prim_uuid) == 0 ||
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? bt_uuid_cmp(&a->uuid, &snd_uuid) == 0)) {
> + ? ? ? ? ? ? ? ? ? ? ? /* 16 bit UUID service definition */
> + ? ? ? ? ? ? ? ? ? ? ? return 0;
> + ? ? ? ? ? ? ? }
> +
> + ? ? ? ? ? ? ? if (a->handle == 0x0001)
> + ? ? ? ? ? ? ? ? ? ? ? return 0;
> +
> + ? ? ? ? ? ? ? end = a->handle - 1;
> + ? ? ? ? ? ? ? pick = TRUE;
> + ? ? ? }
> +
> + ? ? ? if (end - 0x0001 >= nitems)
> + ? ? ? ? ? ? ? return end - nitems + 1;
> +
> ? ? ? ?return 0;
> ?}
>
> --
> 1.7.8.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html


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

2012-01-24 12:47:32

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCH 4/8] attrib-server: Allocate 16-bits UUIDS at the begining of the list

Hi Santiago,

On Tue, Jan 24, 2012 at 7:06 AM, Santiago Carot-Nemesio
<[email protected]> wrote:
> ---
> ?src/attrib-server.c | ? 43 ++++++++++++++++++++++++++++++++++++-------
> ?1 files changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/src/attrib-server.c b/src/attrib-server.c
> index 93b7216..68a0b1c 100644
> --- a/src/attrib-server.c
> +++ b/src/attrib-server.c
> @@ -350,7 +350,6 @@ static struct attribute *attrib_db_add_new(struct gatt_server *server,
>
> ? ? ? ?server->database = g_slist_insert_sorted(server->database, a,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?attribute_cmp);
> -
> ? ? ? ?return a;
> ?}

The change above is bogus.

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

2012-01-24 12:18:27

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCH 3/8] gatt-service: Provide service uuid in attrib_db_find_avail function

Hi Santiago,

On Tue, Jan 24, 2012 at 7:06 AM, Santiago Carot-Nemesio
<[email protected]> wrote:
> We need to provide the service uuid because of GATT server should group
> 16-bit uuid services together and 128-bit uuid services together,
> (Bluetooth 4.0, Vol 3, Part G, 3.1).
> ---
> ?attrib/gatt-service.c ?| ? ?2 +-
> ?plugins/gatt-example.c | ? 16 ++++++++++------
> ?proximity/reporter.c ? | ? ?9 ++++++---
> ?src/attrib-server.c ? ?| ? ?3 ++-
> ?src/attrib-server.h ? ?| ? ?3 ++-
> ?5 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/attrib/gatt-service.c b/attrib/gatt-service.c
> index d00adb0..b27e525 100644
> --- a/attrib/gatt-service.c
> +++ b/attrib/gatt-service.c
> @@ -436,8 +440,8 @@ static void register_weight_service(struct gatt_example_adapter *adapter,
> ? ? ? ?int len;
>
> ? ? ? ?btoh128(&char_weight_uuid_btorder, &char_weight_uuid);
> -
> - ? ? ? start_handle = attrib_db_find_avail(adapter->adapter, svc_size);
> + ? ? ? bt_uuid128_create(&uuid, char_weight_uuid_btorder);
> + ? ? ? start_handle = attrib_db_find_avail(adapter->adapter, &uuid, svc_size);
> ? ? ? ?if (start_handle == 0) {
> ? ? ? ? ? ? ? ?error("Not enough free handles to register service");
> ? ? ? ? ? ? ? ?return;

This is wrong. You are using a *characteristic* UUID as a service
UUID. This example weight service is 16-bit, but it is has one 128-bit
characteristic.

BTW, 16-bit vs. 128-bit characteristic grouping is optional as per
page 1897 (section 3.3.1 "Characteristic Declaration").

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

2012-01-24 12:03:17

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCH 1/8] gatt-service: Add support for 128-bit Bluetooth UUIDs

Hi Santiago,

On Tue, Jan 24, 2012 at 7:06 AM, Santiago Carot-Nemesio
<[email protected]> wrote:
> UUID services in GATT should be either 16-bit or 128-bit. Current
> GATT interface does not allow to provide 128-bit ones. This patch
> enables plugins to register services using 128-bit UUIDs.
> ---
> ?attrib/gatt-service.c ?| ? 57 ++++++++++++++++++++++++++++++++++-------------
> ?attrib/gatt-service.h ?| ? ?2 +-
> ?plugins/gatt-example.c | ? ?7 ++++-
> ?time/server.c ? ? ? ? ?| ? ?6 ++++-
> ?4 files changed, 52 insertions(+), 20 deletions(-)
>
> diff --git a/attrib/gatt-service.c b/attrib/gatt-service.c
> index bfefdee..eeda366 100644
> --- a/attrib/gatt-service.c
> +++ b/attrib/gatt-service.c
> @@ -116,6 +116,28 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
> ? ? ? ?return l;
> ?}
>
> +static struct attribute *add_service_declaration(struct btd_adapter *adapter,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? uint16_t handle, uint16_t scv, bt_uuid_t *uuid)

typo: scv -> svc

> +{
> + ? ? ? bt_uuid_t bt_uuid;
> + ? ? ? uint8_t atval[16];
> + ? ? ? int len;
> +
> + ? ? ? if (uuid->type == BT_UUID16) {
> + ? ? ? ? ? ? ? att_put_u16(uuid->value.u16, &atval[0]);
> + ? ? ? ? ? ? ? len = 2;
> + ? ? ? } else if (uuid->type == BT_UUID128) {
> + ? ? ? ? ? ? ? att_put_u128(uuid->value.u128, &atval[0]);
> + ? ? ? ? ? ? ? len = 16;
> + ? ? ? } else
> + ? ? ? ? ? ? ? return NULL;
> +
> + ? ? ? bt_uuid16_create(&bt_uuid, scv);
> +
> + ? ? ? return attrib_db_add(adapter, handle, &bt_uuid, ATT_NONE,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ATT_NOT_PERMITTED, atval, len);
> +}
> +
> ?static int att_read_reqs(int authorization, int authentication, uint8_t props)
> ?{
> ? ? ? ?if (authorization == GATT_CHR_VALUE_READ ||
> @@ -268,15 +290,21 @@ static void service_attr_del(struct btd_adapter *adapter, uint16_t start_handle,
> ?}
>
> ?gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? uint16_t svc_uuid, gatt_option opt1, ...)
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? bt_uuid_t svc_uuid, gatt_option opt1, ...)

I think it is more efficient to pass svc_uuid as pointer (this avoids
a possible 128-bit copy).

> ?{
> + ? ? ? char uuidstr[MAX_LEN_UUID_STR];
> ? ? ? ?uint16_t start_handle, h;
> ? ? ? ?unsigned int size;
> - ? ? ? bt_uuid_t bt_uuid;
> - ? ? ? uint8_t atval[2];
> ? ? ? ?va_list args;
> ? ? ? ?GSList *chrs, *l;
>
> + ? ? ? bt_uuid_to_string(&svc_uuid, uuidstr, MAX_LEN_UUID_STR);
> +
> + ? ? ? if (svc_uuid.type != BT_UUID16 && svc_uuid.type != BT_UUID128) {
> + ? ? ? ? ? ? ? error("Invalid service uuid: %s", uuidstr);
> + ? ? ? ? ? ? ? return FALSE;
> + ? ? ? }
> +
> ? ? ? ?va_start(args, opt1);
> ? ? ? ?chrs = parse_opts(opt1, args);
> ? ? ? ?/* calculate how many attributes are necessary for this service */

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

2012-02-03 18:48:31

by Johan Hedberg

[permalink] [raw]
Subject: Re: GATT improvements v3

Hi Santiago,

On Wed, Jan 25, 2012, Santiago Carot-Nemesio wrote:
> The same set of patches but including all changes suggested.
>
> [PATCH 1/8] gatt-service: Add support for 128-bit Bluetooth UUIDs
> [PATCH 2/8] gatt-service: Move va_end just after processing the
> [PATCH 3/8] gatt-service: Provide service uuid in
> [PATCH 4/8] attrib-server: Allocate 16-bits UUIDS at the begining of
> [PATCH 5/8] attrib-server: Set database uuids as a double linked
> [PATCH 6/8] glib-compat: Add g_list_free_full to deal with issues in
> [PATCH 7/8] attrib-server: Allocate 128-bits UUIDs using highest
> [PATCH 8/8] gatt-example: Fix g_assert checks when an uint16_t value

All of these patches have been pushed upstream. Thanks.

Johan

2012-02-03 08:37:35

by Santiago Carot

[permalink] [raw]
Subject: Re: GATT improvements v3

ping

2012/1/25 Santiago Carot-Nemesio <[email protected]>:
> The same set of patches but including all changes suggested.
>
> [PATCH 1/8] gatt-service: Add support for 128-bit Bluetooth UUIDs
> [PATCH 2/8] gatt-service: Move va_end just after processing the
> [PATCH 3/8] gatt-service: Provide service uuid in
> [PATCH 4/8] attrib-server: Allocate 16-bits UUIDS at the begining of
> [PATCH 5/8] attrib-server: Set database uuids as a double linked
> [PATCH 6/8] glib-compat: Add g_list_free_full to deal with issues in
> [PATCH 7/8] attrib-server: Allocate 128-bits UUIDs using highest
> [PATCH 8/8] gatt-example: Fix g_assert checks when an uint16_t value