Skip to contentSkip to conversation

attrib/gatt_service_add: support 128-bit uuids for characteristics

PATCH In review Dirk-Jan C. Binnema 3 participants 14 messages Jul 23, 2013 – Jul 25, 2013 Bluetooth drivers

lore
Share & tools
lore

Dirk-Jan C. Binnema

author
#0 lore raw

PATCH 1/1 attrib/gatt_service_add: support 128-bit uuids for characteristics

From: "Dirk-Jan C. Binnema" <[email protected]>

Add GATT_OPT_CHR_BT_UUID_T, which does what GATT_OPT_CHR_UUID does, but instead takes a bt_uuid_t*, so 128-bit uuids are supported as well.

GATT_OPT_CHR_UUID is kept around for backward compatibility.

Signed-off-by Dirk-Jan C. Binnema <[email protected]>
---
 attrib/gatt-service.c | 19 ++++++++++++-------
 attrib/gatt-service.h |  1 +
 2 files changed, 13 insertions(+), 7 deletions(-)
attrib/gatt-service.c +12 -7
index 4b02d39..bc17778 100644
@@ -73,6 +73,11 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 			/* characteristic declaration and value */
 			info->num_attrs += 2;
 			break;
+		case GATT_OPT_CHR_BT_UUID_T:
+			memcpy(&info->uuid, va_arg(args, bt_uuid_t *), sizeof(bt_uuid_t));
+			/* characteristic declaration and value */
+			info->num_attrs += 2;
+			break;
 		case GATT_OPT_CHR_PROPS:
 			info->props = va_arg(args, int);
 

@@ -108,7 +113,7 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 		}
 
 		opt = va_arg(args, gatt_option);
-		if (opt == GATT_OPT_CHR_UUID) {
+		if (opt == GATT_OPT_CHR_UUID || opt == GATT_OPT_CHR_BT_UUID_T) {
 			info = g_new0(struct gatt_info, 1);
 			l = g_slist_append(l, info);
 		}

@@ -177,16 +182,16 @@ static int find_callback(gconstpointer a, gconstpointer b)
 }
 
 static gboolean add_characteristic(struct btd_adapter *adapter,
-				uint16_t *handle, struct gatt_info *info)
+				   uint16_t *handle, struct gatt_info *info)
 {
 	int read_req, write_req;
 	uint16_t h = *handle;
 	struct attribute *a;
 	bt_uuid_t bt_uuid;
-	uint8_t atval[5];
+	uint8_t atval[131];
 	GSList *l;
 
-	if (!info->uuid.value.u16 || !info->props) {
+	if ((!info->uuid.value.u16 && info->uuid.type != BT_UUID128) || !info->props) {
 		error("Characteristic UUID or properties are missing");
 		return FALSE;
 	}

@@ -222,9 +227,9 @@ static gboolean add_characteristic(struct btd_adapter *adapter,
 	bt_uuid16_create(&bt_uuid, GATT_CHARAC_UUID);
 	atval[0] = info->props;
 	att_put_u16(h + 1, &atval[1]);
-	att_put_u16(info->uuid.value.u16, &atval[3]);
+	att_put_uuid(info->uuid, &atval[3]);
 	if (attrib_db_add(adapter, h++, &bt_uuid, ATT_NONE, ATT_NOT_PERMITTED,
-						atval, sizeof(atval)) == NULL)
+			  atval, 3 + info->uuid.type/8) == NULL)
 		return FALSE;
 
 	/* characteristic value */

@@ -341,7 +346,7 @@ gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid,
 	}
 
 	g_assert(size < USHRT_MAX);
-	g_assert(h - start_handle == (uint16_t) size);
+	g_assert((uint16_t)(h - start_handle) == (uint16_t) size);
 	g_slist_free_full(chrs, free_gatt_info);
 
 	return TRUE;
attrib/gatt-service.h +1 -0
index b810e2e..5c3e15d 100644
@@ -25,6 +25,7 @@
 typedef enum {
 	GATT_OPT_INVALID = 0,
 	GATT_OPT_CHR_UUID,
+	GATT_OPT_CHR_BT_UUID_T,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
Signature · 1 line
1.8.3.2
#1 lore raw

Hi Dirk-Jan,

attrib/gatt-service.c static GSList *parse_opts(gatt_option opt1, va_list args) · line 77

Quoted 2 lines · Dirk-Jan C. Binnema
attrib/gatt-service.c +2 -0
+		case GATT_OPT_CHR_BT_UUID_T:
+			memcpy(&info->uuid, va_arg(args, bt_uuid_t *), sizeof(bt_uuid_t));

The above looks like a too long line to me. Please stick to under 80 characters.

attrib/gatt-service.c static int find_callback(gconstpointer a, gconstpointer b) · line 185

Quoted 3 lines

static gboolean add_characteristic(struct btd_adapter *adapter,

attrib/gatt-service.c +1 -1
-				uint16_t *handle, struct gatt_info *info)
+				   uint16_t *handle, struct gatt_info *info)

The above coding style change seems unrelated to this patch.

attrib/gatt-service.c static int find_callback(gconstpointer a, gconstpointer b) · line 194

Quoted 2 lines
attrib/gatt-service.c +1 -1
-	if (!info->uuid.value.u16 || !info->props) {
+	if ((!info->uuid.value.u16 && info->uuid.type != BT_UUID128) || !info->props) {

Looks like a too long line as well.

info->uuid.value is a union so I don't think it's safe to check any particular member before you've checked uuid.type. However, isn't what you want above to check that you have a BT_UUID16 or BT_UUID128? I.e. there should be no need to check for info->uuid.value.u16 directly.

attrib/gatt-service.c static gboolean add_characteristic(struct btd_adapter *adapter, · line 232

Quoted 3 lines
attrib/gatt-service.c +1 -1
 	if (attrib_db_add(adapter, h++, &bt_uuid, ATT_NONE, ATT_NOT_PERMITTED,
-						atval, sizeof(atval)) == NULL)
+			  atval, 3 + info->uuid.type/8) == NULL)

It looks like the above change introduces a mix of spaces and tabs for indentation. The user space style is to use only tabs (indent as much as you can as long as you don't go over 80 characters).

attrib/gatt-service.c gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid, · line 349

Quoted 2 lines
attrib/gatt-service.c +1 -1
-	g_assert(h - start_handle == (uint16_t) size);
+	g_assert((uint16_t)(h - start_handle) == (uint16_t) size);

Be consistent with the coding style here: type casts should have a space between the cast and the variable.

attrib/gatt-service.h line 31

Quoted 7 lines

typedef enum {

attrib/gatt-service.h +1 -0
 	GATT_OPT_INVALID = 0,
 	GATT_OPT_CHR_UUID,
+	GATT_OPT_CHR_BT_UUID_T,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,

To be honest, I'm not 100% sure this is the best name for the new option, but I can't really think of anything much better either. Renaming the existing one to UUID_STR and the new one to UUID (like the old one was) would be one option, but it's one more patch and more code changes.

Johan

author
#2 lore raw

Hi Johan,

Thanks for the quick review! I've updated the too-long lines and taught emacs to avoid mixing tabs/space in bluez code.

Quoted 4 lines · On 2013-07-24 07:57, [email protected] wrote:

info->uuid.value is a union so I don't think it's safe to check any particular member before you've checked uuid.type. However, isn't what you want above to check that you have a BT_UUID16 or BT_UUID128? I.e. there should be no need to check for info->uuid.value.u16 directly.

Makes sense; note that the value check was in the original code, which I didn't want to touch. In any case, I've changed it now.

Quoted 16 lines

Be consistent with the coding style here: type casts should have a space between the cast and the variable.

attrib/gatt-service.h line 31

Quoted 7 lines

typedef enum {

attrib/gatt-service.h +1 -0
 	GATT_OPT_INVALID = 0,
 	GATT_OPT_CHR_UUID,
+	GATT_OPT_CHR_BT_UUID_T,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,

To be honest, I'm not 100% sure this is the best name for the new option, but I can't really think of anything much better either. Renaming the existing one to UUID_STR and the new one to UUID (like the old one was) would be one option, but it's one more patch and more code changes.

Indeed, couldn't come up with a better name myself :/... The old one takes a uint16_t, so maybe we could call it UUID_16, and perhaps in the future add UUID_STR for string-encoded 128-bit uuids.

Anyway, I'll send an updated patch.

Cheers,
Dirk.

Signature · 3 lines
Dirk-Jan C. Binnema                  Helsinki, Finland
e:[email protected]           w:www.djcbsoftware.nl
pgp: D09C E664 897D 7D39 5047 A178 E96A C7A1 017D DA3C
author
#3 lore raw

PATCH 1/1 attrib/gatt_service_add: support 128-bit uuids for characteristics

In reply to Johan Hedberg

From: "Dirk-Jan C. Binnema" <[email protected]>

Add GATT_OPT_CHR_BT_UUID_T, which does what GATT_OPT_CHR_UUID does, but instead takes a bt_uuid_t*, so 128-bit uuids are supported as well.

GATT_OPT_CHR_UUID is kept around for backward compatibility.

---
 attrib/gatt-service.c | 19 +++++++++++++------
 attrib/gatt-service.h |  1 +
 2 files changed, 14 insertions(+), 6 deletions(-)
attrib/gatt-service.c +13 -6
index 4b02d39..b9e5a70 100644
@@ -73,6 +73,12 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 			/* characteristic declaration and value */
 			info->num_attrs += 2;
 			break;
+		case GATT_OPT_CHR_BT_UUID_T:
+			memcpy(&info->uuid, va_arg(args, bt_uuid_t *),
+				sizeof(bt_uuid_t));
+			/* characteristic declaration and value */
+			info->num_attrs += 2;
+			break;
 		case GATT_OPT_CHR_PROPS:
 			info->props = va_arg(args, int);
 

@@ -108,7 +114,7 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 		}
 
 		opt = va_arg(args, gatt_option);
-		if (opt == GATT_OPT_CHR_UUID) {
+		if (opt == GATT_OPT_CHR_UUID || opt == GATT_OPT_CHR_BT_UUID_T) {
 			info = g_new0(struct gatt_info, 1);
 			l = g_slist_append(l, info);
 		}

@@ -183,10 +189,11 @@ static gboolean add_characteristic(struct btd_adapter *adapter,
 	uint16_t h = *handle;
 	struct attribute *a;
 	bt_uuid_t bt_uuid;
-	uint8_t atval[5];
+	uint8_t atval[131];
 	GSList *l;
 
-	if (!info->uuid.value.u16 || !info->props) {
+	if ((info->uuid.type != BT_UUID16 && info->uuid.type != BT_UUID128) ||
+		!info->props) {
 		error("Characteristic UUID or properties are missing");
 		return FALSE;
 	}

@@ -222,9 +229,9 @@ static gboolean add_characteristic(struct btd_adapter *adapter,
 	bt_uuid16_create(&bt_uuid, GATT_CHARAC_UUID);
 	atval[0] = info->props;
 	att_put_u16(h + 1, &atval[1]);
-	att_put_u16(info->uuid.value.u16, &atval[3]);
+	att_put_uuid(info->uuid, &atval[3]);
 	if (attrib_db_add(adapter, h++, &bt_uuid, ATT_NONE, ATT_NOT_PERMITTED,
-						atval, sizeof(atval)) == NULL)
+				atval, 3 + info->uuid.type/8) == NULL)
 		return FALSE;
 
 	/* characteristic value */

@@ -341,7 +348,7 @@ gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid,
 	}
 
 	g_assert(size < USHRT_MAX);
-	g_assert(h - start_handle == (uint16_t) size);
+	g_assert((uint16_t) (h - start_handle) == (uint16_t) size);
 	g_slist_free_full(chrs, free_gatt_info);
 
 	return TRUE;
attrib/gatt-service.h +1 -0
index b810e2e..5c3e15d 100644
@@ -25,6 +25,7 @@
 typedef enum {
 	GATT_OPT_INVALID = 0,
 	GATT_OPT_CHR_UUID,
+	GATT_OPT_CHR_BT_UUID_T,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
Signature · 1 line
1.8.3.2
#4 lore raw

Hi Dirk-Jan,

attrib/gatt-service.c static gboolean add_characteristic(struct btd_adapter *adapter, · line 192

Quoted 6 lines · Dirk-Jan C. Binnema
attrib/gatt-service.c +1 -1
@@ -183,10 +189,11 @@ static gboolean add_characteristic(struct btd_adapter *adapter,
        uint16_t h = *handle;
        struct attribute *a;
        bt_uuid_t bt_uuid;
-       uint8_t atval[5];
+       uint8_t atval[131];

Where "131" comes from? A UUID has 128 bits (i.e. 16 bytes). Anyway, I suggest using atval[ATT_MAX_VALUE_LEN] and not worry about the size here, as long as you pass the exact site to atttrib_db_add(), as you do below.

attrib/gatt-service.c static gboolean add_characteristic(struct btd_adapter *adapter, · line 234

Quoted 9 lines
attrib/gatt-service.c +2 -2
@@ -222,9 +229,9 @@ static gboolean add_characteristic(struct btd_adapter *adapter,
        bt_uuid16_create(&bt_uuid, GATT_CHARAC_UUID);
        atval[0] = info->props;
        att_put_u16(h + 1, &atval[1]);
-       att_put_u16(info->uuid.value.u16, &atval[3]);
+       att_put_uuid(info->uuid, &atval[3]);
        if (attrib_db_add(adapter, h++, &bt_uuid, ATT_NONE, ATT_NOT_PERMITTED,
-                                               atval, sizeof(atval)) == NULL)
+                               atval, 3 + info->uuid.type/8) == NULL)

Add spaces around "/" here, i.e: 3 + info->uuid.type / 8

attrib/gatt-service.c gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid, · line 351

Quoted 9 lines
attrib/gatt-service.c +1 -1
                return FALSE;
 
        /* characteristic value */

@@ -341,7 +348,7 @@ gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid,
        }
 
        g_assert(size < USHRT_MAX);
-       g_assert(h - start_handle == (uint16_t) size);
+       g_assert((uint16_t) (h - start_handle) == (uint16_t) size);

This actually seems unrelated to the patch. Can you put it into a separate patch with the compilation error you got?

attrib/gatt-service.h line 28

Quoted 13 lines
attrib/gatt-service.c +0 -0
        g_slist_free_full(chrs, free_gatt_info);
 
        return TRUE;
[...]
attrib/gatt-service.h +1 -0
index b810e2e..5c3e15d 100644
@@ -25,6 +25,7 @@
 typedef enum {
        GATT_OPT_INVALID = 0,
        GATT_OPT_CHR_UUID,
+       GATT_OPT_CHR_BT_UUID_T,

My suggestion would be to rename the old one to GATT_OPT_CHR_UUID16 and this could be GATT_OPT_CHR_UUID. But as Johan said, this could be in a separate patch.

attrib/gatt-service.h line 31

Quoted 3 lines
attrib/gatt-service.h +0 -0
        GATT_OPT_CHR_PROPS,
        GATT_OPT_CHR_VALUE_CB,
        GATT_OPT_CHR_AUTHENTICATION,

Best Regards,

Signature · 3 lines
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
author
#5 lore raw

Hi Lizardo,

Thanks for the comments!

Quoted 4 lines · On 2013-07-24 13:55, [email protected] wrote:

Hi Dirk-Jan,

On Wed, Jul 24, 2013 at 3:32 AM, Dirk-Jan C. Binnema <[email protected]> wrote:

Quoted 11 lines

attrib/gatt-service.c static gboolean add_characteristic(struct btd_adapter *adapter, · line 192

Quoted 6 lines
attrib/gatt-service.c +1 -1
@@ -183,10 +189,11 @@ static gboolean add_characteristic(struct btd_adapter *adapter,
        uint16_t h = *handle;
        struct attribute *a;
        bt_uuid_t bt_uuid;
-       uint8_t atval[5];
+       uint8_t atval[131];

Where "131" comes from? A UUID has 128 bits (i.e. 16 bytes). Anyway, I suggest using atval[ATT_MAX_VALUE_LEN] and not worry about the size here, as long as you pass the exact site to atttrib_db_add(), as you do below.

Well, a little bit below that, there is:

    att_put_uuid(info->uuid, &atval[3]);

so I guessed I need to reserve the three extra bytes; this is also why it was 5 before, I think. Or?

ATT_MAX_VALUE_LEN is 512, but it seems to be the maximum size of a value, rather than the UUID. It's more than enough at least :)

Quoted 5 lines

attrib/gatt-service.c gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid, · line 351

Quoted 2 lines
attrib/gatt-service.c +1 -1
-       g_assert(h - start_handle == (uint16_t) size);
+       g_assert((uint16_t) (h - start_handle) == (uint16_t) size);

This actually seems unrelated to the patch. Can you put it into a separate patch with the compilation error you got?

When using 128-bit UUIDs, the start handle ultimately comes from find_uuid128_avail, which returns 0xffff - nitems + 1 (for the first item). If I don't explicitly cast it, h - start_handle will be negative number. So, my patch doesn't really work without this.

Quoted 7 lines

My suggestion would be to rename the old one to GATT_OPT_CHR_UUID16 and this could be GATT_OPT_CHR_UUID. But as Johan said, this could be in a separate patch.

Quoted 3 lines
attrib/gatt-service.h +0 -0
        GATT_OPT_CHR_PROPS,
        GATT_OPT_CHR_VALUE_CB,
        GATT_OPT_CHR_AUTHENTICATION,

That makes sense -- but as the newcomer here, I'll happily let the others reach some consensus on what the best naming would be, and I'll make a patch after that :-)

Cheers,
Dirk.

Signature · 3 lines
Dirk-Jan C. Binnema                  Helsinki, Finland
e:[email protected]           w:www.djcbsoftware.nl
pgp: D09C E664 897D 7D39 5047 A178 E96A C7A1 017D DA3C
#6 lore raw

Hi Dirk,

Quoted 11 lines · Dirk-Jan C. Binnema
Quoted 7 lines

My suggestion would be to rename the old one to GATT_OPT_CHR_UUID16 and this could be GATT_OPT_CHR_UUID. But as Johan said, this could be in a separate patch.

Quoted 3 lines

GATT_OPT_CHR_PROPS,
GATT_OPT_CHR_VALUE_CB,
GATT_OPT_CHR_AUTHENTICATION,

That makes sense -- but as the newcomer here, I'll happily let the others reach some consensus on what the best naming would be, and I'll make a patch after that :-)

GATT_OPT_CHR_UUID16 and GATT_OPT_CHR_UUID sound fine to me so you can proceed with those.

Johan

author
#7 lore raw

PATCH 1/2 Add support for 128-bit UUIDs for characteristics.

From: "Dirk-Jan C. Binnema" <[email protected]>

---
 attrib/gatt-service.c | 19 +++++++++++++------
 attrib/gatt-service.h |  1 +
 2 files changed, 14 insertions(+), 6 deletions(-)
attrib/gatt-service.c +13 -6
index 4b02d39..e4ca78c 100644
@@ -73,6 +73,12 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 			/* characteristic declaration and value */
 			info->num_attrs += 2;
 			break;
+		case GATT_OPT_CHR_BT_UUID_T:
+			memcpy(&info->uuid, va_arg(args, bt_uuid_t *),
+				sizeof(bt_uuid_t));
+			/* characteristic declaration and value */
+			info->num_attrs += 2;
+			break;
 		case GATT_OPT_CHR_PROPS:
 			info->props = va_arg(args, int);
 

@@ -108,7 +114,7 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 		}
 
 		opt = va_arg(args, gatt_option);
-		if (opt == GATT_OPT_CHR_UUID) {
+		if (opt == GATT_OPT_CHR_UUID || opt == GATT_OPT_CHR_BT_UUID_T) {
 			info = g_new0(struct gatt_info, 1);
 			l = g_slist_append(l, info);
 		}

@@ -183,10 +189,11 @@ static gboolean add_characteristic(struct btd_adapter *adapter,
 	uint16_t h = *handle;
 	struct attribute *a;
 	bt_uuid_t bt_uuid;
-	uint8_t atval[5];
+	uint8_t atval[131];
 	GSList *l;
 
-	if (!info->uuid.value.u16 || !info->props) {
+	if ((info->uuid.type != BT_UUID16 && info->uuid.type != BT_UUID128) ||
+		!info->props) {
 		error("Characteristic UUID or properties are missing");
 		return FALSE;
 	}

@@ -222,9 +229,9 @@ static gboolean add_characteristic(struct btd_adapter *adapter,
 	bt_uuid16_create(&bt_uuid, GATT_CHARAC_UUID);
 	atval[0] = info->props;
 	att_put_u16(h + 1, &atval[1]);
-	att_put_u16(info->uuid.value.u16, &atval[3]);
+	att_put_uuid(info->uuid, &atval[3]);
 	if (attrib_db_add(adapter, h++, &bt_uuid, ATT_NONE, ATT_NOT_PERMITTED,
-						atval, sizeof(atval)) == NULL)
+				atval, 3 + info->uuid.type / 8) == NULL)
 		return FALSE;
 
 	/* characteristic value */

@@ -341,7 +348,7 @@ gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid,
 	}
 
 	g_assert(size < USHRT_MAX);
-	g_assert(h - start_handle == (uint16_t) size);
+	g_assert((uint16_t) (h - start_handle) == (uint16_t) size);
 	g_slist_free_full(chrs, free_gatt_info);
 
 	return TRUE;
attrib/gatt-service.h +1 -0
index b810e2e..5c3e15d 100644
@@ -25,6 +25,7 @@
 typedef enum {
 	GATT_OPT_INVALID = 0,
 	GATT_OPT_CHR_UUID,
+	GATT_OPT_CHR_BT_UUID_T,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
Signature · 1 line
1.8.3.2
author
#8 lore raw

PATCH 2/2 Use GATT_OPT_CHR_UUID for bt_uuid_t*, GATT_OPT_CHR_UUID16 for uint16 chrs

From: "Dirk-Jan C. Binnema" <[email protected]>

This renames GATT_OPT_CHR_UUID into GATT_OPT_CHR_UUID16, and renames the (temporary) GATT_OPT_CHR_BT_UUID_T into GATT_OPT_CHR_UUID. Also, updates all in-tree users.

---
 attrib/gatt-service.c         |  6 +++---
 attrib/gatt-service.h         |  2 +-
 plugins/gatt-example.c        |  2 +-
 profiles/alert/server.c       | 16 ++++++++--------
 profiles/proximity/immalert.c |  2 +-
 profiles/proximity/linkloss.c |  2 +-
 profiles/time/server.c        |  8 ++++----
 7 files changed, 19 insertions(+), 19 deletions(-)
attrib/gatt-service.c +3 -3
index e4ca78c..86e0093 100644
@@ -68,12 +68,12 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 
 	while (opt != GATT_OPT_INVALID) {
 		switch (opt) {
-		case GATT_OPT_CHR_UUID:
+		case GATT_OPT_CHR_UUID16:
 			bt_uuid16_create(&info->uuid, va_arg(args, int));
 			/* characteristic declaration and value */
 			info->num_attrs += 2;
 			break;
-		case GATT_OPT_CHR_BT_UUID_T:
+		case GATT_OPT_CHR_UUID:
 			memcpy(&info->uuid, va_arg(args, bt_uuid_t *),
 				sizeof(bt_uuid_t));
 			/* characteristic declaration and value */

@@ -114,7 +114,7 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 		}
 
 		opt = va_arg(args, gatt_option);
-		if (opt == GATT_OPT_CHR_UUID || opt == GATT_OPT_CHR_BT_UUID_T) {
+		if (opt == GATT_OPT_CHR_UUID16 || opt == GATT_OPT_CHR_UUID) {
 			info = g_new0(struct gatt_info, 1);
 			l = g_slist_append(l, info);
 		}
attrib/gatt-service.h +1 -1
index 5c3e15d..2c05c7d 100644
@@ -24,8 +24,8 @@
 
 typedef enum {
 	GATT_OPT_INVALID = 0,
+	GATT_OPT_CHR_UUID16,
 	GATT_OPT_CHR_UUID,
-	GATT_OPT_CHR_BT_UUID_T,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
plugins/gatt-example.c +1 -1
index 37972e8..b926947 100644
@@ -114,7 +114,7 @@ static gboolean register_battery_service(struct btd_adapter *adapter)
 
 	return gatt_service_add(adapter, GATT_PRIM_SVC_UUID, &uuid,
 			/* battery state characteristic */
-			GATT_OPT_CHR_UUID, BATTERY_STATE_UUID,
+			GATT_OPT_CHR_UUID16, BATTERY_STATE_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
profiles/alert/server.c +8 -8
index 8da1928..4565470 100644
@@ -789,7 +789,7 @@ static void register_phone_alert_service(struct alert_adapter *al_adapter)
 	/* Phone Alert Status Service */
 	gatt_service_add(al_adapter->adapter, GATT_PRIM_SVC_UUID, &uuid,
 			/* Alert Status characteristic */
-			GATT_OPT_CHR_UUID, ALERT_STATUS_CHR_UUID,
+			GATT_OPT_CHR_UUID16, ALERT_STATUS_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,

@@ -799,12 +799,12 @@ static void register_phone_alert_service(struct alert_adapter *al_adapter)
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->hnd_value[NOTIFY_ALERT_STATUS],
 			/* Ringer Control Point characteristic */
-			GATT_OPT_CHR_UUID, RINGER_CP_CHR_UUID,
+			GATT_OPT_CHR_UUID16, RINGER_CP_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_WRITE_WITHOUT_RESP,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
 			ringer_cp_write, NULL,
 			/* Ringer Setting characteristic */
-			GATT_OPT_CHR_UUID, RINGER_SETTING_CHR_UUID,
+			GATT_OPT_CHR_UUID16, RINGER_SETTING_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,

@@ -892,35 +892,35 @@ static void register_alert_notif_service(struct alert_adapter *al_adapter)
 	/* Alert Notification Service */
 	gatt_service_add(al_adapter->adapter, GATT_PRIM_SVC_UUID, &uuid,
 			/* Supported New Alert Category */
-			GATT_OPT_CHR_UUID, SUPP_NEW_ALERT_CAT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, SUPP_NEW_ALERT_CAT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 			supp_new_alert_cat_read, al_adapter->adapter,
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->supp_new_alert_cat_handle,
 			/* New Alert */
-			GATT_OPT_CHR_UUID, NEW_ALERT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, NEW_ALERT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CCC_GET_HANDLE,
 			&al_adapter->hnd_ccc[NOTIFY_NEW_ALERT],
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->hnd_value[NOTIFY_NEW_ALERT],
 			/* Supported Unread Alert Category */
-			GATT_OPT_CHR_UUID, SUPP_UNREAD_ALERT_CAT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, SUPP_UNREAD_ALERT_CAT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 			supp_unread_alert_cat_read, al_adapter->adapter,
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->supp_unread_alert_cat_handle,
 			/* Unread Alert Status */
-			GATT_OPT_CHR_UUID, UNREAD_ALERT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, UNREAD_ALERT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CCC_GET_HANDLE,
 			&al_adapter->hnd_ccc[NOTIFY_UNREAD_ALERT],
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->hnd_value[NOTIFY_UNREAD_ALERT],
 			/* Alert Notification Control Point */
-			GATT_OPT_CHR_UUID, ALERT_NOTIF_CP_CHR_UUID,
+			GATT_OPT_CHR_UUID16, ALERT_NOTIF_CP_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_WRITE,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
 			alert_notif_cp_write, NULL,
profiles/proximity/immalert.c +1 -1
index 06e8eb8..2f8d4e7 100644
@@ -249,7 +249,7 @@ void imm_alert_register(struct btd_adapter *adapter)
 	svc_added = gatt_service_add(adapter,
 				GATT_PRIM_SVC_UUID, &uuid,
 				/* Alert level characteristic */
-				GATT_OPT_CHR_UUID, ALERT_LEVEL_CHR_UUID,
+				GATT_OPT_CHR_UUID16, ALERT_LEVEL_CHR_UUID,
 				GATT_OPT_CHR_PROPS,
 					ATT_CHAR_PROPER_WRITE_WITHOUT_RESP,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
profiles/proximity/linkloss.c +1 -1
index cb87b12..a7ed96c 100644
@@ -290,7 +290,7 @@ void link_loss_register(struct btd_adapter *adapter)
 	svc_added = gatt_service_add(adapter,
 			GATT_PRIM_SVC_UUID, &uuid,
 			/* Alert level characteristic */
-			GATT_OPT_CHR_UUID, ALERT_LEVEL_CHR_UUID,
+			GATT_OPT_CHR_UUID16, ALERT_LEVEL_CHR_UUID,
 			GATT_OPT_CHR_PROPS,
 				ATT_CHAR_PROPER_READ | ATT_CHAR_PROPER_WRITE,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
profiles/time/server.c +4 -4
index 518a29f..178d4d2 100644
@@ -152,14 +152,14 @@ static gboolean register_current_time_service(struct btd_adapter *adapter)
 	/* Current Time service */
 	return gatt_service_add(adapter, GATT_PRIM_SVC_UUID, &uuid,
 				/* CT Time characteristic */
-				GATT_OPT_CHR_UUID, CT_TIME_CHR_UUID,
+				GATT_OPT_CHR_UUID16, CT_TIME_CHR_UUID,
 				GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 						current_time_read, adapter,
 
 				/* Local Time Information characteristic */
-				GATT_OPT_CHR_UUID, LOCAL_TIME_INFO_CHR_UUID,
+				GATT_OPT_CHR_UUID16, LOCAL_TIME_INFO_CHR_UUID,
 				GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 						local_time_info_read, adapter,

@@ -215,14 +215,14 @@ static gboolean register_ref_time_update_service(struct btd_adapter *adapter)
 	/* Reference Time Update service */
 	return gatt_service_add(adapter, GATT_PRIM_SVC_UUID, &uuid,
 				/* Time Update control point */
-				GATT_OPT_CHR_UUID, TIME_UPDATE_CTRL_CHR_UUID,
+				GATT_OPT_CHR_UUID16, TIME_UPDATE_CTRL_CHR_UUID,
 				GATT_OPT_CHR_PROPS,
 					ATT_CHAR_PROPER_WRITE_WITHOUT_RESP,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
 						time_update_control, adapter,
 
 				/* Time Update status */
-				GATT_OPT_CHR_UUID, TIME_UPDATE_STAT_CHR_UUID,
+				GATT_OPT_CHR_UUID16, TIME_UPDATE_STAT_CHR_UUID,
 				GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 						time_update_status, adapter,
Signature · 1 line
1.8.3.2
#9 lore raw
In reply to Dirk-Jan C. Binnema

Hi Dirk-Jan,

Quoted 14 lines · Dirk-Jan C. Binnema
Quoted 13 lines

attrib/gatt-service.c static gboolean add_characteristic(struct btd_adapter *adapter, · line 192

Quoted 6 lines · Dirk-Jan C. Binnema
attrib/gatt-service.c +1 -1
@@ -183,10 +189,11 @@ static gboolean add_characteristic(struct btd_adapter *adapter,
        uint16_t h = *handle;
        struct attribute *a;
        bt_uuid_t bt_uuid;
-       uint8_t atval[5];
+       uint8_t atval[131];

Where "131" comes from? A UUID has 128 bits (i.e. 16 bytes). Anyway, I suggest using atval[ATT_MAX_VALUE_LEN] and not worry about the size here, as long as you pass the exact site to atttrib_db_add(), as you do below.

Quoted 4 lines

Well, a little bit below that, there is:

    att_put_uuid(info->uuid, &atval[3]);

so I guessed I need to reserve the three extra bytes; this is also why it was 5 before, I think. Or?

Yes, but 128-bit == 16 bytes, so it should have been 19... but using ATT_MAX_VALUE_LEN is better so you don't have to worry about the exact buffer size anyway (it just needs to fit the data).

Quoted 3 lines

ATT_MAX_VALUE_LEN is 512, but it seems to be the maximum size of a value, rather than the UUID. It's more than enough at least :)

The UUID is being saved into an attribute, and attributes have maximum value size of 512, that's why I proposed using this define instead of a hardcoded size.

Quoted 11 lines
Quoted 6 lines

attrib/gatt-service.c gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid, · line 351

Quoted 3 lines
attrib/gatt-service.c +1 -1
-       g_assert(h - start_handle == (uint16_t) size);
+       g_assert((uint16_t) (h - start_handle) == (uint16_t) size);

This actually seems unrelated to the patch. Can you put it into a separate patch with the compilation error you got?

When using 128-bit UUIDs, the start handle ultimately comes from find_uuid128_avail, which returns 0xffff - nitems + 1 (for the first item). If I don't explicitly cast it, h - start_handle will be negative number. So, my patch doesn't really work without this.

Ok, the problem is clearer now. "h" is overflowing for 128-bit services because the handle is incremented after each attribute is added so at the end we have: 0xffff + 1 == 0 (h is uint16_t).

As it seems more complex to try to remove the overflow without affecting the logic too much, can you try the following change instead:

- g_assert(h - start_handle == (uint16_t) size);
+ g_assert(h == 0 || (h - start_handle == (uint16_t) size));

In my opinion, the cast you added is dangerous because it will hide the programming bugs that this g_assert() was supposed to catch.

Given this is a bug on the existing code and not part of the implementation of GATT_OPT_CHR_UUID, please put it into a separate patch. This means you should send 2 patches, the first with this fix and the second with the remaining changes.

Quoted 5 lines
Quoted 4 lines

My suggestion would be to rename the old one to GATT_OPT_CHR_UUID16 and this could be GATT_OPT_CHR_UUID. But as Johan said, this could be in a separate patch.

Quoted 7 lines
Quoted 3 lines

attrib/gatt-service.h line 31

Quoted 3 lines
attrib/gatt-service.h +0 -0
        GATT_OPT_CHR_PROPS,
        GATT_OPT_CHR_VALUE_CB,
        GATT_OPT_CHR_AUTHENTICATION,

That makes sense -- but as the newcomer here, I'll happily let the others reach some consensus on what the best naming would be, and I'll make a patch after that :-)

Johan agreed with the name, so feel free to change the existing occurrences on the code to use CHR_UUID16 instead.

Best Regards,

Signature · 3 lines
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
author
#10 lore raw

Hi,

Was a bit too quick with my updated patch! Anyway...

Quoted 10 lines · On 2013-07-24 20:38, [email protected] wrote:

Yes, but 128-bit == 16 bytes, so it should have been 19... but using ATT_MAX_VALUE_LEN is better so you don't have to worry about the exact buffer size anyway (it just needs to fit the data).

Quoted 2 lines

ATT_MAX_VALUE_LEN is 512, but it seems to be the maximum size of a value, rather than the UUID. It's more than enough at least :)

The UUID is being saved into an attribute, and attributes have maximum value size of 512, that's why I proposed using this define instead of a hardcoded size.

Oh, I see your point. I'll update the patch.

Quoted 22 lines
Quoted 10 lines
Quoted 5 lines

attrib/gatt-service.c gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid, · line 351

Quoted 2 lines
attrib/gatt-service.c +1 -1
-       g_assert(h - start_handle == (uint16_t) size);
+       g_assert((uint16_t) (h - start_handle) == (uint16_t) size);

This actually seems unrelated to the patch. Can you put it into a separate patch with the compilation error you got?

When using 128-bit UUIDs, the start handle ultimately comes from find_uuid128_avail, which returns 0xffff - nitems + 1 (for the first item). If I don't explicitly cast it, h - start_handle will be negative number. So, my patch doesn't really work without this.

Ok, the problem is clearer now. "h" is overflowing for 128-bit services because the handle is incremented after each attribute is added so at the end we have: 0xffff + 1 == 0 (h is uint16_t).

As it seems more complex to try to remove the overflow without affecting the logic too much, can you try the following change instead:

(quoted diff) +1 -1
-       g_assert(h - start_handle == (uint16_t) size);
+       g_assert(h == 0 || (h - start_handle == (uint16_t) size));
Quoted 2 lines

In my opinion, the cast you added is dangerous because it will hide the programming bugs that this g_assert() was supposed to catch.

Sure, will do that, too.

Quoted 4 lines

Given this is a bug on the existing code and not part of the implementation of GATT_OPT_CHR_UUID, please put it into a separate patch. This means you should send 2 patches, the first with this fix and the second with the remaining changes.

Okidoki. I actually split the original patch in two, so their separately applicable.

Cheers,
Dirk.

Signature · 3 lines
Dirk-Jan C. Binnema                  Helsinki, Finland
e:[email protected]           w:www.djcbsoftware.nl
pgp: D09C E664 897D 7D39 5047 A178 E96A C7A1 017D DA3C
author
#11 lore raw

PATCH 1/3 gatt-service: fix assertion in gatt_service_add for 128-bit uuids

In reply to Anderson Lizardo

From: "Dirk-Jan C. Binnema" <[email protected]>

---
 attrib/gatt-service.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
attrib/gatt-service.c +1 -1
index 4b02d39..1b84c89 100644
@@ -341,7 +341,7 @@ gboolean gatt_service_add(struct btd_adapter *adapter, uint16_t uuid,
 	}
 
 	g_assert(size < USHRT_MAX);
-	g_assert(h - start_handle == (uint16_t) size);
+	g_assert(h == 0 || (h - start_handle == (uint16_t) size));
 	g_slist_free_full(chrs, free_gatt_info);
 
 	return TRUE;
Signature · 1 line
1.8.3.2
author
#12 lore raw

PATCH 2/3 gatt_service_add: add support for 128-bit uuids for characteristics

From: "Dirk-Jan C. Binnema" <[email protected]>

---
 attrib/gatt-service.c | 17 ++++++++++++-----
 attrib/gatt-service.h |  1 +
 2 files changed, 13 insertions(+), 5 deletions(-)
attrib/gatt-service.c +12 -5
index 1b84c89..3d9414d 100644
@@ -73,6 +73,12 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 			/* characteristic declaration and value */
 			info->num_attrs += 2;
 			break;
+		case GATT_OPT_CHR_BT_UUID_T:
+			memcpy(&info->uuid, va_arg(args, bt_uuid_t *),
+				sizeof(bt_uuid_t));
+			/* characteristic declaration and value */
+			info->num_attrs += 2;
+			break;
 		case GATT_OPT_CHR_PROPS:
 			info->props = va_arg(args, int);
 

@@ -108,7 +114,7 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 		}
 
 		opt = va_arg(args, gatt_option);
-		if (opt == GATT_OPT_CHR_UUID) {
+		if (opt == GATT_OPT_CHR_UUID || opt == GATT_OPT_CHR_BT_UUID_T) {
 			info = g_new0(struct gatt_info, 1);
 			l = g_slist_append(l, info);
 		}

@@ -183,10 +189,11 @@ static gboolean add_characteristic(struct btd_adapter *adapter,
 	uint16_t h = *handle;
 	struct attribute *a;
 	bt_uuid_t bt_uuid;
-	uint8_t atval[5];
+	uint8_t atval[ATT_MAX_VALUE_LEN];
 	GSList *l;
 
-	if (!info->uuid.value.u16 || !info->props) {
+	if ((info->uuid.type != BT_UUID16 && info->uuid.type != BT_UUID128) ||
+		!info->props) {
 		error("Characteristic UUID or properties are missing");
 		return FALSE;
 	}

@@ -222,9 +229,9 @@ static gboolean add_characteristic(struct btd_adapter *adapter,
 	bt_uuid16_create(&bt_uuid, GATT_CHARAC_UUID);
 	atval[0] = info->props;
 	att_put_u16(h + 1, &atval[1]);
-	att_put_u16(info->uuid.value.u16, &atval[3]);
+	att_put_uuid(info->uuid, &atval[3]);
 	if (attrib_db_add(adapter, h++, &bt_uuid, ATT_NONE, ATT_NOT_PERMITTED,
-						atval, sizeof(atval)) == NULL)
+				atval, 3 + info->uuid.type / 8) == NULL)
 		return FALSE;
 
 	/* characteristic value */
attrib/gatt-service.h +1 -0
index b810e2e..5c3e15d 100644
@@ -25,6 +25,7 @@
 typedef enum {
 	GATT_OPT_INVALID = 0,
 	GATT_OPT_CHR_UUID,
+	GATT_OPT_CHR_BT_UUID_T,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
Signature · 1 line
1.8.3.2
author
#13 lore raw

PATCH 3/3 Use GATT_OPT_CHR_UUID for bt_uuid_t*, GATT_OPT_CHR_UUID16 for uint16 chrs

In reply to Dirk-Jan C. Binnema

From: "Dirk-Jan C. Binnema" <[email protected]>

This renames GATT_OPT_CHR_UUID into GATT_OPT_CHR_UUID16, and renames the (temporary) GATT_OPT_CHR_BT_UUID_T into GATT_OPT_CHR_UUID. Also, updates all in-tree users.

---
 attrib/gatt-service.c         |  6 +++---
 attrib/gatt-service.h         |  2 +-
 plugins/gatt-example.c        |  2 +-
 profiles/alert/server.c       | 16 ++++++++--------
 profiles/proximity/immalert.c |  2 +-
 profiles/proximity/linkloss.c |  2 +-
 profiles/time/server.c        |  8 ++++----
 7 files changed, 19 insertions(+), 19 deletions(-)
attrib/gatt-service.c +3 -3
index 3d9414d..4cd300f 100644
@@ -68,12 +68,12 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 
 	while (opt != GATT_OPT_INVALID) {
 		switch (opt) {
-		case GATT_OPT_CHR_UUID:
+		case GATT_OPT_CHR_UUID16:
 			bt_uuid16_create(&info->uuid, va_arg(args, int));
 			/* characteristic declaration and value */
 			info->num_attrs += 2;
 			break;
-		case GATT_OPT_CHR_BT_UUID_T:
+		case GATT_OPT_CHR_UUID:
 			memcpy(&info->uuid, va_arg(args, bt_uuid_t *),
 				sizeof(bt_uuid_t));
 			/* characteristic declaration and value */

@@ -114,7 +114,7 @@ static GSList *parse_opts(gatt_option opt1, va_list args)
 		}
 
 		opt = va_arg(args, gatt_option);
-		if (opt == GATT_OPT_CHR_UUID || opt == GATT_OPT_CHR_BT_UUID_T) {
+		if (opt == GATT_OPT_CHR_UUID16 || opt == GATT_OPT_CHR_UUID) {
 			info = g_new0(struct gatt_info, 1);
 			l = g_slist_append(l, info);
 		}
attrib/gatt-service.h +1 -1
index 5c3e15d..2c05c7d 100644
@@ -24,8 +24,8 @@
 
 typedef enum {
 	GATT_OPT_INVALID = 0,
+	GATT_OPT_CHR_UUID16,
 	GATT_OPT_CHR_UUID,
-	GATT_OPT_CHR_BT_UUID_T,
 	GATT_OPT_CHR_PROPS,
 	GATT_OPT_CHR_VALUE_CB,
 	GATT_OPT_CHR_AUTHENTICATION,
plugins/gatt-example.c +1 -1
index 37972e8..b926947 100644
@@ -114,7 +114,7 @@ static gboolean register_battery_service(struct btd_adapter *adapter)
 
 	return gatt_service_add(adapter, GATT_PRIM_SVC_UUID, &uuid,
 			/* battery state characteristic */
-			GATT_OPT_CHR_UUID, BATTERY_STATE_UUID,
+			GATT_OPT_CHR_UUID16, BATTERY_STATE_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
profiles/alert/server.c +8 -8
index 8da1928..4565470 100644
@@ -789,7 +789,7 @@ static void register_phone_alert_service(struct alert_adapter *al_adapter)
 	/* Phone Alert Status Service */
 	gatt_service_add(al_adapter->adapter, GATT_PRIM_SVC_UUID, &uuid,
 			/* Alert Status characteristic */
-			GATT_OPT_CHR_UUID, ALERT_STATUS_CHR_UUID,
+			GATT_OPT_CHR_UUID16, ALERT_STATUS_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,

@@ -799,12 +799,12 @@ static void register_phone_alert_service(struct alert_adapter *al_adapter)
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->hnd_value[NOTIFY_ALERT_STATUS],
 			/* Ringer Control Point characteristic */
-			GATT_OPT_CHR_UUID, RINGER_CP_CHR_UUID,
+			GATT_OPT_CHR_UUID16, RINGER_CP_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_WRITE_WITHOUT_RESP,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
 			ringer_cp_write, NULL,
 			/* Ringer Setting characteristic */
-			GATT_OPT_CHR_UUID, RINGER_SETTING_CHR_UUID,
+			GATT_OPT_CHR_UUID16, RINGER_SETTING_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,

@@ -892,35 +892,35 @@ static void register_alert_notif_service(struct alert_adapter *al_adapter)
 	/* Alert Notification Service */
 	gatt_service_add(al_adapter->adapter, GATT_PRIM_SVC_UUID, &uuid,
 			/* Supported New Alert Category */
-			GATT_OPT_CHR_UUID, SUPP_NEW_ALERT_CAT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, SUPP_NEW_ALERT_CAT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 			supp_new_alert_cat_read, al_adapter->adapter,
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->supp_new_alert_cat_handle,
 			/* New Alert */
-			GATT_OPT_CHR_UUID, NEW_ALERT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, NEW_ALERT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CCC_GET_HANDLE,
 			&al_adapter->hnd_ccc[NOTIFY_NEW_ALERT],
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->hnd_value[NOTIFY_NEW_ALERT],
 			/* Supported Unread Alert Category */
-			GATT_OPT_CHR_UUID, SUPP_UNREAD_ALERT_CAT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, SUPP_UNREAD_ALERT_CAT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 			supp_unread_alert_cat_read, al_adapter->adapter,
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->supp_unread_alert_cat_handle,
 			/* Unread Alert Status */
-			GATT_OPT_CHR_UUID, UNREAD_ALERT_CHR_UUID,
+			GATT_OPT_CHR_UUID16, UNREAD_ALERT_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_NOTIFY,
 			GATT_OPT_CCC_GET_HANDLE,
 			&al_adapter->hnd_ccc[NOTIFY_UNREAD_ALERT],
 			GATT_OPT_CHR_VALUE_GET_HANDLE,
 			&al_adapter->hnd_value[NOTIFY_UNREAD_ALERT],
 			/* Alert Notification Control Point */
-			GATT_OPT_CHR_UUID, ALERT_NOTIF_CP_CHR_UUID,
+			GATT_OPT_CHR_UUID16, ALERT_NOTIF_CP_CHR_UUID,
 			GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_WRITE,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
 			alert_notif_cp_write, NULL,
profiles/proximity/immalert.c +1 -1
index 06e8eb8..2f8d4e7 100644
@@ -249,7 +249,7 @@ void imm_alert_register(struct btd_adapter *adapter)
 	svc_added = gatt_service_add(adapter,
 				GATT_PRIM_SVC_UUID, &uuid,
 				/* Alert level characteristic */
-				GATT_OPT_CHR_UUID, ALERT_LEVEL_CHR_UUID,
+				GATT_OPT_CHR_UUID16, ALERT_LEVEL_CHR_UUID,
 				GATT_OPT_CHR_PROPS,
 					ATT_CHAR_PROPER_WRITE_WITHOUT_RESP,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
profiles/proximity/linkloss.c +1 -1
index cb87b12..a7ed96c 100644
@@ -290,7 +290,7 @@ void link_loss_register(struct btd_adapter *adapter)
 	svc_added = gatt_service_add(adapter,
 			GATT_PRIM_SVC_UUID, &uuid,
 			/* Alert level characteristic */
-			GATT_OPT_CHR_UUID, ALERT_LEVEL_CHR_UUID,
+			GATT_OPT_CHR_UUID16, ALERT_LEVEL_CHR_UUID,
 			GATT_OPT_CHR_PROPS,
 				ATT_CHAR_PROPER_READ | ATT_CHAR_PROPER_WRITE,
 			GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
profiles/time/server.c +4 -4
index 518a29f..178d4d2 100644
@@ -152,14 +152,14 @@ static gboolean register_current_time_service(struct btd_adapter *adapter)
 	/* Current Time service */
 	return gatt_service_add(adapter, GATT_PRIM_SVC_UUID, &uuid,
 				/* CT Time characteristic */
-				GATT_OPT_CHR_UUID, CT_TIME_CHR_UUID,
+				GATT_OPT_CHR_UUID16, CT_TIME_CHR_UUID,
 				GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ |
 							ATT_CHAR_PROPER_NOTIFY,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 						current_time_read, adapter,
 
 				/* Local Time Information characteristic */
-				GATT_OPT_CHR_UUID, LOCAL_TIME_INFO_CHR_UUID,
+				GATT_OPT_CHR_UUID16, LOCAL_TIME_INFO_CHR_UUID,
 				GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 						local_time_info_read, adapter,

@@ -215,14 +215,14 @@ static gboolean register_ref_time_update_service(struct btd_adapter *adapter)
 	/* Reference Time Update service */
 	return gatt_service_add(adapter, GATT_PRIM_SVC_UUID, &uuid,
 				/* Time Update control point */
-				GATT_OPT_CHR_UUID, TIME_UPDATE_CTRL_CHR_UUID,
+				GATT_OPT_CHR_UUID16, TIME_UPDATE_CTRL_CHR_UUID,
 				GATT_OPT_CHR_PROPS,
 					ATT_CHAR_PROPER_WRITE_WITHOUT_RESP,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_WRITE,
 						time_update_control, adapter,
 
 				/* Time Update status */
-				GATT_OPT_CHR_UUID, TIME_UPDATE_STAT_CHR_UUID,
+				GATT_OPT_CHR_UUID16, TIME_UPDATE_STAT_CHR_UUID,
 				GATT_OPT_CHR_PROPS, ATT_CHAR_PROPER_READ,
 				GATT_OPT_CHR_VALUE_CB, ATTRIB_READ,
 						time_update_status, adapter,
Signature · 1 line
1.8.3.2