Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH 5/6] android/hal: Add device state changed event handler
From: Andrei Emeltchenko @ 2013-10-29 12:36 UTC (permalink / raw)
  To: Jakub Tyszkowski; +Cc: linux-bluetooth
In-Reply-To: <1383049016-23371-5-git-send-email-jakub.tyszkowski@tieto.com>

Hi Jakub,

On Tue, Oct 29, 2013 at 01:16:55PM +0100, Jakub Tyszkowski wrote:
> This is used to report property change of already reported remote
> device.
> 
> ---
>  android/hal-bluetooth.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> index 261ae85..0fef680 100644
> --- a/android/hal-bluetooth.c
> +++ b/android/hal-bluetooth.c
> @@ -114,6 +114,29 @@ static void handle_device_found(void *buf)
>  	bt_hal_cbacks->device_found_cb(ev->num_props, send_props);
>  }
>  
> +static void handle_device_state_changed(void *buf)
> +{
> +	uint8_t i;

I've got comment myself that first we put variables with assignments.

> +	struct hal_ev_remote_device_props *ev = buf;
> +	bt_property_t send_props[ev->num_props];
> +	struct hal_property *prop = ev->props;
> +
> +	if (!bt_hal_cbacks->remote_device_properties_cb)
> +		return;
> +
> +	/* repack props */
> +	for (i = 0; i < ev->num_props; ++i) {
> +		send_props[i].type = prop->type;
> +		send_props[i].len = prop->len;
> +		send_props[i].val = prop->val;
> +
> +		prop = (void *) prop + (sizeof(*prop) + prop->len);
> +	}

I would put empty line here. This looks a bit more readable then
handle_adapter_props_changed. Do you think those 2 might have reused code?

Best regards 
Andrei Emeltchenko 


> +	bt_hal_cbacks->remote_device_properties_cb(ev->status,
> +						(bt_bdaddr_t *)ev->bdaddr,
> +						ev->num_props, send_props);
> +}
> +
>  /* will be called from notification thread context */
>  void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
>  {
> @@ -133,6 +156,9 @@ void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
>  	case HAL_EV_DEVICE_FOUND:
>  		handle_device_found(buf);
>  		break;
> +	case HAL_EV_REMOTE_DEVICE_PROPS:
> +		handle_device_state_changed(buf);
> +		break;
>  	default:
>  		DBG("Unhandled callback opcode=0x%x", opcode);
>  		break;
> -- 
> 1.8.4.1
> 
> --
> 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: [RFC] Bluetooth: Set ISOC altsetting from within notify callback
From: Timo Müller @ 2013-10-29 12:39 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <525BB77A.7030803@timomueller.eu>

Hi Marcel,

Timo Mueller wrote, On 14.10.2013 11:20:
> Hi Marcel,
>
> Am 11.10.2013 14:36, schrieb Marcel Holtmann:
>> Hi Timo,
>>
>>>> Since the event handling is done within a workqueue, the notify
>>>> callback can now sleep. So no need to trigger a separate workqueue
>>>> from within the Bluetooth USB driver.
>>>>
>>>> This should give a little bit better latency with the SCO packet
>>>> processing since the ISOC altsetting is correct from the beginning.
>>>>
>>>> However I am not sure if we can actually sleep in the USB reset
>>>> handler and what we need to do to restore the correct altsetting
>>>> in there. This could potentially fail, so please test ;)
>>>>
>>>> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
>>>> ---
>>>>   drivers/bluetooth/btusb.c | 34 ++++++++++++++--------------------
>>>>   1 file changed, 14 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>>>> index f3dfc0a..32cae73 100644
>>>> --- a/drivers/bluetooth/btusb.c
>>>> +++ b/drivers/bluetooth/btusb.c
>>>> @@ -245,7 +245,6 @@ struct btusb_data {
>>>>
>>>>       unsigned long flags;
>>>>
>>>> -    struct work_struct work;
>>>>       struct work_struct waker;
>>>>
>>>>       struct usb_anchor tx_anchor;
>>>> @@ -685,7 +684,6 @@ static int btusb_close(struct hci_dev *hdev)
>>>>       if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
>>>>           return 0;
>>>>
>>>> -    cancel_work_sync(&data->work);
>>>>       cancel_work_sync(&data->waker);
>>>>
>>>>       clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
>>>> @@ -827,18 +825,6 @@ done:
>>>>       return err;
>>>>   }
>>>>
>>>> -static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
>>>> -{
>>>> -    struct btusb_data *data = hci_get_drvdata(hdev);
>>>> -
>>>> -    BT_DBG("%s evt %d", hdev->name, evt);
>>>> -
>>>> -    if (hdev->conn_hash.sco_num != data->sco_num) {
>>>> -        data->sco_num = hdev->conn_hash.sco_num;
>>>> -        schedule_work(&data->work);
>>>> -    }
>>>> -}
>>>> -
>>>>   static inline int __set_isoc_interface(struct hci_dev *hdev, int
>>>> altsetting)
>>>>   {
>>>>       struct btusb_data *data = hci_get_drvdata(hdev);
>>>> @@ -882,9 +868,8 @@ static inline int __set_isoc_interface(struct
>>>> hci_dev *hdev, int altsetting)
>>>>       return 0;
>>>>   }
>>>>
>>>> -static void btusb_work(struct work_struct *work)
>>>> +static void btusb_update_isoc_altsetting(struct btusb_data *data)
>>>>   {
>>>> -    struct btusb_data *data = container_of(work, struct btusb_data,
>>>> work);
>>>>       struct hci_dev *hdev = data->hdev;
>>>>       int new_alts;
>>>>       int err;
>>>> @@ -932,6 +917,18 @@ static void btusb_work(struct work_struct *work)
>>>>       }
>>>>   }
>>>>
>>>> +static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
>>>> +{
>>>> +    struct btusb_data *data = hci_get_drvdata(hdev);
>>>> +
>>>> +    BT_DBG("%s evt %d", hdev->name, evt);
>>>> +
>>>> +    if (hdev->conn_hash.sco_num != data->sco_num) {
>>>> +        data->sco_num = hdev->conn_hash.sco_num;
>>>> +        btusb_update_isoc_altsetting(data);
>>>> +    }
>>>> +}
>>>> +
>>>>   static void btusb_waker(struct work_struct *work)
>>>>   {
>>>>       struct btusb_data *data = container_of(work, struct
>>>> btusb_data, waker);
>>>> @@ -1404,7 +1401,6 @@ static int btusb_probe(struct usb_interface
>>>> *intf,
>>>>
>>>>       spin_lock_init(&data->lock);
>>>>
>>>> -    INIT_WORK(&data->work, btusb_work);
>>>>       INIT_WORK(&data->waker, btusb_waker);
>>>>       spin_lock_init(&data->txlock);
>>>>
>>>> @@ -1540,8 +1536,6 @@ static int btusb_suspend(struct usb_interface
>>>> *intf, pm_message_t message)
>>>>           return -EBUSY;
>>>>       }
>>>>
>>>> -    cancel_work_sync(&data->work);
>>>> -
>>>>       btusb_stop_traffic(data);
>>>>       usb_kill_anchored_urbs(&data->tx_anchor);
>>>>
>>>> @@ -1606,8 +1600,8 @@ static int btusb_resume(struct usb_interface
>>>> *intf)
>>>>       play_deferred(data);
>>>>       clear_bit(BTUSB_SUSPENDING, &data->flags);
>>>>       spin_unlock_irq(&data->txlock);
>>>> -    schedule_work(&data->work);
>>>>
>>>> +    btusb_update_isoc_altsetting(data);
>>>>       return 0;
>>>>
>>>>   failed:
>>>>
>>> I have been testing this patch for the last two days at the UPF in
>>> Vienna. It was running fine most of the time, but I experienced two
>>> crashes. Both crashes appeared when there was an active call and the
>>> phone transferred audio to the phone and back. Both times I wasn't
>>> able to reproduce, when I restarted everything and tested again it
>>> worked fine.
>>>
>>> Unfortunately the kernel log is not complete but, when it failed the
>>> kernel reported:
>>> [147.344546] Bluetooth: hci0 SCO packet for unknown connection handle 5
>>> [147.354515] Bluetooth: hci0 SCO packet for unknown connection handle 21
>>> [147.354537] Bluetooth: hci0 SCO packet for unknown connection handle 29
>>> [147.354548] Bluetooth: hci0 SCO packet for unknown connection handle
>>> 65534
>>> [147.364574] Bluetooth: hci0 SCO packet for unknown connection handle
>>> 65532
>>> [147.364581] Bluetooth: hci0 SCO packet for unknown connection handle 27
>>> …
>> what kind of hardware where you testing with?
>
> It was nothing special, an off-the-shelf CSR USB BT Dongle.
>
>>
>> This handle mismatch normally means that our SCO packet frames are out
>> of sync. I think we are not doing a good job trying to keep them
>> nicely lined up.
>>
>> For the crash, do you happen to have a backtrace of the crash.
>> Personally I was worried about the reset handling and not the actual
>> alternate setting switching.
>
> Unfortunately I haven't. To be precise the first crash wasn't even a
> crash, as everything was continuing to run, but without a working audio
> of course. The second time my system completely froze, but due to my
> fault I wasn't able to pull the backtrace. I'll see if I can reproduce
> this week and send a backtrace if I manage to.

I've been unsuccessfully trying to reproduce the system crash with an 
iPhone 3 and a Nexus 4. Routing the Audio to the phone and back 
occasionally leads to the observed handle mismatches, but no 
crashes/freezes whatsoever. Sometimes the mismatch is even recovered.

>>
>> Regards
>>
>> Marcel
>
> Regards,
> Timo

Regards,
Timo

^ permalink raw reply

* Re: [PATCH v3 4/5] android/hal: Add support for handling pin request event
From: Szymon Janc @ 2013-10-29 12:40 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <20131029121945.GA1008@x220.p-661hnu-f1>

Hi Johan,

> Hi Szymon,
> 
> On Tue, Oct 29, 2013, Szymon Janc wrote:
> > ---
> >  android/hal-bluetooth.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> 
> I've applied the first three patches in this set, but are you sure the
> following isn't prone to unaligned access issues:
> 
> > +	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
> > +	bt_bdname_t *name = (bt_bdname_t *) ev->name;
> 
> I.e. wouldn't this be an issue if the expected alignment of bt_bdaddr_t
> and bt_bdname_t pointers is different from the alignment of ev->bdaddr
> and ev->name.

bt_bdaddr_t and bt_bdname_t are declared as packed structures so this is safe.
I'll add comment in v4 as suggested.

-- 
BR
Szymon Janc


^ permalink raw reply

* [PATCH] android: Fix build errors
From: Andrei Emeltchenko @ 2013-10-29 12:48 UTC (permalink / raw)
  To: linux-bluetooth

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

The patch fixes following issues when building:

Make links to sco.h and rfcomm.h needed for Android sockets.
...
btio.c:39:30: fatal error: bluetooth/rfcomm.h: No such file or directory
compilation terminated.
...
btio.c:40:27: fatal error: bluetooth/sco.h: No such file or directory
compilation terminated.
...
---
 android/Android.mk |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/android/Android.mk b/android/Android.mk
index 28ec465..d8f9d0d 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -52,6 +52,8 @@ lib_headers := \
 	l2cap.h \
 	sdp_lib.h \
 	sdp.h \
+	rfcomm.h \
+	sco.h \
 
 $(shell mkdir -p $(LOCAL_PATH)/../lib/bluetooth)
 
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH v4 1/2] android/hal: Add support for handling pin request event
From: Szymon Janc @ 2013-10-29 12:49 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

---
v4: Added comment clarifying that pointer casting is safe here

 android/hal-bluetooth.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index a3f5038..230f742 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -78,6 +78,17 @@ static void handle_bond_state_change(void *buf)
 								ev->state);
 }
 
+static void handle_pin_request(void *buf)
+{
+	struct hal_ev_pin_request *ev = buf;
+	/* Those are declared as packed, so it's safe to assign pointers */
+	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
+	bt_bdname_t *name = (bt_bdname_t *) ev->name;
+
+	if (bt_hal_cbacks->pin_request_cb)
+		bt_hal_cbacks->pin_request_cb(addr, name, ev->class_of_dev);
+}
+
 void bt_thread_associate(void)
 {
 	if (bt_hal_cbacks->thread_evt_cb)
@@ -111,6 +122,9 @@ void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
 	case HAL_EV_BOND_STATE_CHANGED:
 		handle_bond_state_change(buf);
 		break;
+	case HAL_EV_PIN_REQUEST:
+		handle_pin_request(buf);
+		break;
 	default:
 		DBG("Unhandled callback opcode=0x%x", opcode);
 		break;
-- 
1.8.4.1


^ permalink raw reply related

* [PATCH v4 2/2] android/hal: Add support for handling SSP request event
From: Szymon Janc @ 2013-10-29 12:49 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1383050945-7984-1-git-send-email-szymon.janc@tieto.com>

---
v4: Added comment clarifying that pointer casting is safe here

 android/hal-bluetooth.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 230f742..ffaf8da 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -89,6 +89,19 @@ static void handle_pin_request(void *buf)
 		bt_hal_cbacks->pin_request_cb(addr, name, ev->class_of_dev);
 }
 
+static void handle_ssp_request(void *buf)
+{
+	struct hal_ev_ssp_request *ev = buf;
+	/* Those are declared as packed, so it's safe to assign pointers */
+	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
+	bt_bdname_t *name = (bt_bdname_t *) ev->name;
+
+	if (bt_hal_cbacks->ssp_request_cb)
+		bt_hal_cbacks->ssp_request_cb(addr, name, ev->class_of_dev,
+							ev->pairing_variant,
+							ev->passkey);
+}
+
 void bt_thread_associate(void)
 {
 	if (bt_hal_cbacks->thread_evt_cb)
@@ -125,6 +138,9 @@ void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
 	case HAL_EV_PIN_REQUEST:
 		handle_pin_request(buf);
 		break;
+	case HAL_EV_SSP_REQUEST:
+		handle_ssp_request(buf);
+		break;
 	default:
 		DBG("Unhandled callback opcode=0x%x", opcode);
 		break;
-- 
1.8.4.1


^ permalink raw reply related

* Re: [RFC] Bluetooth: Set ISOC altsetting from within notify callback
From: Marcel Holtmann @ 2013-10-29 12:53 UTC (permalink / raw)
  To: Timo Müller; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <526FAC6C.1090306@timomueller.eu>

Hi Timo,

>>>>> Since the event handling is done within a workqueue, the notify
>>>>> callback can now sleep. So no need to trigger a separate workqueue
>>>>> from within the Bluetooth USB driver.
>>>>> 
>>>>> This should give a little bit better latency with the SCO packet
>>>>> processing since the ISOC altsetting is correct from the beginning.
>>>>> 
>>>>> However I am not sure if we can actually sleep in the USB reset
>>>>> handler and what we need to do to restore the correct altsetting
>>>>> in there. This could potentially fail, so please test ;)
>>>>> 
>>>>> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
>>>>> ---
>>>>>  drivers/bluetooth/btusb.c | 34 ++++++++++++++--------------------
>>>>>  1 file changed, 14 insertions(+), 20 deletions(-)
>>>>> 
>>>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>>>>> index f3dfc0a..32cae73 100644
>>>>> --- a/drivers/bluetooth/btusb.c
>>>>> +++ b/drivers/bluetooth/btusb.c
>>>>> @@ -245,7 +245,6 @@ struct btusb_data {
>>>>> 
>>>>>      unsigned long flags;
>>>>> 
>>>>> -    struct work_struct work;
>>>>>      struct work_struct waker;
>>>>> 
>>>>>      struct usb_anchor tx_anchor;
>>>>> @@ -685,7 +684,6 @@ static int btusb_close(struct hci_dev *hdev)
>>>>>      if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
>>>>>          return 0;
>>>>> 
>>>>> -    cancel_work_sync(&data->work);
>>>>>      cancel_work_sync(&data->waker);
>>>>> 
>>>>>      clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
>>>>> @@ -827,18 +825,6 @@ done:
>>>>>      return err;
>>>>>  }
>>>>> 
>>>>> -static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
>>>>> -{
>>>>> -    struct btusb_data *data = hci_get_drvdata(hdev);
>>>>> -
>>>>> -    BT_DBG("%s evt %d", hdev->name, evt);
>>>>> -
>>>>> -    if (hdev->conn_hash.sco_num != data->sco_num) {
>>>>> -        data->sco_num = hdev->conn_hash.sco_num;
>>>>> -        schedule_work(&data->work);
>>>>> -    }
>>>>> -}
>>>>> -
>>>>>  static inline int __set_isoc_interface(struct hci_dev *hdev, int
>>>>> altsetting)
>>>>>  {
>>>>>      struct btusb_data *data = hci_get_drvdata(hdev);
>>>>> @@ -882,9 +868,8 @@ static inline int __set_isoc_interface(struct
>>>>> hci_dev *hdev, int altsetting)
>>>>>      return 0;
>>>>>  }
>>>>> 
>>>>> -static void btusb_work(struct work_struct *work)
>>>>> +static void btusb_update_isoc_altsetting(struct btusb_data *data)
>>>>>  {
>>>>> -    struct btusb_data *data = container_of(work, struct btusb_data,
>>>>> work);
>>>>>      struct hci_dev *hdev = data->hdev;
>>>>>      int new_alts;
>>>>>      int err;
>>>>> @@ -932,6 +917,18 @@ static void btusb_work(struct work_struct *work)
>>>>>      }
>>>>>  }
>>>>> 
>>>>> +static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
>>>>> +{
>>>>> +    struct btusb_data *data = hci_get_drvdata(hdev);
>>>>> +
>>>>> +    BT_DBG("%s evt %d", hdev->name, evt);
>>>>> +
>>>>> +    if (hdev->conn_hash.sco_num != data->sco_num) {
>>>>> +        data->sco_num = hdev->conn_hash.sco_num;
>>>>> +        btusb_update_isoc_altsetting(data);
>>>>> +    }
>>>>> +}
>>>>> +
>>>>>  static void btusb_waker(struct work_struct *work)
>>>>>  {
>>>>>      struct btusb_data *data = container_of(work, struct
>>>>> btusb_data, waker);
>>>>> @@ -1404,7 +1401,6 @@ static int btusb_probe(struct usb_interface
>>>>> *intf,
>>>>> 
>>>>>      spin_lock_init(&data->lock);
>>>>> 
>>>>> -    INIT_WORK(&data->work, btusb_work);
>>>>>      INIT_WORK(&data->waker, btusb_waker);
>>>>>      spin_lock_init(&data->txlock);
>>>>> 
>>>>> @@ -1540,8 +1536,6 @@ static int btusb_suspend(struct usb_interface
>>>>> *intf, pm_message_t message)
>>>>>          return -EBUSY;
>>>>>      }
>>>>> 
>>>>> -    cancel_work_sync(&data->work);
>>>>> -
>>>>>      btusb_stop_traffic(data);
>>>>>      usb_kill_anchored_urbs(&data->tx_anchor);
>>>>> 
>>>>> @@ -1606,8 +1600,8 @@ static int btusb_resume(struct usb_interface
>>>>> *intf)
>>>>>      play_deferred(data);
>>>>>      clear_bit(BTUSB_SUSPENDING, &data->flags);
>>>>>      spin_unlock_irq(&data->txlock);
>>>>> -    schedule_work(&data->work);
>>>>> 
>>>>> +    btusb_update_isoc_altsetting(data);
>>>>>      return 0;
>>>>> 
>>>>>  failed:
>>>>> 
>>>> I have been testing this patch for the last two days at the UPF in
>>>> Vienna. It was running fine most of the time, but I experienced two
>>>> crashes. Both crashes appeared when there was an active call and the
>>>> phone transferred audio to the phone and back. Both times I wasn't
>>>> able to reproduce, when I restarted everything and tested again it
>>>> worked fine.
>>>> 
>>>> Unfortunately the kernel log is not complete but, when it failed the
>>>> kernel reported:
>>>> [147.344546] Bluetooth: hci0 SCO packet for unknown connection handle 5
>>>> [147.354515] Bluetooth: hci0 SCO packet for unknown connection handle 21
>>>> [147.354537] Bluetooth: hci0 SCO packet for unknown connection handle 29
>>>> [147.354548] Bluetooth: hci0 SCO packet for unknown connection handle
>>>> 65534
>>>> [147.364574] Bluetooth: hci0 SCO packet for unknown connection handle
>>>> 65532
>>>> [147.364581] Bluetooth: hci0 SCO packet for unknown connection handle 27
>>>> …
>>> what kind of hardware where you testing with?
>> 
>> It was nothing special, an off-the-shelf CSR USB BT Dongle.
>> 
>>> 
>>> This handle mismatch normally means that our SCO packet frames are out
>>> of sync. I think we are not doing a good job trying to keep them
>>> nicely lined up.
>>> 
>>> For the crash, do you happen to have a backtrace of the crash.
>>> Personally I was worried about the reset handling and not the actual
>>> alternate setting switching.
>> 
>> Unfortunately I haven't. To be precise the first crash wasn't even a
>> crash, as everything was continuing to run, but without a working audio
>> of course. The second time my system completely froze, but due to my
>> fault I wasn't able to pull the backtrace. I'll see if I can reproduce
>> this week and send a backtrace if I manage to.
> 
> I've been unsuccessfully trying to reproduce the system crash with an iPhone 3 and a Nexus 4. Routing the Audio to the phone and back occasionally leads to the observed handle mismatches, but no crashes/freezes whatsoever. Sometimes the mismatch is even recovered.

we could start more than 2 ISOC URBs and see if that improves the behavior. However at the end of the day, we might just have to deal with the fact that with ISOC URBs, we sometimes loose a few bytes. So btusb.ko needs to re-sync by itself.

My wild guess is that HFP 1.5 in most of the cases we just got lucky that the stream of PCM data is pretty constant. And once throwing HFP 1.6 and mSBC into the mix these problem manifest itself.

Then again, my main concern with this patch is the reset and resume handling. I have no idea if the URB allocation is correct there.

One other thing we could be doing is that we tell the driver first to switch the alternate endpoints before even establishing the eSCO connection. That might bring this a lot better into sync as well. We would still then need to filter out the packets for unknown handles since their might be some. One problem however might be that we not always know ahead of time what kind of voice setting we actually get.

Regards

Marcel


^ permalink raw reply

* [PATCH v2] android: Add README file with instructions
From: Szymon Janc @ 2013-10-29 12:55 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This file cotains help on how BlueZ for Android should be build, run
and test. Some hints and examples on how BlueZ can be intergrated into
Android are present as well.
---
V2:
 - rebased against master
 - clarify requirements on Android init system  

 android/Makefile.am |  2 +-
 android/README      | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 android/README

diff --git a/android/Makefile.am b/android/Makefile.am
index 22002be..5690e93 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -96,7 +96,7 @@ EXTRA_DIST += android/client/terminal.c \
 		android/client/history.h \
 		android/client/terminal.h
 
-EXTRA_DIST += android/hal-ipc-api.txt
+EXTRA_DIST += android/hal-ipc-api.txt android/README
 
 EXTRA_DIST += android/hardware/bluetooth.h \
 		android/hardware/bt_av.h \
diff --git a/android/README b/android/README
new file mode 100644
index 0000000..ada2c2f
--- /dev/null
+++ b/android/README
@@ -0,0 +1,87 @@
+BlueZ for Android
+*****************
+
+Since Android 4.2 there exists a well standardized HAL interface that the
+Bluetooth stack is expected to provide and which enables the easy replacement
+of the stack of choice on Android. Android BlueZ is intended as a drop-in
+replacement to Android provided Bluetooth stack.
+
+More details about BlueZ for Android architecture and components can be found
+in android/hal-apc-api.txt file.
+
+===============================
+Building and running on Android
+===============================
+
+Build requirements
+==================
+
+- GLib - Android 4.2 or later don't provide GLib and one must provide it in
+'external/bluetooth/glib' folder of Android tree. Sample Android GLib port
+is available at https://code.google.com/p/android-bluez.glib/
+
+- Bionic support - BlueZ requires signalfd and timerfd APIs to be provided
+by libc library. Currently only 'master' branch available at
+https://android.googlesource.com/platform/bionic provides all required
+functionality and running BlueZ on older branch requires backporting missing
+features. Sample Bionic for Android on Intel Architecture (Android-IA) with all
+required features backported is available at
+https://code.google.com/p/android-bluez.bionic/
+
+Runtime requirements
+====================
+
+BlueZ HAL library requires 'bluetoothd' service to be available on Android
+system. This can be done by defining service in init.rc file of targeted board:
+
+service bluetoothd logwrapper /system/bin/bluetoothd
+  class main
+  group bluetooth net_bt_stack
+  disabled
+  oneshot
+
+It is required that bluetooth user could start and stop bluetoothd service by
+setting 'ctl.start' or 'ctl.stop' property.
+
+Required Android init system modifications can be found at
+https://code.google.com/p/android-bluez.system-core/
+
+Downloading and building
+========================
+
+Building for Android requires full Android AOSP source tree. Sample Android-IA
+tree with all required components present is available at
+http://code.google.com/p/android-bluez/
+
+Downloading:
+repo init -u https://code.google.com/p/android-bluez.manifest/ -m topics/bluez
+repo sync
+
+Build for Intel ultrabook:
+'source build/envsetup.sh'
+'lunch core_mesa-eng'
+'make allimages -j8'
+
+After full build is done it is possible to rebuild only BlueZ:
+'cd external/bluetooth/bluez/android/'
+'mm' (or 'mm -B' to force rebuilding of all files)
+'adb sync' to update target device.
+
+=============================
+Building and running on Linux
+=============================
+It is possible to build and test BlueZ for Android daemon on Linux (eg. PC).
+Simply follow instructions available at README file in BlueZ top directory.
+Android daemon binary is located at android/bluetoothd.
+
+=======
+Testing
+=======
+
+BT HAL test tools located in android/haltest is provided for HAL level testing
+of both Android daemon and HAL library. Start it and type 'adapter init' in
+prompt to initialize HAL library. On Android required bluetoothd service will
+be started automatically. On Linux it is required to start android/bluetoothd
+manually before init command timeout. To deinitialize HAL library and stop
+daemon type 'adapter cleanup'. Type 'help' for more information. Tab completion
+is also supported.
\ No newline at end of file
-- 
1.8.4.1


^ permalink raw reply related

* [PATCHv3 0/9] Improve logging for Android
From: Andrei Emeltchenko @ 2013-10-29 12:57 UTC (permalink / raw)
  To: linux-bluetooth

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

Logging on Android is really important since we do not have specification
how and in which order hal functions are called, sometimes the only way is
to examine logs or read Java Bluetooth code.

Changes:
	* v3: Merged first two patches.
	* v2: Added thread-safe helpers for printing properties, bdaddr, etc
	after comments that simple printing is not thread-safe. The idea is to
	use TLS (thread local storage) like bionic is doing for strerror for
	example. More info can be found on manpage for pthread_key_create.

This patch series uses debug functions defined already for haltest and
allows to print very helpful logs on Android target like shown below:

...
hal-bluetooth.c:set_adapter_property() prop: type=BT_PROPERTY_ADAPTER_SCAN_MODE len=4 val=BT_SCAN_MODE_NONE
...

Andrei Emeltchenko (9):
  android/haltest: Export print property
  android/haltest: Use pointer as parameter for debug
  android/hal: Print full property in debug
  android/hal: Add extra logs
  android/hal: Print adapter state
  android/hal: Print adapter property in callback
  android: Add thread-safe helpers
  android: Use thread-safe helpers
  android: Fix build errors

 android/Android.mk        |    2 +
 android/client/if-bt.c    |  112 +------------------------------------------
 android/client/textconv.c |  115 +++++++++++++++++++++++++++++++++++++++++++++
 android/client/textconv.h |    3 ++
 android/hal-bluetooth.c   |   28 ++++++-----
 android/pthread-local.h   |   58 +++++++++++++++++++++++
 6 files changed, 196 insertions(+), 122 deletions(-)
 create mode 100644 android/pthread-local.h

-- 
1.7.10.4


^ permalink raw reply

* [PATCHv3 1/9] android/haltest: Export print property
From: Andrei Emeltchenko @ 2013-10-29 12:57 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383051459-29495-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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

Export property printing debug function.
---
 android/client/if-bt.c    |  110 ---------------------------------------------
 android/client/textconv.c |  110 +++++++++++++++++++++++++++++++++++++++++++++
 android/client/textconv.h |    3 ++
 3 files changed, 113 insertions(+), 110 deletions(-)

diff --git a/android/client/if-bt.c b/android/client/if-bt.c
index a20a7c6..e9edec5 100644
--- a/android/client/if-bt.c
+++ b/android/client/if-bt.c
@@ -29,20 +29,6 @@ const bt_interface_t *if_bluetooth;
 		} \
 	} while (0)
 
-static char *bdaddr2str(const bt_bdaddr_t *bd_addr)
-{
-	static char buf[MAX_ADDR_STR_LEN];
-
-	return bt_bdaddr_t2str(bd_addr, buf);
-}
-
-static char *btuuid2str(const bt_uuid_t *uuid)
-{
-	static char buf[MAX_UUID_STR_LEN];
-
-	return bt_uuid_t2str(uuid, buf);
-}
-
 static bt_scan_mode_t str2btscanmode(const char *str)
 {
 	bt_scan_mode_t v = str2bt_scan_mode_t(str);
@@ -76,102 +62,6 @@ static bt_property_type_t str2btpropertytype(const char *str)
 	return (bt_property_type_t) atoi(str);
 }
 
-static char *btproperty2str(bt_property_t property)
-{
-	static char buf[4096];
-	char *p;
-
-	p = buf + sprintf(buf, "type=%s len=%d val=",
-					bt_property_type_t2str(property.type),
-					property.len);
-
-	switch (property.type) {
-	case BT_PROPERTY_BDNAME:
-	case BT_PROPERTY_REMOTE_FRIENDLY_NAME:
-		sprintf(p, "%*s", property.len,
-					((bt_bdname_t *) property.val)->name);
-		break;
-
-	case BT_PROPERTY_BDADDR:
-		sprintf(p, "%s", bdaddr2str((bt_bdaddr_t *) property.val));
-		break;
-
-	case BT_PROPERTY_CLASS_OF_DEVICE:
-		sprintf(p, "%06x", *((int *) property.val));
-		break;
-
-	case BT_PROPERTY_TYPE_OF_DEVICE:
-		sprintf(p, "%s", bt_device_type_t2str(
-					*((bt_device_type_t *) property.val)));
-		break;
-
-	case BT_PROPERTY_REMOTE_RSSI:
-		sprintf(p, "%d", *((char *) property.val));
-		break;
-
-	case BT_PROPERTY_ADAPTER_SCAN_MODE:
-		sprintf(p, "%s",
-			bt_scan_mode_t2str(*((bt_scan_mode_t *) property.val)));
-		break;
-
-	case BT_PROPERTY_ADAPTER_DISCOVERY_TIMEOUT:
-		sprintf(p, "%d", *((int *) property.val));
-		break;
-
-	case BT_PROPERTY_ADAPTER_BONDED_DEVICES:
-		{
-			int count = property.len / sizeof(bt_bdaddr_t);
-			char *ptr = property.val;
-
-			strcat(p, "{");
-
-			while (count--) {
-				strcat(p, bdaddr2str((bt_bdaddr_t *) ptr));
-				if (count)
-					strcat(p, ", ");
-				ptr += sizeof(bt_bdaddr_t);
-			}
-
-			strcat(p, "}");
-
-		}
-		break;
-
-	case BT_PROPERTY_UUIDS:
-		{
-			int count = property.len / sizeof(bt_uuid_t);
-			char *ptr = property.val;
-
-			strcat(p, "{");
-
-			while (count--) {
-				strcat(p, btuuid2str((bt_uuid_t *) ptr));
-				if (count)
-					strcat(p, ", ");
-				ptr += sizeof(bt_uuid_t);
-			}
-
-			strcat(p, "}");
-
-		}
-		break;
-
-	case BT_PROPERTY_SERVICE_RECORD:
-		{
-			bt_service_record_t *rec = property.val;
-
-			sprintf(p, "{%s, %d, %s}", btuuid2str(&rec->uuid),
-						rec->channel, rec->name);
-		}
-		break;
-
-	default:
-		sprintf(p, "%p", property.val);
-	}
-
-	return buf;
-}
-
 static void dump_properties(int num_properties, bt_property_t *properties)
 {
 	int i;
diff --git a/android/client/textconv.c b/android/client/textconv.c
index 32b1cab..8f27948 100644
--- a/android/client/textconv.c
+++ b/android/client/textconv.c
@@ -226,3 +226,113 @@ const char *enum_one_string(void *v, int i)
 
 	return (i == 0) && (m[0] != 0) ? m : NULL;
 }
+
+char *bdaddr2str(const bt_bdaddr_t *bd_addr)
+{
+	static char buf[MAX_ADDR_STR_LEN];
+
+	return bt_bdaddr_t2str(bd_addr, buf);
+}
+
+static char *btuuid2str(const bt_uuid_t *uuid)
+{
+	static char buf[MAX_UUID_STR_LEN];
+
+	return bt_uuid_t2str(uuid, buf);
+}
+
+char *btproperty2str(bt_property_t property)
+{
+	static char buf[4096];
+	char *p;
+
+	p = buf + sprintf(buf, "type=%s len=%d val=",
+					bt_property_type_t2str(property.type),
+					property.len);
+
+	switch (property.type) {
+	case BT_PROPERTY_BDNAME:
+	case BT_PROPERTY_REMOTE_FRIENDLY_NAME:
+		sprintf(p, "%*s", property.len,
+					((bt_bdname_t *) property.val)->name);
+		break;
+
+	case BT_PROPERTY_BDADDR:
+		sprintf(p, "%s", bdaddr2str((bt_bdaddr_t *) property.val));
+		break;
+
+	case BT_PROPERTY_CLASS_OF_DEVICE:
+		sprintf(p, "%06x", *((int *) property.val));
+		break;
+
+	case BT_PROPERTY_TYPE_OF_DEVICE:
+		sprintf(p, "%s", bt_device_type_t2str(
+					*((bt_device_type_t *) property.val)));
+		break;
+
+	case BT_PROPERTY_REMOTE_RSSI:
+		sprintf(p, "%d", *((char *) property.val));
+		break;
+
+	case BT_PROPERTY_ADAPTER_SCAN_MODE:
+		sprintf(p, "%s",
+			bt_scan_mode_t2str(*((bt_scan_mode_t *) property.val)));
+		break;
+
+	case BT_PROPERTY_ADAPTER_DISCOVERY_TIMEOUT:
+		sprintf(p, "%d", *((int *) property.val));
+		break;
+
+	case BT_PROPERTY_ADAPTER_BONDED_DEVICES:
+		{
+			int count = property.len / sizeof(bt_bdaddr_t);
+			char *ptr = property.val;
+
+			strcat(p, "{");
+
+			while (count--) {
+				strcat(p, bdaddr2str((bt_bdaddr_t *) ptr));
+				if (count)
+					strcat(p, ", ");
+				ptr += sizeof(bt_bdaddr_t);
+			}
+
+			strcat(p, "}");
+
+		}
+		break;
+
+	case BT_PROPERTY_UUIDS:
+		{
+			int count = property.len / sizeof(bt_uuid_t);
+			char *ptr = property.val;
+
+			strcat(p, "{");
+
+			while (count--) {
+				strcat(p, btuuid2str((bt_uuid_t *) ptr));
+				if (count)
+					strcat(p, ", ");
+				ptr += sizeof(bt_uuid_t);
+			}
+
+			strcat(p, "}");
+
+		}
+		break;
+
+	case BT_PROPERTY_SERVICE_RECORD:
+		{
+			bt_service_record_t *rec = property.val;
+
+			sprintf(p, "{%s, %d, %s}", btuuid2str(&rec->uuid),
+						rec->channel, rec->name);
+		}
+		break;
+
+	default:
+		sprintf(p, "%p", property.val);
+	}
+
+	return buf;
+}
diff --git a/android/client/textconv.h b/android/client/textconv.h
index 085b141..89b29c6 100644
--- a/android/client/textconv.h
+++ b/android/client/textconv.h
@@ -107,6 +107,9 @@ void str2bt_bdaddr_t(const char *str, bt_bdaddr_t *bd_addr);
 char *bt_uuid_t2str(const bt_uuid_t *uuid, char *buf);
 void str2bt_uuid_t(const char *str, bt_uuid_t *uuid);
 
+char *btproperty2str(bt_property_t property);
+char *bdaddr2str(const bt_bdaddr_t *bd_addr);
+
 DECINTMAP(bt_status_t);
 DECINTMAP(bt_state_t);
 DECINTMAP(bt_device_type_t);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv3 2/9] android/haltest: Use pointer as parameter for debug
From: Andrei Emeltchenko @ 2013-10-29 12:57 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383051459-29495-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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

Pass structure as pointer. This makes it consistent with the rest of
the code and helps to reuse this function in other parts.
---
 android/client/if-bt.c    |    2 +-
 android/client/textconv.c |   36 ++++++++++++++++++------------------
 android/client/textconv.h |    2 +-
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/android/client/if-bt.c b/android/client/if-bt.c
index e9edec5..cbb828b 100644
--- a/android/client/if-bt.c
+++ b/android/client/if-bt.c
@@ -74,7 +74,7 @@ static void dump_properties(int num_properties, bt_property_t *properties)
 		bt_property_t prop;
 		memcpy(&prop, properties + i, sizeof(prop));
 
-		haltest_info("prop: %s\n", btproperty2str(prop));
+		haltest_info("prop: %s\n", btproperty2str(&prop));
 	}
 }
 
diff --git a/android/client/textconv.c b/android/client/textconv.c
index 8f27948..1dc6ad0 100644
--- a/android/client/textconv.c
+++ b/android/client/textconv.c
@@ -241,52 +241,52 @@ static char *btuuid2str(const bt_uuid_t *uuid)
 	return bt_uuid_t2str(uuid, buf);
 }
 
-char *btproperty2str(bt_property_t property)
+char *btproperty2str(const bt_property_t *property)
 {
 	static char buf[4096];
 	char *p;
 
 	p = buf + sprintf(buf, "type=%s len=%d val=",
-					bt_property_type_t2str(property.type),
-					property.len);
+					bt_property_type_t2str(property->type),
+					property->len);
 
-	switch (property.type) {
+	switch (property->type) {
 	case BT_PROPERTY_BDNAME:
 	case BT_PROPERTY_REMOTE_FRIENDLY_NAME:
-		sprintf(p, "%*s", property.len,
-					((bt_bdname_t *) property.val)->name);
+		sprintf(p, "%*s", property->len,
+					((bt_bdname_t *) property->val)->name);
 		break;
 
 	case BT_PROPERTY_BDADDR:
-		sprintf(p, "%s", bdaddr2str((bt_bdaddr_t *) property.val));
+		sprintf(p, "%s", bdaddr2str((bt_bdaddr_t *) property->val));
 		break;
 
 	case BT_PROPERTY_CLASS_OF_DEVICE:
-		sprintf(p, "%06x", *((int *) property.val));
+		sprintf(p, "%06x", *((int *) property->val));
 		break;
 
 	case BT_PROPERTY_TYPE_OF_DEVICE:
 		sprintf(p, "%s", bt_device_type_t2str(
-					*((bt_device_type_t *) property.val)));
+				*((bt_device_type_t *) property->val)));
 		break;
 
 	case BT_PROPERTY_REMOTE_RSSI:
-		sprintf(p, "%d", *((char *) property.val));
+		sprintf(p, "%d", *((char *) property->val));
 		break;
 
 	case BT_PROPERTY_ADAPTER_SCAN_MODE:
 		sprintf(p, "%s",
-			bt_scan_mode_t2str(*((bt_scan_mode_t *) property.val)));
+			bt_scan_mode_t2str(*((bt_scan_mode_t *) property->val)));
 		break;
 
 	case BT_PROPERTY_ADAPTER_DISCOVERY_TIMEOUT:
-		sprintf(p, "%d", *((int *) property.val));
+		sprintf(p, "%d", *((int *) property->val));
 		break;
 
 	case BT_PROPERTY_ADAPTER_BONDED_DEVICES:
 		{
-			int count = property.len / sizeof(bt_bdaddr_t);
-			char *ptr = property.val;
+			int count = property->len / sizeof(bt_bdaddr_t);
+			char *ptr = property->val;
 
 			strcat(p, "{");
 
@@ -304,8 +304,8 @@ char *btproperty2str(bt_property_t property)
 
 	case BT_PROPERTY_UUIDS:
 		{
-			int count = property.len / sizeof(bt_uuid_t);
-			char *ptr = property.val;
+			int count = property->len / sizeof(bt_uuid_t);
+			char *ptr = property->val;
 
 			strcat(p, "{");
 
@@ -323,7 +323,7 @@ char *btproperty2str(bt_property_t property)
 
 	case BT_PROPERTY_SERVICE_RECORD:
 		{
-			bt_service_record_t *rec = property.val;
+			bt_service_record_t *rec = property->val;
 
 			sprintf(p, "{%s, %d, %s}", btuuid2str(&rec->uuid),
 						rec->channel, rec->name);
@@ -331,7 +331,7 @@ char *btproperty2str(bt_property_t property)
 		break;
 
 	default:
-		sprintf(p, "%p", property.val);
+		sprintf(p, "%p", property->val);
 	}
 
 	return buf;
diff --git a/android/client/textconv.h b/android/client/textconv.h
index 89b29c6..1c848ef 100644
--- a/android/client/textconv.h
+++ b/android/client/textconv.h
@@ -107,7 +107,7 @@ void str2bt_bdaddr_t(const char *str, bt_bdaddr_t *bd_addr);
 char *bt_uuid_t2str(const bt_uuid_t *uuid, char *buf);
 void str2bt_uuid_t(const char *str, bt_uuid_t *uuid);
 
-char *btproperty2str(bt_property_t property);
+char *btproperty2str(const bt_property_t *property);
 char *bdaddr2str(const bt_bdaddr_t *bd_addr);
 
 DECINTMAP(bt_status_t);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv3 3/9] android/hal: Print full property in debug
From: Andrei Emeltchenko @ 2013-10-29 12:57 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383051459-29495-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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

Instead of printing property type print type and value. Use exported
function from hal test tool.
---
 android/hal-bluetooth.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index a3f5038..0b46465 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -241,7 +241,7 @@ static int set_adapter_property(const bt_property_t *property)
 	char buf[sizeof(struct hal_cmd_set_adapter_prop) + property->len];
 	struct hal_cmd_set_adapter_prop *cmd = (void *) buf;
 
-	DBG("prop: %s", bt_property_type_t2str(property->type));
+	DBG("prop: %s", btproperty2str(property));
 
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv3 4/9] android/hal: Add extra logs
From: Andrei Emeltchenko @ 2013-10-29 12:57 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383051459-29495-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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

Add extra log prints for printing properties and bluetooth addresses.
---
 android/hal-bluetooth.c |   22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 0b46465..85ccded 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -266,7 +266,7 @@ static int set_adapter_property(const bt_property_t *property)
 
 static int get_remote_device_properties(bt_bdaddr_t *remote_addr)
 {
-	DBG("");
+	DBG("bdaddr: %s", bdaddr2str(remote_addr));
 
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
@@ -277,7 +277,8 @@ static int get_remote_device_properties(bt_bdaddr_t *remote_addr)
 static int get_remote_device_property(bt_bdaddr_t *remote_addr,
 						bt_property_type_t type)
 {
-	DBG("");
+	DBG("bdaddr: %s prop: %s", bdaddr2str(remote_addr),
+						bt_property_type_t2str(type));
 
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
@@ -288,7 +289,8 @@ static int get_remote_device_property(bt_bdaddr_t *remote_addr,
 static int set_remote_device_property(bt_bdaddr_t *remote_addr,
 						const bt_property_t *property)
 {
-	DBG("");
+	DBG("bdaddr: %s prop: %s", bdaddr2str(remote_addr),
+						btproperty2str(property));
 
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
@@ -298,7 +300,7 @@ static int set_remote_device_property(bt_bdaddr_t *remote_addr,
 
 static int get_remote_service_record(bt_bdaddr_t *remote_addr, bt_uuid_t *uuid)
 {
-	DBG("");
+	DBG("bdaddr: %s", bdaddr2str(remote_addr));
 
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
@@ -308,7 +310,7 @@ static int get_remote_service_record(bt_bdaddr_t *remote_addr, bt_uuid_t *uuid)
 
 static int get_remote_services(bt_bdaddr_t *remote_addr)
 {
-	DBG("");
+	DBG("bdaddr: %s", bdaddr2str(remote_addr));
 
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
@@ -340,7 +342,7 @@ static int create_bond(const bt_bdaddr_t *bd_addr)
 {
 	struct hal_cmd_create_bond cmd;
 
-	DBG("");
+	DBG("bdaddr: %s", bdaddr2str(bd_addr));
 
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
@@ -355,7 +357,7 @@ static int cancel_bond(const bt_bdaddr_t *bd_addr)
 {
 	struct hal_cmd_cancel_bond cmd;
 
-	DBG("");
+	DBG("bdaddr: %s", bdaddr2str(bd_addr));
 
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
@@ -370,7 +372,7 @@ static int remove_bond(const bt_bdaddr_t *bd_addr)
 {
 	struct hal_cmd_remove_bond cmd;
 
-	DBG("");
+	DBG("bdaddr: %s", bdaddr2str(bd_addr));
 
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
@@ -386,7 +388,7 @@ static int pin_reply(const bt_bdaddr_t *bd_addr, uint8_t accept,
 {
 	struct hal_cmd_pin_reply cmd;
 
-	DBG("");
+	DBG("bdaddr: %s", bdaddr2str(bd_addr));
 
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
@@ -405,7 +407,7 @@ static int ssp_reply(const bt_bdaddr_t *bd_addr, bt_ssp_variant_t variant,
 {
 	struct hal_cmd_ssp_reply cmd;
 
-	DBG("");
+	DBG("bdaddr: %s", bdaddr2str(bd_addr));
 
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv3 5/9] android/hal: Print adapter state
From: Andrei Emeltchenko @ 2013-10-29 12:57 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383051459-29495-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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

---
 android/hal-bluetooth.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 85ccded..4d467ee 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -33,6 +33,8 @@ static void handle_adapter_state_changed(void *buf)
 {
 	struct hal_ev_adapter_state_changed *ev = buf;
 
+	DBG("state: %s", bt_state_t2str(ev->state));
+
 	if (bt_hal_cbacks->adapter_state_changed_cb)
 		bt_hal_cbacks->adapter_state_changed_cb(ev->state);
 }
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv3 6/9] android/hal: Print adapter property in callback
From: Andrei Emeltchenko @ 2013-10-29 12:57 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383051459-29495-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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

---
 android/hal-bluetooth.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 4d467ee..cc63bd9 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -65,6 +65,8 @@ static void handle_adapter_props_changed(void *buf, uint16_t len)
 
 		p += sizeof(*hal_prop) + hal_prop->len;
 		hal_prop = p;
+
+		DBG("prop[%d]: %s", i, btproperty2str(&props[i]));
 	}
 
 	bt_hal_cbacks->adapter_properties_cb(ev->status, ev->num_props, props);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv3 7/9] android: Add thread-safe helpers
From: Andrei Emeltchenko @ 2013-10-29 12:57 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383051459-29495-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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

Add thread safe helpers to make HAL debug printing thread-safe. The code
is inherited from Android bionic and it is used for strerror, strsignal,
etc.
---
 android/pthread-local.h |   58 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 android/pthread-local.h

diff --git a/android/pthread-local.h b/android/pthread-local.h
new file mode 100644
index 0000000..bc3c0b3
--- /dev/null
+++ b/android/pthread-local.h
@@ -0,0 +1,58 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ * Copyright (C) 2013 Intel Corp.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *  * Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  * Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <pthread.h>
+#include <stdlib.h>
+
+#define GLOBAL_INIT_THREAD_LOCAL_BUFFER(name) \
+	static pthread_key_t __tls_ ## name ## _key; \
+	static void __tls_ ## name ## _key_destroy(void *buffer) \
+	{ \
+		free(buffer); \
+	} \
+	static void __attribute__((constructor)) __tls_ ## name ## _key_init() \
+	{ \
+		pthread_key_create(&__tls_ ## name ## _key, \
+					__tls_ ## name ## _key_destroy); \
+	}
+
+/*
+ * Leaves "name_tls_buffer" and "name_tls_buffer_size" defined and initialized.
+ */
+#define LOCAL_INIT_THREAD_LOCAL_BUFFER(type, name, byte_count) \
+	const size_t name ## _tls_buffer_size \
+					__attribute__((unused)) = byte_count; \
+	type name ## _tls_buffer = \
+		(pthread_getspecific(__tls_ ## name ## _key)); \
+	if (name ## _tls_buffer == NULL) { \
+		name ## _tls_buffer = (calloc(1, byte_count)); \
+		pthread_setspecific(__tls_ ## name ## _key, \
+							name ## _tls_buffer); \
+	}
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv3 8/9] android: Use thread-safe helpers
From: Andrei Emeltchenko @ 2013-10-29 12:57 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383051459-29495-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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

Make use of thread-safe helpers.
---
 android/client/textconv.c |   15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/android/client/textconv.c b/android/client/textconv.c
index 1dc6ad0..effd1b3 100644
--- a/android/client/textconv.c
+++ b/android/client/textconv.c
@@ -19,6 +19,8 @@
 #include <stdio.h>
 #include <hardware/bluetooth.h>
 
+#include "../pthread-local.h"
+
 #include "textconv.h"
 
 /*
@@ -227,11 +229,12 @@ const char *enum_one_string(void *v, int i)
 	return (i == 0) && (m[0] != 0) ? m : NULL;
 }
 
+GLOBAL_INIT_THREAD_LOCAL_BUFFER(bdaddr);
 char *bdaddr2str(const bt_bdaddr_t *bd_addr)
 {
-	static char buf[MAX_ADDR_STR_LEN];
+	LOCAL_INIT_THREAD_LOCAL_BUFFER(char*, bdaddr, MAX_ADDR_STR_LEN);
 
-	return bt_bdaddr_t2str(bd_addr, buf);
+	return bt_bdaddr_t2str(bd_addr, bdaddr_tls_buffer);
 }
 
 static char *btuuid2str(const bt_uuid_t *uuid)
@@ -241,12 +244,14 @@ static char *btuuid2str(const bt_uuid_t *uuid)
 	return bt_uuid_t2str(uuid, buf);
 }
 
+GLOBAL_INIT_THREAD_LOCAL_BUFFER(property);
 char *btproperty2str(const bt_property_t *property)
 {
-	static char buf[4096];
 	char *p;
+	LOCAL_INIT_THREAD_LOCAL_BUFFER(char*, property, 4096);
 
-	p = buf + sprintf(buf, "type=%s len=%d val=",
+	p = property_tls_buffer + sprintf(property_tls_buffer,
+					"type=%s len=%d val=",
 					bt_property_type_t2str(property->type),
 					property->len);
 
@@ -334,5 +339,5 @@ char *btproperty2str(const bt_property_t *property)
 		sprintf(p, "%p", property->val);
 	}
 
-	return buf;
+	return property_tls_buffer;
 }
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv3 9/9] android: Fix build errors
From: Andrei Emeltchenko @ 2013-10-29 12:57 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383051459-29495-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

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

The patch fixes following issues when building:

Make links to sco.h and rfcomm.h needed for Android sockets.
...
btio.c:39:30: fatal error: bluetooth/rfcomm.h: No such file or directory
compilation terminated.
...
btio.c:40:27: fatal error: bluetooth/sco.h: No such file or directory
compilation terminated.
...
---
 android/Android.mk |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/android/Android.mk b/android/Android.mk
index 28ec465..d8f9d0d 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -52,6 +52,8 @@ lib_headers := \
 	l2cap.h \
 	sdp_lib.h \
 	sdp.h \
+	rfcomm.h \
+	sco.h \
 
 $(shell mkdir -p $(LOCAL_PATH)/../lib/bluetooth)
 
-- 
1.7.10.4


^ permalink raw reply related

* Re: [PATCH 1/6] android: Remove reduntant structure
From: Jakub Tyszkowski @ 2013-10-29 13:01 UTC (permalink / raw)
  To: Jakub Tyszkowski, linux-bluetooth
In-Reply-To: <20131029122827.GA12345@x220.p-661hnu-f1>

Hi Johan,

Please ignore this patch then. I'll fix this or Szymon will.

On 29 October 2013 13:28, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Jakub,
>
> On Tue, Oct 29, 2013, Jakub Tyszkowski wrote:
>> ---
>>  android/adapter.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/android/adapter.c b/android/adapter.c
>> index 15b65e5..7e5c1a1 100644
>> --- a/android/adapter.c
>> +++ b/android/adapter.c
>> @@ -234,12 +234,11 @@ static void load_link_keys(GSList *keys)
>>  {
>>       struct mgmt_cp_load_link_keys *cp;
>>       size_t key_len = g_slist_length(keys);
>> -     struct mgmt_link_key_info *key;
>>       size_t len;
>>
>>       DBG("");
>>
>> -     len = sizeof(*cp) + key_len * sizeof(*key);
>> +     len = sizeof(*cp) + key_len * sizeof(struct mgmt_link_key_info);
>>       cp = g_malloc0(len);
>>
>>       cp->debug_keys = 0;
>
> If the point of the keys list is to contain struct mgmt_link_key_info
> entries I'd rather fix this function to properly fill in the mgmt
> command with those since right now it's broken if the list contains any
> entries at all.
>
> Johan

^ permalink raw reply

* Re: [PATCH] android: Fix build errors
From: Johan Hedberg @ 2013-10-29 13:05 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1383050922-27758-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Tue, Oct 29, 2013, Andrei Emeltchenko wrote:
> The patch fixes following issues when building:
> 
> Make links to sco.h and rfcomm.h needed for Android sockets.
> ...
> btio.c:39:30: fatal error: bluetooth/rfcomm.h: No such file or directory
> compilation terminated.
> ...
> btio.c:40:27: fatal error: bluetooth/sco.h: No such file or directory
> compilation terminated.
> ...
> ---
>  android/Android.mk |    2 ++
>  1 file changed, 2 insertions(+)

Applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH v4 1/2] android/hal: Add support for handling pin request event
From: Johan Hedberg @ 2013-10-29 13:06 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1383050945-7984-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Tue, Oct 29, 2013, Szymon Janc wrote:
> ---
> v4: Added comment clarifying that pointer casting is safe here
> 
>  android/hal-bluetooth.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Both patches have been applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH v2] android: Add README file with instructions
From: Andrei Emeltchenko @ 2013-10-29 13:07 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1383051302-9754-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Tue, Oct 29, 2013 at 01:55:02PM +0100, Szymon Janc wrote:
> This file cotains help on how BlueZ for Android should be build, run
> and test. Some hints and examples on how BlueZ can be intergrated into
> Android are present as well.
> ---
> V2:
>  - rebased against master
>  - clarify requirements on Android init system  
> 
>  android/Makefile.am |  2 +-
>  android/README      | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 88 insertions(+), 1 deletion(-)
>  create mode 100644 android/README
> 
> diff --git a/android/Makefile.am b/android/Makefile.am
> index 22002be..5690e93 100644
> --- a/android/Makefile.am
> +++ b/android/Makefile.am
> @@ -96,7 +96,7 @@ EXTRA_DIST += android/client/terminal.c \
>  		android/client/history.h \
>  		android/client/terminal.h
>  
> -EXTRA_DIST += android/hal-ipc-api.txt
> +EXTRA_DIST += android/hal-ipc-api.txt android/README
>  
>  EXTRA_DIST += android/hardware/bluetooth.h \
>  		android/hardware/bt_av.h \
> diff --git a/android/README b/android/README
> new file mode 100644
> index 0000000..ada2c2f
> --- /dev/null
> +++ b/android/README
> @@ -0,0 +1,87 @@
> +BlueZ for Android
> +*****************
> +
> +Since Android 4.2 there exists a well standardized HAL interface that the
> +Bluetooth stack is expected to provide and which enables the easy replacement
> +of the stack of choice on Android. Android BlueZ is intended as a drop-in
> +replacement to Android provided Bluetooth stack.
> +
> +More details about BlueZ for Android architecture and components can be found
> +in android/hal-apc-api.txt file.
> +
> +===============================
> +Building and running on Android
> +===============================
> +
> +Build requirements
> +==================
> +
> +- GLib - Android 4.2 or later don't provide GLib and one must provide it in
> +'external/bluetooth/glib' folder of Android tree. Sample Android GLib port
> +is available at https://code.google.com/p/android-bluez.glib/
> +
> +- Bionic support - BlueZ requires signalfd and timerfd APIs to be provided
> +by libc library. Currently only 'master' branch available at
> +https://android.googlesource.com/platform/bionic provides all required
> +functionality and running BlueZ on older branch requires backporting missing
> +features. Sample Bionic for Android on Intel Architecture (Android-IA) with all
> +required features backported is available at
> +https://code.google.com/p/android-bluez.bionic/
> +
> +Runtime requirements
> +====================
> +
> +BlueZ HAL library requires 'bluetoothd' service to be available on Android
> +system. This can be done by defining service in init.rc file of targeted board:
> +
> +service bluetoothd logwrapper /system/bin/bluetoothd

logwrapper cannot be found without the full path:

service bluetoothd /system/bin/logwrapper /system/bin/bluetoothd

> +  class main
> +  group bluetooth net_bt_stack

net_bt_stack seems to be bluedroid specific, currently we use

group bluetooth net_admin

otherwise looks good

Best regards 
Andrei Emeltchenko 

> +  disabled
> +  oneshot
> +
> +It is required that bluetooth user could start and stop bluetoothd service by
> +setting 'ctl.start' or 'ctl.stop' property.
> +
> +Required Android init system modifications can be found at
> +https://code.google.com/p/android-bluez.system-core/
> +
> +Downloading and building
> +========================
> +
> +Building for Android requires full Android AOSP source tree. Sample Android-IA
> +tree with all required components present is available at
> +http://code.google.com/p/android-bluez/
> +
> +Downloading:
> +repo init -u https://code.google.com/p/android-bluez.manifest/ -m topics/bluez
> +repo sync
> +
> +Build for Intel ultrabook:
> +'source build/envsetup.sh'
> +'lunch core_mesa-eng'
> +'make allimages -j8'
> +
> +After full build is done it is possible to rebuild only BlueZ:
> +'cd external/bluetooth/bluez/android/'
> +'mm' (or 'mm -B' to force rebuilding of all files)
> +'adb sync' to update target device.
> +
> +=============================
> +Building and running on Linux
> +=============================
> +It is possible to build and test BlueZ for Android daemon on Linux (eg. PC).
> +Simply follow instructions available at README file in BlueZ top directory.
> +Android daemon binary is located at android/bluetoothd.
> +
> +=======
> +Testing
> +=======
> +
> +BT HAL test tools located in android/haltest is provided for HAL level testing
> +of both Android daemon and HAL library. Start it and type 'adapter init' in
> +prompt to initialize HAL library. On Android required bluetoothd service will
> +be started automatically. On Linux it is required to start android/bluetoothd
> +manually before init command timeout. To deinitialize HAL library and stop
> +daemon type 'adapter cleanup'. Type 'help' for more information. Tab completion
> +is also supported.
> \ No newline at end of file
> -- 
> 1.8.4.1
> 
> --
> 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: [PATCH 5/6] android/hal: Add device state changed event handler
From: Jakub Tyszkowski @ 2013-10-29 13:18 UTC (permalink / raw)
  To: Andrei Emeltchenko, Jakub Tyszkowski, linux-bluetooth
In-Reply-To: <20131029123622.GF27517@aemeltch-MOBL1>

Hi Andrei,

I think the props packing part can be extracted and reused. Not sure
if other events will benefit from this.. probably just adapter props
event?
Thanks, I'll fix those.

On 29 October 2013 13:36, Andrei Emeltchenko
<andrei.emeltchenko.news@gmail.com> wrote:
> Hi Jakub,
>
> On Tue, Oct 29, 2013 at 01:16:55PM +0100, Jakub Tyszkowski wrote:
>> This is used to report property change of already reported remote
>> device.
>>
>> ---
>>  android/hal-bluetooth.c | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>
>> diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
>> index 261ae85..0fef680 100644
>> --- a/android/hal-bluetooth.c
>> +++ b/android/hal-bluetooth.c
>> @@ -114,6 +114,29 @@ static void handle_device_found(void *buf)
>>       bt_hal_cbacks->device_found_cb(ev->num_props, send_props);
>>  }
>>
>> +static void handle_device_state_changed(void *buf)
>> +{
>> +     uint8_t i;
>
> I've got comment myself that first we put variables with assignments.
>
>> +     struct hal_ev_remote_device_props *ev = buf;
>> +     bt_property_t send_props[ev->num_props];
>> +     struct hal_property *prop = ev->props;
>> +
>> +     if (!bt_hal_cbacks->remote_device_properties_cb)
>> +             return;
>> +
>> +     /* repack props */
>> +     for (i = 0; i < ev->num_props; ++i) {
>> +             send_props[i].type = prop->type;
>> +             send_props[i].len = prop->len;
>> +             send_props[i].val = prop->val;
>> +
>> +             prop = (void *) prop + (sizeof(*prop) + prop->len);
>> +     }
>
> I would put empty line here. This looks a bit more readable then
> handle_adapter_props_changed. Do you think those 2 might have reused code?
>
> Best regards
> Andrei Emeltchenko
>
>
>> +     bt_hal_cbacks->remote_device_properties_cb(ev->status,
>> +                                             (bt_bdaddr_t *)ev->bdaddr,
>> +                                             ev->num_props, send_props);
>> +}
>> +
>>  /* will be called from notification thread context */
>>  void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
>>  {
>> @@ -133,6 +156,9 @@ void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
>>       case HAL_EV_DEVICE_FOUND:
>>               handle_device_found(buf);
>>               break;
>> +     case HAL_EV_REMOTE_DEVICE_PROPS:
>> +             handle_device_state_changed(buf);
>> +             break;
>>       default:
>>               DBG("Unhandled callback opcode=0x%x", opcode);
>>               break;
>> --
>> 1.8.4.1
>>
>> --
>> 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

* [RFC v2 00/15] LE auto connection and connection parameters
From: Andre Guedes @ 2013-10-29 13:25 UTC (permalink / raw)
  To: linux-bluetooth

Hi all,

Sorry about the delay of this patch set.

This v2 patch set implements Marcel's comments from the previous one. The main
changes are:
* The patch set was reorganized, all refactoring and improving was placed at
  the beginning of the set.
* Minimum and maximum connection interval are saved in hci_conn so we are able
  to know the connection parameters that are currently being used.
* The trigger/untrigger background scanning mechanism was replaced by a list
  of pending auto connection. We keep background scan running as long as we
  have elements in the list.
* New mgmt commands patches were moved to the end of the set.

It was suggested we change connect() behavior to use LE auto connection
infrastructure. To achieve that, we would have to change hci_connect_le() to
add a pending auto connection and hci_conn_timeout() to remove that pending
auto connection. However, this change is more complicated than what it seems.
The trick part is handling connection timeout. To properly handle connection
timeout, we have to delete the hci_conn object inside hci_conn_timeout() but
we cannot do it since since we get a dead lock. The problem is: a delayed work
thread executes hci_conn_timeout() which calls hci_conn_del(). hci_conn_del()
calls cancel_delayed_work_sync() which waits for itself.

Thus, since this change is not required to support LE auto connection and it
would delay even more this v2 patch set, connect() behavior was not changed.
If this change is really worth, we can do it in a separate patch set.

This patch set has been extensively tested with dongles that don't support
scanning and connection at the same time (e.g. PTS dongle) as well as with
dongles that support it. For testing purposes, patched btmgmt tool to support
the new mgmt commands (you can find patches in [1]).

Finally, this patch set is organized as follows:
* Patch 1-3: Refactoring and improvements.
* Patch 4-6: Use connection parameters specified by user.
* Patch 7-11: Add support for LE auto connection infrastructure
* Patch 12: Add support for auto connection options 
* Patch 13-15: Add mgmt commands

Regards,

Andre

[1] - https://github.com/aguedes/bluez/commits/auto-connect

Andre Guedes (15):
  Bluetooth: Refactor hci_disconn_complete_evt
  Bluetooth: Save connection interval parameters in hci_conn
  Bluetooth: Stop scanning on connection
  Bluetooth: Introduce connection parameters list
  Bluetooth: Make find_conn_param() helper non-local
  Bluetooth: Use connection parameters if any
  Bluetooth: Introduce hdev->pending_auto_conn list
  Bluetooth: Move is_scan_and_conn_supported() to hci_core
  Bluetooth: Introduce LE auto connection infrastructure
  Bluetooth: Temporarily stop background scanning on discovery
  Bluetooth: Auto connection and power on
  Bleutooth: Add support for auto connect options
  Bluetooth: Add thread-safe version of helpers
  Bluetooth: Mgmt command for adding connection parameters
  Bluetooth: Mgmt command for removing connection parameters

 include/net/bluetooth/hci_core.h |  51 +++++
 include/net/bluetooth/mgmt.h     |  15 ++
 net/bluetooth/hci_conn.c         |  40 +++-
 net/bluetooth/hci_core.c         | 403 +++++++++++++++++++++++++++++++++++++++
 net/bluetooth/hci_event.c        | 129 ++++++++++---
 net/bluetooth/mgmt.c             |  93 ++++++++-
 6 files changed, 696 insertions(+), 35 deletions(-)

-- 
1.8.4


^ permalink raw reply

* [RFC v2 01/15] Bluetooth: Refactor hci_disconn_complete_evt
From: Andre Guedes @ 2013-10-29 13:25 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383053160-10175-1-git-send-email-andre.guedes@openbossa.org>

hci_disconn_complete_evt() logic is more complicated than what it
should be, making it hard to follow and add new features.

So this patch does some code refactoring by handling the error cases
in the beginning of the function and by moving the main flow into the
first level of function scope. No change is done in the event handling
logic itself.

Besides organizing this messy code, this patch makes easier to add
code for handling LE auto connection (which will be added in a further
patch).

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_event.c | 62 +++++++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 29 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 5935f74..8b7cd37 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1783,6 +1783,8 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_ev_disconn_complete *ev = (void *) skb->data;
 	struct hci_conn *conn;
+	u8 type;
+	bool send_mgmt_event = false;
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);
 
@@ -1792,44 +1794,46 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	if (!conn)
 		goto unlock;
 
-	if (ev->status == 0)
-		conn->state = BT_CLOSED;
-
 	if (test_and_clear_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags) &&
-	    (conn->type == ACL_LINK || conn->type == LE_LINK)) {
-		if (ev->status) {
+	    (conn->type == ACL_LINK || conn->type == LE_LINK))
+		send_mgmt_event = true;
+
+	if (ev->status) {
+		if (send_mgmt_event)
 			mgmt_disconnect_failed(hdev, &conn->dst, conn->type,
 					       conn->dst_type, ev->status);
-		} else {
-			u8 reason = hci_to_mgmt_reason(ev->reason);
-
-			mgmt_device_disconnected(hdev, &conn->dst, conn->type,
-						 conn->dst_type, reason);
-		}
+		return;
 	}
 
-	if (ev->status == 0) {
-		u8 type = conn->type;
+	conn->state = BT_CLOSED;
 
-		if (type == ACL_LINK && conn->flush_key)
-			hci_remove_link_key(hdev, &conn->dst);
-		hci_proto_disconn_cfm(conn, ev->reason);
-		hci_conn_del(conn);
+	if (send_mgmt_event) {
+		u8 reason = hci_to_mgmt_reason(ev->reason);
 
-		/* Re-enable advertising if necessary, since it might
-		 * have been disabled by the connection. From the
-		 * HCI_LE_Set_Advertise_Enable command description in
-		 * the core specification (v4.0):
-		 * "The Controller shall continue advertising until the Host
-		 * issues an LE_Set_Advertise_Enable command with
-		 * Advertising_Enable set to 0x00 (Advertising is disabled)
-		 * or until a connection is created or until the Advertising
-		 * is timed out due to Directed Advertising."
-		 */
-		if (type == LE_LINK)
-			mgmt_reenable_advertising(hdev);
+		mgmt_device_disconnected(hdev, &conn->dst, conn->type,
+					 conn->dst_type, reason);
 	}
 
+	if (conn->type == ACL_LINK && conn->flush_key)
+		hci_remove_link_key(hdev, &conn->dst);
+
+	type = conn->type;
+	hci_proto_disconn_cfm(conn, ev->reason);
+	hci_conn_del(conn);
+
+	/* Re-enable advertising if necessary, since it might
+	 * have been disabled by the connection. From the
+	 * HCI_LE_Set_Advertise_Enable command description in
+	 * the core specification (v4.0):
+	 * "The Controller shall continue advertising until the Host
+	 * issues an LE_Set_Advertise_Enable command with
+	 * Advertising_Enable set to 0x00 (Advertising is disabled)
+	 * or until a connection is created or until the Advertising
+	 * is timed out due to Directed Advertising."
+	 */
+	if (type == LE_LINK)
+		mgmt_reenable_advertising(hdev);
+
 unlock:
 	hci_dev_unlock(hdev);
 }
-- 
1.8.4


^ 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