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: [PATCH 1/6] android: Remove reduntant structure
From: Johan Hedberg @ 2013-10-29 12:28 UTC (permalink / raw)
  To: Jakub Tyszkowski; +Cc: linux-bluetooth
In-Reply-To: <1383049016-23371-1-git-send-email-jakub.tyszkowski@tieto.com>

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 v3 4/5] android/hal: Add support for handling pin request event
From: Johan Hedberg @ 2013-10-29 12:19 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1383047562-4799-5-git-send-email-szymon.janc@tieto.com>

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.

Johan

^ permalink raw reply

* Re: [PATCHv2 0/9] Improve logging for Android
From: Andrei Emeltchenko @ 2013-10-29 12:18 UTC (permalink / raw)
  To: Jerzy Kasenberg; +Cc: linux-bluetooth
In-Reply-To: <CAHCYCoz4tSeimwrwVNENWsGf-ZDq9Huq9jB9dOgwccrPiF8Fow@mail.gmail.com>

Hi Jerzy,

On Tue, Oct 29, 2013 at 12:43:32PM +0100, Jerzy Kasenberg wrote:
> Hi Andrei,
> 
> On 29 October 2013 11:21, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >
> > Changes:
> >         *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
> 
> Implementation of strerror in bionic and what is proposed here is
> thread safe but not
> useful if one wants to use one print to for two addresses or two
> properties (one buffer
> in one thread for two strings).

Then just print 2 times.

> In my opinion there should be safe functions that explicitly get
> string buffer as
> argument. For address and uuid they already exist, property printing
> was used in haltest
> only in one place and moving buffer form btproperty2str to function
> that calls it
> makes sense to me.

Then every function calling need to define a buffer and for property the
buffer is not small.

For example current code

...
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", btproperty2str(property));
...

would need to define 4K buffer !!!, the same for many other functions.

> 
> If there is need for shortcut as for code lines that could be removed
> in compilation,
> wrapper function with pthread stuff can still be used (provided that
> there is no same
> type conversion call used in one print)
> 
> BTW strerror() works fine when called with value in range 0..131.
> But when you call it with higher number there will be a problem
> printf("%s\n%s\n\%s\n%s\n", strerror(1), strerror(2), strerror(132),
> strerror(133));
> prints:
> Operation not permitted
> No such file or directory
> Unknown error 133
> Unknown error 133

Yes, just print 2 times :)

Best regards 
Andrei Emeltchenko 


^ permalink raw reply

* [PATCH 6/6] android: Add missing discovery state definitions to IPC header
From: Jakub Tyszkowski @ 2013-10-29 12:16 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383049016-23371-1-git-send-email-jakub.tyszkowski@tieto.com>

---
 android/hal-msg.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/android/hal-msg.h b/android/hal-msg.h
index a4eb2a8..ec4b62a 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -348,6 +348,9 @@ struct hal_ev_device_found {
 	struct hal_property props[0];
 } __attribute__((packed));
 
+#define HAL_DISCOVERY_STATE_STOPPED	0x00
+#define HAL_DISCOVERY_STATE_STARTED	0x01
+
 #define HAL_EV_DISCOVERY_STATE_CHANGED	0x85
 struct hal_ev_discovery_state_changed {
 	uint8_t state;
-- 
1.8.4.1


^ permalink raw reply related

* [PATCH 5/6] android/hal: Add device state changed event handler
From: Jakub Tyszkowski @ 2013-10-29 12:16 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383049016-23371-1-git-send-email-jakub.tyszkowski@tieto.com>

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;
+	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);
+	}
+	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


^ permalink raw reply related

* [PATCH 4/6] android/hal: Add device found event handler
From: Jakub Tyszkowski @ 2013-10-29 12:16 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383049016-23371-1-git-send-email-jakub.tyszkowski@tieto.com>

This is called when new remote device is found.

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

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index b13f4d7..261ae85 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -93,6 +93,27 @@ static void handle_discovery_state_changed(void *buf)
 		bt_hal_cbacks->discovery_state_changed_cb(ev->state);
 }
 
+static void handle_device_found(void *buf)
+{
+	uint8_t i;
+	struct hal_ev_device_found *ev = buf;
+	bt_property_t send_props[ev->num_props];
+	struct hal_property *prop = ev->props;
+
+	if (!bt_hal_cbacks->device_found_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);
+	}
+	bt_hal_cbacks->device_found_cb(ev->num_props, send_props);
+}
+
 /* will be called from notification thread context */
 void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
 {
@@ -109,6 +130,9 @@ void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
 	case HAL_EV_DISCOVERY_STATE_CHANGED:
 		handle_discovery_state_changed(buf);
 		break;
+	case HAL_EV_DEVICE_FOUND:
+		handle_device_found(buf);
+		break;
 	default:
 		DBG("Unhandled callback opcode=0x%x", opcode);
 		break;
-- 
1.8.4.1


^ permalink raw reply related

* [PATCH 3/6] android/hal: Add support for handling discovery state change event
From: Jakub Tyszkowski @ 2013-10-29 12:16 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383049016-23371-1-git-send-email-jakub.tyszkowski@tieto.com>

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

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index aa104ba..b13f4d7 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -85,6 +85,14 @@ static bool interface_ready(void)
 	return bt_hal_cbacks != NULL;
 }
 
+static void handle_discovery_state_changed(void *buf)
+{
+	struct hal_ev_discovery_state_changed *ev = buf;
+
+	if (bt_hal_cbacks->discovery_state_changed_cb)
+		bt_hal_cbacks->discovery_state_changed_cb(ev->state);
+}
+
 /* will be called from notification thread context */
 void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
 {
@@ -98,6 +106,9 @@ void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
 	case HAL_EV_ADAPTER_PROPS_CHANGED:
 		handle_adapter_props_changed(buf, len);
 		break;
+	case HAL_EV_DISCOVERY_STATE_CHANGED:
+		handle_discovery_state_changed(buf);
+		break;
 	default:
 		DBG("Unhandled callback opcode=0x%x", opcode);
 		break;
-- 
1.8.4.1


^ permalink raw reply related

* [PATCH 2/6] android/hal: Add support for start/cancel device discovery commands
From: Jakub Tyszkowski @ 2013-10-29 12:16 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383049016-23371-1-git-send-email-jakub.tyszkowski@tieto.com>

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

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 5929fff..aa104ba 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -308,7 +308,9 @@ static int start_discovery(void)
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
 
-	return BT_STATUS_UNSUPPORTED;
+	return hal_ipc_cmd(HAL_SERVICE_ID_BLUETOOTH,
+				HAL_OP_START_DISCOVERY, 0, NULL, 0,
+				NULL, NULL);
 }
 
 static int cancel_discovery(void)
@@ -318,7 +320,9 @@ static int cancel_discovery(void)
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
 
-	return BT_STATUS_UNSUPPORTED;
+	return hal_ipc_cmd(HAL_SERVICE_ID_BLUETOOTH,
+				HAL_OP_CANCEL_DISCOVERY, 0, NULL, 0,
+				NULL, NULL);
 }
 
 static int create_bond(const bt_bdaddr_t *bd_addr)
-- 
1.8.4.1


^ permalink raw reply related

* [PATCH 1/6] android: Remove reduntant structure
From: Jakub Tyszkowski @ 2013-10-29 12:16 UTC (permalink / raw)
  To: linux-bluetooth

---
 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;
-- 
1.8.4.1


^ permalink raw reply related

* Re: [PATCH] build: Move Makefile.android to android/Makefile.am
From: Johan Hedberg @ 2013-10-29 12:11 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1382978226-28073-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Mon, Oct 28, 2013, Szymon Janc wrote:
> This will keep all Android related changes within android directory.
> ---
> 
> Will also fix messing with Marcel's tab completion habits :-)
> 
>  Makefile.am         |   2 +-
>  Makefile.android    | 113 ----------------------------------------------------
>  android/Makefile.am | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 114 insertions(+), 114 deletions(-)
>  delete mode 100644 Makefile.android
>  create mode 100644 android/Makefile.am

Applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] android/hal: Set callbacks before initializing IPC
From: Johan Hedberg @ 2013-10-29 12:04 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1382975137-12604-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Mon, Oct 28, 2013, Szymon Janc wrote:
> Notification thread is started on IPC init and it can be scheduled
> before main thread.
> 
> Fix following crash on HAL init:
> 
> pid: 3392, tid: 3492, name: droid.bluetooth  >>> com.android.bluetooth <<<
> signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 00000028
>     eax 00000000  ebx ce047f64  ecx ce0405d0  edx f7763724
>     esi cdf3af00  edi f837fb90
>     xcs 00000023  xds 0000002b  xes 0000002b  xfs 00000000  xss 0000002b
>     eip ce042289  ebp cdf3a9f8  esp cdf3a9e0  flags 00010292
> 
> backtrace:
>     #00  pc 00003289  /system/lib/hw/bluetooth.default.so (bt_thread_associate+25)
>     #01  pc 000015f8  /system/lib/hw/bluetooth.default.so (notification_handler+40)
>     #02  pc 0000f804  /system/lib/libc.so (__thread_entry+276)
>     #03  pc 0002999d  /system/lib/libc.so
>     #04  pc 00082ae7  /system/lib/libdvm.so (dvmThreadSelf()+39)
>     #05  pc 00010db9  /system/lib/libc.so (pthread_mutex_unlock+25)
> ---
>  android/hal-bluetooth.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

Applied. Thanks.

Johan

^ permalink raw reply

* [PATCH v3 5/5] android/hal: Add support for handling SSP request event
From: Szymon Janc @ 2013-10-29 11:52 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1383047562-4799-1-git-send-email-szymon.janc@tieto.com>

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

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index a1a4ef4..f14fbb5 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -88,6 +88,18 @@ 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;
+	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)
@@ -124,6 +136,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

* [PATCH v3 4/5] android/hal: Add support for handling pin request event
From: Szymon Janc @ 2013-10-29 11:52 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1383047562-4799-1-git-send-email-szymon.janc@tieto.com>

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

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index a3f5038..a1a4ef4 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -78,6 +78,16 @@ static void handle_bond_state_change(void *buf)
 								ev->state);
 }
 
+static void handle_pin_request(void *buf)
+{
+	struct hal_ev_pin_request *ev = buf;
+	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 +121,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 v3 3/5] android/hal: Add support for handling bond state change event
From: Szymon Janc @ 2013-10-29 11:52 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1383047562-4799-1-git-send-email-szymon.janc@tieto.com>

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

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 5f6dcbe..a3f5038 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -68,6 +68,16 @@ static void handle_adapter_props_changed(void *buf, uint16_t len)
 	bt_hal_cbacks->adapter_properties_cb(ev->status, ev->num_props, props);
 }
 
+static void handle_bond_state_change(void *buf)
+{
+	struct hal_ev_bond_state_changed *ev = buf;
+	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
+
+	if (bt_hal_cbacks->bond_state_changed_cb)
+		bt_hal_cbacks->bond_state_changed_cb(ev->status, addr,
+								ev->state);
+}
+
 void bt_thread_associate(void)
 {
 	if (bt_hal_cbacks->thread_evt_cb)
@@ -98,6 +108,9 @@ void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
 	case HAL_EV_ADAPTER_PROPS_CHANGED:
 		handle_adapter_props_changed(buf, len);
 		break;
+	case HAL_EV_BOND_STATE_CHANGED:
+		handle_bond_state_change(buf);
+		break;
 	default:
 		DBG("Unhandled callback opcode=0x%x", opcode);
 		break;
-- 
1.8.4.1


^ permalink raw reply related

* [PATCH v3 2/5] android: Update IPC headers to match SSP and PIN requests events
From: Szymon Janc @ 2013-10-29 11:52 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1383047562-4799-1-git-send-email-szymon.janc@tieto.com>

Name should be 249 bytes so it is always NULL terminated string.
Class of device is send as uint32. This will allow to make simple
passing of data in HAL library without need of copying data.
---
 android/hal-msg.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/android/hal-msg.h b/android/hal-msg.h
index a4eb2a8..80b47d6 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -356,15 +356,15 @@ struct hal_ev_discovery_state_changed {
 #define HAL_EV_PIN_REQUEST		0x86
 struct hal_ev_pin_request {
 	uint8_t bdaddr[6];
-	uint8_t name[249 - 1];
-	uint8_t class_of_dev[3];
+	uint8_t name[249];
+	uint32_t class_of_dev;
 } __attribute__((packed));
 
 #define HAL_EV_SSP_REQUEST		0x87
 struct hal_ev_ssp_request {
 	uint8_t  bdaddr[6];
-	uint8_t  name[249 - 1];
-	uint8_t  class_of_dev[3];
+	uint8_t  name[249];
+	uint32_t  class_of_dev;
 	uint8_t  pairing_variant;
 	uint32_t passkey;
 } __attribute__((packed));
-- 
1.8.4.1


^ permalink raw reply related

* [PATCH v3 1/5] android: Define class of device as four bytes in IPC doc
From: Szymon Janc @ 2013-10-29 11:52 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1383047562-4799-1-git-send-email-szymon.janc@tieto.com>

For PIN and SSP requests callback define CoD as 4 bytes. This will
allow HAL library to pass CoD direclty to callback. Will also match
how CoD is passed as property.
---
 android/hal-ipc-api.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
index dc0d067..e7af8a3 100644
--- a/android/hal-ipc-api.txt
+++ b/android/hal-ipc-api.txt
@@ -354,13 +354,13 @@ Notifications:
 
 		Notification parameters: Remote address (6 octets)
 		                         Remote name (249 octets)
-		                         Class of device (3 octets)
+		                         Class of device (4 octets)
 
 	Opcode 0x87 - SSP Request notification
 
 		Notification parameters: Remote address (6 octets)
 		                         Remote name (249 octets)
-		                         Class of device (3 octets)
+		                         Class of device (4 octets)
 		                         Pairing variant (1 octet)
 		                         Passkey (4 octets)
 
-- 
1.8.4.1


^ permalink raw reply related

* [PATCH v3 0/5] initial code for pairing support
From: Szymon Janc @ 2013-10-29 11:52 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

V3: Fixed cback calling convention pointed by Andrei.

Szymon Janc (5):
  android: Define class of device as four bytes in IPC doc
  android: Update IPC headers to match SSP and PIN requests events
  android/hal: Add support for handling bond state change event
  android/hal: Add support for handling pin request event
  android/hal: Add support for handling SSP request event

 android/hal-bluetooth.c | 41 +++++++++++++++++++++++++++++++++++++++++
 android/hal-ipc-api.txt |  4 ++--
 android/hal-msg.h       |  8 ++++----
 3 files changed, 47 insertions(+), 6 deletions(-)

-- 
1.8.4.1


^ permalink raw reply

* Re: [PATCHv2 0/9] Improve logging for Android
From: Jerzy Kasenberg @ 2013-10-29 11:43 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1383042118-21205-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On 29 October 2013 11:21, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Changes:
>         *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

Implementation of strerror in bionic and what is proposed here is
thread safe but not
useful if one wants to use one print to for two addresses or two
properties (one buffer
in one thread for two strings).

In my opinion there should be safe functions that explicitly get
string buffer as
argument. For address and uuid they already exist, property printing
was used in haltest
only in one place and moving buffer form btproperty2str to function
that calls it
makes sense to me.

If there is need for shortcut as for code lines that could be removed
in compilation,
wrapper function with pthread stuff can still be used (provided that
there is no same
type conversion call used in one print)

BTW strerror() works fine when called with value in range 0..131.
But when you call it with higher number there will be a problem
printf("%s\n%s\n\%s\n%s\n", strerror(1), strerror(2), strerror(132),
strerror(133));
prints:
Operation not permitted
No such file or directory
Unknown error 133
Unknown error 133

>         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):
...

-- 
Jerzy

^ permalink raw reply

* Re: Intel 7260 bluetooth malfunction when it is connected to EHCI bus
From: Marcel Holtmann @ 2013-10-29 11:16 UTC (permalink / raw)
  To: Hui Wang
  Cc: Tedd An, Johan Hedberg, xiong.y.zhang, Gustavo F. Padovan,
	linux-bluetooth@vger.kernel.org development
In-Reply-To: <526F8804.9060906@canonical.com>

Hi Hui,

> The problem is:
> On the machine which has Intel 7260 BT module, i use it to connect a bluetooth headset,
> it can successfully scan and connect to the headset, when i play sound to the bt headset,
> the problem comes, if the bt module is connected to the XHCI, it can work very well.
> 
> u@u-Lenovo-B4400:~$ lsusb -t
> 1-7:1.0: No such file or directory
> /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 5000M
> /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/14p, 480M
>    |__ Port 6: Dev 8, If 0, Class=HID, Driver=usbhid, 1.5M
>    |__ Port 7: Dev 2, If 0, Class=vend., Driver=, 12M
>    |__ Port 11: Dev 3, If 0, Class='bInterfaceClass 0xe0 not yet handled', Driver=btusb, 12M
>    |__ Port 11: Dev 3, If 1, Class='bInterfaceClass 0xe0 not yet handled', Driver=btusb, 12M
> 
> But if the bt module is connected to the EHCI, it always fails to play sound.
> 
> u@u-Lenovo-B4400:~$ lsusb -t
> /: Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 5000M
> /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/14p, 480M
> /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=ehci_hcd/3p, 480M
>    |__ Port 1: Dev 2, If 0, Class=hub, Driver=hub/8p, 480M
> /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci_hcd/3p, 480M
>    |__ Port 1: Dev 2, If 0, Class=hub, Driver=hub/6p, 480M
>        |__ Port 5: Dev 3, If 0, Class='bInterfaceClass 0xe0 not yet handled', Driver=btusb, 12M
>        |__ Port 5: Dev 3, If 1, Class='bInterfaceClass 0xe0 not yet handled', Driver=btusb, 12M
>        |__ Port 6: Dev 4, If 0, Class='bInterfaceClass 0x0e not yet handled', Driver=uvcvideo, 480M
>        |__ Port 6: Dev 4, If 1, Class='bInterfaceClass 0x0e not yet handled', Driver=uvcvideo, 480M

can you paste /sys/kernel/debug/usb/devices here.

If the uvcvideo driver is also using ISOC URBs, we might have just exhausted the bandwidth of the USB bus here or these two devices just do not play nice with each other.

Try to unload the uvcvideo driver and try it again.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH_v3 1/2] android: Add initial HID connect implementation
From: Luiz Augusto von Dentz @ 2013-10-29 11:15 UTC (permalink / raw)
  To: Ravi kumar Veeramally; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1383040097-15475-1-git-send-email-ravikumar.veeramally@linux.intel.com>

Hi Ravi,

On Tue, Oct 29, 2013 at 11:48 AM, Ravi kumar Veeramally
<ravikumar.veeramally@linux.intel.com> wrote:
> Implemented basic HID connect method. Host connects to
> bt device at L2CAP level.
> ---
> v3: Updated patches as per Luiz comments (remove unnecessary
>     type cast and tabs)
>
> v2: Updated patches as per Luiz comments
>
> v1: Patchset adds hid connect and disconnect mechanisms at
>     L2CAP level. It opens the control channel and interrupt
>     channel and listens on io events. UHID, hid server and
> ---
>  Makefile.android   |    3 +-
>  android/Android.mk |    1 +
>  android/hid.c      |  245 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 248 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile.android b/Makefile.android
> index 2b57daa..22002be 100644
> --- a/Makefile.android
> +++ b/Makefile.android
> @@ -12,7 +12,8 @@ android_bluetoothd_SOURCES =  android/main.c \
>                                 android/adapter.h android/adapter.c \
>                                 android/hid.h android/hid.c \
>                                 android/ipc.h android/ipc.c \
> -                               android/socket.h android/socket.c
> +                               android/socket.h android/socket.c \
> +                               btio/btio.h btio/btio.c
>
>  android_bluetoothd_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
>
> diff --git a/android/Android.mk b/android/Android.mk
> index 22208e0..28ec465 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -28,6 +28,7 @@ LOCAL_SRC_FILES := \
>         ../lib/sdp.c \
>         ../lib/bluetooth.c \
>         ../lib/hci.c \
> +       ../btio/btio.c
>
>  LOCAL_C_INCLUDES := \
>         $(call include-path-for, glib) \
> diff --git a/android/hid.c b/android/hid.c
> index f2da0d3..913dab4 100644
> --- a/android/hid.c
> +++ b/android/hid.c
> @@ -23,16 +23,260 @@
>
>  #include <stdint.h>
>  #include <stdbool.h>
> +#include <errno.h>
> +#include <unistd.h>
> +#include <fcntl.h>
>
>  #include <glib.h>
>
> +#include "btio/btio.h"
>  #include "lib/bluetooth.h"
> +#include "src/shared/mgmt.h"
> +
>  #include "log.h"
>  #include "hal-msg.h"
>  #include "ipc.h"
>  #include "hid.h"
> +#include "adapter.h"
> +#include "utils.h"
> +
> +#define L2CAP_PSM_HIDP_CTRL    0x11
> +#define L2CAP_PSM_HIDP_INTR    0x13
> +#define MAX_READ_BUFFER                4096
>
>  static GIOChannel *notification_io = NULL;
> +static GSList *devices = NULL;
> +
> +struct hid_device {
> +       bdaddr_t        dst;
> +       GIOChannel      *ctrl_io;
> +       GIOChannel      *intr_io;
> +       guint           ctrl_watch;
> +       guint           intr_watch;
> +};
> +
> +static int device_cmp(gconstpointer s, gconstpointer user_data)
> +{
> +       const struct hid_device *hdev = s;
> +       const bdaddr_t *dst = user_data;
> +
> +       return bacmp(&hdev->dst, dst);
> +}
> +
> +static void hid_device_free(struct hid_device *hdev)
> +{
> +       if (hdev->ctrl_watch > 0)
> +               g_source_remove(hdev->ctrl_watch);
> +
> +       if (hdev->intr_watch > 0)
> +               g_source_remove(hdev->intr_watch);
> +
> +       if (hdev->intr_io)
> +               g_io_channel_unref(hdev->intr_io);
> +
> +       if (hdev->ctrl_io)
> +               g_io_channel_unref(hdev->ctrl_io);
> +
> +       devices = g_slist_remove(devices, hdev);
> +       g_free(hdev);
> +}
> +
> +static gboolean intr_io_watch_cb(GIOChannel *chan, gpointer data)
> +{
> +       char buf[MAX_READ_BUFFER];
> +       int fd, bread;
> +
> +       fd = g_io_channel_unix_get_fd(chan);
> +       bread = read(fd, buf, sizeof(buf));
> +       if (bread < 0) {
> +               error("read: %s(%d)", strerror(errno), -errno);
> +               return TRUE;
> +       }
> +
> +       DBG("bytes read %d", bread);
> +
> +       /* TODO: At this moment only baseband is connected, i.e. mouse
> +        * movements keyboard events doesn't effect on UI. Have to send
> +        * this data to uhid fd for profile connection. */
> +
> +       return TRUE;
> +}
> +
> +static gboolean intr_watch_cb(GIOChannel *chan, GIOCondition cond,
> +                                                               gpointer data)
> +{
> +       struct hid_device *hdev = data;
> +       char address[18];
> +
> +       if (cond & G_IO_IN)
> +               return intr_io_watch_cb(chan, data);
> +
> +       ba2str(&hdev->dst, address);
> +       DBG("Device %s disconnected", address);
> +
> +       /* Checking for ctrl_watch avoids a double g_io_channel_shutdown since
> +        * it's likely that ctrl_watch_cb has been queued for dispatching in
> +        * this mainloop iteration */
> +       if ((cond & (G_IO_HUP | G_IO_ERR)) && hdev->ctrl_watch)
> +               g_io_channel_shutdown(chan, TRUE, NULL);
> +
> +       hdev->intr_watch = 0;
> +
> +       if (hdev->intr_io) {
> +               g_io_channel_unref(hdev->intr_io);
> +               hdev->intr_io = NULL;
> +       }
> +
> +       /* Close control channel */
> +       if (hdev->ctrl_io && !(cond & G_IO_NVAL))
> +               g_io_channel_shutdown(hdev->ctrl_io, TRUE, NULL);
> +
> +       return FALSE;
> +}
> +
> +static gboolean ctrl_watch_cb(GIOChannel *chan, GIOCondition cond,
> +                                                               gpointer data)
> +{
> +       struct hid_device *hdev = data;
> +       char address[18];
> +
> +       ba2str(&hdev->dst, address);
> +       DBG("Device %s disconnected", address);
> +
> +       /* Checking for intr_watch avoids a double g_io_channel_shutdown since
> +        * it's likely that intr_watch_cb has been queued for dispatching in
> +        * this mainloop iteration */
> +       if ((cond & (G_IO_HUP | G_IO_ERR)) && hdev->intr_watch)
> +               g_io_channel_shutdown(chan, TRUE, NULL);
> +
> +       hdev->ctrl_watch = 0;
> +
> +       if (hdev->ctrl_io) {
> +               g_io_channel_unref(hdev->ctrl_io);
> +               hdev->ctrl_io = NULL;
> +       }
> +
> +       if (hdev->intr_io && !(cond & G_IO_NVAL))
> +               g_io_channel_shutdown(hdev->intr_io, TRUE, NULL);
> +
> +       return FALSE;
> +}
> +
> +static void interrupt_connect_cb(GIOChannel *chan, GError *conn_err,
> +                                                       gpointer user_data)
> +{
> +       struct hid_device *hdev = user_data;
> +
> +       DBG("");
> +
> +       if (conn_err)
> +               goto failed;
> +
> +       /*TODO: Get device details through SDP and create UHID fd and start
> +        * listening on uhid events */
> +       hdev->intr_watch = g_io_add_watch(hdev->intr_io,
> +                               G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
> +                               intr_watch_cb, hdev);
> +
> +       return;
> +
> +failed:
> +       /* So we guarantee the interrupt channel is closed before the
> +        * control channel (if we only do unref GLib will close it only
> +        * after returning control to the mainloop */
> +       if (!conn_err)
> +               g_io_channel_shutdown(hdev->intr_io, FALSE, NULL);
> +
> +       g_io_channel_unref(hdev->intr_io);
> +       hdev->intr_io = NULL;
> +
> +       if (hdev->ctrl_io) {
> +               g_io_channel_unref(hdev->ctrl_io);
> +               hdev->ctrl_io = NULL;
> +       }
> +}
> +
> +static void control_connect_cb(GIOChannel *chan, GError *conn_err,
> +                                                       gpointer user_data)
> +{
> +       struct hid_device *hdev = user_data;
> +       GError *err = NULL;
> +       const bdaddr_t *src = bt_adapter_get_address();
> +
> +       DBG("");
> +
> +       if (conn_err) {
> +               error("%s", conn_err->message);
> +               goto failed;
> +       }
> +
> +       /* Connect to the HID interrupt channel */
> +       hdev->intr_io = bt_io_connect(interrupt_connect_cb, hdev, NULL, &err,
> +                                       BT_IO_OPT_SOURCE_BDADDR, src,
> +                                       BT_IO_OPT_DEST_BDADDR, &hdev->dst,
> +                                       BT_IO_OPT_PSM, L2CAP_PSM_HIDP_INTR,
> +                                       BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
> +                                       BT_IO_OPT_INVALID);
> +       if (!hdev->intr_io) {
> +               error("%s", err->message);
> +               g_error_free(err);
> +               goto failed;
> +       }
> +
> +       hdev->ctrl_watch = g_io_add_watch(hdev->ctrl_io,
> +                                       G_IO_HUP | G_IO_ERR | G_IO_NVAL,
> +                                       ctrl_watch_cb, hdev);
> +
> +       return;
> +
> +failed:
> +       g_io_channel_unref(hdev->ctrl_io);
> +       hdev->ctrl_io = NULL;
> +}
> +
> +static uint8_t bt_hid_connect(struct hal_cmd_hid_connect *cmd, uint16_t len)
> +{
> +       struct hid_device *hdev;
> +       char addr[18];
> +       bdaddr_t dst;
> +       GSList *l;
> +       GError *err = NULL;
> +       const bdaddr_t *src = bt_adapter_get_address();
> +
> +       DBG("");
> +
> +       if (len < sizeof(*cmd))
> +               return HAL_STATUS_INVALID;
> +
> +       android2bdaddr(&cmd->bdaddr, &dst);
> +
> +       l = g_slist_find_custom(devices, &dst, device_cmp);
> +       if (l)
> +               return HAL_STATUS_FAILED;
> +
> +       hdev = g_new0(struct hid_device, 1);
> +       android2bdaddr(&cmd->bdaddr, &hdev->dst);
> +       ba2str(&hdev->dst, addr);
> +
> +       DBG("connecting to %s", addr);
> +
> +       hdev->ctrl_io = bt_io_connect(control_connect_cb, hdev, NULL, &err,
> +                                       BT_IO_OPT_SOURCE_BDADDR, src,
> +                                       BT_IO_OPT_DEST_BDADDR, &hdev->dst,
> +                                       BT_IO_OPT_PSM, L2CAP_PSM_HIDP_CTRL,
> +                                       BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
> +                                       BT_IO_OPT_INVALID);
> +       if (err) {
> +               error("%s", err->message);
> +               g_error_free(err);
> +               hid_device_free(hdev);
> +               return HAL_STATUS_FAILED;
> +       }
> +
> +       devices = g_slist_append(devices, hdev);
> +
> +       return HAL_STATUS_SUCCESS;
> +}
>
>  void bt_hid_handle_cmd(GIOChannel *io, uint8_t opcode, void *buf, uint16_t len)
>  {
> @@ -40,6 +284,7 @@ void bt_hid_handle_cmd(GIOChannel *io, uint8_t opcode, void *buf, uint16_t len)
>
>         switch (opcode) {
>         case HAL_OP_HID_CONNECT:
> +               status = bt_hid_connect(buf, len);
>                 break;
>         case HAL_OP_HID_DISCONNECT:
>                 break;
> --
> 1.7.9.5

Pushed, thanks.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH v2 3/5] android/hal: Add support for handling bond state change event
From: Szymon Janc @ 2013-10-29 10:52 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <20131029103800.GD27517@aemeltch-MOBL1>

Hi Andrei,

> On Tue, Oct 29, 2013 at 11:32:29AM +0100, Szymon Janc wrote:
> > Hi Andrei,
> > 
> > On Tuesday 29 of October 2013 12:27:24 Andrei Emeltchenko wrote:
> > > Hi Szymon,
> > > 
> > > On Tue, Oct 29, 2013 at 11:16:27AM +0100, Szymon Janc wrote:
> > > > ---
> > > >  android/hal-bluetooth.c | 14 ++++++++++++++
> > > >  1 file changed, 14 insertions(+)
> > > > 
> > > > diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> > > > index 5f6dcbe..067f420 100644
> > > > --- a/android/hal-bluetooth.c
> > > > +++ b/android/hal-bluetooth.c
> > > > @@ -68,6 +68,17 @@ static void handle_adapter_props_changed(void *buf, uint16_t len)
> > > >  	bt_hal_cbacks->adapter_properties_cb(ev->status, ev->num_props, props);
> > > >  }
> > > >  
> > > > +static void handle_bond_state_change(void *buf)
> > > > +{
> > > > +	struct hal_ev_bond_state_changed *ev = buf;
> > > > +	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
> > > > +
> > > > +	if (!bt_hal_cbacks->bond_state_changed_cb)
> > > > +		return;
> > > > +
> > > > +	bt_hal_cbacks->bond_state_changed_cb(ev->status, addr, ev->state);
> > > 
> > > We shall use the same style like for other callbacks.
> > 
> > In that case reverting check allow to not break function call into 2 lines.
> > I find it more readable (and there will be reverted check for less trivial
> > callbacks e.g. with properties)
> 
> Those checks at least need to be consistent. Your next patch use other
> way. We shall agree about best way for this check, maybe some #define like
> I have in my patches sent some time ago? We could also add debug traces
> then.

In next patch reverting check wouldn't give any benefit as function call
doesn't fit 1 line. Not that this matter a lot since this is trivial callback
handler. I'll send v3 with this check changed if this really bothers you :)

As for macro, I would prefer to not have any as this makes code harder to
follow especially for non-trivial handlers  (where you can return early in case
callback is null).

(I was even thinking about open-coding interface_ready() functions... )

-- 
BR
Szymon Janc



^ permalink raw reply

* Re: [PATCH v2 3/5] android/hal: Add support for handling bond state change event
From: Andrei Emeltchenko @ 2013-10-29 10:38 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <32939690.A882f3Cjsu@uw000953>

On Tue, Oct 29, 2013 at 11:32:29AM +0100, Szymon Janc wrote:
> Hi Andrei,
> 
> On Tuesday 29 of October 2013 12:27:24 Andrei Emeltchenko wrote:
> > Hi Szymon,
> > 
> > On Tue, Oct 29, 2013 at 11:16:27AM +0100, Szymon Janc wrote:
> > > ---
> > >  android/hal-bluetooth.c | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> > > index 5f6dcbe..067f420 100644
> > > --- a/android/hal-bluetooth.c
> > > +++ b/android/hal-bluetooth.c
> > > @@ -68,6 +68,17 @@ static void handle_adapter_props_changed(void *buf, uint16_t len)
> > >  	bt_hal_cbacks->adapter_properties_cb(ev->status, ev->num_props, props);
> > >  }
> > >  
> > > +static void handle_bond_state_change(void *buf)
> > > +{
> > > +	struct hal_ev_bond_state_changed *ev = buf;
> > > +	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
> > > +
> > > +	if (!bt_hal_cbacks->bond_state_changed_cb)
> > > +		return;
> > > +
> > > +	bt_hal_cbacks->bond_state_changed_cb(ev->status, addr, ev->state);
> > 
> > We shall use the same style like for other callbacks.
> 
> In that case reverting check allow to not break function call into 2 lines.
> I find it more readable (and there will be reverted check for less trivial
> callbacks e.g. with properties)

Those checks at least need to be consistent. Your next patch use other
way. We shall agree about best way for this check, maybe some #define like
I have in my patches sent some time ago? We could also add debug traces
then.

Best regards 
Andrei Emeltchenko 


^ permalink raw reply

* Re: [PATCH v2 3/5] android/hal: Add support for handling bond state change event
From: Szymon Janc @ 2013-10-29 10:32 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <20131029102722.GB27517@aemeltch-MOBL1>

Hi Andrei,

On Tuesday 29 of October 2013 12:27:24 Andrei Emeltchenko wrote:
> Hi Szymon,
> 
> On Tue, Oct 29, 2013 at 11:16:27AM +0100, Szymon Janc wrote:
> > ---
> >  android/hal-bluetooth.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> > index 5f6dcbe..067f420 100644
> > --- a/android/hal-bluetooth.c
> > +++ b/android/hal-bluetooth.c
> > @@ -68,6 +68,17 @@ static void handle_adapter_props_changed(void *buf, uint16_t len)
> >  	bt_hal_cbacks->adapter_properties_cb(ev->status, ev->num_props, props);
> >  }
> >  
> > +static void handle_bond_state_change(void *buf)
> > +{
> > +	struct hal_ev_bond_state_changed *ev = buf;
> > +	bt_bdaddr_t *addr = (bt_bdaddr_t *) ev->bdaddr;
> > +
> > +	if (!bt_hal_cbacks->bond_state_changed_cb)
> > +		return;
> > +
> > +	bt_hal_cbacks->bond_state_changed_cb(ev->status, addr, ev->state);
> 
> We shall use the same style like for other callbacks.

In that case reverting check allow to not break function call into 2 lines.
I find it more readable (and there will be reverted check for less trivial
callbacks e.g. with properties)

-- 
BR
Szymon Janc


^ permalink raw reply

* Re: [PATCHv2 2/9] android/haltest: Fix compile error making function static
From: Andrei Emeltchenko @ 2013-10-29 10:31 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1383042118-21205-3-git-send-email-Andrei.Emeltchenko.news@gmail.com>

sorry forgot to merge this fix

On Tue, Oct 29, 2013 at 12:21:51PM +0200, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> This fixes following error during compilation:
> ...
> android/client/textconv.c:240: error: no previous declaration for ‘btuuid2str’
> make[1]: *** [android/client/android_haltest-textconv.o] Error 1
> ...
> ---
>  android/client/textconv.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/android/client/textconv.c b/android/client/textconv.c
> index a3e10ee..8f27948 100644
> --- a/android/client/textconv.c
> +++ b/android/client/textconv.c
> @@ -234,7 +234,7 @@ char *bdaddr2str(const bt_bdaddr_t *bd_addr)
>  	return bt_bdaddr_t2str(bd_addr, buf);
>  }
>  
> -char *btuuid2str(const bt_uuid_t *uuid)
> +static char *btuuid2str(const bt_uuid_t *uuid)
>  {
>  	static char buf[MAX_UUID_STR_LEN];
>  
> -- 
> 1.7.10.4
> 
> --
> 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


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