Re: Re: [PATCH] [Bluetooth 4.1][32bit-UUID Support] Added support for 32 bit uuid for the Attribute type in ATT and GATT for GATT Client devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Marcel,

I would like to remind that 32-UUID support is added in the ADV data and new AD Types were defined for this.

?0x1F	?«List of 32-bit Service Solicitation UUIDs»
0x20	?«Service Data - 32-bit UUID»

Neverthless, Specification talks about 32-bit UUID for attribute type. Since there are only 10 GATT attribute types defined in BT 4.1 spec, can't understand the need of 32-bit UUID just to denote attribute type.
Using 32-bit UUIDs to represent the attribute itself makes sence.

We raised concerned errata (Errata ID 5559) on this already which is still open.

Please let me know your opinion on this.

Regards,
Bhargava

------- Original Message -------
Sender : Marcel Holtmann<marcel@xxxxxxxxxxxx> 
Date   : Apr 08, 2014 19:46 (GMT+05:30)
Title  : Re: [PATCH] [Bluetooth 4.1][32bit-UUID Support] Added support for 32
 bit uuid for the Attribute type in ATT and GATT 		 for GATT Client devices

Hi Sudha,

> 1> As per 4.0 spec support for only 16 and 128 bit support was provided for attribute type in GATT.
> 2> As a part of core 4.1 spec, 32 bit uuid support is also added. The code changes for the same
>   in ATT and GATT is added as a part of this patch for GATT client role.
> ---
> attrib/att.c  |   12 ++++++++++++
> attrib/gatt.c |   11 +++++++++++
> 2 files changed, 23 insertions(+)
> 
> diff --git a/attrib/att.c b/attrib/att.c
> index 8e9c06d..2225255 100644
> --- a/attrib/att.c
> +++ b/attrib/att.c
> @@ -42,6 +42,8 @@ static inline void put_uuid_le(const bt_uuid_t *src, void *dst)
> {
> 	if (src->type == BT_UUID16)
> 		put_le16(src->value.u16, dst);
> +	else if (src->type == BT_UUID32)
> +		put_le32(src->value.u32, dst);

I have no idea when this would ever happen. The wire protocol of ATT does not support 32-bit UUIDs and they all need to be converted into 128-bit UUIDs.

> 	else
> 		/* Convert from 128-bit BE to LE */
> 		bswap_128(&src->value.u128, dst);
> @@ -134,6 +136,8 @@ static void get_uuid(uint8_t type, const void *val, bt_uuid_t *uuid)
> {
> 	if (type == BT_UUID16)
> 		bt_uuid16_create(uuid, get_le16(val));
> +	else if (type == BT_UUID32)
> +		bt_uuid32_create(uuid, get_le32(val));

Same here. There will be no 32-bit UUID coming from the wire.

> 	else {
> 		uint128_t u128;
> 
> @@ -153,6 +157,8 @@ uint16_t enc_read_by_grp_req(uint16_t start, uint16_t end, bt_uuid_t *uuid,
> 
> 	if (uuid->type == BT_UUID16)
> 		uuid_len = 2;
> +	else if (uuid->type == BT_UUID32)
> +		uuid_len = 4;

This is not valid. It needs to be convert to a 128-bit UUID.

> 	else if (uuid->type == BT_UUID128)
> 		uuid_len = 16;
> 	else
> @@ -187,6 +193,8 @@ uint16_t dec_read_by_grp_req(const uint8_t *pdu, size_t len, uint16_t *start,
> 
> 	if (len == (min_len + 2))
> 		type = BT_UUID16;
> +	else if (len == (min_len + 4))
> +		type = BT_UUID32;

Same here, no 32-bit UUID on the wire, they will all be 128-bit.

> 	else if (len == (min_len + 16))
> 		type = BT_UUID128;
> 	else
> @@ -398,6 +406,8 @@ uint16_t enc_read_by_type_req(uint16_t start, uint16_t end, bt_uuid_t *uuid,
> 
> 	if (uuid->type == BT_UUID16)
> 		uuid_len = 2;
> +	else if (uuid->type == BT_UUID32)
> +		uuid_len = 4;
> 	else if (uuid->type == BT_UUID128)
> 		uuid_len = 16;
> 	else
> @@ -429,6 +439,8 @@ uint16_t dec_read_by_type_req(const uint8_t *pdu, size_t len, uint16_t *start,
> 
> 	if (len == (min_len + 2))
> 		type = BT_UUID16;
> +	if (len == (min_len + 4))
> +		type = BT_UUID32;

These two as well, I do not see a reference in the spec. that you can encode or decode 32-bit sized UUID in the wire protocol.

> 	else if (len == (min_len + 16))
> 		type = BT_UUID128;
> 	else
> diff --git a/attrib/gatt.c b/attrib/gatt.c
> index 49cd1a3..bb2330a 100644
> --- a/attrib/gatt.c
> +++ b/attrib/gatt.c
> @@ -143,6 +143,8 @@ static void put_uuid_le(const bt_uuid_t *uuid, void *dst)
> {
> 	if (uuid->type == BT_UUID16)
> 		put_le16(uuid->value.u16, dst);
> +	else if (uuid->type == BT_UUID32)
> +		put_le32(uuid->value.u32, dst);
> 	else
> 		/* Convert from 128-bit BE to LE */
> 		bswap_128(&uuid->value.u128, dst);
> @@ -155,6 +157,11 @@ static void get_uuid128(uint8_t type, const void *val, bt_uuid_t *uuid)
> 
> 		bt_uuid16_create(&uuid16, get_le16(val));
> 		bt_uuid_to_uuid128(&uuid16, uuid);
> +	} else if (type == BT_UUID32) {
> +		bt_uuid_t uuid32;
> +
> +		bt_uuid32_create(&uuid32, get_le32(val));
> +		bt_uuid_to_uuid128(&uuid32, uuid);
> 	} else {
> 		uint128_t u128;
> 
> @@ -256,6 +263,8 @@ static void primary_all_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
> 
> 	if (list->len == 6)
> 		type = BT_UUID16;
> +	else if (list->len == 8)
> +		type = BT_UUID32;
> 	else if (list->len == 20)
> 		type = BT_UUID128;
> 	else {
> @@ -519,6 +528,8 @@ static void char_discovered_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
> 
> 	if (list->len == 7)
> 		type = BT_UUID16;
> +	else if (list->len == 9)
> +		type = BT_UUID32;
> 	else
> 		type = BT_UUID128;

I just do not see how this patch is correct at all. I am reading the Bluetooth 4.1 spec and the 32-bit UUID is not part of the wire protocol. It is an internal representation that always needs to be converted into a 128-bit before it goes over the wire. Which makes sense since otherwise GATT from Bluetooth 4.1 would not be backwards compatible with Bluetooth 4.0 specification.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ¥Šwÿº{.nÇ+‰·¥Š{±ý¹nzÚ(¶âžØ^n‡r¡ö¦zË?ëh™¨è­Ú&£ûàz¿äz¹Þ—ú+€Ê+zf£¢·hšˆ§~†­†Ûiÿÿï?êÿ‘êçz_è®æj:+v‰¨þ)ߣøm





[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux