Linux bluetooth development
 help / color / mirror / Atom feed
* [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