* [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support
@ 2011-03-25 13:25 johan.hedberg
2011-03-25 15:36 ` Anderson Lizardo
0 siblings, 1 reply; 7+ messages in thread
From: johan.hedberg @ 2011-03-25 13:25 UTC (permalink / raw)
To: linux-bluetooth
From: Johan Hedberg <johan.hedberg@nokia.com>
This patch adds automated creation of the local EIR data based on what
16-bit UUIDs are registered and what the device name is. This should
cover the majority use cases, however things like 32/128-bit UUIDs, TX
power and Device ID will need to be added later to be on par with what
bluetoothd is capable of doing (without the Management interface).
Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
include/net/bluetooth/hci.h | 8 ++
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/mgmt.c | 164 ++++++++++++++++++++++++++++++++++++++
3 files changed, 173 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index b989a8c..6846ec0 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -614,6 +614,14 @@ struct hci_cp_host_buffer_size {
#define HCI_OP_WRITE_INQUIRY_MODE 0x0c45
+#define HCI_MAX_EIR_LENGTH 240
+
+#define HCI_OP_WRITE_EIR 0x0c52
+struct hci_cp_write_eir {
+ uint8_t fec;
+ uint8_t data[HCI_MAX_EIR_LENGTH];
+} __packed;
+
#define HCI_OP_READ_SSP_MODE 0x0c55
struct hci_rp_read_ssp_mode {
__u8 status;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 87bff51..3b2f09d 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -102,6 +102,7 @@ struct hci_dev {
__u8 dev_type;
bdaddr_t bdaddr;
__u8 dev_name[HCI_MAX_NAME_LENGTH];
+ __u8 eir[HCI_MAX_EIR_LENGTH];
__u8 dev_class[3];
__u8 major_class;
__u8 minor_class;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index a42dc8c..d8cbf4d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -544,6 +544,151 @@ failed:
return err;
}
+#define EIR_FLAGS 0x01 /* flags */
+#define EIR_UUID16_SOME 0x02 /* 16-bit UUID, more available */
+#define EIR_UUID16_ALL 0x03 /* 16-bit UUID, all listed */
+#define EIR_UUID32_SOME 0x04 /* 32-bit UUID, more available */
+#define EIR_UUID32_ALL 0x05 /* 32-bit UUID, all listed */
+#define EIR_UUID128_SOME 0x06 /* 128-bit UUID, more available */
+#define EIR_UUID128_ALL 0x07 /* 128-bit UUID, all listed */
+#define EIR_NAME_SHORT 0x08 /* shortened local name */
+#define EIR_NAME_COMPLETE 0x09 /* complete local name */
+#define EIR_TX_POWER 0x0A /* transmit power level */
+#define EIR_DEVICE_ID 0x10 /* device ID */
+
+#define PNP_INFO_SVCLASS_ID 0x1200
+
+static u8 bluetooth_base_uuid[] = {
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00,
+ 0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB,
+};
+
+static u16 get_uuid16(u8 *uuid128)
+{
+ u8 *b = bluetooth_base_uuid;
+ u32 val;
+ int i;
+
+ for (i = 4; i < 16; i++) {
+ if (b[i] != uuid128[i])
+ return 0;
+ }
+
+ memcpy(&val, uuid128, 4);
+
+ val = be32_to_cpu(val);
+ if (val > 0xffff)
+ return 0;
+
+ return (u16) val;
+}
+
+static void create_eir(struct hci_dev *hdev, u8 *data)
+{
+ u8 *ptr = data;
+ u16 eir_len = 0;
+ u16 uuid16_list[HCI_MAX_EIR_LENGTH / sizeof(u16)];
+ int i, truncated = 0;
+ struct list_head *p;
+ size_t name_len;
+
+ name_len = strlen(hdev->dev_name);
+
+ if (name_len > 0) {
+ /* EIR Data type */
+ if (name_len > 48) {
+ name_len = 48;
+ ptr[1] = EIR_NAME_SHORT;
+ } else
+ ptr[1] = EIR_NAME_COMPLETE;
+
+ /* EIR Data length */
+ ptr[0] = name_len + 1;
+
+ memcpy(ptr + 2, hdev->dev_name, name_len);
+
+ eir_len += (name_len + 2);
+ ptr += (name_len + 2);
+ }
+
+ memset(uuid16_list, 0, sizeof(uuid16_list));
+
+ /* Group all UUID16 types */
+ list_for_each(p, &hdev->uuids) {
+ struct bt_uuid *uuid = list_entry(p, struct bt_uuid, list);
+ u16 uuid16;
+
+ uuid16 = get_uuid16(uuid->uuid);
+ if (uuid16 == 0)
+ return;
+
+ if (uuid16 < 0x1100)
+ continue;
+
+ if (uuid16 == PNP_INFO_SVCLASS_ID)
+ continue;
+
+ /* Stop if not enough space to put next UUID */
+ if (eir_len + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
+ truncated = 1;
+ break;
+ }
+
+ /* Check for duplicates */
+ for (i = 0; uuid16_list[i] != 0; i++)
+ if (uuid16_list[i] == uuid16)
+ break;
+
+ if (uuid16_list[i] == 0) {
+ uuid16_list[i] = uuid16;
+ eir_len += sizeof(u16);
+ }
+ }
+
+ if (uuid16_list[0] != 0) {
+ u8 *length = ptr;
+
+ /* EIR Data type */
+ ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;
+
+ ptr += 2;
+ eir_len += 2;
+
+ for (i = 0; uuid16_list[i] != 0; i++) {
+ *ptr++ = (uuid16_list[i] & 0x00ff);
+ *ptr++ = (uuid16_list[i] & 0xff00) >> 8;
+ }
+
+ /* EIR Data length */
+ *length = (i * sizeof(u16)) + 1;
+ }
+}
+
+static int update_eir(struct hci_dev *hdev)
+{
+ struct hci_cp_write_eir cp;
+
+ if (!(hdev->features[6] & LMP_EXT_INQ))
+ return 0;
+
+ if (hdev->ssp_mode == 0)
+ return 0;
+
+ if (test_bit(HCI_SERVICE_CACHE, &hdev->flags))
+ return 0;
+
+ memset(&cp, 0, sizeof(cp));
+
+ create_eir(hdev, cp.data);
+
+ if (memcmp(cp.data, hdev->eir, sizeof(cp.data)) == 0)
+ return 0;
+
+ memcpy(hdev->eir, cp.data, sizeof(cp.data));
+
+ return hci_send_cmd(hdev, HCI_OP_WRITE_EIR, sizeof(cp), &cp);
+}
+
static u8 get_service_classes(struct hci_dev *hdev)
{
struct list_head *p;
@@ -612,6 +757,10 @@ static int add_uuid(struct sock *sk, u16 index, unsigned char *data, u16 len)
if (err < 0)
goto failed;
+ err = update_eir(hdev);
+ if (err < 0)
+ goto failed;
+
err = cmd_complete(sk, index, MGMT_OP_ADD_UUID, NULL, 0);
failed:
@@ -668,6 +817,10 @@ static int remove_uuid(struct sock *sk, u16 index, unsigned char *data, u16 len)
if (err < 0)
goto unlock;
+ err = update_eir(hdev);
+ if (err < 0)
+ goto unlock;
+
err = cmd_complete(sk, index, MGMT_OP_REMOVE_UUID, NULL, 0);
unlock:
@@ -737,6 +890,8 @@ static int set_service_cache(struct sock *sk, u16 index, unsigned char *data,
} else {
clear_bit(HCI_SERVICE_CACHE, &hdev->flags);
err = update_class(hdev);
+ if (err == 0)
+ err = update_eir(hdev);
}
if (err == 0)
@@ -1822,6 +1977,7 @@ int mgmt_auth_failed(u16 index, bdaddr_t *bdaddr, u8 status)
int mgmt_set_local_name_complete(u16 index, u8 *name, u8 status)
{
struct pending_cmd *cmd;
+ struct hci_dev *hdev;
struct mgmt_cp_set_local_name ev;
int err;
@@ -1837,6 +1993,14 @@ int mgmt_set_local_name_complete(u16 index, u8 *name, u8 status)
goto failed;
}
+ hdev = hci_dev_get(index);
+ if (hdev) {
+ hci_dev_lock_bh(hdev);
+ update_eir(hdev);
+ hci_dev_unlock_bh(hdev);
+ hci_dev_put(hdev);
+ }
+
err = cmd_complete(cmd->sk, index, MGMT_OP_SET_LOCAL_NAME, &ev,
sizeof(ev));
if (err < 0)
--
1.7.4.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support
2011-03-25 13:25 [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support johan.hedberg
@ 2011-03-25 15:36 ` Anderson Lizardo
2011-03-25 16:35 ` Johan Hedberg
0 siblings, 1 reply; 7+ messages in thread
From: Anderson Lizardo @ 2011-03-25 15:36 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
Hi Johan,
On Fri, Mar 25, 2011 at 9:25 AM, <johan.hedberg@gmail.com> wrote:
> +static u8 bluetooth_base_uuid[] = {
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00,
> + 0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB,
> +};
> +
> +static u16 get_uuid16(u8 *uuid128)
> +{
> + u8 *b = bluetooth_base_uuid;
> + u32 val;
> + int i;
> +
> + for (i = 4; i < 16; i++) {
> + if (b[i] != uuid128[i])
> + return 0;
Don't you need to check bytes 0 and 1 as well, i.e. only bytes 2 and 3
are changed?
> + }
> +
> + memcpy(&val, uuid128, 4);
> +
> + val = be32_to_cpu(val);
I noticed just know the those UUID handling functions from management
API are assuming UUIDs in network order. We will need to change this,
or take extra care when reusing them for Advertising Data and GATT
uuids.
> + if (val > 0xffff)
> + return 0;
> +
> + return (u16) val;
> +}
> +
> +static void create_eir(struct hci_dev *hdev, u8 *data)
> +{
> + u8 *ptr = data;
> + u16 eir_len = 0;
> + u16 uuid16_list[HCI_MAX_EIR_LENGTH / sizeof(u16)];
> + int i, truncated = 0;
> + struct list_head *p;
> + size_t name_len;
> +
> + name_len = strlen(hdev->dev_name);
> +
> + if (name_len > 0) {
> + /* EIR Data type */
> + if (name_len > 48) {
> + name_len = 48;
> + ptr[1] = EIR_NAME_SHORT;
> + } else
> + ptr[1] = EIR_NAME_COMPLETE;
> +
> + /* EIR Data length */
> + ptr[0] = name_len + 1;
> +
> + memcpy(ptr + 2, hdev->dev_name, name_len);
> +
> + eir_len += (name_len + 2);
> + ptr += (name_len + 2);
> + }
> +
> + memset(uuid16_list, 0, sizeof(uuid16_list));
> +
> + /* Group all UUID16 types */
> + list_for_each(p, &hdev->uuids) {
> + struct bt_uuid *uuid = list_entry(p, struct bt_uuid, list);
> + u16 uuid16;
> +
> + uuid16 = get_uuid16(uuid->uuid);
> + if (uuid16 == 0)
> + return;
> +
> + if (uuid16 < 0x1100)
> + continue;
> +
> + if (uuid16 == PNP_INFO_SVCLASS_ID)
> + continue;
> +
> + /* Stop if not enough space to put next UUID */
> + if (eir_len + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
> + truncated = 1;
> + break;
> + }
> +
> + /* Check for duplicates */
> + for (i = 0; uuid16_list[i] != 0; i++)
> + if (uuid16_list[i] == uuid16)
> + break;
> +
> + if (uuid16_list[i] == 0) {
> + uuid16_list[i] = uuid16;
> + eir_len += sizeof(u16);
> + }
> + }
> +
> + if (uuid16_list[0] != 0) {
> + u8 *length = ptr;
> +
> + /* EIR Data type */
> + ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;
> +
> + ptr += 2;
> + eir_len += 2;
Here you increment eir_len but does not use it anymore. Maybe it would
be nice to add some assert (BUG_ON ?) on it just to check for
programming errors if someone tries to change this code later.
> +
> + for (i = 0; uuid16_list[i] != 0; i++) {
> + *ptr++ = (uuid16_list[i] & 0x00ff);
> + *ptr++ = (uuid16_list[i] & 0xff00) >> 8;
> + }
> +
> + /* EIR Data length */
> + *length = (i * sizeof(u16)) + 1;
> + }
> +}
> +
> +static int update_eir(struct hci_dev *hdev)
> +{
> + struct hci_cp_write_eir cp;
> +
> + if (!(hdev->features[6] & LMP_EXT_INQ))
> + return 0;
> +
> + if (hdev->ssp_mode == 0)
> + return 0;
> +
> + if (test_bit(HCI_SERVICE_CACHE, &hdev->flags))
> + return 0;
> +
> + memset(&cp, 0, sizeof(cp));
> +
> + create_eir(hdev, cp.data);
> +
> + if (memcmp(cp.data, hdev->eir, sizeof(cp.data)) == 0)
> + return 0;
What about making create_eir() return "int" and check for eir_len and
the end? That would avoid the memcmp() here.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support
2011-03-25 15:36 ` Anderson Lizardo
@ 2011-03-25 16:35 ` Johan Hedberg
2011-03-25 16:49 ` Johan Hedberg
2011-03-25 17:23 ` Anderson Lizardo
0 siblings, 2 replies; 7+ messages in thread
From: Johan Hedberg @ 2011-03-25 16:35 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
Hi Lizardo,
On Fri, Mar 25, 2011, Anderson Lizardo wrote:
> > +static u16 get_uuid16(u8 *uuid128)
> > +{
> > + u8 *b = bluetooth_base_uuid;
> > + u32 val;
> > + int i;
> > +
> > + for (i = 4; i < 16; i++) {
> > + if (b[i] != uuid128[i])
> > + return 0;
>
> Don't you need to check bytes 0 and 1 as well, i.e. only bytes 2 and 3
> are changed?
The first four bytes can have any value for Bluetooth UUIDs. The purpose
of this part of the function is to determine whether it's a Bluetooth
UUID or not (i.e. derived from the Bluetooth base UUID), so the four
first bytes don't matter.
> > + }
> > +
> > + memcpy(&val, uuid128, 4);
> > +
> > + val = be32_to_cpu(val);
>
> I noticed just know the those UUID handling functions from management
> API are assuming UUIDs in network order. We will need to change this,
> or take extra care when reusing them for Advertising Data and GATT
> uuids.
Right. This code is pretty much a copy of what user space already has
(hciops.c), and that code is relying on SDP (i.e. network) byte order
UUID-128s.
> > + /* Group all UUID16 types */
> > + list_for_each(p, &hdev->uuids) {
> > + struct bt_uuid *uuid = list_entry(p, struct bt_uuid, list);
> > + u16 uuid16;
> > +
> > + uuid16 = get_uuid16(uuid->uuid);
> > + if (uuid16 == 0)
> > + return;
> > +
> > + if (uuid16 < 0x1100)
> > + continue;
> > +
> > + if (uuid16 == PNP_INFO_SVCLASS_ID)
> > + continue;
> > +
> > + /* Stop if not enough space to put next UUID */
> > + if (eir_len + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
> > + truncated = 1;
> > + break;
> > + }
> > +
> > + /* Check for duplicates */
> > + for (i = 0; uuid16_list[i] != 0; i++)
> > + if (uuid16_list[i] == uuid16)
> > + break;
> > +
> > + if (uuid16_list[i] == 0) {
> > + uuid16_list[i] = uuid16;
> > + eir_len += sizeof(u16);
> > + }
> > + }
> > +
> > + if (uuid16_list[0] != 0) {
> > + u8 *length = ptr;
> > +
> > + /* EIR Data type */
> > + ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;
> > +
> > + ptr += 2;
> > + eir_len += 2;
>
> Here you increment eir_len but does not use it anymore. Maybe it would
> be nice to add some assert (BUG_ON ?) on it just to check for
> programming errors if someone tries to change this code later.
This is a straight copy from user space with UUID-128, TX power and
Device ID info support stripped away. Once those get added the eir_len
variable will be important later in the function.
> > +static int update_eir(struct hci_dev *hdev)
> > +{
> > + struct hci_cp_write_eir cp;
> > +
> > + if (!(hdev->features[6] & LMP_EXT_INQ))
> > + return 0;
> > +
> > + if (hdev->ssp_mode == 0)
> > + return 0;
> > +
> > + if (test_bit(HCI_SERVICE_CACHE, &hdev->flags))
> > + return 0;
> > +
> > + memset(&cp, 0, sizeof(cp));
> > +
> > + create_eir(hdev, cp.data);
> > +
> > + if (memcmp(cp.data, hdev->eir, sizeof(cp.data)) == 0)
> > + return 0;
>
> What about making create_eir() return "int" and check for eir_len and
> the end? That would avoid the memcmp() here.
I'm not completely sure what you're proposing, but if it's to check for
matching lengths of cp.data and hdev->eir before doing the memcmp then
hdev would need a new eir_length member in addition to eir. Could be
worth it, but it does slightly increase the complexity and since this is
not a performance critical piece of code (won't be called too often) I'm
not sure if it's worth it.
Johan
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support
2011-03-25 16:35 ` Johan Hedberg
@ 2011-03-25 16:49 ` Johan Hedberg
2011-03-25 17:23 ` Anderson Lizardo
1 sibling, 0 replies; 7+ messages in thread
From: Johan Hedberg @ 2011-03-25 16:49 UTC (permalink / raw)
To: Anderson Lizardo, linux-bluetooth
Hi,
On Fri, Mar 25, 2011, Johan Hedberg wrote:
> > > + }
> > > +
> > > + memcpy(&val, uuid128, 4);
> > > +
> > > + val = be32_to_cpu(val);
> >
> > I noticed just know the those UUID handling functions from management
> > API are assuming UUIDs in network order. We will need to change this,
> > or take extra care when reusing them for Advertising Data and GATT
> > uuids.
>
> Right. This code is pretty much a copy of what user space already has
> (hciops.c), and that code is relying on SDP (i.e. network) byte order
> UUID-128s.
Actually now that I think about it, for consistency with everything else
in the management interface the byte order, at least in the management
protocol, should probably stay little endian (mimicing HCI). We *could*
store the UUIDs in hdev->uuids in host byte order, but then we'll need
to add all the byte-order dependent code that we've recently created for
user space with bt_uuid.
The really important part is the management protocol since that's what's
visible to user space. The internal storage of these UUIDs in the kernel
is secondary. Personally, I'd like to avoid any 128-bit byte order
conversions if possible but I can see how this can be argued in both
ways.
Johan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support
2011-03-25 16:35 ` Johan Hedberg
2011-03-25 16:49 ` Johan Hedberg
@ 2011-03-25 17:23 ` Anderson Lizardo
2011-03-25 17:26 ` Anderson Lizardo
2011-03-25 17:28 ` Johan Hedberg
1 sibling, 2 replies; 7+ messages in thread
From: Anderson Lizardo @ 2011-03-25 17:23 UTC (permalink / raw)
To: Anderson Lizardo, linux-bluetooth; +Cc: Johan Hedberg
Hi Johan,
On Fri, Mar 25, 2011 at 12:35 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Lizardo,
>
> On Fri, Mar 25, 2011, Anderson Lizardo wrote:
>> > +static u16 get_uuid16(u8 *uuid128)
>> > +{
>> > + u8 *b = bluetooth_base_uuid;
>> > + u32 val;
>> > + int i;
>> > +
>> > + for (i = 4; i < 16; i++) {
>> > + if (b[i] != uuid128[i])
>> > + return 0;
>>
>> Don't you need to check bytes 0 and 1 as well, i.e. only bytes 2 and 3
>> are changed?
>
> The first four bytes can have any value for Bluetooth UUIDs. The purpose
> of this part of the function is to determine whether it's a Bluetooth
> UUID or not (i.e. derived from the Bluetooth base UUID), so the four
> first bytes don't matter.
At least for UUIDs used in attribute types I see this on the spec (page 1835):
"Or, to put it more simply, the 16-bit Attribute UUID replaces the x’s
in the follow-
ing:
0000xxxx-0000-1000-8000-00805F9B34FB"
I.e. , only third and fourth bytes being part of 16-bit UUID.
>> > +static int update_eir(struct hci_dev *hdev)
>> > +{
>> > + struct hci_cp_write_eir cp;
>> > +
>> > + if (!(hdev->features[6] & LMP_EXT_INQ))
>> > + return 0;
>> > +
>> > + if (hdev->ssp_mode == 0)
>> > + return 0;
>> > +
>> > + if (test_bit(HCI_SERVICE_CACHE, &hdev->flags))
>> > + return 0;
>> > +
>> > + memset(&cp, 0, sizeof(cp));
>> > +
>> > + create_eir(hdev, cp.data);
>> > +
>> > + if (memcmp(cp.data, hdev->eir, sizeof(cp.data)) == 0)
>> > + return 0;
>>
>> What about making create_eir() return "int" and check for eir_len and
>> the end? That would avoid the memcmp() here.
>
> I'm not completely sure what you're proposing, but if it's to check for
> matching lengths of cp.data and hdev->eir before doing the memcmp then
> hdev would need a new eir_length member in addition to eir. Could be
> worth it, but it does slightly increase the complexity and since this is
> not a performance critical piece of code (won't be called too often) I'm
> not sure if it's worth it.
Actually I was proposing to get rid of the memcmp(), but now I see it
is necessary to not do HCI_OP_WRITE_EIR if EIR has not changed.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support
2011-03-25 17:23 ` Anderson Lizardo
@ 2011-03-25 17:26 ` Anderson Lizardo
2011-03-25 17:28 ` Johan Hedberg
1 sibling, 0 replies; 7+ messages in thread
From: Anderson Lizardo @ 2011-03-25 17:26 UTC (permalink / raw)
To: Anderson Lizardo, linux-bluetooth; +Cc: Johan Hedberg
Hi Johan,
On Fri, Mar 25, 2011 at 1:23 PM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> Hi Johan,
>
> On Fri, Mar 25, 2011 at 12:35 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> Hi Lizardo,
>>
>> On Fri, Mar 25, 2011, Anderson Lizardo wrote:
>>> > +static u16 get_uuid16(u8 *uuid128)
>>> > +{
>>> > + u8 *b = bluetooth_base_uuid;
>>> > + u32 val;
>>> > + int i;
>>> > +
>>> > + for (i = 4; i < 16; i++) {
>>> > + if (b[i] != uuid128[i])
>>> > + return 0;
>>>
>>> Don't you need to check bytes 0 and 1 as well, i.e. only bytes 2 and 3
>>> are changed?
>>
>> The first four bytes can have any value for Bluetooth UUIDs. The purpose
>> of this part of the function is to determine whether it's a Bluetooth
>> UUID or not (i.e. derived from the Bluetooth base UUID), so the four
>> first bytes don't matter.
>
> At least for UUIDs used in attribute types I see this on the spec (page 1835):
>
> "Or, to put it more simply, the 16-bit Attribute UUID replaces the x’s
> in the follow-
> ing:
> 0000xxxx-0000-1000-8000-00805F9B34FB"
>
> I.e. , only third and fourth bytes being part of 16-bit UUID.
Hmm, now I see you implicitly do what I was proposing by using:
+ if (val > 0xffff)
+ return 0;
So you can just ignore this comment :)
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support
2011-03-25 17:23 ` Anderson Lizardo
2011-03-25 17:26 ` Anderson Lizardo
@ 2011-03-25 17:28 ` Johan Hedberg
1 sibling, 0 replies; 7+ messages in thread
From: Johan Hedberg @ 2011-03-25 17:28 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
Hi Lizardo,
On Fri, Mar 25, 2011, Anderson Lizardo wrote:
> > On Fri, Mar 25, 2011, Anderson Lizardo wrote:
> >> > +static u16 get_uuid16(u8 *uuid128)
> >> > +{
> >> > + u8 *b = bluetooth_base_uuid;
> >> > + u32 val;
> >> > + int i;
> >> > +
> >> > + for (i = 4; i < 16; i++) {
> >> > + if (b[i] != uuid128[i])
> >> > + return 0;
> >>
> >> Don't you need to check bytes 0 and 1 as well, i.e. only bytes 2 and 3
> >> are changed?
> >
> > The first four bytes can have any value for Bluetooth UUIDs. The purpose
> > of this part of the function is to determine whether it's a Bluetooth
> > UUID or not (i.e. derived from the Bluetooth base UUID), so the four
> > first bytes don't matter.
>
> At least for UUIDs used in attribute types I see this on the spec (page 1835):
>
> "Or, to put it more simply, the 16-bit Attribute UUID replaces the x’s
> in the follow-
> ing:
> 0000xxxx-0000-1000-8000-00805F9B34FB"
>
> I.e. , only third and fourth bytes being part of 16-bit UUID.
That's correct, and we're not contradicting each other here. The
function simply in the first stage doesn't care if it's a 32 or 16 bit
Bluetooth UUID and only after that it does the > 0xffff comparison.
Johan
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-03-25 17:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-25 13:25 [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support johan.hedberg
2011-03-25 15:36 ` Anderson Lizardo
2011-03-25 16:35 ` Johan Hedberg
2011-03-25 16:49 ` Johan Hedberg
2011-03-25 17:23 ` Anderson Lizardo
2011-03-25 17:26 ` Anderson Lizardo
2011-03-25 17:28 ` Johan Hedberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox