Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH] hog: Fill ev.u.create.{phys,uniq} for UHID_CREATE
From: Petri Gynther @ 2014-02-19 21:15 UTC (permalink / raw)
  To: Szymon Janc, Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <3638568.rUrzPLdPh8@athlon>

I used hogdev->device directly in:
http://marc.info/?l=linux-bluetooth&m=139105411019568&w=2

But Johan commented on that:
http://marc.info/?l=linux-bluetooth&m=139228316405777&w=2

So that's why I went this route and added name, src, dst.

-- Petri

On Wed, Feb 19, 2014 at 12:30 PM, Szymon Janc <szymon.janc@gmail.com> wrote:
> Hi Petri,
>
> On Wednesday 19 February 2014 12:15:45 Petri Gynther wrote:
>> Fill ev.u.create.{phys,uniq} fields when uHID device is created.
>> These values are copied to kernel hid_device structure.
>>
>> linux/include/linux/hid.h:
>> struct hid_device {
>>   ...
>>   char name[128];  /* Device name */
>>   char phys[64];   /* Device physical location */
>>   char uniq[64];   /* Device unique identifier (serial #) */
>> ---
>>  profiles/input/hog.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/profiles/input/hog.c b/profiles/input/hog.c
>> index b9a14b9..35b973b 100644
>> --- a/profiles/input/hog.c
>> +++ b/profiles/input/hog.c
>> @@ -92,6 +92,8 @@ struct hog_device {
>>       uint16_t                ctrlpt_handle;
>>       uint8_t                 flags;
>>       char                    *name;
>> +     bdaddr_t                src;
>> +     bdaddr_t                dst;
>>  };
>>
>
> struct hog_device already holds reference to btd_device and both src and dst
> can be taken from it where needed. No need to extend struct hog_device.
>
> BTW same goes with name member which could be removed.
>
>>  struct report {
>> @@ -396,6 +398,8 @@ static void report_map_read_cb(guint8 status, const
>> guint8 *pdu, guint16 plen, strncpy((char *) ev.u.create.name, hogdev->name
>> ?
>>                                       hogdev->name : "bluez-hog-device",
>>                                       sizeof(ev.u.create.name) - 1);
>> +     ba2str(&hogdev->src, (char *) ev.u.create.phys);
>> +     ba2str(&hogdev->dst, (char *) ev.u.create.uniq);
>>       ev.u.create.vendor = vendor;
>>       ev.u.create.product = product;
>>       ev.u.create.version = version;
>> @@ -724,6 +728,7 @@ static void attio_disconnected_cb(gpointer user_data)
>>  static struct hog_device *hog_new_device(struct btd_device *device,
>>                                                               uint16_t id)
>>  {
>> +     struct btd_adapter *adapter = device_get_adapter(device);
>>       struct hog_device *hogdev;
>>       char name[HCI_MAX_NAME_LENGTH + 1];
>>
>> @@ -736,6 +741,8 @@ static struct hog_device *hog_new_device(struct
>> btd_device *device, device_get_name(device, name, sizeof(name));
>>       if (strlen(name) > 0)
>>               hogdev->name = g_strdup(name);
>> +     bacpy(&hogdev->src, btd_adapter_get_address(adapter));
>> +     bacpy(&hogdev->dst, device_get_address(device));
>>
>>       return hogdev;
>>  }
>
> --
> Szymon K. Janc
> szymon.janc@gmail.com

^ permalink raw reply

* Re: [PATCH] hog: Fill ev.u.create.{phys,uniq} for UHID_CREATE
From: Szymon Janc @ 2014-02-19 21:24 UTC (permalink / raw)
  To: Petri Gynther; +Cc: Johan Hedberg, linux-bluetooth
In-Reply-To: <CAGXr9JGz-B6yLHsDMKZrp3iuWP4Uaj78kqiGtNUytnku+wczAA@mail.gmail.com>

Hi Petri,

On Wednesday 19 February 2014 13:15:32 Petri Gynther wrote:
> I used hogdev->device directly in:
> http://marc.info/?l=linux-bluetooth&m=139105411019568&w=2
> 
> But Johan commented on that:
> http://marc.info/?l=linux-bluetooth&m=139228316405777&w=2
> 
> So that's why I went this route and added name, src, dst.

I suppose it was about using local adapter variable to avoid those nested 
calls as this is indeed quite hard to read.

> 
> -- Petri
> 
> On Wed, Feb 19, 2014 at 12:30 PM, Szymon Janc <szymon.janc@gmail.com> wrote:
> > Hi Petri,
> > 
> > On Wednesday 19 February 2014 12:15:45 Petri Gynther wrote:
> >> Fill ev.u.create.{phys,uniq} fields when uHID device is created.
> >> These values are copied to kernel hid_device structure.
> >> 
> >> linux/include/linux/hid.h:
> >> struct hid_device {
> >> 
> >>   ...
> >>   char name[128];  /* Device name */
> >>   char phys[64];   /* Device physical location */
> >>   char uniq[64];   /* Device unique identifier (serial #) */
> >> 
> >> ---
> >> 
> >>  profiles/input/hog.c | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >> 
> >> diff --git a/profiles/input/hog.c b/profiles/input/hog.c
> >> index b9a14b9..35b973b 100644
> >> --- a/profiles/input/hog.c
> >> +++ b/profiles/input/hog.c
> >> @@ -92,6 +92,8 @@ struct hog_device {
> >> 
> >>       uint16_t                ctrlpt_handle;
> >>       uint8_t                 flags;
> >>       char                    *name;
> >> 
> >> +     bdaddr_t                src;
> >> +     bdaddr_t                dst;
> >> 
> >>  };
> > 
> > struct hog_device already holds reference to btd_device and both src and
> > dst can be taken from it where needed. No need to extend struct
> > hog_device.
> > 
> > BTW same goes with name member which could be removed.
> > 
> >>  struct report {
> >> 
> >> @@ -396,6 +398,8 @@ static void report_map_read_cb(guint8 status, const
> >> guint8 *pdu, guint16 plen, strncpy((char *) ev.u.create.name,
> >> hogdev->name
> >> ?
> >> 
> >>                                       hogdev->name : "bluez-hog-device",
> >>                                       sizeof(ev.u.create.name) - 1);
> >> 
> >> +     ba2str(&hogdev->src, (char *) ev.u.create.phys);
> >> +     ba2str(&hogdev->dst, (char *) ev.u.create.uniq);
> >> 
> >>       ev.u.create.vendor = vendor;
> >>       ev.u.create.product = product;
> >>       ev.u.create.version = version;
> >> 
> >> @@ -724,6 +728,7 @@ static void attio_disconnected_cb(gpointer user_data)
> >> 
> >>  static struct hog_device *hog_new_device(struct btd_device *device,
> >>  
> >>                                                               uint16_t
> >>                                                               id)
> >>  
> >>  {
> >> 
> >> +     struct btd_adapter *adapter = device_get_adapter(device);
> >> 
> >>       struct hog_device *hogdev;
> >>       char name[HCI_MAX_NAME_LENGTH + 1];
> >> 
> >> @@ -736,6 +741,8 @@ static struct hog_device *hog_new_device(struct
> >> btd_device *device, device_get_name(device, name, sizeof(name));
> >> 
> >>       if (strlen(name) > 0)
> >>       
> >>               hogdev->name = g_strdup(name);
> >> 
> >> +     bacpy(&hogdev->src, btd_adapter_get_address(adapter));
> >> +     bacpy(&hogdev->dst, device_get_address(device));
> >> 
> >>       return hogdev;
> >>  
> >>  }
> > 
> > --
> > Szymon K. Janc
> > szymon.janc@gmail.com

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply

* Re: [PATCH] hog: Fill ev.u.create.{phys,uniq} for UHID_CREATE
From: Petri Gynther @ 2014-02-19 21:47 UTC (permalink / raw)
  To: Szymon Janc; +Cc: Johan Hedberg, linux-bluetooth
In-Reply-To: <2850259.ILDlU5AmMy@athlon>

Hi Szymon,

On Wed, Feb 19, 2014 at 1:24 PM, Szymon Janc <szymon.janc@gmail.com> wrote:
> Hi Petri,
>
> On Wednesday 19 February 2014 13:15:32 Petri Gynther wrote:
>> I used hogdev->device directly in:
>> http://marc.info/?l=linux-bluetooth&m=139105411019568&w=2
>>
>> But Johan commented on that:
>> http://marc.info/?l=linux-bluetooth&m=139228316405777&w=2
>>
>> So that's why I went this route and added name, src, dst.
>
> I suppose it was about using local adapter variable to avoid those nested
> calls as this is indeed quite hard to read.
>

I think I misunderstood the comment. I ended up adding name, src, dst
to hog_device to mimic input_device, which has those fields. However,
I just realized that input_device also has a pointer to btd_device.

I'll rework the HoG code. And, I'll see if it is easy to do the same
for input_device.

>>
>> -- Petri
>>
>> On Wed, Feb 19, 2014 at 12:30 PM, Szymon Janc <szymon.janc@gmail.com> wrote:
>> > Hi Petri,
>> >
>> > On Wednesday 19 February 2014 12:15:45 Petri Gynther wrote:
>> >> Fill ev.u.create.{phys,uniq} fields when uHID device is created.
>> >> These values are copied to kernel hid_device structure.
>> >>
>> >> linux/include/linux/hid.h:
>> >> struct hid_device {
>> >>
>> >>   ...
>> >>   char name[128];  /* Device name */
>> >>   char phys[64];   /* Device physical location */
>> >>   char uniq[64];   /* Device unique identifier (serial #) */
>> >>
>> >> ---
>> >>
>> >>  profiles/input/hog.c | 7 +++++++
>> >>  1 file changed, 7 insertions(+)
>> >>
>> >> diff --git a/profiles/input/hog.c b/profiles/input/hog.c
>> >> index b9a14b9..35b973b 100644
>> >> --- a/profiles/input/hog.c
>> >> +++ b/profiles/input/hog.c
>> >> @@ -92,6 +92,8 @@ struct hog_device {
>> >>
>> >>       uint16_t                ctrlpt_handle;
>> >>       uint8_t                 flags;
>> >>       char                    *name;
>> >>
>> >> +     bdaddr_t                src;
>> >> +     bdaddr_t                dst;
>> >>
>> >>  };
>> >
>> > struct hog_device already holds reference to btd_device and both src and
>> > dst can be taken from it where needed. No need to extend struct
>> > hog_device.
>> >
>> > BTW same goes with name member which could be removed.
>> >
>> >>  struct report {
>> >>
>> >> @@ -396,6 +398,8 @@ static void report_map_read_cb(guint8 status, const
>> >> guint8 *pdu, guint16 plen, strncpy((char *) ev.u.create.name,
>> >> hogdev->name
>> >> ?
>> >>
>> >>                                       hogdev->name : "bluez-hog-device",
>> >>                                       sizeof(ev.u.create.name) - 1);
>> >>
>> >> +     ba2str(&hogdev->src, (char *) ev.u.create.phys);
>> >> +     ba2str(&hogdev->dst, (char *) ev.u.create.uniq);
>> >>
>> >>       ev.u.create.vendor = vendor;
>> >>       ev.u.create.product = product;
>> >>       ev.u.create.version = version;
>> >>
>> >> @@ -724,6 +728,7 @@ static void attio_disconnected_cb(gpointer user_data)
>> >>
>> >>  static struct hog_device *hog_new_device(struct btd_device *device,
>> >>
>> >>                                                               uint16_t
>> >>                                                               id)
>> >>
>> >>  {
>> >>
>> >> +     struct btd_adapter *adapter = device_get_adapter(device);
>> >>
>> >>       struct hog_device *hogdev;
>> >>       char name[HCI_MAX_NAME_LENGTH + 1];
>> >>
>> >> @@ -736,6 +741,8 @@ static struct hog_device *hog_new_device(struct
>> >> btd_device *device, device_get_name(device, name, sizeof(name));
>> >>
>> >>       if (strlen(name) > 0)
>> >>
>> >>               hogdev->name = g_strdup(name);
>> >>
>> >> +     bacpy(&hogdev->src, btd_adapter_get_address(adapter));
>> >> +     bacpy(&hogdev->dst, device_get_address(device));
>> >>
>> >>       return hogdev;
>> >>
>> >>  }
>> >
>> > --
>> > Szymon K. Janc
>> > szymon.janc@gmail.com
>
> --
> Szymon K. Janc
> szymon.janc@gmail.com

^ permalink raw reply

* [PATCH] Bluetooth: Add comment explainging store hint for long term keys
From: Marcel Holtmann @ 2014-02-20  1:11 UTC (permalink / raw)
  To: linux-bluetooth

The code itself is not descriptive on what store hint is used for
long term keys and why. So add some extensive comment here. Similar
to what has already been done for identity resolving key store hint.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/mgmt.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index e8b9d2f261ee..5f5e388716ec 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4771,6 +4771,17 @@ void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key)
 
 	memset(&ev, 0, sizeof(ev));
 
+	/* Devices using resolvable or non-resolvable random addresses
+	 * without providing an indentity resolving key don't require
+	 * to store long term keys. Their addresses will change the
+	 * next time around.
+	 *
+	 * Only when a remote device provides an identity address
+	 * make sure the long term key is stored. If the remote
+	 * identity is known, the long term keys are internally
+	 * mapped to the identity address. So allow static random
+	 * and public addresses here.
+	 */
 	if (key->bdaddr_type == ADDR_LE_DEV_RANDOM &&
 	    (key->bdaddr.b[5] & 0xc0) != 0xc0)
 		ev.store_hint = 0x00;
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH] Bluetooth: Replace own_address_type with force_static_address debugfs
From: Marcel Holtmann @ 2014-02-20  3:31 UTC (permalink / raw)
  To: linux-bluetooth

The own_address_type debugfs option does not providing enough
flexibity for interacting with the upcoming LE privacy support.

What really is needed is an option to force using the static address
compared to the public address. The new force_static_address debugfs
option does exactly that. In addition it is also only available when
the controller does actually have a public address. For single mode
LE only controllers this option will not be available.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 include/net/bluetooth/hci.h |  1 +
 net/bluetooth/hci_core.c    | 95 +++++++++++++++++++++++++++++----------------
 2 files changed, 63 insertions(+), 33 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index d3a8fff50f69..fe4b06bfc150 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -118,6 +118,7 @@ enum {
 	HCI_DEBUG_KEYS,
 	HCI_DUT_MODE,
 	HCI_FORCE_SC,
+	HCI_FORCE_STATIC_ADDR,
 	HCI_UNREGISTER,
 	HCI_USER_CHANNEL,
 
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 3711c7626cb2..b25a36c3064b 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -571,33 +571,52 @@ static const struct file_operations static_address_fops = {
 	.release	= single_release,
 };
 
-static int own_address_type_set(void *data, u64 val)
+static ssize_t force_static_address_read(struct file *file,
+					 char __user *user_buf,
+					 size_t count, loff_t *ppos)
 {
-	struct hci_dev *hdev = data;
-
-	if (val != 0 && val != 1)
-		return -EINVAL;
-
-	hci_dev_lock(hdev);
-	hdev->own_addr_type = val;
-	hci_dev_unlock(hdev);
+	struct hci_dev *hdev = file->private_data;
+	char buf[3];
 
-	return 0;
+	buf[0] = test_bit(HCI_FORCE_STATIC_ADDR, &hdev->dev_flags) ? 'Y': 'N';
+	buf[1] = '\n';
+	buf[2] = '\0';
+	return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
 }
 
-static int own_address_type_get(void *data, u64 *val)
+static ssize_t force_static_address_write(struct file *file,
+					  const char __user *user_buf,
+					  size_t count, loff_t *ppos)
 {
-	struct hci_dev *hdev = data;
+	struct hci_dev *hdev = file->private_data;
+	char buf[32];
+	size_t buf_size = min(count, (sizeof(buf)-1));
+	bool enable;
 
-	hci_dev_lock(hdev);
-	*val = hdev->own_addr_type;
-	hci_dev_unlock(hdev);
+	if (test_bit(HCI_UP, &hdev->flags))
+		return -EBUSY;
 
-	return 0;
+	if (copy_from_user(buf, user_buf, buf_size))
+		return -EFAULT;
+
+	buf[buf_size] = '\0';
+	if (strtobool(buf, &enable))
+		return -EINVAL;
+
+	if (enable == test_bit(HCI_FORCE_STATIC_ADDR, &hdev->dev_flags))
+		return -EALREADY;
+
+	change_bit(HCI_FORCE_STATIC_ADDR, &hdev->dev_flags);
+
+	return count;
 }
 
-DEFINE_SIMPLE_ATTRIBUTE(own_address_type_fops, own_address_type_get,
-			own_address_type_set, "%llu\n");
+static const struct file_operations force_static_address_fops = {
+	.open		= simple_open,
+	.read		= force_static_address_read,
+	.write		= force_static_address_write,
+	.llseek		= default_llseek,
+};
 
 static int identity_resolving_keys_show(struct seq_file *f, void *ptr)
 {
@@ -1406,17 +1425,19 @@ static void hci_init3_req(struct hci_request *req, unsigned long opt)
 		hci_setup_link_policy(req);
 
 	if (lmp_le_capable(hdev)) {
-		if (test_bit(HCI_SETUP, &hdev->dev_flags)) {
-			/* If the controller has a public BD_ADDR, then
-			 * by default use that one. If this is a LE only
-			 * controller without a public address, default
-			 * to the random address.
-			 */
-			if (bacmp(&hdev->bdaddr, BDADDR_ANY))
-				hdev->own_addr_type = ADDR_LE_DEV_PUBLIC;
-			else
-				hdev->own_addr_type = ADDR_LE_DEV_RANDOM;
-		}
+		/* If the controller has a public BD_ADDR, then by default
+		 * use that one. If this is a LE only controller without
+		 * a public address, default to the random address.
+		 *
+		 * For debugging purposes it is possible to force
+		 * controllers with a public address to use the
+		 * random address instead.
+		 */
+		if (test_bit(HCI_FORCE_STATIC_ADDR, &hdev->dev_flags) ||
+		    !bacmp(&hdev->bdaddr, BDADDR_ANY))
+			hdev->own_addr_type = ADDR_LE_DEV_RANDOM;
+		else
+			hdev->own_addr_type = ADDR_LE_DEV_PUBLIC;
 
 		hci_set_le_support(req);
 	}
@@ -1536,12 +1557,20 @@ static int __hci_init(struct hci_dev *hdev)
 	}
 
 	if (lmp_le_capable(hdev)) {
+		debugfs_create_file("static_address", 0444, hdev->debugfs,
+				    hdev, &static_address_fops);
+
+		/* For controllers with a public address, provide a debug
+		 * option to force the usage of the configured static
+		 * address. By default the public address is used.
+		 */
+		if (bacmp(&hdev->bdaddr, BDADDR_ANY))
+			debugfs_create_file("force_static_address", 0644,
+					    hdev->debugfs, hdev,
+					    &force_static_address_fops);
+
 		debugfs_create_u8("white_list_size", 0444, hdev->debugfs,
 				  &hdev->le_white_list_size);
-		debugfs_create_file("static_address", 0444, hdev->debugfs,
-				   hdev, &static_address_fops);
-		debugfs_create_file("own_address_type", 0644, hdev->debugfs,
-				    hdev, &own_address_type_fops);
 		debugfs_create_file("identity_resolving_keys", 0400,
 				    hdev->debugfs, hdev,
 				    &identity_resolving_keys_fops);
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH] Bluetooth: Track the current configured random address
From: Marcel Holtmann @ 2014-02-20  3:52 UTC (permalink / raw)
  To: linux-bluetooth

For Bluetooth controllers with LE support, track the value of the
currently configured random address. It is important to know what
the current random address is to avoid unneeded attempts to set
a new address. This will become important when introducing the
LE privacy support in the future.

In addition expose the current configured random address via
debugfs for debugging purposes.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 include/net/bluetooth/hci_core.h |  1 +
 net/bluetooth/hci_core.c         | 26 ++++++++++++++++++++++++++
 net/bluetooth/hci_event.c        | 24 ++++++++++++++++++++++++
 3 files changed, 51 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 3be2905010cd..3a8e22e9b25d 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -151,6 +151,7 @@ struct hci_dev {
 	__u8		bus;
 	__u8		dev_type;
 	bdaddr_t	bdaddr;
+	bdaddr_t	random_addr;
 	bdaddr_t	static_addr;
 	__u8		own_addr_type;
 	__u8		dev_name[HCI_MAX_NAME_LENGTH];
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index b25a36c3064b..877330b4876f 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -548,6 +548,29 @@ static int sniff_max_interval_get(void *data, u64 *val)
 DEFINE_SIMPLE_ATTRIBUTE(sniff_max_interval_fops, sniff_max_interval_get,
 			sniff_max_interval_set, "%llu\n");
 
+static int random_address_show(struct seq_file *f, void *p)
+{
+	struct hci_dev *hdev = f->private;
+
+	hci_dev_lock(hdev);
+	seq_printf(f, "%pMR\n", &hdev->random_addr);
+	hci_dev_unlock(hdev);
+
+	return 0;
+}
+
+static int random_address_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, random_address_show, inode->i_private);
+}
+
+static const struct file_operations random_address_fops = {
+	.open		= random_address_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
 static int static_address_show(struct seq_file *f, void *p)
 {
 	struct hci_dev *hdev = f->private;
@@ -1557,6 +1580,8 @@ static int __hci_init(struct hci_dev *hdev)
 	}
 
 	if (lmp_le_capable(hdev)) {
+		debugfs_create_file("random_address", 0444, hdev->debugfs,
+				    hdev, &random_address_fops);
 		debugfs_create_file("static_address", 0444, hdev->debugfs,
 				    hdev, &static_address_fops);
 
@@ -2205,6 +2230,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 
 	memset(hdev->eir, 0, sizeof(hdev->eir));
 	memset(hdev->dev_class, 0, sizeof(hdev->dev_class));
+	bacpy(&hdev->random_addr, BDADDR_ANY);
 
 	hci_req_unlock(hdev);
 
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 7228fa100b1f..4327b129d38e 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -959,6 +959,26 @@ static void hci_cc_read_local_oob_ext_data(struct hci_dev *hdev,
 	hci_dev_unlock(hdev);
 }
 
+
+static void hci_cc_le_set_random_addr(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	__u8 status = *((__u8 *) skb->data);
+	bdaddr_t *sent;
+
+	BT_DBG("%s status 0x%2.2x", hdev->name, status);
+
+	sent = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_RANDOM_ADDR);
+	if (!sent)
+		return;
+
+	hci_dev_lock(hdev);
+
+	if (!status)
+		bacpy(&hdev->random_addr, sent);
+
+	hci_dev_unlock(hdev);
+}
+
 static void hci_cc_le_set_adv_enable(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	__u8 *sent, status = *((__u8 *) skb->data);
@@ -2308,6 +2328,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_cc_user_passkey_neg_reply(hdev, skb);
 		break;
 
+	case HCI_OP_LE_SET_RANDOM_ADDR:
+		hci_cc_le_set_random_addr(hdev, skb);
+		break;
+
 	case HCI_OP_LE_SET_ADV_ENABLE:
 		hci_cc_le_set_adv_enable(hdev, skb);
 		break;
-- 
1.8.5.3


^ permalink raw reply related

* Re: [PATCH] Bluetooth: Don't send store hint for devices using identity addresses
From: Johan Hedberg @ 2014-02-20  6:15 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1392839514-36896-1-git-send-email-marcel@holtmann.org>

Hi Marcel,

On Wed, Feb 19, 2014, Marcel Holtmann wrote:
> The identity resolving keys should only be stored for devices using
> resolvable random addresses. If the device is already using an
> identity address, inform it about the new identity resolving key,
> but tell userspace that this key is not persistent.
> 
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  net/bluetooth/mgmt.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)

Applied to bluetooth-next. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] Bluetooth: Add comment explainging store hint for long term keys
From: Johan Hedberg @ 2014-02-20  6:32 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1392858718-40052-1-git-send-email-marcel@holtmann.org>

Hi Marcel,

On Wed, Feb 19, 2014, Marcel Holtmann wrote:
> The code itself is not descriptive on what store hint is used for
> long term keys and why. So add some extensive comment here. Similar
> to what has already been done for identity resolving key store hint.
> 
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  net/bluetooth/mgmt.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)

Applied to bluetooth-next. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] Bluetooth: Replace own_address_type with force_static_address debugfs
From: Johan Hedberg @ 2014-02-20  6:32 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1392867086-23388-1-git-send-email-marcel@holtmann.org>

Hi Marcel,

On Wed, Feb 19, 2014, Marcel Holtmann wrote:
> The own_address_type debugfs option does not providing enough
> flexibity for interacting with the upcoming LE privacy support.
> 
> What really is needed is an option to force using the static address
> compared to the public address. The new force_static_address debugfs
> option does exactly that. In addition it is also only available when
> the controller does actually have a public address. For single mode
> LE only controllers this option will not be available.
> 
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  include/net/bluetooth/hci.h |  1 +
>  net/bluetooth/hci_core.c    | 95 +++++++++++++++++++++++++++++----------------
>  2 files changed, 63 insertions(+), 33 deletions(-)

Applied to bluetooth-next. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] Bluetooth: Track the current configured random address
From: Johan Hedberg @ 2014-02-20  6:32 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1392868333-37479-1-git-send-email-marcel@holtmann.org>

Hi Marcel,

On Wed, Feb 19, 2014, Marcel Holtmann wrote:
> For Bluetooth controllers with LE support, track the value of the
> currently configured random address. It is important to know what
> the current random address is to avoid unneeded attempts to set
> a new address. This will become important when introducing the
> LE privacy support in the future.
> 
> In addition expose the current configured random address via
> debugfs for debugging purposes.
> 
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  include/net/bluetooth/hci_core.h |  1 +
>  net/bluetooth/hci_core.c         | 26 ++++++++++++++++++++++++++
>  net/bluetooth/hci_event.c        | 24 ++++++++++++++++++++++++
>  3 files changed, 51 insertions(+)

Applied to bluetooth-next. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH v2] Bluetooth: Fix channel check when binding RFCOMM sock
From: Andrzej Kaczmarek @ 2014-02-20  7:39 UTC (permalink / raw)
  To: linux-bluetooth, Marcel Holtmann
In-Reply-To: <1392300832-14142-1-git-send-email-andrzej.kaczmarek@tieto.com>

Hi,

On 13 February 2014 15:13, Andrzej Kaczmarek
<andrzej.kaczmarek@tieto.com> wrote:
> When binding RFCOMM socket with non-zero channel we're checking if
> there is already any other socket which has the same channel number
> assigned and then fail. This check does not consider situation where
> we have another socket connected to remote device on given channel
> number in which case we still should be able to bind local socket.
>
> This patch changes __rfcomm_get_sock_by_addr() to return only sockets
> in either BT_BOUND or BT_LISTEN states, also name is updated to better
> describe what this function does now.
>
> Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com>

ping...?

BR,
Andrzej

^ permalink raw reply

* Re: [PATCH] tools/rfcomm-tester: Add bind after connected test case
From: Andrzej Kaczmarek @ 2014-02-20  7:40 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1392300683-13835-1-git-send-email-andrzej.kaczmarek@tieto.com>

Hi,

On 13 February 2014 15:11, Andrzej Kaczmarek
<andrzej.kaczmarek@tieto.com> wrote:
> This testcase will check if it's possible to bind socket on the same
> channel number as used by some other socket connected to remote device.

ping...?

BR,
Andrzej

^ permalink raw reply

* Re: [PATCH v2] Bluetooth: Fix channel check when binding RFCOMM sock
From: Marcel Holtmann @ 2014-02-20  8:47 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: bluez mailin list (linux-bluetooth@vger.kernel.org)
In-Reply-To: <1392300832-14142-1-git-send-email-andrzej.kaczmarek@tieto.com>

Hi Andrzej,

> When binding RFCOMM socket with non-zero channel we're checking if
> there is already any other socket which has the same channel number
> assigned and then fail. This check does not consider situation where
> we have another socket connected to remote device on given channel
> number in which case we still should be able to bind local socket.
> 
> This patch changes __rfcomm_get_sock_by_addr() to return only sockets
> in either BT_BOUND or BT_LISTEN states, also name is updated to better
> describe what this function does now.
> 
> Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com>
> ---
> net/bluetooth/rfcomm/sock.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
> index 00573fb..1a59ddd 100644
> --- a/net/bluetooth/rfcomm/sock.c
> +++ b/net/bluetooth/rfcomm/sock.c
> @@ -105,13 +105,18 @@ static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err)
> }
> 
> /* ---- Socket functions ---- */
> -static struct sock *__rfcomm_get_sock_by_addr(u8 channel, bdaddr_t *src)
> +static struct sock *__rfcomm_get_listen_sock_by_addr(u8 channel, bdaddr_t *src)
> {
> 	struct sock *sk = NULL;
> 
> 	sk_for_each(sk, &rfcomm_sk_list.head) {
> -		if (rfcomm_pi(sk)->channel == channel &&
> -				!bacmp(&rfcomm_pi(sk)->src, src))
> +		if (rfcomm_pi(sk)->channel != channel)
> +			continue;
> +
> +		if (bacmp(&rfcomm_pi(sk)->src, src))
> +			continue;
> +
> +		if (sk->sk_state == BT_BOUND || sk->sk_state == BT_LISTEN)
> 			break;
> 	}
> 
> @@ -352,7 +357,8 @@ static int rfcomm_sock_bind(struct socket *sock, struct sockaddr *addr, int addr
> 
> 	write_lock(&rfcomm_sk_list.lock);
> 
> -	if (sa->rc_channel && __rfcomm_get_sock_by_addr(sa->rc_channel, &sa->rc_bdaddr)) {
> +	if (sa->rc_channel && __rfcomm_get_listen_sock_by_addr(sa->rc_channel,
> +							&sa->rc_bdaddr)) {

you need to fix the indentation here. If that is a problem, then maybe we need a shorter function name.

> 		err = -EADDRINUSE;
> 	} else {
> 		/* Save source address */
> @@ -439,7 +445,7 @@ static int rfcomm_sock_listen(struct socket *sock, int backlog)
> 		write_lock(&rfcomm_sk_list.lock);
> 
> 		for (channel = 1; channel < 31; channel++)
> -			if (!__rfcomm_get_sock_by_addr(channel, src)) {
> +			if (!__rfcomm_get_listen_sock_by_addr(channel, src)) {
> 				rfcomm_pi(sk)->channel = channel;
> 				err = 0;
> 				break;

Regards

Marcel


^ permalink raw reply

* Re: [PATCH BlueZ v2 1/4] shared: Add initial AVRCP code
From: Andrei Emeltchenko @ 2014-02-20  9:03 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1392734435-2182-1-git-send-email-luiz.dentz@gmail.com>

Hi

On Tue, Feb 18, 2014 at 04:40:32PM +0200, Luiz Augusto von Dentz wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> The patch makes AVRCP to be transport agnostic so that it can be used in
> with socket pair to build unit tests.
> 
> The idea is that all AVRCP specific logic will stay on src/shared/avrcp
> and connecting handling elsewhere.

Any comments about the patch? We need this one to start doing AVRCP unit tests.

Best regards 
Andrei Emeltchenko 


> ---
> v2: Fix not updating Android.mk with new location of the files
> 
>  android/Android.mk              |  1 +
>  android/Makefile.am             |  1 +
>  android/avrcp.c                 | 42 ++++++++++++-----------
>  src/shared/avrcp.c              | 75 +++++++++++++++++++++++++++++++++++++++++
>  {android => src/shared}/avrcp.h | 13 ++++---
>  5 files changed, 108 insertions(+), 24 deletions(-)
>  create mode 100644 src/shared/avrcp.c
>  copy {android => src/shared}/avrcp.h (65%)
> 
> diff --git a/android/Android.mk b/android/Android.mk
> index 2481a2c..4f8a17a 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -49,6 +49,7 @@ LOCAL_SRC_FILES := \
>  	bluez/src/shared/ringbuf.c \
>  	bluez/src/shared/hfp.c \
>  	bluez/src/shared/io-glib.c \
> +	bluez/src/shared/avrcp.c \
>  	bluez/src/sdpd-database.c \
>  	bluez/src/sdpd-service.c \
>  	bluez/src/sdpd-request.c \
> diff --git a/android/Makefile.am b/android/Makefile.am
> index 1913b42..3cc0687 100644
> --- a/android/Makefile.am
> +++ b/android/Makefile.am
> @@ -29,6 +29,7 @@ android_bluetoothd_SOURCES = android/main.c \
>  				src/shared/mgmt.h src/shared/mgmt.c \
>  				src/shared/ringbuf.h src/shared/ringbuf.c \
>  				src/shared/hfp.h src/shared/hfp.c \
> +				src/shared/avrcp.h src/shared/avrcp.c \
>  				android/bluetooth.h android/bluetooth.c \
>  				android/hidhost.h android/hidhost.c \
>  				android/ipc.h android/ipc.c \
> diff --git a/android/avrcp.c b/android/avrcp.c
> index b8304f5..65b3417 100644
> --- a/android/avrcp.c
> +++ b/android/avrcp.c
> @@ -32,12 +32,12 @@
>  #include "lib/bluetooth.h"
>  #include "lib/sdp.h"
>  #include "lib/sdp_lib.h"
> +#include "src/shared/avrcp.h"
>  #include "src/log.h"
>  #include "bluetooth.h"
>  #include "avrcp.h"
>  #include "hal-msg.h"
>  #include "ipc.h"
> -#include "avctp.h"
>  
>  #define L2CAP_PSM_AVCTP 0x17
>  
> @@ -53,7 +53,7 @@ static GIOChannel *server = NULL;
>  
>  struct avrcp_device {
>  	bdaddr_t	dst;
> -	struct avctp	*session;
> +	struct avrcp	*session;
>  	GIOChannel	*io;
>  };
>  
> @@ -133,7 +133,7 @@ static void avrcp_device_free(void *data)
>  	struct avrcp_device *dev = data;
>  
>  	if (dev->session)
> -		avctp_shutdown(dev->session);
> +		avrcp_shutdown(dev->session);
>  
>  	if (dev->io) {
>  		g_io_channel_shutdown(dev->io, FALSE, NULL);
> @@ -168,6 +168,17 @@ static int device_cmp(gconstpointer s, gconstpointer user_data)
>  	return bacmp(&dev->dst, dst);
>  }
>  
> +static struct avrcp_device *avrcp_device_find(const bdaddr_t *dst)
> +{
> +	GSList *l;
> +
> +	l = g_slist_find_custom(devices, dst, device_cmp);
> +	if (!l)
> +		return NULL;
> +
> +	return l->data;
> +}
> +
>  static void disconnect_cb(void *data)
>  {
>  	struct avrcp_device *dev = data;
> @@ -186,7 +197,6 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
>  	char address[18];
>  	uint16_t imtu, omtu;
>  	GError *gerr = NULL;
> -	GSList *l;
>  	int fd;
>  
>  	if (err) {
> @@ -209,9 +219,8 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
>  
>  	ba2str(&dst, address);
>  
> -	l = g_slist_find_custom(devices, &dst, device_cmp);
> -	if (l) {
> -		dev = l->data;
> +	dev = avrcp_device_find(&dst);
> +	if (dev) {
>  		if (dev->session) {
>  			error("Unexpected connection");
>  			return;
> @@ -222,17 +231,17 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
>  	}
>  
>  	fd = g_io_channel_unix_get_fd(chan);
> -	dev->session = avctp_new(fd, imtu, omtu, 0x0100);
>  
> +	dev->session = avrcp_new(fd, imtu, omtu, 0x0100);
>  	if (!dev->session) {
>  		avrcp_device_free(dev);
>  		return;
>  	}
>  
> -	avctp_set_destroy_cb(dev->session, disconnect_cb, dev);
> +	avrcp_set_destroy_cb(dev->session, disconnect_cb, dev);
>  
>  	/* FIXME: get the real name of the device */
> -	avctp_init_uinput(dev->session, "bluetooth", address);
> +	avrcp_init_uinput(dev->session, "bluetooth", address);
>  
>  	g_io_channel_set_close_on_unref(chan, FALSE);
>  
> @@ -331,12 +340,10 @@ void bt_avrcp_connect(const bdaddr_t *dst)
>  {
>  	struct avrcp_device *dev;
>  	char addr[18];
> -	GSList *l;
>  
>  	DBG("");
>  
> -	l = g_slist_find_custom(devices, dst, device_cmp);
> -	if (l)
> +	if (avrcp_device_find(dst))
>  		return;
>  
>  	dev = avrcp_device_new(dst);
> @@ -352,18 +359,15 @@ void bt_avrcp_connect(const bdaddr_t *dst)
>  void bt_avrcp_disconnect(const bdaddr_t *dst)
>  {
>  	struct avrcp_device *dev;
> -	GSList *l;
>  
>  	DBG("");
>  
> -	l = g_slist_find_custom(devices, dst, device_cmp);
> -	if (!l)
> +	dev = avrcp_device_find(dst);
> +	if (!dev)
>  		return;
>  
> -	dev = l->data;
> -
>  	if (dev->session) {
> -		avctp_shutdown(dev->session);
> +		avrcp_shutdown(dev->session);
>  		return;
>  	}
>  
> diff --git a/src/shared/avrcp.c b/src/shared/avrcp.c
> new file mode 100644
> index 0000000..32bd703
> --- /dev/null
> +++ b/src/shared/avrcp.c
> @@ -0,0 +1,75 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2014  Intel Corporation. All rights reserved.
> + *
> + *
> + *  This library is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU Lesser General Public
> + *  License as published by the Free Software Foundation; either
> + *  version 2.1 of the License, or (at your option) any later version.
> + *
> + *  This library is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  Lesser General Public License for more details.
> + *
> + *  You should have received a copy of the GNU Lesser General Public
> + *  License along with this library; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <stdbool.h>
> +#include <glib.h>
> +
> +#include "lib/bluetooth.h"
> +
> +#include "src/log.h"
> +
> +#include "android/avctp.h"
> +#include "avrcp.h"
> +
> +struct avrcp {
> +	struct avctp	*session;
> +};
> +
> +void avrcp_shutdown(struct avrcp *session)
> +{
> +	if (session->session)
> +		avctp_shutdown(session->session);
> +
> +	g_free(session);
> +}
> +
> +struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
> +{
> +	struct avrcp *session;
> +
> +	session = g_new0(struct avrcp, 1);
> +
> +	session->session = avctp_new(fd, imtu, omtu, version);
> +	if (!session->session) {
> +		g_free(session);
> +		return NULL;
> +	}
> +
> +	return session;
> +}
> +
> +void avrcp_set_destroy_cb(struct avrcp *session, avrcp_destroy_cb_t cb,
> +							void *user_data)
> +{
> +	avctp_set_destroy_cb(session->session, cb, user_data);
> +}
> +
> +int avrcp_init_uinput(struct avrcp *session, const char *name,
> +							const char *address)
> +{
> +	return avctp_init_uinput(session->session, name, address);
> +}
> diff --git a/android/avrcp.h b/src/shared/avrcp.h
> similarity index 65%
> copy from android/avrcp.h
> copy to src/shared/avrcp.h
> index 1fcd953..7955d56 100644
> --- a/android/avrcp.h
> +++ b/src/shared/avrcp.h
> @@ -2,7 +2,7 @@
>   *
>   *  BlueZ - Bluetooth protocol stack for Linux
>   *
> - *  Copyright (C) 2013-2014  Intel Corporation. All rights reserved.
> + *  Copyright (C) 2014  Intel Corporation. All rights reserved.
>   *
>   *
>   *  This library is free software; you can redistribute it and/or
> @@ -21,8 +21,11 @@
>   *
>   */
>  
> -bool bt_avrcp_register(const bdaddr_t *addr);
> -void bt_avrcp_unregister(void);
> +typedef void (*avrcp_destroy_cb_t) (void *user_data);
>  
> -void bt_avrcp_connect(const bdaddr_t *dst);
> -void bt_avrcp_disconnect(const bdaddr_t *dst);
> +struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version);
> +void avrcp_shutdown(struct avrcp *session);
> +void avrcp_set_destroy_cb(struct avrcp *session, avrcp_destroy_cb_t cb,
> +							void *user_data);
> +int avrcp_init_uinput(struct avrcp *session, const char *name,
> +							const char *address);
> -- 
> 1.8.5.3
> 
> --
> 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

^ permalink raw reply

* Re: partial success with PS3 sixaxis
From: Szymon Janc @ 2014-02-20  9:47 UTC (permalink / raw)
  To: Andrea; +Cc: linux-bluetooth
In-Reply-To: <52FFD116.2080902@gmail.com>

Hi Andrea,

On Saturday 15 of February 2014 20:41:58 Andrea wrote:
> On 13/02/14 09:25, Szymon Janc wrote:
> > Hi Andrea,
> > 
> > 
> > I assume you are using bluetoothctl so do:
> > agent on
> > default-agent
> > 
> > Then you should get authorization request in bluetoothctl when DS3 is connecting.
> > 
> 
> Hi,
> 
> I have tried on a different laptop (no more the raspberry pi, where I still have the issues I
> reported in the other email), running Fedora 19.
> 
> I compiled bluez 5.14 and run it.
> It is more stable and /dev/input/js0 is usable.
> 
> It is still not able to set the LEDs. This gets written in the output
> 
> Feb 15 16:55:02 localhost.localdomain bluetoothd[24620]: sixaxis: compatible device connected:
> PLAYSTATION(R)3 Contr
> Feb 15 16:55:02 localhost.localdomain kernel: input: PLAYSTATION(R)3 Controller as
> /devices/pci0000:00/0000:00:1d.1/
> Feb 15 16:55:02 localhost.localdomain kernel: sony 0005:054C:0268.000E: input,hidraw0: BLUETOOTH HID
> v1.00 Joystick
> Feb 15 16:55:02 localhost.localdomain bluetoothd[24620]: sixaxis: failed to set LEDS (0 bytes written)
> 
> I have tried to log the traffic for bluez and sixad (where the LEDs work), to see you can find any
> difference.

This is a known regression on kernels >3.9. Should be fixed in 3.15 AFAIR.

> http://pastebin.com/pExywtUk  (this is the log from bluez where LEDs keep flashing)
> http://pastebin.com/BgRUnamS  (log from sixad where LEDs are set)
> 
> Happy to provide more if it can be of any help.
> 
> Andrea
> 

-- 
Best regards, 
Szymon Janc

^ permalink raw reply

* Re: [PATCH BlueZ 8/8] doc/obex-api: Update documentation
From: Patrick Ohly @ 2014-02-20 10:52 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZL82SJPZws+b0syb0nx8OVHCwfB4Tr3cDem9JWBvy5oCQ@mail.gmail.com>

On Wed, 2014-02-19 at 19:11 +0200, Luiz Augusto von Dentz wrote:
> Hi Patrick,
> 
> On Tue, Feb 18, 2014 at 6:54 PM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> > On Tue, 2014-02-18 at 11:30 +0200, Luiz Augusto von Dentz wrote:
> >> Hi Patrick,
> >>
> >> On Tue, Feb 18, 2014 at 11:09 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> > I think I would prefer to *not* get errors when Suspend() is called on
> > an already suspended transfer or when Resume() is called on a running
> > transfer. The rationale is that the caller will typically only care
> > about the end result and not so much whether it was the one who
> > triggered the change. If we remove NotInProgress resp. InProgress errors
> > from the API, then such a caller can treat all errors as fatal problem,
> > without having to check the exact error.
> 
> Actually the NotInProgress error is for transfer queued, I guess for
> this case it makes sense to return an error since otherwise we would
> have to queue commands to be send when the transfer becomes active I
> believe this should probably be considered an error.

Agreed, if suspending can't be done, then an error needs to be returned.

It would be good to add information about the error to the
documentation, so that apps get at least some hint what they are
supposed to do in case of such an error (like aborting the queued
transfer (most likely) or waiting for it to start before suspending it
(seems a bit odd, but perhaps it is useful sometimes)).

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.




^ permalink raw reply

* Re: [PATCH BlueZ 8/8] doc/obex-api: Update documentation
From: Luiz Augusto von Dentz @ 2014-02-20 12:09 UTC (permalink / raw)
  To: Patrick Ohly; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1392893523.5772.2.camel@pohly-mobl1.fritz.box>

Hi Patrick,

On Thu, Feb 20, 2014 at 12:52 PM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> On Wed, 2014-02-19 at 19:11 +0200, Luiz Augusto von Dentz wrote:
>> Hi Patrick,
>>
>> On Tue, Feb 18, 2014 at 6:54 PM, Patrick Ohly <patrick.ohly@intel.com> wrote:
>> > On Tue, 2014-02-18 at 11:30 +0200, Luiz Augusto von Dentz wrote:
>> >> Hi Patrick,
>> >>
>> >> On Tue, Feb 18, 2014 at 11:09 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
>> > I think I would prefer to *not* get errors when Suspend() is called on
>> > an already suspended transfer or when Resume() is called on a running
>> > transfer. The rationale is that the caller will typically only care
>> > about the end result and not so much whether it was the one who
>> > triggered the change. If we remove NotInProgress resp. InProgress errors
>> > from the API, then such a caller can treat all errors as fatal problem,
>> > without having to check the exact error.
>>
>> Actually the NotInProgress error is for transfer queued, I guess for
>> this case it makes sense to return an error since otherwise we would
>> have to queue commands to be send when the transfer becomes active I
>> believe this should probably be considered an error.
>
> Agreed, if suspending can't be done, then an error needs to be returned.
>
> It would be good to add information about the error to the
> documentation, so that apps get at least some hint what they are
> supposed to do in case of such an error (like aborting the queued
> transfer (most likely) or waiting for it to start before suspending it
> (seems a bit odd, but perhaps it is useful sometimes)).

I went ahead and pushed with a note about NotInProgress error, if we
figure that is not enough we can reword it to be more clear, but at
least in terms of API and functionality it should be fine.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* [PATCHv1 0/4] AVRCP first unit tests
From: Andrei Emeltchenko @ 2014-02-20 13:22 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

This is first set of AVRCP unit tests to get feedback about the proposed approach.

Andrei Emeltchenko (4):
  unit/avrcp: First unit test for AVRCP profile
  unit/avrcp: Add support for browsing AVCTP channel
  unit/avrcp: Add TP/MPS/BV-03-C test case
  unit/avrcp: Add TP/MPS/BV-08-C test case

 Makefile.am         |   9 ++
 android/avrcp-lib.c | 129 ++++++++++++++++++
 android/avrcp-lib.h |   6 +
 unit/test-avrcp.c   | 370 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 514 insertions(+)
 create mode 100644 unit/test-avrcp.c

-- 
1.8.3.2


^ permalink raw reply

* [PATCHv1 1/4] unit/avrcp: First unit test for AVRCP profile
From: Andrei Emeltchenko @ 2014-02-20 13:22 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1392902559-23987-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Test TP/MPS/BV-01-C [SetAddressedPlayer - CT] verifies
SetAddressedPlayer command.
---
 Makefile.am         |   9 ++
 android/avrcp-lib.c |  59 ++++++++++++
 android/avrcp-lib.h |   1 +
 unit/test-avrcp.c   | 270 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 339 insertions(+)
 create mode 100644 unit/test-avrcp.c

diff --git a/Makefile.am b/Makefile.am
index 11f2aa1..5fe58eb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -282,6 +282,15 @@ unit_test_avctp_SOURCES = unit/test-avctp.c \
 				android/avctp.c android/avctp.h
 unit_test_avctp_LDADD = @GLIB_LIBS@
 
+unit_tests += unit/test-avrcp
+
+unit_test_avrcp_SOURCES = unit/test-avrcp.c \
+				src/shared/util.h src/shared/util.c \
+				src/log.h src/log.c \
+				android/avctp.c android/avctp.h \
+				android/avrcp-lib.c android/avrcp-lib.h
+unit_test_avrcp_LDADD = @GLIB_LIBS@ lib/libbluetooth-internal.la
+
 unit_tests += unit/test-gdbus-client
 
 unit_test_gdbus_client_SOURCES = unit/test-gdbus-client.c
diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index ef48fa7..adb42be 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -35,6 +35,38 @@
 #include "avctp.h"
 #include "avrcp-lib.h"
 
+#define IEEEID_BTSIG		0x001958
+
+#define AVRCP_SET_ADDRESSED_PLAYER	0x60
+
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+
+struct avrcp_header {
+	uint8_t company_id[3];
+	uint8_t pdu_id;
+	uint8_t packet_type:2;
+	uint8_t rsvd:6;
+	uint16_t params_len;
+	uint8_t params[0];
+} __attribute__ ((packed));
+#define AVRCP_HEADER_LENGTH 7
+
+#elif __BYTE_ORDER == __BIG_ENDIAN
+
+struct avrcp_header {
+	uint8_t company_id[3];
+	uint8_t pdu_id;
+	uint8_t rsvd:6;
+	uint8_t packet_type:2;
+	uint16_t params_len;
+	uint8_t params[0];
+} __attribute__ ((packed));
+#define AVRCP_HEADER_LENGTH 7
+
+#else
+#error "Unknown byte order"
+#endif
+
 struct avrcp {
 	struct avctp	*session;
 };
@@ -73,3 +105,30 @@ int avrcp_init_uinput(struct avrcp *session, const char *name,
 {
 	return avctp_init_uinput(session->session, name, address);
 }
+
+static void set_company_id(uint8_t cid[3], const uint32_t cid_in)
+{
+	cid[0] = cid_in >> 16;
+	cid[1] = cid_in >> 8;
+	cid[2] = cid_in;
+}
+
+int avrcp_set_addr_player(struct avrcp *avrcp_session, uint16_t id)
+{
+	uint8_t buf[AVRCP_HEADER_LENGTH + 2];
+	struct avrcp_header *pdu = (void *) buf;
+	struct avctp *session = avrcp_session->session;
+
+	memset(buf, 0, sizeof(buf));
+
+	set_company_id(pdu->company_id, IEEEID_BTSIG);
+
+	pdu->pdu_id = AVRCP_SET_ADDRESSED_PLAYER;
+
+	bt_put_be16(id, &pdu->params[0]);
+	pdu->params_len = htons(sizeof(id));
+
+	return avctp_send_vendordep_req(session, AVC_CTYPE_CONTROL,
+					AVC_SUBUNIT_PANEL, buf, sizeof(buf),
+					NULL, NULL);
+}
diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
index 7955d56..4207cce 100644
--- a/android/avrcp-lib.h
+++ b/android/avrcp-lib.h
@@ -29,3 +29,4 @@ void avrcp_set_destroy_cb(struct avrcp *session, avrcp_destroy_cb_t cb,
 							void *user_data);
 int avrcp_init_uinput(struct avrcp *session, const char *name,
 							const char *address);
+int avrcp_set_addr_player(struct avrcp *avrcp_session, uint16_t id);
diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
new file mode 100644
index 0000000..9491631
--- /dev/null
+++ b/unit/test-avrcp.c
@@ -0,0 +1,270 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Intel Corporation. All rights reserved.
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <inttypes.h>
+#include <string.h>
+#include <fcntl.h>
+#include <sys/socket.h>
+
+#include <glib.h>
+
+#include "src/shared/util.h"
+#include "src/log.h"
+#include "lib/bluetooth.h"
+
+#include "android/avctp.h"
+#include "android/avrcp-lib.h"
+
+struct test_pdu {
+	bool valid;
+	bool fragmented;
+	const uint8_t *data;
+	size_t size;
+};
+
+struct test_data {
+	char *test_name;
+	struct test_pdu *pdu_list;
+};
+
+struct context {
+	GMainLoop *main_loop;
+	struct avrcp *session;
+	guint source;
+	guint process;
+	int fd;
+	unsigned int pdu_offset;
+	const struct test_data *data;
+};
+
+#define data(args...) ((const unsigned char[]) { args })
+
+#define raw_pdu(args...)					\
+	{							\
+		.valid = true,					\
+		.data = data(args),				\
+		.size = sizeof(data(args)),			\
+	}
+
+#define frg_pdu(args...)					\
+	{							\
+		.valid = true,					\
+		.fragmented = true,				\
+		.data = data(args),				\
+		.size = sizeof(data(args)),			\
+	}
+
+#define define_test(name, function, args...)				\
+	do {								\
+		const struct test_pdu pdus[] = {			\
+			args, { }					\
+		};							\
+		static struct test_data data;				\
+		data.test_name = g_strdup(name);			\
+		data.pdu_list = g_malloc(sizeof(pdus));			\
+		memcpy(data.pdu_list, pdus, sizeof(pdus));		\
+		g_test_add_data_func(name, &data, function);		\
+	} while (0)
+
+static void test_debug(const char *str, void *user_data)
+{
+	const char *prefix = user_data;
+
+	g_print("%s%s\n", prefix, str);
+}
+
+static void test_free(gconstpointer user_data)
+{
+	const struct test_data *data = user_data;
+
+	g_free(data->test_name);
+	g_free(data->pdu_list);
+}
+
+static gboolean context_quit(gpointer user_data)
+{
+	struct context *context = user_data;
+
+	if (context->process > 0)
+		g_source_remove(context->process);
+
+	g_main_loop_quit(context->main_loop);
+
+	return FALSE;
+}
+
+static gboolean send_pdu(gpointer user_data)
+{
+	struct context *context = user_data;
+	const struct test_pdu *pdu;
+	ssize_t len;
+
+	pdu = &context->data->pdu_list[context->pdu_offset++];
+
+	len = write(context->fd, pdu->data, pdu->size);
+
+	if (g_test_verbose())
+		util_hexdump('<', pdu->data, len, test_debug, "AVCTP: ");
+
+	g_assert_cmpint(len, ==, pdu->size);
+
+	if (pdu->fragmented)
+		return send_pdu(user_data);
+
+	context->process = 0;
+	return FALSE;
+}
+
+static void context_process(struct context *context)
+{
+	if (!context->data->pdu_list[context->pdu_offset].valid) {
+		context_quit(context);
+		return;
+	}
+
+	context->process = g_idle_add(send_pdu, context);
+}
+
+static gboolean test_handler(GIOChannel *channel, GIOCondition cond,
+							gpointer user_data)
+{
+	struct context *context = user_data;
+	const struct test_pdu *pdu;
+	unsigned char buf[512];
+	ssize_t len;
+	int fd;
+
+	pdu = &context->data->pdu_list[context->pdu_offset++];
+
+	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+		context->source = 0;
+		g_print("%s: cond %x\n", __func__, cond);
+		return FALSE;
+	}
+
+	fd = g_io_channel_unix_get_fd(channel);
+
+	len = read(fd, buf, sizeof(buf));
+
+	g_assert(len > 0);
+
+	if (g_test_verbose())
+		util_hexdump('>', buf, len, test_debug, "AVCTP: ");
+
+	g_assert_cmpint(len, ==, pdu->size);
+
+	g_assert(memcmp(buf, pdu->data, pdu->size) == 0);
+
+	if (!pdu->fragmented)
+		context_process(context);
+
+	return TRUE;
+}
+
+static struct context *create_context(uint16_t version, gconstpointer data)
+{
+	struct context *context = g_new0(struct context, 1);
+	GIOChannel *channel;
+	int err, sv[2];
+
+	context->main_loop = g_main_loop_new(NULL, FALSE);
+	g_assert(context->main_loop);
+
+	err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
+	g_assert(err == 0);
+
+	context->session = avrcp_new(sv[0], 672, 672, version);
+	g_assert(context->session != NULL);
+
+	channel = g_io_channel_unix_new(sv[1]);
+
+	g_io_channel_set_close_on_unref(channel, TRUE);
+	g_io_channel_set_encoding(channel, NULL, NULL);
+	g_io_channel_set_buffered(channel, FALSE);
+
+	context->source = g_io_add_watch(channel,
+				G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+				test_handler, context);
+	g_assert(context->source > 0);
+
+	g_io_channel_unref(channel);
+
+	context->fd = sv[1];
+	context->data = data;
+
+	return context;
+}
+
+static void execute_context(struct context *context)
+{
+	g_main_loop_run(context->main_loop);
+
+	if (context->source > 0)
+		g_source_remove(context->source);
+
+	avrcp_shutdown(context->session);
+
+	g_main_loop_unref(context->main_loop);
+
+	test_free(context->data);
+	g_free(context);
+}
+
+static void test_client(gconstpointer data)
+{
+	struct context *context = create_context(0x0100, data);
+	int ret = 0;
+
+	if (g_str_equal(context->data->test_name, "/TP/MPS/BV-01-C"))
+		ret = avrcp_set_addr_player(context->session, 0xabcd);
+
+	DBG("ret = %d", ret);
+
+	g_assert(!ret);
+
+	execute_context(context);
+}
+
+int main(int argc, char *argv[])
+{
+	g_test_init(&argc, &argv, NULL);
+
+	if (g_test_verbose())
+		__btd_log_init("*", 0);
+
+	/* Media Player Selection Commands and Notifications tests */
+
+	define_test("/TP/MPS/BV-01-C", test_client,
+			raw_pdu(0x00, 0x11, 0x0e, 0x00, 0x48, 0x00,
+				0x00, 0x19, 0x58, 0x60, 0x00, 0x00,
+				0x02, 0xab, 0xcd));
+
+	return g_test_run();
+}
-- 
1.8.3.2


^ permalink raw reply related

* [PATCHv1 2/4] unit/avrcp: Add support for browsing AVCTP channel
From: Andrei Emeltchenko @ 2014-02-20 13:22 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1392902559-23987-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/avrcp-lib.c |  6 ++++
 android/avrcp-lib.h |  2 ++
 unit/test-avrcp.c   | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 94 insertions(+), 2 deletions(-)

diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index adb42be..13bfd79 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -94,6 +94,12 @@ struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 	return session;
 }
 
+int avrcp_connect_browsing(struct avrcp *session, int fd, size_t imtu,
+								size_t omtu)
+{
+	return avctp_connect_browsing(session->session, fd, imtu, omtu);
+}
+
 void avrcp_set_destroy_cb(struct avrcp *session, avrcp_destroy_cb_t cb,
 							void *user_data)
 {
diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
index 4207cce..7b0aaef 100644
--- a/android/avrcp-lib.h
+++ b/android/avrcp-lib.h
@@ -27,6 +27,8 @@ struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version);
 void avrcp_shutdown(struct avrcp *session);
 void avrcp_set_destroy_cb(struct avrcp *session, avrcp_destroy_cb_t cb,
 							void *user_data);
+int avrcp_connect_browsing(struct avrcp *session, int fd, size_t imtu,
+								size_t omtu);
 int avrcp_init_uinput(struct avrcp *session, const char *name,
 							const char *address);
 int avrcp_set_addr_player(struct avrcp *avrcp_session, uint16_t id);
diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index 9491631..9d4eede 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -45,6 +45,7 @@
 struct test_pdu {
 	bool valid;
 	bool fragmented;
+	bool browse;
 	const uint8_t *data;
 	size_t size;
 };
@@ -58,8 +59,10 @@ struct context {
 	GMainLoop *main_loop;
 	struct avrcp *session;
 	guint source;
+	guint browse_source;
 	guint process;
 	int fd;
+	int browse_fd;
 	unsigned int pdu_offset;
 	const struct test_data *data;
 };
@@ -73,6 +76,14 @@ struct context {
 		.size = sizeof(data(args)),			\
 	}
 
+#define brs_pdu(args...)					\
+	{							\
+		.valid = true,					\
+		.browse = true,					\
+		.data = data(args),				\
+		.size = sizeof(data(args)),			\
+	}
+
 #define frg_pdu(args...)					\
 	{							\
 		.valid = true,					\
@@ -128,7 +139,10 @@ static gboolean send_pdu(gpointer user_data)
 
 	pdu = &context->data->pdu_list[context->pdu_offset++];
 
-	len = write(context->fd, pdu->data, pdu->size);
+	if (pdu->browse)
+		len = write(context->browse_fd, pdu->data, pdu->size);
+	else
+		len = write(context->fd, pdu->data, pdu->size);
 
 	if (g_test_verbose())
 		util_hexdump('<', pdu->data, len, test_debug, "AVCTP: ");
@@ -161,6 +175,8 @@ static gboolean test_handler(GIOChannel *channel, GIOCondition cond,
 	ssize_t len;
 	int fd;
 
+	DBG("");
+
 	pdu = &context->data->pdu_list[context->pdu_offset++];
 
 	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
@@ -188,17 +204,58 @@ static gboolean test_handler(GIOChannel *channel, GIOCondition cond,
 	return TRUE;
 }
 
+static gboolean browse_test_handler(GIOChannel *channel, GIOCondition cond,
+							gpointer user_data)
+{
+	struct context *context = user_data;
+	const struct test_pdu *pdu;
+	unsigned char buf[512];
+	ssize_t len;
+	int fd;
+
+	pdu = &context->data->pdu_list[context->pdu_offset++];
+
+	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+		context->browse_source = 0;
+		g_print("%s: cond %x\n", __func__, cond);
+		return FALSE;
+	}
+
+	fd = g_io_channel_unix_get_fd(channel);
+
+	len = read(fd, buf, sizeof(buf));
+
+	g_assert(len > 0);
+
+	if (g_test_verbose())
+		util_hexdump('>', buf, len, test_debug, "AVCTP: ");
+
+	g_assert_cmpint(len, ==, pdu->size);
+
+	g_assert(memcmp(buf, pdu->data, pdu->size) == 0);
+
+	if (!pdu->fragmented)
+		context_process(context);
+
+	return TRUE;
+}
+
 static struct context *create_context(uint16_t version, gconstpointer data)
 {
 	struct context *context = g_new0(struct context, 1);
 	GIOChannel *channel;
 	int err, sv[2];
+	int ret;
+
+	DBG("");
 
 	context->main_loop = g_main_loop_new(NULL, FALSE);
 	g_assert(context->main_loop);
 
+	/* Control channel setup */
+
 	err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
-	g_assert(err == 0);
+	g_assert(!err);
 
 	context->session = avrcp_new(sv[0], 672, 672, version);
 	g_assert(context->session != NULL);
@@ -217,6 +274,30 @@ static struct context *create_context(uint16_t version, gconstpointer data)
 	g_io_channel_unref(channel);
 
 	context->fd = sv[1];
+
+	/* Browsing channel setup */
+
+	err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
+	g_assert(!err);
+
+	ret = avrcp_connect_browsing(context->session, sv[0], 672, 672);
+	g_assert(!ret);
+
+	channel = g_io_channel_unix_new(sv[1]);
+
+	g_io_channel_set_close_on_unref(channel, TRUE);
+	g_io_channel_set_encoding(channel, NULL, NULL);
+	g_io_channel_set_buffered(channel, FALSE);
+
+	context->browse_source = g_io_add_watch(channel,
+				G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+				browse_test_handler, context);
+	g_assert(context->browse_source > 0);
+
+	g_io_channel_unref(channel);
+
+	context->browse_fd = sv[1];
+
 	context->data = data;
 
 	return context;
@@ -231,6 +312,9 @@ static void execute_context(struct context *context)
 
 	avrcp_shutdown(context->session);
 
+	if (context->browse_source > 0)
+		g_source_remove(context->browse_source);
+
 	g_main_loop_unref(context->main_loop);
 
 	test_free(context->data);
-- 
1.8.3.2


^ permalink raw reply related

* [PATCHv1 3/4] unit/avrcp: Add TP/MPS/BV-03-C test case
From: Andrei Emeltchenko @ 2014-02-20 13:22 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1392902559-23987-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Test verifies that the Set Browsed Player command issued by the
AVRCP controller.
---
 android/avrcp-lib.c | 37 +++++++++++++++++++++++++++++++++++++
 android/avrcp-lib.h |  1 +
 unit/test-avrcp.c   |  6 ++++++
 3 files changed, 44 insertions(+)

diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index 13bfd79..92d2ac8 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -38,6 +38,7 @@
 #define IEEEID_BTSIG		0x001958
 
 #define AVRCP_SET_ADDRESSED_PLAYER	0x60
+#define AVRCP_SET_BROWSED_PLAYER	0x70
 
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 
@@ -67,6 +68,13 @@ struct avrcp_header {
 #error "Unknown byte order"
 #endif
 
+struct avrcp_browsing_header {
+	uint8_t pdu_id;
+	uint16_t param_len;
+	uint8_t params[0];
+} __attribute__ ((packed));
+#define AVRCP_BROWSING_HEADER_LENGTH 3
+
 struct avrcp {
 	struct avctp	*session;
 };
@@ -138,3 +146,32 @@ int avrcp_set_addr_player(struct avrcp *avrcp_session, uint16_t id)
 					AVC_SUBUNIT_PANEL, buf, sizeof(buf),
 					NULL, NULL);
 }
+
+static gboolean avrcp_set_browsed_player_rsp(struct avctp *conn,
+						uint8_t *operands,
+						size_t operand_count,
+						void *user_data)
+{
+	DBG("");
+
+	return FALSE;
+}
+
+int avrcp_set_browsed_player(struct avrcp *avrcp_session, uint16_t id)
+{
+	uint8_t buf[AVRCP_BROWSING_HEADER_LENGTH + 2];
+	struct avrcp_browsing_header *pdu = (void *) buf;
+	struct avctp *session = avrcp_session->session;
+
+	DBG("");
+
+	memset(buf, 0, sizeof(buf));
+
+	pdu->pdu_id = AVRCP_SET_BROWSED_PLAYER;
+
+	bt_put_be16(id, &pdu->params[0]);
+	pdu->param_len = htons(sizeof(id));
+
+	return avctp_send_browsing_req(session, buf, sizeof(buf),
+					avrcp_set_browsed_player_rsp, session);
+}
diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
index 7b0aaef..2eb020b 100644
--- a/android/avrcp-lib.h
+++ b/android/avrcp-lib.h
@@ -32,3 +32,4 @@ int avrcp_connect_browsing(struct avrcp *session, int fd, size_t imtu,
 int avrcp_init_uinput(struct avrcp *session, const char *name,
 							const char *address);
 int avrcp_set_addr_player(struct avrcp *avrcp_session, uint16_t id);
+int avrcp_set_browsed_player(struct avrcp *avrcp_session, uint16_t id);
diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index 9d4eede..692652c 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -329,6 +329,9 @@ static void test_client(gconstpointer data)
 	if (g_str_equal(context->data->test_name, "/TP/MPS/BV-01-C"))
 		ret = avrcp_set_addr_player(context->session, 0xabcd);
 
+	if (g_str_equal(context->data->test_name, "/TP/MPS/BV-03-C"))
+		ret = avrcp_set_browsed_player(context->session, 0xabcd);
+
 	DBG("ret = %d", ret);
 
 	g_assert(!ret);
@@ -350,5 +353,8 @@ int main(int argc, char *argv[])
 				0x00, 0x19, 0x58, 0x60, 0x00, 0x00,
 				0x02, 0xab, 0xcd));
 
+	define_test("/TP/MPS/BV-03-C", test_client,
+			raw_pdu(0x00, 0x11, 0x0e, 0x70, 0x00, 0x02,
+				0xab, 0xcd));
 	return g_test_run();
 }
-- 
1.8.3.2


^ permalink raw reply related

* [PATCHv1 4/4] unit/avrcp: Add TP/MPS/BV-08-C test case
From: Andrei Emeltchenko @ 2014-02-20 13:22 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1392902559-23987-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

This verifies GetFolderItems command for controller.
---
 android/avrcp-lib.c | 27 +++++++++++++++++++++++++++
 android/avrcp-lib.h |  2 ++
 unit/test-avrcp.c   | 10 ++++++++++
 3 files changed, 39 insertions(+)

diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index 92d2ac8..4f92780 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -39,6 +39,9 @@
 
 #define AVRCP_SET_ADDRESSED_PLAYER	0x60
 #define AVRCP_SET_BROWSED_PLAYER	0x70
+#define AVRCP_GET_FOLDER_ITEMS		0x71
+
+#define AVRCP_MEDIA_ATTRIBUTE_TITLE	0x01
 
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 
@@ -175,3 +178,27 @@ int avrcp_set_browsed_player(struct avrcp *avrcp_session, uint16_t id)
 	return avctp_send_browsing_req(session, buf, sizeof(buf),
 					avrcp_set_browsed_player_rsp, session);
 }
+
+int avrcp_get_folder_items(struct avrcp *avrcp_session, uint32_t start,
+								uint32_t end)
+{
+	uint8_t buf[AVRCP_BROWSING_HEADER_LENGTH + 10 + sizeof(uint32_t)];
+	struct avrcp_browsing_header *pdu = (void *) buf;
+	struct avctp *session = avrcp_session->session;
+
+	memset(buf, 0, sizeof(buf));
+
+	pdu->pdu_id = AVRCP_GET_FOLDER_ITEMS;
+	pdu->param_len = htons(10 + sizeof(uint32_t));
+
+	bt_put_be32(start, &pdu->params[1]);
+	bt_put_be32(end, &pdu->params[5]);
+
+	pdu->params[9] = 1;
+
+	/* Only the title (0x01) is mandatory. This can be extended to
+	 * support AVRCP_MEDIA_ATTRIBUTE_* attributes */
+	bt_put_be32(AVRCP_MEDIA_ATTRIBUTE_TITLE, &pdu->params[10]);
+
+	return avctp_send_browsing_req(session, buf, sizeof(buf), NULL, NULL);
+}
diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
index 2eb020b..b5d52b6 100644
--- a/android/avrcp-lib.h
+++ b/android/avrcp-lib.h
@@ -33,3 +33,5 @@ int avrcp_init_uinput(struct avrcp *session, const char *name,
 							const char *address);
 int avrcp_set_addr_player(struct avrcp *avrcp_session, uint16_t id);
 int avrcp_set_browsed_player(struct avrcp *avrcp_session, uint16_t id);
+int avrcp_get_folder_items(struct avrcp *avrcp_session, uint32_t start,
+								uint32_t end);
diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index 692652c..3971bd0 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -332,6 +332,9 @@ static void test_client(gconstpointer data)
 	if (g_str_equal(context->data->test_name, "/TP/MPS/BV-03-C"))
 		ret = avrcp_set_browsed_player(context->session, 0xabcd);
 
+	if (g_str_equal(context->data->test_name, "/TP/MPS/BV-08-C"))
+		ret = avrcp_get_folder_items(context->session, 0, 2);
+
 	DBG("ret = %d", ret);
 
 	g_assert(!ret);
@@ -356,5 +359,12 @@ int main(int argc, char *argv[])
 	define_test("/TP/MPS/BV-03-C", test_client,
 			raw_pdu(0x00, 0x11, 0x0e, 0x70, 0x00, 0x02,
 				0xab, 0xcd));
+
+	define_test("/TP/MPS/BV-08-C", test_client,
+			raw_pdu(0x00, 0x11, 0x0e, 0x71, 0x00, 0x0e,
+				0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+				0x00, 0x00, 0x02, 0x01, 0x00, 0x00,
+				0x00, 0x01));
+
 	return g_test_run();
 }
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH BlueZ 1/3] android/avrcp-lib: Rename AVCTP session to conn
From: Luiz Augusto von Dentz @ 2014-02-20 14:40 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

---
 android/avrcp-lib.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index ef48fa7..8450480 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -36,13 +36,13 @@
 #include "avrcp-lib.h"
 
 struct avrcp {
-	struct avctp	*session;
+	struct avctp	*conn;
 };
 
 void avrcp_shutdown(struct avrcp *session)
 {
-	if (session->session)
-		avctp_shutdown(session->session);
+	if (session->conn)
+		avctp_shutdown(session->conn);
 
 	g_free(session);
 }
@@ -53,8 +53,8 @@ struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 
 	session = g_new0(struct avrcp, 1);
 
-	session->session = avctp_new(fd, imtu, omtu, version);
-	if (!session->session) {
+	session->conn = avctp_new(fd, imtu, omtu, version);
+	if (!session->conn) {
 		g_free(session);
 		return NULL;
 	}
@@ -65,11 +65,11 @@ struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 void avrcp_set_destroy_cb(struct avrcp *session, avrcp_destroy_cb_t cb,
 							void *user_data)
 {
-	avctp_set_destroy_cb(session->session, cb, user_data);
+	avctp_set_destroy_cb(session->conn, cb, user_data);
 }
 
 int avrcp_init_uinput(struct avrcp *session, const char *name,
 							const char *address)
 {
-	return avctp_init_uinput(session->session, name, address);
+	return avctp_init_uinput(session->conn, name, address);
 }
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH BlueZ 2/3] android/avrcp-lib: Add avrcp_set_control_handlers
From: Luiz Augusto von Dentz @ 2014-02-20 14:40 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1392907255-29758-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds avrcp_set_control_handlers which can be used to set control
PDU handlers table and user data.
---
 android/avrcp-lib.c | 138 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 android/avrcp-lib.h |  37 ++++++++++++++
 2 files changed, 173 insertions(+), 2 deletions(-)

diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index 8450480..c508516 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -35,18 +35,139 @@
 #include "avctp.h"
 #include "avrcp-lib.h"
 
+/* Company IDs for vendor dependent commands */
+#define IEEEID_BTSIG		0x001958
+
+/* Status codes */
+#define AVRCP_STATUS_INVALID_COMMAND		0x00
+#define AVRCP_STATUS_INVALID_PARAM		0x01
+#define AVRCP_STATUS_PARAM_NOT_FOUND		0x02
+#define AVRCP_STATUS_INTERNAL_ERROR		0x03
+#define AVRCP_STATUS_SUCCESS			0x04
+#define AVRCP_STATUS_OUT_OF_BOUNDS		0x0b
+#define AVRCP_STATUS_INVALID_PLAYER_ID		0x11
+#define AVRCP_STATUS_PLAYER_NOT_BROWSABLE	0x12
+#define AVRCP_STATUS_NO_AVAILABLE_PLAYERS	0x15
+#define AVRCP_STATUS_ADDRESSED_PLAYER_CHANGED	0x16
+
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+
+struct avrcp_header {
+	uint8_t company_id[3];
+	uint8_t pdu_id;
+	uint8_t packet_type:2;
+	uint8_t rsvd:6;
+	uint16_t params_len;
+	uint8_t params[0];
+} __attribute__ ((packed));
+#define AVRCP_HEADER_LENGTH 7
+
+static inline uint32_t ntoh24(const uint8_t src[3])
+{
+	return src[0] << 16 | src[1] << 8 | src[2];
+}
+
+#elif __BYTE_ORDER == __BIG_ENDIAN
+
+struct avrcp_header {
+	uint8_t company_id[3];
+	uint8_t pdu_id;
+	uint8_t rsvd:6;
+	uint8_t packet_type:2;
+	uint16_t params_len;
+	uint8_t params[0];
+} __attribute__ ((packed));
+#define AVRCP_HEADER_LENGTH 7
+
+static inline uint32_t ntoh24(const uint8_t src[3])
+{
+	uint32_t dst;
+
+	memcpy(&dst, src, sizeof(src));
+
+	return dst;
+}
+
+#else
+#error "Unknown byte order"
+#endif
+
 struct avrcp {
-	struct avctp	*conn;
+	struct avctp *conn;
+
+	const struct avrcp_control_handler *control_handlers;
+	void *control_data;
+	unsigned int control_id;
 };
 
 void avrcp_shutdown(struct avrcp *session)
 {
-	if (session->conn)
+	if (session->conn) {
+		if (session->control_id > 0)
+			avctp_unregister_pdu_handler(session->conn,
+							session->control_id);
 		avctp_shutdown(session->conn);
+	}
 
 	g_free(session);
 }
 
+static size_t handle_vendordep_pdu(struct avctp *conn, uint8_t transaction,
+					uint8_t *code, uint8_t *subunit,
+					uint8_t *operands, size_t operand_count,
+					void *user_data)
+{
+	struct avrcp *session = user_data;
+	const struct avrcp_control_handler *handler;
+	struct avrcp_header *pdu = (void *) operands;
+	uint32_t company_id = ntoh24(pdu->company_id);
+
+	if (company_id != IEEEID_BTSIG) {
+		*code = AVC_CTYPE_NOT_IMPLEMENTED;
+		return 0;
+	}
+
+	DBG("AVRCP PDU 0x%02X, len 0x%04X", pdu->pdu_id,
+						ntohs(pdu->params_len));
+
+	pdu->packet_type = 0;
+	pdu->rsvd = 0;
+
+	if (operand_count < AVRCP_HEADER_LENGTH) {
+		pdu->params[0] = AVRCP_STATUS_INVALID_COMMAND;
+		goto reject;
+	}
+
+	if (!session->control_handlers)
+		goto reject;
+
+	for (handler = session->control_handlers; handler->id; handler++) {
+		if (handler->id == pdu->pdu_id)
+			break;
+	}
+
+	if (handler->id != pdu->pdu_id || handler->code != *code) {
+		pdu->params[0] = AVRCP_STATUS_INVALID_COMMAND;
+		goto reject;
+	}
+
+	if (!handler->func) {
+		pdu->params[0] = AVRCP_STATUS_INVALID_PARAM;
+		goto reject;
+	}
+
+	*code = handler->func(session, transaction, &pdu->params_len,
+					pdu->params, session->control_data);
+
+	return AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
+
+reject:
+	pdu->params_len = htons(1);
+	*code = AVC_CTYPE_REJECTED;
+
+	return AVRCP_HEADER_LENGTH + 1;
+}
+
 struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 {
 	struct avrcp *session;
@@ -59,6 +180,11 @@ struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 		return NULL;
 	}
 
+	session->control_id = avctp_register_pdu_handler(session->conn,
+							AVC_OP_VENDORDEP,
+							handle_vendordep_pdu,
+							session);
+
 	return session;
 }
 
@@ -68,6 +194,14 @@ void avrcp_set_destroy_cb(struct avrcp *session, avrcp_destroy_cb_t cb,
 	avctp_set_destroy_cb(session->conn, cb, user_data);
 }
 
+void avrcp_set_control_handlers(struct avrcp *session,
+				const struct avrcp_control_handler *handlers,
+				void *user_data)
+{
+	session->control_handlers = handlers;
+	session->control_data = user_data;
+}
+
 int avrcp_init_uinput(struct avrcp *session, const char *name,
 							const char *address)
 {
diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
index 7955d56..1a135cc 100644
--- a/android/avrcp-lib.h
+++ b/android/avrcp-lib.h
@@ -21,11 +21,48 @@
  *
  */
 
+/* Control PDU ids */
+#define AVRCP_GET_CAPABILITIES		0x10
+#define AVRCP_LIST_PLAYER_ATTRIBUTES	0X11
+#define AVRCP_LIST_PLAYER_VALUES	0x12
+#define AVRCP_GET_CURRENT_PLAYER_VALUE	0x13
+#define AVRCP_SET_PLAYER_VALUE		0x14
+#define AVRCP_GET_PLAYER_ATTRIBUTE_TEXT	0x15
+#define AVRCP_GET_PLAYER_VALUE_TEXT	0x16
+#define AVRCP_DISPLAYABLE_CHARSET	0x17
+#define AVRCP_CT_BATTERY_STATUS		0x18
+#define AVRCP_GET_ELEMENT_ATTRIBUTES	0x20
+#define AVRCP_GET_PLAY_STATUS		0x30
+#define AVRCP_REGISTER_NOTIFICATION	0x31
+#define AVRCP_REQUEST_CONTINUING	0x40
+#define AVRCP_ABORT_CONTINUING		0x41
+#define AVRCP_SET_ABSOLUTE_VOLUME	0x50
+#define AVRCP_SET_BROWSED_PLAYER	0x70
+#define AVRCP_GET_FOLDER_ITEMS		0x71
+#define AVRCP_CHANGE_PATH		0x72
+#define AVRCP_GET_ITEM_ATTRIBUTES	0x73
+#define AVRCP_PLAY_ITEM			0x74
+#define AVRCP_SEARCH			0x80
+#define AVRCP_ADD_TO_NOW_PLAYING	0x90
+#define AVRCP_GENERAL_REJECT		0xA0
+
+struct avrcp;
+
+struct avrcp_control_handler {
+	uint8_t id;
+	uint8_t code;
+	uint8_t (*func) (struct avrcp *session, uint8_t transaction,
+			uint16_t *params_len, uint8_t *params, void *user_data);
+};
+
 typedef void (*avrcp_destroy_cb_t) (void *user_data);
 
 struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version);
 void avrcp_shutdown(struct avrcp *session);
 void avrcp_set_destroy_cb(struct avrcp *session, avrcp_destroy_cb_t cb,
 							void *user_data);
+void avrcp_set_control_handlers(struct avrcp *session,
+				const struct avrcp_control_handler *handlers,
+				void *user_data);
 int avrcp_init_uinput(struct avrcp *session, const char *name,
 							const char *address);
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH BlueZ 3/3] android/avrcp-lib: Add avrcp_set_passthrough_handlers
From: Luiz Augusto von Dentz @ 2014-02-20 14:40 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1392907255-29758-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds avrcp_set_passthrough_handlers which can be used to set
pass-through PDU handlers table and user data.
---
 android/avrcp-lib.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 android/avrcp-lib.h |  8 ++++++++
 2 files changed, 52 insertions(+)

diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index c508516..d46e3d3 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -98,6 +98,10 @@ struct avrcp {
 	const struct avrcp_control_handler *control_handlers;
 	void *control_data;
 	unsigned int control_id;
+
+	const struct avrcp_passthrough_handler *passthrough_handlers;
+	void *passthrough_data;
+	unsigned int passthrough_id;
 };
 
 void avrcp_shutdown(struct avrcp *session)
@@ -106,6 +110,9 @@ void avrcp_shutdown(struct avrcp *session)
 		if (session->control_id > 0)
 			avctp_unregister_pdu_handler(session->conn,
 							session->control_id);
+		if (session->passthrough_id > 0)
+			avctp_unregister_passthrough_handler(session->conn,
+						session->passthrough_id);
 		avctp_shutdown(session->conn);
 	}
 
@@ -168,6 +175,31 @@ reject:
 	return AVRCP_HEADER_LENGTH + 1;
 }
 
+static bool handle_passthrough_pdu(struct avctp *conn, uint8_t op,
+						bool pressed, void *user_data)
+{
+	struct avrcp *session = user_data;
+	const struct avrcp_passthrough_handler *handler;
+
+	if (!session->passthrough_handlers)
+		return false;
+
+	for (handler = session->passthrough_handlers; handler->func;
+								handler++) {
+		if (handler->op == op)
+			break;
+	}
+
+	if (handler->func == NULL)
+		return false;
+
+	/* Do not trigger handler on release */
+	if (!pressed)
+		return true;
+
+	return handler->func(session);
+}
+
 struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 {
 	struct avrcp *session;
@@ -180,6 +212,10 @@ struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 		return NULL;
 	}
 
+	session->passthrough_id = avctp_register_passthrough_handler(
+							session->conn,
+							handle_passthrough_pdu,
+							session);
 	session->control_id = avctp_register_pdu_handler(session->conn,
 							AVC_OP_VENDORDEP,
 							handle_vendordep_pdu,
@@ -202,6 +238,14 @@ void avrcp_set_control_handlers(struct avrcp *session,
 	session->control_data = user_data;
 }
 
+void avrcp_set_passthrough_handlers(struct avrcp *session,
+			const struct avrcp_passthrough_handler *handlers,
+			void *user_data)
+{
+	session->passthrough_handlers = handlers;
+	session->passthrough_data = user_data;
+}
+
 int avrcp_init_uinput(struct avrcp *session, const char *name,
 							const char *address)
 {
diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
index 1a135cc..2337429 100644
--- a/android/avrcp-lib.h
+++ b/android/avrcp-lib.h
@@ -55,6 +55,11 @@ struct avrcp_control_handler {
 			uint16_t *params_len, uint8_t *params, void *user_data);
 };
 
+struct avrcp_passthrough_handler {
+	uint8_t op;
+	bool (*func) (struct avrcp *session);
+};
+
 typedef void (*avrcp_destroy_cb_t) (void *user_data);
 
 struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version);
@@ -64,5 +69,8 @@ void avrcp_set_destroy_cb(struct avrcp *session, avrcp_destroy_cb_t cb,
 void avrcp_set_control_handlers(struct avrcp *session,
 				const struct avrcp_control_handler *handlers,
 				void *user_data);
+void avrcp_set_passthrough_handlers(struct avrcp *session,
+			const struct avrcp_passthrough_handler *handlers,
+			void *user_data);
 int avrcp_init_uinput(struct avrcp *session, const char *name,
 							const char *address);
-- 
1.8.5.3


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox