All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] attrib-server: Use bt_uuid_to_le instead of local function
@ 2015-03-27 15:39 Grzegorz Kolodziejczyk
  2015-03-27 15:39 ` [PATCH 2/2] shared/gatt-db: " Grzegorz Kolodziejczyk
  2015-03-30  7:55 ` [PATCH 1/2] attrib-server: " Luiz Augusto von Dentz
  0 siblings, 2 replies; 7+ messages in thread
From: Grzegorz Kolodziejczyk @ 2015-03-27 15:39 UTC (permalink / raw)
  To: linux-bluetooth

This patch use lib/uuid library for uuids instead of local function.
---
 src/attrib-server.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index fe127e6..0f2cced 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -98,15 +98,6 @@ static bt_uuid_t ccc_uuid = {
 			.value.u16 = GATT_CLIENT_CHARAC_CFG_UUID
 };
 
-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
-		/* Convert from 128-bit BE to LE */
-		bswap_128(&src->value.u128, dst);
-}
-
 static void attrib_free(void *data)
 {
 	struct attribute *a = data;
@@ -697,7 +688,7 @@ static uint16_t find_info(struct gatt_channel *channel, uint16_t start,
 		put_le16(a->handle, value);
 
 		/* Attribute Value */
-		put_uuid_le(&a->uuid, &value[2]);
+		bt_uuid_to_le(&a->uuid, &value[2]);
 	}
 
 	length = enc_find_info_resp(format, adl, pdu, len);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] shared/gatt-db: Use bt_uuid_to_le instead of local function
  2015-03-27 15:39 [PATCH 1/2] attrib-server: Use bt_uuid_to_le instead of local function Grzegorz Kolodziejczyk
@ 2015-03-27 15:39 ` Grzegorz Kolodziejczyk
  2015-03-27 15:42   ` Grzegorz Kolodziejczyk
  2015-03-30  7:55 ` [PATCH 1/2] attrib-server: " Luiz Augusto von Dentz
  1 sibling, 1 reply; 7+ messages in thread
From: Grzegorz Kolodziejczyk @ 2015-03-27 15:39 UTC (permalink / raw)
  To: linux-bluetooth

This patch use lib/uuid library for uuids instead of local functions.
---
 src/shared/gatt-db.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index 2b2090c..323ce91 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -335,20 +335,6 @@ bool gatt_db_isempty(struct gatt_db *db)
 	return queue_isempty(db->services);
 }
 
-static int uuid_to_le(const bt_uuid_t *uuid, uint8_t *dst)
-{
-	bt_uuid_t uuid128;
-
-	if (uuid->type == BT_UUID16) {
-		put_le16(uuid->value.u16, dst);
-		return bt_uuid_len(uuid);
-	}
-
-	bt_uuid_to_uuid128(uuid, &uuid128);
-	bswap_128(&uuid128.value.u128, dst);
-	return bt_uuid_len(&uuid128);
-}
-
 static bool le_to_uuid(const uint8_t *src, size_t len, bt_uuid_t *uuid)
 {
 	uint128_t u128;
@@ -379,8 +365,8 @@ static struct gatt_db_service *gatt_db_service_create(const bt_uuid_t *uuid,
 {
 	struct gatt_db_service *service;
 	const bt_uuid_t *type;
+	bt_uuid_t uuid128;
 	uint8_t value[16];
-	uint16_t len;
 
 	if (num_handles < 1)
 		return NULL;
@@ -400,10 +386,23 @@ static struct gatt_db_service *gatt_db_service_create(const bt_uuid_t *uuid,
 	else
 		type = &secondary_service_uuid;
 
-	len = uuid_to_le(uuid, value);
+	switch (uuid->type) {
+	case BT_UUID16:
+	case BT_UUID128:
+		bt_uuid_to_le(uuid, value);
+		break;
+	case BT_UUID32:
+		bt_uuid_to_uuid128(uuid, &uuid128);
+		bt_uuid_to_le(&uuid128, value);
+		break;
+	case BT_UUID_UNSPEC:
+	default:
+		gatt_db_service_destroy(service);
+		return NULL;
+	}
 
 	service->attributes[0] = new_attribute(service, handle, type, value,
-									len);
+								uuid->type/8);
 	if (!service->attributes[0]) {
 		gatt_db_service_destroy(service);
 		return NULL;
@@ -698,7 +697,8 @@ service_insert_characteristic(struct gatt_db_service *service,
 	/* We set handle of characteristic value, which will be added next */
 	put_le16(handle, &value[1]);
 	len += sizeof(uint16_t);
-	len += uuid_to_le(uuid, &value[3]);
+	len += uuid->type/8;
+	bt_uuid_to_le(uuid, &value[3]);
 
 	service->attributes[i] = new_attribute(service, handle - 1,
 							&characteristic_uuid,
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] shared/gatt-db: Use bt_uuid_to_le instead of local function
  2015-03-27 15:39 ` [PATCH 2/2] shared/gatt-db: " Grzegorz Kolodziejczyk
@ 2015-03-27 15:42   ` Grzegorz Kolodziejczyk
  2015-03-30  7:49     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 7+ messages in thread
From: Grzegorz Kolodziejczyk @ 2015-03-27 15:42 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

One more comment here.

On 27 March 2015 at 16:39, Grzegorz Kolodziejczyk
<grzegorz.kolodziejczyk@tieto.com> wrote:
> This patch use lib/uuid library for uuids instead of local functions.
> ---
>  src/shared/gatt-db.c | 36 ++++++++++++++++++------------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
> index 2b2090c..323ce91 100644
> --- a/src/shared/gatt-db.c
> +++ b/src/shared/gatt-db.c
> @@ -335,20 +335,6 @@ bool gatt_db_isempty(struct gatt_db *db)
>         return queue_isempty(db->services);
>  }
>
> -static int uuid_to_le(const bt_uuid_t *uuid, uint8_t *dst)
> -{
> -       bt_uuid_t uuid128;
> -
> -       if (uuid->type == BT_UUID16) {
> -               put_le16(uuid->value.u16, dst);
> -               return bt_uuid_len(uuid);
> -       }
> -
> -       bt_uuid_to_uuid128(uuid, &uuid128);
> -       bswap_128(&uuid128.value.u128, dst);
> -       return bt_uuid_len(&uuid128);
> -}
> -
>  static bool le_to_uuid(const uint8_t *src, size_t len, bt_uuid_t *uuid)
>  {
>         uint128_t u128;
> @@ -379,8 +365,8 @@ static struct gatt_db_service *gatt_db_service_create(const bt_uuid_t *uuid,
>  {
>         struct gatt_db_service *service;
>         const bt_uuid_t *type;
> +       bt_uuid_t uuid128;
>         uint8_t value[16];
> -       uint16_t len;
>
>         if (num_handles < 1)
>                 return NULL;
> @@ -400,10 +386,23 @@ static struct gatt_db_service *gatt_db_service_create(const bt_uuid_t *uuid,
>         else
>                 type = &secondary_service_uuid;
>
> -       len = uuid_to_le(uuid, value);
> +       switch (uuid->type) {
> +       case BT_UUID16:
> +       case BT_UUID128:
> +               bt_uuid_to_le(uuid, value);
> +               break;
> +       case BT_UUID32:
> +               bt_uuid_to_uuid128(uuid, &uuid128);
> +               bt_uuid_to_le(&uuid128, value);
> +               break;
> +       case BT_UUID_UNSPEC:
> +       default:
> +               gatt_db_service_destroy(service);
> +               return NULL;
> +       }
I'm not sure if we should generally handle 32bit uuids.
>
>         service->attributes[0] = new_attribute(service, handle, type, value,
> -                                                                       len);
> +                                                               uuid->type/8);
>         if (!service->attributes[0]) {
>                 gatt_db_service_destroy(service);
>                 return NULL;
> @@ -698,7 +697,8 @@ service_insert_characteristic(struct gatt_db_service *service,
>         /* We set handle of characteristic value, which will be added next */
>         put_le16(handle, &value[1]);
>         len += sizeof(uint16_t);
> -       len += uuid_to_le(uuid, &value[3]);
> +       len += uuid->type/8;
> +       bt_uuid_to_le(uuid, &value[3]);
>
>         service->attributes[i] = new_attribute(service, handle - 1,
>                                                         &characteristic_uuid,
> --
> 2.1.0
>

BR,
Grzegorz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] shared/gatt-db: Use bt_uuid_to_le instead of local function
  2015-03-27 15:42   ` Grzegorz Kolodziejczyk
@ 2015-03-30  7:49     ` Luiz Augusto von Dentz
  2015-03-30  9:38       ` Grzegorz Kolodziejczyk
  0 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2015-03-30  7:49 UTC (permalink / raw)
  To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth

Hi Grzegorz,

On Fri, Mar 27, 2015 at 5:42 PM, Grzegorz Kolodziejczyk
<grzegorz.kolodziejczyk@tieto.com> wrote:
> Hi,
>
> One more comment here.
>
> On 27 March 2015 at 16:39, Grzegorz Kolodziejczyk
> <grzegorz.kolodziejczyk@tieto.com> wrote:
>> This patch use lib/uuid library for uuids instead of local functions.
>> ---
>>  src/shared/gatt-db.c | 36 ++++++++++++++++++------------------
>>  1 file changed, 18 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
>> index 2b2090c..323ce91 100644
>> --- a/src/shared/gatt-db.c
>> +++ b/src/shared/gatt-db.c
>> @@ -335,20 +335,6 @@ bool gatt_db_isempty(struct gatt_db *db)
>>         return queue_isempty(db->services);
>>  }
>>
>> -static int uuid_to_le(const bt_uuid_t *uuid, uint8_t *dst)
>> -{
>> -       bt_uuid_t uuid128;
>> -
>> -       if (uuid->type == BT_UUID16) {
>> -               put_le16(uuid->value.u16, dst);
>> -               return bt_uuid_len(uuid);
>> -       }
>> -
>> -       bt_uuid_to_uuid128(uuid, &uuid128);
>> -       bswap_128(&uuid128.value.u128, dst);
>> -       return bt_uuid_len(&uuid128);
>> -}
>> -
>>  static bool le_to_uuid(const uint8_t *src, size_t len, bt_uuid_t *uuid)
>>  {
>>         uint128_t u128;
>> @@ -379,8 +365,8 @@ static struct gatt_db_service *gatt_db_service_create(const bt_uuid_t *uuid,
>>  {
>>         struct gatt_db_service *service;
>>         const bt_uuid_t *type;
>> +       bt_uuid_t uuid128;
>>         uint8_t value[16];
>> -       uint16_t len;
>>
>>         if (num_handles < 1)
>>                 return NULL;
>> @@ -400,10 +386,23 @@ static struct gatt_db_service *gatt_db_service_create(const bt_uuid_t *uuid,
>>         else
>>                 type = &secondary_service_uuid;
>>
>> -       len = uuid_to_le(uuid, value);
>> +       switch (uuid->type) {
>> +       case BT_UUID16:
>> +       case BT_UUID128:
>> +               bt_uuid_to_le(uuid, value);
>> +               break;
>> +       case BT_UUID32:
>> +               bt_uuid_to_uuid128(uuid, &uuid128);
>> +               bt_uuid_to_le(&uuid128, value);

bt_uuid_to_le already does this conversion, so this switch is not
actually needed.

>> +               break;
>> +       case BT_UUID_UNSPEC:
>> +       default:
>> +               gatt_db_service_destroy(service);
>> +               return NULL;
>> +       }
> I'm not sure if we should generally handle 32bit uuids.
>>
>>         service->attributes[0] = new_attribute(service, handle, type, value,
>> -                                                                       len);
>> +                                                               uuid->type/8);

Maybe we should make bt_uuid_to_le return how much it has written so
we don't have to guess with uuid->type / 8.

>>         if (!service->attributes[0]) {
>>                 gatt_db_service_destroy(service);
>>                 return NULL;
>> @@ -698,7 +697,8 @@ service_insert_characteristic(struct gatt_db_service *service,
>>         /* We set handle of characteristic value, which will be added next */
>>         put_le16(handle, &value[1]);
>>         len += sizeof(uint16_t);
>> -       len += uuid_to_le(uuid, &value[3]);
>> +       len += uuid->type/8;
>> +       bt_uuid_to_le(uuid, &value[3]);
>>
>>         service->attributes[i] = new_attribute(service, handle - 1,
>>                                                         &characteristic_uuid,
>> --
>> 2.1.0
>>
>
> BR,
> Grzegorz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] attrib-server: Use bt_uuid_to_le instead of local function
  2015-03-27 15:39 [PATCH 1/2] attrib-server: Use bt_uuid_to_le instead of local function Grzegorz Kolodziejczyk
  2015-03-27 15:39 ` [PATCH 2/2] shared/gatt-db: " Grzegorz Kolodziejczyk
@ 2015-03-30  7:55 ` Luiz Augusto von Dentz
  1 sibling, 0 replies; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2015-03-30  7:55 UTC (permalink / raw)
  To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth

Hi Grzegorz,


On Fri, Mar 27, 2015 at 5:39 PM, Grzegorz Kolodziejczyk
<grzegorz.kolodziejczyk@tieto.com> wrote:
> This patch use lib/uuid library for uuids instead of local function.
> ---
>  src/attrib-server.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/src/attrib-server.c b/src/attrib-server.c
> index fe127e6..0f2cced 100644
> --- a/src/attrib-server.c
> +++ b/src/attrib-server.c
> @@ -98,15 +98,6 @@ static bt_uuid_t ccc_uuid = {
>                         .value.u16 = GATT_CLIENT_CHARAC_CFG_UUID
>  };
>
> -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
> -               /* Convert from 128-bit BE to LE */
> -               bswap_128(&src->value.u128, dst);
> -}
> -
>  static void attrib_free(void *data)
>  {
>         struct attribute *a = data;
> @@ -697,7 +688,7 @@ static uint16_t find_info(struct gatt_channel *channel, uint16_t start,
>                 put_le16(a->handle, value);
>
>                 /* Attribute Value */
> -               put_uuid_le(&a->uuid, &value[2]);
> +               bt_uuid_to_le(&a->uuid, &value[2]);
>         }
>
>         length = enc_find_info_resp(format, adl, pdu, len);
> --
> 2.1.0

Patch 1/2 applied, please check the comments for 2/2.


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] shared/gatt-db: Use bt_uuid_to_le instead of local function
  2015-03-30  7:49     ` Luiz Augusto von Dentz
@ 2015-03-30  9:38       ` Grzegorz Kolodziejczyk
  2015-03-30  9:51         ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 7+ messages in thread
From: Grzegorz Kolodziejczyk @ 2015-03-30  9:38 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

On 30 March 2015 at 09:49, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Grzegorz,
>
> On Fri, Mar 27, 2015 at 5:42 PM, Grzegorz Kolodziejczyk
> <grzegorz.kolodziejczyk@tieto.com> wrote:
>> Hi,
>>
>> One more comment here.
>>
>> On 27 March 2015 at 16:39, Grzegorz Kolodziejczyk
>> <grzegorz.kolodziejczyk@tieto.com> wrote:
>>> This patch use lib/uuid library for uuids instead of local functions.
>>> ---
>>>  src/shared/gatt-db.c | 36 ++++++++++++++++++------------------
>>>  1 file changed, 18 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
>>> index 2b2090c..323ce91 100644
>>> --- a/src/shared/gatt-db.c
>>> +++ b/src/shared/gatt-db.c
>>> @@ -335,20 +335,6 @@ bool gatt_db_isempty(struct gatt_db *db)
>>>         return queue_isempty(db->services);
>>>  }
>>>
>>> -static int uuid_to_le(const bt_uuid_t *uuid, uint8_t *dst)
>>> -{
>>> -       bt_uuid_t uuid128;
>>> -
>>> -       if (uuid->type == BT_UUID16) {
>>> -               put_le16(uuid->value.u16, dst);
>>> -               return bt_uuid_len(uuid);
>>> -       }
>>> -
>>> -       bt_uuid_to_uuid128(uuid, &uuid128);
>>> -       bswap_128(&uuid128.value.u128, dst);
>>> -       return bt_uuid_len(&uuid128);
>>> -}
>>> -
>>>  static bool le_to_uuid(const uint8_t *src, size_t len, bt_uuid_t *uuid)
>>>  {
>>>         uint128_t u128;
>>> @@ -379,8 +365,8 @@ static struct gatt_db_service *gatt_db_service_create(const bt_uuid_t *uuid,
>>>  {
>>>         struct gatt_db_service *service;
>>>         const bt_uuid_t *type;
>>> +       bt_uuid_t uuid128;
>>>         uint8_t value[16];
>>> -       uint16_t len;
>>>
>>>         if (num_handles < 1)
>>>                 return NULL;
>>> @@ -400,10 +386,23 @@ static struct gatt_db_service *gatt_db_service_create(const bt_uuid_t *uuid,
>>>         else
>>>                 type = &secondary_service_uuid;
>>>
>>> -       len = uuid_to_le(uuid, value);
>>> +       switch (uuid->type) {
>>> +       case BT_UUID16:
>>> +       case BT_UUID128:
>>> +               bt_uuid_to_le(uuid, value);
>>> +               break;
>>> +       case BT_UUID32:
>>> +               bt_uuid_to_uuid128(uuid, &uuid128);
>>> +               bt_uuid_to_le(&uuid128, value);
>
> bt_uuid_to_le already does this conversion, so this switch is not
> actually needed.
>
lib/uuid library is only gatt oriented ? Specification 2.5.4 - says
that "All 32-bit UUIDs shall be converted to 128-bit UUIDs when the
UUID is contained in an ATT PDU". I've sent patch, few minutes ago
which changes functionality of bt_uuid_to_le. I think it should only
convert value to le order.
>>> +               break;
>>> +       case BT_UUID_UNSPEC:
>>> +       default:
>>> +               gatt_db_service_destroy(service);
>>> +               return NULL;
>>> +       }
>> I'm not sure if we should generally handle 32bit uuids.
>>>
>>>         service->attributes[0] = new_attribute(service, handle, type, value,
>>> -                                                                       len);
>>> +                                                               uuid->type/8);
>
> Maybe we should make bt_uuid_to_le return how much it has written so
> we don't have to guess with uuid->type / 8.
>
Yes, I agree.
>>>         if (!service->attributes[0]) {
>>>                 gatt_db_service_destroy(service);
>>>                 return NULL;
>>> @@ -698,7 +697,8 @@ service_insert_characteristic(struct gatt_db_service *service,
>>>         /* We set handle of characteristic value, which will be added next */
>>>         put_le16(handle, &value[1]);
>>>         len += sizeof(uint16_t);
>>> -       len += uuid_to_le(uuid, &value[3]);
>>> +       len += uuid->type/8;
>>> +       bt_uuid_to_le(uuid, &value[3]);
>>>
>>>         service->attributes[i] = new_attribute(service, handle - 1,
>>>                                                         &characteristic_uuid,
>>> --
>>> 2.1.0
>>>
>>
>> BR,
>> Grzegorz
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz

BR,
Grzegorz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] shared/gatt-db: Use bt_uuid_to_le instead of local function
  2015-03-30  9:38       ` Grzegorz Kolodziejczyk
@ 2015-03-30  9:51         ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2015-03-30  9:51 UTC (permalink / raw)
  To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth

Hi Grzegorz,

On Mon, Mar 30, 2015 at 12:38 PM, Grzegorz Kolodziejczyk
<grzegorz.kolodziejczyk@tieto.com> wrote:
> Hi Luiz,
>
> On 30 March 2015 at 09:49, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>> Hi Grzegorz,
>>
>> On Fri, Mar 27, 2015 at 5:42 PM, Grzegorz Kolodziejczyk
>> <grzegorz.kolodziejczyk@tieto.com> wrote:
>>> Hi,
>>>
>>> One more comment here.
>>>
>>> On 27 March 2015 at 16:39, Grzegorz Kolodziejczyk
>>> <grzegorz.kolodziejczyk@tieto.com> wrote:
>>>> This patch use lib/uuid library for uuids instead of local functions.
>>>> ---
>>>>  src/shared/gatt-db.c | 36 ++++++++++++++++++------------------
>>>>  1 file changed, 18 insertions(+), 18 deletions(-)
>>>>
>>>> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
>>>> index 2b2090c..323ce91 100644
>>>> --- a/src/shared/gatt-db.c
>>>> +++ b/src/shared/gatt-db.c
>>>> @@ -335,20 +335,6 @@ bool gatt_db_isempty(struct gatt_db *db)
>>>>         return queue_isempty(db->services);
>>>>  }
>>>>
>>>> -static int uuid_to_le(const bt_uuid_t *uuid, uint8_t *dst)
>>>> -{
>>>> -       bt_uuid_t uuid128;
>>>> -
>>>> -       if (uuid->type == BT_UUID16) {
>>>> -               put_le16(uuid->value.u16, dst);
>>>> -               return bt_uuid_len(uuid);
>>>> -       }
>>>> -
>>>> -       bt_uuid_to_uuid128(uuid, &uuid128);
>>>> -       bswap_128(&uuid128.value.u128, dst);
>>>> -       return bt_uuid_len(&uuid128);
>>>> -}
>>>> -
>>>>  static bool le_to_uuid(const uint8_t *src, size_t len, bt_uuid_t *uuid)
>>>>  {
>>>>         uint128_t u128;
>>>> @@ -379,8 +365,8 @@ static struct gatt_db_service *gatt_db_service_create(const bt_uuid_t *uuid,
>>>>  {
>>>>         struct gatt_db_service *service;
>>>>         const bt_uuid_t *type;
>>>> +       bt_uuid_t uuid128;
>>>>         uint8_t value[16];
>>>> -       uint16_t len;
>>>>
>>>>         if (num_handles < 1)
>>>>                 return NULL;
>>>> @@ -400,10 +386,23 @@ static struct gatt_db_service *gatt_db_service_create(const bt_uuid_t *uuid,
>>>>         else
>>>>                 type = &secondary_service_uuid;
>>>>
>>>> -       len = uuid_to_le(uuid, value);
>>>> +       switch (uuid->type) {
>>>> +       case BT_UUID16:
>>>> +       case BT_UUID128:
>>>> +               bt_uuid_to_le(uuid, value);
>>>> +               break;
>>>> +       case BT_UUID32:
>>>> +               bt_uuid_to_uuid128(uuid, &uuid128);
>>>> +               bt_uuid_to_le(&uuid128, value);
>>
>> bt_uuid_to_le already does this conversion, so this switch is not
>> actually needed.
>>
> lib/uuid library is only gatt oriented ? Specification 2.5.4 - says
> that "All 32-bit UUIDs shall be converted to 128-bit UUIDs when the
> UUID is contained in an ATT PDU". I've sent patch, few minutes ago
> which changes functionality of bt_uuid_to_le. I think it should only
> convert value to le order.

That why I said we should keep the conversion from 32 to 128 bits on
bt_uuid_to_le, and yes the only user of bt_uuid_to_le is GATT.

>>>> +               break;
>>>> +       case BT_UUID_UNSPEC:
>>>> +       default:
>>>> +               gatt_db_service_destroy(service);
>>>> +               return NULL;
>>>> +       }
>>> I'm not sure if we should generally handle 32bit uuids.
>>>>
>>>>         service->attributes[0] = new_attribute(service, handle, type, value,
>>>> -                                                                       len);
>>>> +                                                               uuid->type/8);
>>
>> Maybe we should make bt_uuid_to_le return how much it has written so
>> we don't have to guess with uuid->type / 8.
>>
> Yes, I agree.
>>>>         if (!service->attributes[0]) {
>>>>                 gatt_db_service_destroy(service);
>>>>                 return NULL;
>>>> @@ -698,7 +697,8 @@ service_insert_characteristic(struct gatt_db_service *service,
>>>>         /* We set handle of characteristic value, which will be added next */
>>>>         put_le16(handle, &value[1]);
>>>>         len += sizeof(uint16_t);
>>>> -       len += uuid_to_le(uuid, &value[3]);
>>>> +       len += uuid->type/8;
>>>> +       bt_uuid_to_le(uuid, &value[3]);
>>>>
>>>>         service->attributes[i] = new_attribute(service, handle - 1,
>>>>                                                         &characteristic_uuid,
>>>> --
>>>> 2.1.0
>>>>
>>>
>>> BR,
>>> Grzegorz
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
>
> BR,
> Grzegorz



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-03-30  9:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-27 15:39 [PATCH 1/2] attrib-server: Use bt_uuid_to_le instead of local function Grzegorz Kolodziejczyk
2015-03-27 15:39 ` [PATCH 2/2] shared/gatt-db: " Grzegorz Kolodziejczyk
2015-03-27 15:42   ` Grzegorz Kolodziejczyk
2015-03-30  7:49     ` Luiz Augusto von Dentz
2015-03-30  9:38       ` Grzegorz Kolodziejczyk
2015-03-30  9:51         ` Luiz Augusto von Dentz
2015-03-30  7:55 ` [PATCH 1/2] attrib-server: " Luiz Augusto von Dentz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.