Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH] android/tester: Fix memory leak
From: Johan Hedberg @ 2013-12-20 12:42 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1387542935-15736-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Fri, Dec 20, 2013, Andrei Emeltchenko wrote:
> Call del_hook() after add_hook(). This fixes valgrind warnings:
> 
> ...
> ==15303==
> ==15303== HEAP SUMMARY:
> ==15303==     in use at exit: 3,060 bytes in 27 blocks
> ==15303==   total heap usage: 6,410 allocs, 6,383 frees, 332,477 bytes
> allocated
> ==15303==
> ==15303== 24 bytes in 1 blocks are definitely lost in loss record 9 of
> 27
> ==15303==    at 0x4C2A2DB: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==15303==    by 0x406E20: btdev_add_hook (btdev.c:2166)
> ==15303==    by 0x40BFC2: test_discovery_start_done
> (android-tester.c:1401)
> ==15303==    by 0x409C65: run_callback (tester.c:385)
> ==15303==    by 0x4E7C3B5: g_main_context_dispatch (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.3800.1)
> ==15303==    by 0x4E7C707: ??? (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.3800.1)
> ==15303==    by 0x4E7CB09: g_main_loop_run (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.3800.1)
> ==15303==    by 0x40A85C: tester_run (tester.c:784)
> ==15303==    by 0x40368B: main (android-tester.c:1654)
> ==15303==
> ==15303== 24 bytes in 1 blocks are definitely lost in loss record 10 of
> 27
> ==15303==    at 0x4C2A2DB: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==15303==    by 0x406E20: btdev_add_hook (btdev.c:2166)
> ==15303==    by 0x40BF12: test_discovery_stop_success
> (android-tester.c:1386)
> ==15303==    by 0x409C65: run_callback (tester.c:385)
> ==15303==    by 0x4E7C3B5: g_main_context_dispatch (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.3800.1)
> ==15303==    by 0x4E7C707: ??? (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.3800.1)
> ==15303==    by 0x4E7CB09: g_main_loop_run (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.3800.1)
> ==15303==    by 0x40A85C: tester_run (tester.c:784)
> ==15303==    by 0x40368B: main (android-tester.c:1654)
> ==15303==
> ...
> ---
>  android/android-tester.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Applied. Thanks.

Johan

^ permalink raw reply

* [PATCH 1/2] android/bluetooth-hal: Fix using wrong struct for buffer size
From: Szymon Janc @ 2013-12-20 12:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Buffer is for hal_cmd_le_test_mode command.
---
 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 2232ebe..3dbc435 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -812,7 +812,7 @@ static int dut_mode_send(uint16_t opcode, uint8_t *buf, uint8_t len)
 #if PLATFORM_SDK_VERSION > 17
 static int le_test_mode(uint16_t opcode, uint8_t *buf, uint8_t len)
 {
-	uint8_t cmd_buf[sizeof(struct hal_cmd_dut_mode_send) + len];
+	uint8_t cmd_buf[sizeof(struct hal_cmd_le_test_mode) + len];
 	struct hal_cmd_le_test_mode *cmd = (void *) cmd_buf;
 
 	DBG("opcode %u len %u", opcode, len);
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 2/2] android/bluetooth: Remove not needed local variable
From: Szymon Janc @ 2013-12-20 12:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1387543842-6489-1-git-send-email-szymon.janc@tieto.com>

Both manufacturer and sub_version already have correct type and struct
is marked as packed so there is no need to memcpy to temp variable.
---
 android/hal-bluetooth.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 3dbc435..9f9814a 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -150,17 +150,14 @@ static void device_props_to_hal(bt_property_t *send_props,
 		{
 			static bt_remote_version_t e;
 			const struct hal_prop_device_info *p;
-			uint16_t tmp;
 
 			send_props[i].val = &e;
 			send_props[i].len = sizeof(e);
 
 			p = (struct hal_prop_device_info *) prop->val;
 
-			memcpy(&tmp, &p->manufacturer, sizeof(tmp));
-			e.manufacturer = tmp;
-			memcpy(&tmp, &p->sub_version, sizeof(tmp));
-			e.sub_ver = tmp;
+			e.manufacturer = p->manufacturer;
+			e.sub_ver = p->sub_version;
 			e.version = p->version;
 		}
 			break;
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCH_v7 1/5] bnep: Rename struct bnep_conn to bnep and vars for better readability
From: Luiz Augusto von Dentz @ 2013-12-20 12:56 UTC (permalink / raw)
  To: Ravi kumar Veeramally; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1387539653-7271-1-git-send-email-ravikumar.veeramally@linux.intel.com>

Hi Ravi,

On Fri, Dec 20, 2013 at 1:40 PM, Ravi kumar Veeramally
<ravikumar.veeramally@linux.intel.com> wrote:
> ---
>  profiles/network/bnep.c | 80 ++++++++++++++++++++++++-------------------------
>  1 file changed, 40 insertions(+), 40 deletions(-)
>
> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> index 08037e6..edf258b 100644
> --- a/profiles/network/bnep.c
> +++ b/profiles/network/bnep.c
> @@ -67,7 +67,7 @@ struct __service_16 {
>         uint16_t src;
>  } __attribute__ ((packed));
>
> -struct bnep_conn {
> +struct bnep {
>         GIOChannel      *io;
>         uint16_t        src;
>         uint16_t        dst;
> @@ -77,17 +77,17 @@ struct bnep_conn {
>         bnep_connect_cb conn_cb;
>  };
>
> -static void free_bnep_connect(struct bnep_conn *bc)
> +static void free_bnep_connect(struct bnep *session)
>  {
> -       if (!bc)
> +       if (!session)
>                 return;
>
> -       if (bc->io) {
> -               g_io_channel_unref(bc->io);
> -               bc->io = NULL;
> +       if (session->io) {
> +               g_io_channel_unref(session->io);
> +               session->io = NULL;
>         }
>
> -       g_free(bc);
> +       g_free(session);
>  }
>
>  uint16_t bnep_service_id(const char *svc)
> @@ -249,7 +249,7 @@ int bnep_if_down(const char *devname)
>  static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
>                                                                 gpointer data)
>  {
> -       struct bnep_conn *bc = data;
> +       struct bnep *session = data;
>         struct bnep_control_rsp *rsp;
>         struct timeval timeo;
>         char pkt[BNEP_MTU];
> @@ -260,9 +260,9 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
>         if (cond & G_IO_NVAL)
>                 goto failed;
>
> -       if (bc->setup_to > 0) {
> -               g_source_remove(bc->setup_to);
> -               bc->setup_to = 0;
> +       if (session->setup_to > 0) {
> +               g_source_remove(session->setup_to);
> +               session->setup_to = 0;
>         }
>
>         if (cond & (G_IO_HUP | G_IO_ERR)) {
> @@ -309,8 +309,8 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
>         timeo.tv_sec = 0;
>         setsockopt(sk, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo));
>
> -       sk = g_io_channel_unix_get_fd(bc->io);
> -       if (bnep_connadd(sk, bc->src, iface)) {
> +       sk = g_io_channel_unix_get_fd(session->io);
> +       if (bnep_connadd(sk, session->src, iface)) {
>                 error("bnep conn could not be added");
>                 goto failed;
>         }
> @@ -320,19 +320,19 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
>                 goto failed;
>         }
>
> -       bc->conn_cb(chan, iface, 0, bc->data);
> -       free_bnep_connect(bc);
> +       session->conn_cb(chan, iface, 0, session->data);
> +       free_bnep_connect(session);
>
>         return FALSE;
>
>  failed:
> -       bc->conn_cb(NULL, NULL, -EIO, bc->data);
> -       free_bnep_connect(bc);
> +       session->conn_cb(NULL, NULL, -EIO, session->data);
> +       free_bnep_connect(session);
>
>         return FALSE;
>  }
>
> -static int bnep_setup_conn_req(struct bnep_conn *bc)
> +static int bnep_setup_conn_req(struct bnep *session)
>  {
>         struct bnep_setup_conn_req *req;
>         struct __service_16 *s;
> @@ -345,34 +345,34 @@ static int bnep_setup_conn_req(struct bnep_conn *bc)
>         req->ctrl = BNEP_SETUP_CONN_REQ;
>         req->uuid_size = 2;     /* 16bit UUID */
>         s = (void *) req->service;
> -       s->src = htons(bc->src);
> -       s->dst = htons(bc->dst);
> +       s->src = htons(session->src);
> +       s->dst = htons(session->dst);
>
> -       fd = g_io_channel_unix_get_fd(bc->io);
> +       fd = g_io_channel_unix_get_fd(session->io);
>         if (write(fd, pkt, sizeof(*req) + sizeof(*s)) < 0) {
>                 error("bnep connection req send failed: %s", strerror(errno));
>                 return -errno;
>         }
>
> -       bc->attempts++;
> +       session->attempts++;
>
>         return 0;
>  }
>
>  static gboolean bnep_conn_req_to(gpointer user_data)
>  {
> -       struct bnep_conn *bc = user_data;
> +       struct bnep *session = user_data;
>
> -       if (bc->attempts == CON_SETUP_RETRIES) {
> +       if (session->attempts == CON_SETUP_RETRIES) {
>                 error("Too many bnep connection attempts");
>         } else {
>                 error("bnep connection setup TO, retrying...");
> -               if (bnep_setup_conn_req(bc) == 0)
> +               if (bnep_setup_conn_req(session) == 0)
>                         return TRUE;
>         }
>
> -       bc->conn_cb(NULL, NULL, -ETIMEDOUT, bc->data);
> -       free_bnep_connect(bc);
> +       session->conn_cb(NULL, NULL, -ETIMEDOUT, session->data);
> +       free_bnep_connect(session);
>
>         return FALSE;
>  }
> @@ -380,28 +380,28 @@ static gboolean bnep_conn_req_to(gpointer user_data)
>  int bnep_connect(int sk, uint16_t src, uint16_t dst, bnep_connect_cb conn_cb,
>                                                                 void *data)
>  {
> -       struct bnep_conn *bc;
> +       struct bnep *session;
>         int err;
>
>         if (!conn_cb)
>                 return -EINVAL;
>
> -       bc = g_new0(struct bnep_conn, 1);
> -       bc->io = g_io_channel_unix_new(sk);
> -       bc->attempts = 0;
> -       bc->src = src;
> -       bc->dst = dst;
> -       bc->conn_cb = conn_cb;
> -       bc->data = data;
> +       session = g_new0(struct bnep, 1);
> +       session->io = g_io_channel_unix_new(sk);
> +       session->attempts = 0;
> +       session->src = src;
> +       session->dst = dst;
> +       session->conn_cb = conn_cb;
> +       session->data = data;
>
> -       err = bnep_setup_conn_req(bc);
> +       err = bnep_setup_conn_req(session);
>         if (err < 0)
>                 return err;
>
> -       bc->setup_to = g_timeout_add_seconds(CON_SETUP_TO,
> -                                                       bnep_conn_req_to, bc);
> -       g_io_add_watch(bc->io, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
> -                                                       bnep_setup_cb, bc);
> +       session->setup_to = g_timeout_add_seconds(CON_SETUP_TO,
> +                                               bnep_conn_req_to, session);
> +       g_io_add_watch(session->io, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
> +                                                       bnep_setup_cb, session);
>         return 0;
>  }
>
> --
> 1.8.3.2

Pushed, thanks.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* [PATCH] android/tester: Add remote device found success test case
From: Jakub Tyszkowski @ 2013-12-20 12:57 UTC (permalink / raw)
  To: linux-bluetooth

Add checking for remote device being found.

---
 android/android-tester.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 100 insertions(+), 2 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 3fbcb2f..9a5824c 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -70,7 +70,8 @@ enum hal_bluetooth_callbacks_id {
 	ADAPTER_PROP_SERVICE_RECORD,
 	ADAPTER_PROP_BONDED_DEVICES,
 	ADAPTER_DISCOVERY_STATE_ON,
-	ADAPTER_DISCOVERY_STATE_OFF
+	ADAPTER_DISCOVERY_STATE_OFF,
+	REMOTE_DEVICE_FOUND
 };
 
 struct generic_data {
@@ -141,6 +142,32 @@ static void test_update_state(void)
 	tester_test_passed();
 }
 
+static void test_device_property(bt_property_t *property,
+			bt_property_type_t type, const void *value, int len)
+{
+	if (value == NULL) {
+		tester_warn("NULL property passed");
+		tester_test_failed();
+	}
+
+	if (property->type != type) {
+		tester_warn("Wrong remote property type %d, expected %d",
+							type, property->type);
+		tester_test_failed();
+	}
+
+	if (property->len != len) {
+		tester_warn("Wrong remote property len %d, expected %d",
+							len, property->len);
+		tester_test_failed();
+	}
+
+	if (memcmp(property->val, value, len)) {
+		tester_warn("Wrong remote property value");
+		tester_test_failed();
+	}
+}
+
 static void test_mgmt_settings_set(struct test_data *data)
 {
 	data->mgmt_settings_set = true;
@@ -567,6 +594,55 @@ static void discovery_state_changed_cb(bt_discovery_state_t state)
 	}
 }
 
+static void device_found_cb(int num_properties, bt_property_t *properties)
+{
+	struct test_data *data = tester_get_data();
+	const uint8_t *remote_bdaddr =
+					hciemu_get_client_bdaddr(data->hciemu);
+	const uint32_t emu_remote_type = BT_DEVICE_DEVTYPE_BREDR;
+	const int32_t emu_remote_rssi = -60;
+	bt_bdaddr_t emu_remote_bdaddr;
+	int i;
+
+	update_hal_cb_list(REMOTE_DEVICE_FOUND);
+
+	if (num_properties < 1)
+		tester_test_failed();
+
+	bdaddr2android((const bdaddr_t *) remote_bdaddr, &emu_remote_bdaddr);
+
+	for (i = 0; i < num_properties; i++) {
+		int prop_len;
+		const void *prop_data;
+
+		switch (properties[i].type) {
+		case BT_PROPERTY_BDADDR:
+			prop_len = sizeof(emu_remote_bdaddr);
+			prop_data = &emu_remote_bdaddr;
+
+			break;
+		case BT_PROPERTY_TYPE_OF_DEVICE:
+			prop_len = sizeof(emu_remote_type);
+			prop_data = &emu_remote_type;
+
+			break;
+		case BT_PROPERTY_REMOTE_RSSI:
+			prop_len = sizeof(emu_remote_rssi);
+			prop_data = &emu_remote_rssi;
+
+			break;
+		default:
+			prop_len = 0;
+			prop_data = NULL;
+
+			break;
+		}
+
+		test_device_property(&properties[i], properties[i].type,
+							prop_data, prop_len);
+	}
+}
+
 static void adapter_properties_cb(bt_status_t status, int num_properties,
 						bt_property_t *properties)
 {
@@ -765,12 +841,20 @@ static const struct generic_data bluetooth_discovery_stop_success_test = {
 	.expected_adapter_status = BT_STATUS_SUCCESS
 };
 
+static const struct generic_data bluetooth_discovery_device_found_test = {
+	.expected_hal_callbacks = { ADAPTER_DISCOVERY_STATE_ON,
+						REMOTE_DEVICE_FOUND,
+						ADAPTER_DISCOVERY_STATE_OFF,
+						ADAPTER_TEST_END },
+	.expected_adapter_status = BT_STATUS_NOT_EXPECTED
+};
+
 static bt_callbacks_t bt_callbacks = {
 	.size = sizeof(bt_callbacks),
 	.adapter_state_changed_cb = adapter_state_changed_cb,
 	.adapter_properties_cb = adapter_properties_cb,
 	.remote_device_properties_cb = NULL,
-	.device_found_cb = NULL,
+	.device_found_cb = device_found_cb,
 	.discovery_state_changed_cb = discovery_state_changed_cb,
 	.pin_request_cb = NULL,
 	.ssp_request_cb = NULL,
@@ -1409,6 +1493,15 @@ static void test_discovery_start_done(const void *test_data)
 	data->if_bluetooth->start_discovery();
 }
 
+static void test_discovery_device_found(const void *test_data)
+{
+	struct test_data *data = tester_get_data();
+
+	init_test_conditions(data);
+
+	data->if_bluetooth->start_discovery();
+}
+
 static gboolean socket_chan_cb(GIOChannel *io, GIOCondition cond,
 							gpointer user_data)
 {
@@ -1600,6 +1693,11 @@ int main(int argc, char *argv[])
 				setup_enabled_adapter,
 				test_discovery_stop_success, teardown);
 
+	test_bredrle("Bluetooth BREDR Discovery Device Found",
+				&bluetooth_discovery_device_found_test,
+				setup_enabled_adapter,
+				test_discovery_device_found, teardown);
+
 	test_bredrle("Socket Init", NULL, setup_socket_interface,
 						test_dummy, teardown);
 
-- 
1.8.5


^ permalink raw reply related

* Re: [PATCH] android: Include btmon and l2test in userdebug builds
From: Szymon Janc @ 2013-12-20 12:58 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1387542919-16070-1-git-send-email-andrzej.kaczmarek@tieto.com>

Hi Andrzej,

> This patch changes btmon and l2test module tags from 'eng' to 'debug' so
> they are automatically installed also for userdebug variant which is
> default for AOSP devices.
> 
> ---
>  android/Android.mk | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/android/Android.mk b/android/Android.mk
> index 2865c08..d9bae40 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -200,7 +200,7 @@ LOCAL_C_INCLUDES += \
>  LOCAL_CFLAGS := $(BLUEZ_COMMON_CFLAGS)
>  
>  LOCAL_MODULE_PATH := $(TARGET_OUT_OPTIONAL_EXECUTABLES)
> -LOCAL_MODULE_TAGS := eng
> +LOCAL_MODULE_TAGS := debug
>  LOCAL_MODULE := btmon
>  
>  include $(BUILD_EXECUTABLE)
> @@ -247,7 +247,7 @@ LOCAL_C_INCLUDES := \
>  LOCAL_CFLAGS := $(BLUEZ_COMMON_CFLAGS)
>  
>  LOCAL_MODULE_PATH := $(TARGET_OUT_OPTIONAL_EXECUTABLES)
> -LOCAL_MODULE_TAGS := eng
> +LOCAL_MODULE_TAGS := debug
>  LOCAL_MODULE := l2test
>  
>  include $(BUILD_EXECUTABLE)
> 

Applied, thanks.

-- 
BR
Szymon Janc

^ permalink raw reply

* Re: [PATCH BlueZ] android/AVDTP: Duplicate fd passed to avdtp_new
From: Marcel Holtmann @ 2013-12-20 13:16 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1387533387-6724-1-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

> This use dup to create a new fd to be used by AVDTP session leaving the
> caller free to close the original fd. Note that even if the caller
> decides to keep the original fd it will still be notified when
> avdtp_shutdown is called since it uses shutdown.

I would be more curious on why this is needed.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 4/5] android/hal-bluetooth: Add support for device service record property
From: Marcel Holtmann @ 2013-12-20 13:18 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1387535140-6223-5-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

> This allows to correctly handle device service record property.
> ---
> android/hal-bluetooth.c | 15 +++++++++++++++
> android/hal-msg.h       |  7 +++++++
> 2 files changed, 22 insertions(+)
> 
> diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> index d0c1c01..4ef2ebe 100644
> --- a/android/hal-bluetooth.c
> +++ b/android/hal-bluetooth.c
> @@ -166,6 +166,21 @@ static void device_props_to_hal(bt_property_t *send_props,
> 			break;
> #endif
> 		case HAL_PROP_DEVICE_SERVICE_REC:
> +		{
> +			static bt_service_record_t e;

why static here. Since we are essentially multi-threaded, this can be dangerous.

> +			const struct hal_prop_device_service_rec *p;
> +
> +			send_props[i].val = &e;
> +			send_props[i].len = sizeof(e);
> +
> +			p = (struct hal_prop_device_service_rec *) prop->val;
> +
> +			memset(&e, 0, sizeof(e));
> +			memcpy(&e.channel, &p->channel, sizeof(e.channel));
> +			memcpy(e.uuid.uu, p->uuid, sizeof(e.uuid.uu));
> +			memcpy(e.name, p->name, p->name_len);
> +		}
> +			break;
> 		default:
> 			send_props[i].len = prop->len;
> 			send_props[i].val = prop->val;

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 1/2] android/bluetooth-hal: Fix using wrong struct for buffer size
From: Johan Hedberg @ 2013-12-20 13:24 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1387543842-6489-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Fri, Dec 20, 2013, Szymon Janc wrote:
> Buffer is for hal_cmd_le_test_mode command.
> ---
>  android/hal-bluetooth.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Both patches have been applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] android/tester: Add remote device found success test case
From: Johan Hedberg @ 2013-12-20 13:28 UTC (permalink / raw)
  To: Jakub Tyszkowski; +Cc: linux-bluetooth
In-Reply-To: <1387544266-3159-1-git-send-email-jakub.tyszkowski@tieto.com>

Hi Jakub,

On Fri, Dec 20, 2013, Jakub Tyszkowski wrote:
> Add checking for remote device being found.
> 
> ---
>  android/android-tester.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 100 insertions(+), 2 deletions(-)
> 
> diff --git a/android/android-tester.c b/android/android-tester.c
> index 3fbcb2f..9a5824c 100644
> --- a/android/android-tester.c
> +++ b/android/android-tester.c
> @@ -70,7 +70,8 @@ enum hal_bluetooth_callbacks_id {
>  	ADAPTER_PROP_SERVICE_RECORD,
>  	ADAPTER_PROP_BONDED_DEVICES,
>  	ADAPTER_DISCOVERY_STATE_ON,
> -	ADAPTER_DISCOVERY_STATE_OFF
> +	ADAPTER_DISCOVERY_STATE_OFF,
> +	REMOTE_DEVICE_FOUND
>  };

Just add the ',' even to the last entry so subsequent additions become
more readable in patch form. I fixed this myself this time and applied
the patch.

Johan

^ permalink raw reply

* Re: [PATCH] Bluetooth: hidp: make sure input buffers are big enough
From: Marcel Holtmann @ 2013-12-20 13:36 UTC (permalink / raw)
  To: David Herrmann
  Cc: linux-input, Jiri Kosina,
	linux-bluetooth@vger.kernel.org development, Gustavo F. Padovan
In-Reply-To: <1387451372-6881-1-git-send-email-dh.herrmann@gmail.com>

Hi David,

> HID core expects the input buffers to be at least of size 4096
> (HID_MAX_BUFFER_SIZE). Other sizes will result in buffer-overflows if an
> input-report is smaller than advertised. We could, like i2c, compute the
> biggest report-size instead of using HID_MAX_BUFFER_SIZE, but this will
> blow up if report-descriptors are changed after ->start() has been called.
> So lets be safe and just use the biggest buffer we have.
> 
> Note that this adds an additional copy to the HIDP input path. If there is
> a way to make sure the skb-buf is big enough, we should use that instead.
> 
> The best way would be to make hid-core honor the @size argument, though,
> that sounds easier than it is. So lets just fix the buffer-overflows for
> now and afterwards look for a faster way for all transport drivers.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
> Hi
> 
> Any ideas how to improve this patch? I'd like to avoid the extra copy but I have
> no clue how the skb stuff works exactly.

the buffers are coming straight from L2CAP. They might come in fragments. We have to reassemble them and while there are large packets for sure, we rather not want to allocate 4096 for every reassembled packet to make HID happy.

I am not super familiar with the underlying memory management of SKBs, but I assume they use slices of some sort internally. So uses large SKBs for 20 byte HID report will cause an issue with all other protocols running on top of L2CAP>

> I also haven't figured out a nice way to make HID-core honor the "size"
> parameter. hid-input depends on getting the whole input-report.

I think this needs clearly fixing.

> Comments welcome!
> David
> 
> net/bluetooth/hidp/core.c | 16 ++++++++++++++--
> net/bluetooth/hidp/hidp.h |  4 ++++
> 2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 292e619..d9fb934 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -430,6 +430,16 @@ static void hidp_del_timer(struct hidp_session *session)
> 		del_timer(&session->timer);
> }
> 
> +static void hidp_process_report(struct hidp_session *session,
> +				int type, const u8 *data, int len, int intr)
> +{
> +	if (len > HID_MAX_BUFFER_SIZE)
> +		len = HID_MAX_BUFFER_SIZE;
> +
> +	memcpy(session->input_buf, data, len);
> +	hid_input_report(session->hid, type, session->input_buf, len, intr);
> +}
> +
> static void hidp_process_handshake(struct hidp_session *session,
> 					unsigned char param)
> {
> @@ -502,7 +512,8 @@ static int hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
> 			hidp_input_report(session, skb);
> 
> 		if (session->hid)
> -			hid_input_report(session->hid, HID_INPUT_REPORT, skb->data, skb->len, 0);
> +			hidp_process_report(session, HID_INPUT_REPORT,
> +					    skb->data, skb->len, 0);
> 		break;
> 
> 	case HIDP_DATA_RTYPE_OTHER:
> @@ -584,7 +595,8 @@ static void hidp_recv_intr_frame(struct hidp_session *session,
> 			hidp_input_report(session, skb);
> 
> 		if (session->hid) {
> -			hid_input_report(session->hid, HID_INPUT_REPORT, skb->data, skb->len, 1);
> +			hidp_process_report(session, HID_INPUT_REPORT,
> +					    skb->data, skb->len, 1);
> 			BT_DBG("report len %d", skb->len);
> 		}
> 	} else {
> diff --git a/net/bluetooth/hidp/hidp.h b/net/bluetooth/hidp/hidp.h
> index ab52414..8798492 100644
> --- a/net/bluetooth/hidp/hidp.h
> +++ b/net/bluetooth/hidp/hidp.h
> @@ -24,6 +24,7 @@
> #define __HIDP_H
> 
> #include <linux/types.h>
> +#include <linux/hid.h>
> #include <linux/kref.h>
> #include <net/bluetooth/bluetooth.h>
> #include <net/bluetooth/l2cap.h>
> @@ -179,6 +180,9 @@ struct hidp_session {
> 
> 	/* Used in hidp_output_raw_report() */
> 	int output_report_success; /* boolean */
> +
> +	/* temporary input buffer */
> +	u8 input_buf[HID_MAX_BUFFER_SIZE];
> };

The report does not need any specific alignment?

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 4/5] android/hal-bluetooth: Add support for device service record property
From: Szymon Janc @ 2013-12-20 13:38 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <D18535D5-351D-4C61-9D7E-6486E8558859@holtmann.org>

Hi Marcel,

> Hi Szymon,
> 
> > This allows to correctly handle device service record property.
> > ---
> > android/hal-bluetooth.c | 15 +++++++++++++++
> > android/hal-msg.h       |  7 +++++++
> > 2 files changed, 22 insertions(+)
> > 
> > diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> > index d0c1c01..4ef2ebe 100644
> > --- a/android/hal-bluetooth.c
> > +++ b/android/hal-bluetooth.c
> > @@ -166,6 +166,21 @@ static void device_props_to_hal(bt_property_t *send_props,
> > 			break;
> > #endif
> > 		case HAL_PROP_DEVICE_SERVICE_REC:
> > +		{
> > +			static bt_service_record_t e;
> 
> why static here. Since we are essentially multi-threaded, this can be dangerous.

This is executed only in notification thread context, so not multi-threading here.
(yes, commit message could be better)

-- 
BR
Szymon Janc

^ permalink raw reply

* Re: [PATCH 4/5] android/hal-bluetooth: Add support for device service record property
From: Marcel Holtmann @ 2013-12-20 13:43 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <3121104.trYEc2PX04@uw000953>

Hi Szymon,

>>> This allows to correctly handle device service record property.
>>> ---
>>> android/hal-bluetooth.c | 15 +++++++++++++++
>>> android/hal-msg.h       |  7 +++++++
>>> 2 files changed, 22 insertions(+)
>>> 
>>> diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
>>> index d0c1c01..4ef2ebe 100644
>>> --- a/android/hal-bluetooth.c
>>> +++ b/android/hal-bluetooth.c
>>> @@ -166,6 +166,21 @@ static void device_props_to_hal(bt_property_t *send_props,
>>> 			break;
>>> #endif
>>> 		case HAL_PROP_DEVICE_SERVICE_REC:
>>> +		{
>>> +			static bt_service_record_t e;
>> 
>> why static here. Since we are essentially multi-threaded, this can be dangerous.
> 
> This is executed only in notification thread context, so not multi-threading here.
> (yes, commit message could be better)

and what is wrong with having it on the stack.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH BlueZ] android/AVDTP: Duplicate fd passed to avdtp_new
From: Luiz Augusto von Dentz @ 2013-12-20 13:44 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <6E8A8CD1-5CB6-4D65-9BD7-DF4CF23F274C@holtmann.org>

Hi Marcel,

On Fri, Dec 20, 2013 at 3:16 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> This use dup to create a new fd to be used by AVDTP session leaving the
>> caller free to close the original fd. Note that even if the caller
>> decides to keep the original fd it will still be notified when
>> avdtp_shutdown is called since it uses shutdown.
>
> I would be more curious on why this is needed.

It is basically to make the caller able to release any resources, such
as GIOChannel managed by btio, without causing a disconnect.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH BlueZ] android/AVDTP: Duplicate fd passed to avdtp_new
From: Marcel Holtmann @ 2013-12-20 13:52 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <CABBYNZKcsaSSEXLV+gQuxXMzu8ePCyiyU5cJcNQ8rTjzsP-PUg@mail.gmail.com>

Hi Luiz,

>>> This use dup to create a new fd to be used by AVDTP session leaving the
>>> caller free to close the original fd. Note that even if the caller
>>> decides to keep the original fd it will still be notified when
>>> avdtp_shutdown is called since it uses shutdown.
>> 
>> I would be more curious on why this is needed.
> 
> It is basically to make the caller able to release any resources, such
> as GIOChannel managed by btio, without causing a disconnect.

you do not have to set close_on_unref with the GIOChannel. You can just unset that one. Then GIOChannel will not touch it and will not call close on it.

Regards

Marcel


^ permalink raw reply

* [PATCH] android/pan: Add pan sdp record for NAP role
From: Ravi kumar Veeramally @ 2013-12-20 13:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ravi kumar Veeramally

---
 android/pan.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/android/pan.c b/android/pan.c
index 689c7ef..f322f20 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -44,9 +44,12 @@
 #include "utils.h"
 #include "bluetooth.h"
 
+#define SVC_HINT_CAPTURING 0x08
+
 static bdaddr_t adapter_addr;
 GSList *devices = NULL;
 uint8_t local_role = HAL_PAN_ROLE_NONE;
+static uint32_t record_id = 0;
 
 struct pan_device {
 	char		iface[16];
@@ -335,20 +338,106 @@ static const struct ipc_handler cmd_handlers[] = {
 	{ bt_pan_disconnect, false, sizeof(struct hal_cmd_pan_disconnect) },
 };
 
+static sdp_record_t *pan_record(void)
+{
+	sdp_list_t *svclass, *pfseq, *apseq, *root, *aproto;
+	uuid_t root_uuid, pan, l2cap, bnep;
+	sdp_profile_desc_t profile[1];
+	sdp_list_t *proto[2];
+	sdp_data_t *v, *p;
+	uint16_t psm = BNEP_PSM, version = 0x0100;
+	uint16_t security = 0x0001, type = 0xfffe;
+	uint32_t rate = 0;
+	const char *desc = "Network service", *name = "bnep";
+	sdp_record_t *record;
+	uint16_t ptype[] = { 0x0800, /* IPv4 */ 0x0806,  /* ARP */ };
+	sdp_data_t *head, *pseq, *data;
+
+	record = sdp_record_alloc();
+	if (!record)
+		return NULL;
+
+	record->attrlist = NULL;
+	record->pattern = NULL;
+
+	sdp_uuid16_create(&pan, NAP_SVCLASS_ID);
+	svclass = sdp_list_append(NULL, &pan);
+	sdp_set_service_classes(record, svclass);
+	sdp_uuid16_create(&profile[0].uuid, NAP_PROFILE_ID);
+	profile[0].version = 0x0100;
+	pfseq = sdp_list_append(NULL, &profile[0]);
+	sdp_set_profile_descs(record, pfseq);
+	sdp_set_info_attr(record, name, NULL, desc);
+	sdp_attr_add_new(record, SDP_ATTR_NET_ACCESS_TYPE, SDP_UINT16, &type);
+	sdp_attr_add_new(record, SDP_ATTR_MAX_NET_ACCESSRATE,
+							SDP_UINT32, &rate);
+	sdp_uuid16_create(&root_uuid, PUBLIC_BROWSE_GROUP);
+	root = sdp_list_append(NULL, &root_uuid);
+	sdp_set_browse_groups(record, root);
+	sdp_uuid16_create(&l2cap, L2CAP_UUID);
+	proto[0] = sdp_list_append(NULL, &l2cap);
+	p = sdp_data_alloc(SDP_UINT16, &psm);
+	proto[0] = sdp_list_append(proto[0], p);
+	apseq = sdp_list_append(NULL, proto[0]);
+	sdp_uuid16_create(&bnep, BNEP_UUID);
+	proto[1] = sdp_list_append(NULL, &bnep);
+	v = sdp_data_alloc(SDP_UINT16, &version);
+	proto[1] = sdp_list_append(proto[1], v);
+
+	head = sdp_data_alloc(SDP_UINT16, &ptype[0]);
+	data = sdp_data_alloc(SDP_UINT16, &ptype[1]);
+	sdp_seq_append(head, data);
+
+	pseq = sdp_data_alloc(SDP_SEQ16, head);
+	proto[1] = sdp_list_append(proto[1], pseq);
+	apseq = sdp_list_append(apseq, proto[1]);
+	aproto = sdp_list_append(NULL, apseq);
+	sdp_set_access_protos(record, aproto);
+	sdp_add_lang_attr(record);
+	sdp_attr_add_new(record, SDP_ATTR_SECURITY_DESC, SDP_UINT16, &security);
+
+	sdp_data_free(p);
+	sdp_data_free(v);
+	sdp_list_free(apseq, NULL);
+	sdp_list_free(root, NULL);
+	sdp_list_free(aproto, NULL);
+	sdp_list_free(proto[0], NULL);
+	sdp_list_free(proto[1], NULL);
+	sdp_list_free(svclass, NULL);
+	sdp_list_free(pfseq, NULL);
+
+	return record;
+}
+
 bool bt_pan_register(const bdaddr_t *addr)
 {
+	sdp_record_t *rec;
 	int err;
 
 	DBG("");
 
 	bacpy(&adapter_addr, addr);
 
+	rec = pan_record();
+	if (!rec) {
+		error("Failed to allocate PAN record");
+		return false;
+	}
+
+	if (bt_adapter_add_record(rec, SVC_HINT_CAPTURING) < 0) {
+		error("Failed to register PAN record");
+		sdp_record_free(rec);
+		return false;
+	}
+
 	err = bnep_init();
 	if (err) {
 		error("bnep init failed");
+		sdp_record_free(rec);
 		return false;
 	}
 
+	record_id = rec->handle;
 	ipc_register(HAL_SERVICE_ID_PAN, cmd_handlers,
 						G_N_ELEMENTS(cmd_handlers));
 
@@ -362,4 +451,6 @@ void bt_pan_unregister(void)
 	bnep_cleanup();
 
 	ipc_unregister(HAL_SERVICE_ID_PAN);
+	bt_adapter_remove_record(record_id);
+	record_id = 0;
 }
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCH 4/5] android/hal-bluetooth: Add support for device service record property
From: Szymon Janc @ 2013-12-20 14:00 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <FE09C777-868D-4FBB-99BF-739CA67C603E@holtmann.org>

Hi Marcel,

> Hi Szymon,
> 
> >>> This allows to correctly handle device service record property.
> >>> ---
> >>> android/hal-bluetooth.c | 15 +++++++++++++++
> >>> android/hal-msg.h       |  7 +++++++
> >>> 2 files changed, 22 insertions(+)
> >>> 
> >>> diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> >>> index d0c1c01..4ef2ebe 100644
> >>> --- a/android/hal-bluetooth.c
> >>> +++ b/android/hal-bluetooth.c
> >>> @@ -166,6 +166,21 @@ static void device_props_to_hal(bt_property_t *send_props,
> >>> 			break;
> >>> #endif
> >>> 		case HAL_PROP_DEVICE_SERVICE_REC:
> >>> +		{
> >>> +			static bt_service_record_t e;
> >> 
> >> why static here. Since we are essentially multi-threaded, this can be dangerous.
> > 
> > This is executed only in notification thread context, so not multi-threading here.
> > (yes, commit message could be better)
> 
> and what is wrong with having it on the stack.

This function is used to convert from what we get on IPC to HAL, so that data
needs to be also accessible outside of this function scope (so Java can copy it).
Since we won't get more than 1 prop of each type in single notification this is
safe and allows for simpler code.

-- 
BR
Szymon Janc

^ permalink raw reply

* Re: [PATCH BlueZ] android/AVDTP: Duplicate fd passed to avdtp_new
From: Luiz Augusto von Dentz @ 2013-12-20 14:09 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <68A01E8B-5BF3-41AD-AF79-0DF619566069@holtmann.org>

Hi Marcel,

On Fri, Dec 20, 2013 at 3:52 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>>>> This use dup to create a new fd to be used by AVDTP session leaving the
>>>> caller free to close the original fd. Note that even if the caller
>>>> decides to keep the original fd it will still be notified when
>>>> avdtp_shutdown is called since it uses shutdown.
>>>
>>> I would be more curious on why this is needed.
>>
>> It is basically to make the caller able to release any resources, such
>> as GIOChannel managed by btio, without causing a disconnect.
>
> you do not have to set close_on_unref with the GIOChannel. You can just unset that one. Then GIOChannel will not touch it and will not call close on it.

That is what we used to do, but it is not very convenient since there
it is still possible to call g_io_channel_shutdown which would cause a
disconnect. Also dup turn out to be handful to check if the fd is
valid and free the user to do whatever it pleases with the original
fd, in the end what we are doing with the fd is very similar to what
we would do to a GIOChannel as dup just creates another reference the
underline socket is the same.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH BlueZ] android/AVDTP: Duplicate fd passed to avdtp_new
From: Marcel Holtmann @ 2013-12-20 14:39 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <CABBYNZLm6kvHuNJsCwN57CQraWs8pgmQviWP2_LySH2c=284Lg@mail.gmail.com>

Hi Luiz,

>>>>> This use dup to create a new fd to be used by AVDTP session leaving the
>>>>> caller free to close the original fd. Note that even if the caller
>>>>> decides to keep the original fd it will still be notified when
>>>>> avdtp_shutdown is called since it uses shutdown.
>>>> 
>>>> I would be more curious on why this is needed.
>>> 
>>> It is basically to make the caller able to release any resources, such
>>> as GIOChannel managed by btio, without causing a disconnect.
>> 
>> you do not have to set close_on_unref with the GIOChannel. You can just unset that one. Then GIOChannel will not touch it and will not call close on it.
> 
> That is what we used to do, but it is not very convenient since there
> it is still possible to call g_io_channel_shutdown which would cause a
> disconnect. Also dup turn out to be handful to check if the fd is
> valid and free the user to do whatever it pleases with the original
> fd, in the end what we are doing with the fd is very similar to what
> we would do to a GIOChannel as dup just creates another reference the
> underline socket is the same.

if the original caller calls g_io_channel_shutdown it will still cause a disconnect. No matter if you duped the fd or not. Not getting your point. If the caller is doing something stupid, then you have a problem not matter what.

This duped fd is different from everything else we have in src/shared/ in any of the projects. You hand over the fd and then it is owned by that part. After that you are not suppose to touch it anymore. That is the semantic that I want. And not another complicated concept of reference counting fd in multiple levels of the code.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH BlueZ] android/AVDTP: Duplicate fd passed to avdtp_new
From: Luiz Augusto von Dentz @ 2013-12-20 15:40 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <F4C8A536-4F92-42DF-8492-B81330118461@holtmann.org>

Hi Marcel,

On Fri, Dec 20, 2013 at 4:39 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Luiz,
>
>>>>>> This use dup to create a new fd to be used by AVDTP session leaving =
the
>>>>>> caller free to close the original fd. Note that even if the caller
>>>>>> decides to keep the original fd it will still be notified when
>>>>>> avdtp_shutdown is called since it uses shutdown.
>>>>>
>>>>> I would be more curious on why this is needed.
>>>>
>>>> It is basically to make the caller able to release any resources, such
>>>> as GIOChannel managed by btio, without causing a disconnect.
>>>
>>> you do not have to set close_on_unref with the GIOChannel. You can just=
 unset that one. Then GIOChannel will not touch it and will not call close =
on it.
>>
>> That is what we used to do, but it is not very convenient since there
>> it is still possible to call g_io_channel_shutdown which would cause a
>> disconnect. Also dup turn out to be handful to check if the fd is
>> valid and free the user to do whatever it pleases with the original
>> fd, in the end what we are doing with the fd is very similar to what
>> we would do to a GIOChannel as dup just creates another reference the
>> underline socket is the same.
>
> if the original caller calls g_io_channel_shutdown it will still cause a =
disconnect. No matter if you duped the fd or not. Not getting your point. I=
f the caller is doing something stupid, then you have a problem not matter =
what.

Nope, g_io_channel_shutdown does not call shutdown, it only does close
and therefore do not affect in case of duped fd. The point is
convenience, having same code path regardless if the fd has been hand
over or not.

> This duped fd is different from everything else we have in src/shared/ in=
 any of the projects. You hand over the fd and then it is owned by that par=
t. After that you are not suppose to touch it anymore. That is the semantic=
 that I want. And not another complicated concept of reference counting fd =
in multiple levels of the code.

Well I don't think we had defined any semantic for fd ownership,
mgmt_new is the only one that actually have such a thing but it does
not work with GIOChannel/btio handling the connection setup, and if
you look at other examples like libdbus it does make use of dup when
passing fds around (and no this is not necessary with SCM_RIGTHS since
the kernel does take a reference while transferring the fd to another
process).

We could perhaps do the close_on_unref function similar to the mgmt,
which perhaps was inspired in the GIOChannel that also take ownership
and thus requires close_on_unref helper function to instrument how it
should deal with the fd, imo dup is probably less code since you can
always close on unref.

--=20
Luiz Augusto von Dentz

^ permalink raw reply

* Unable to use Sony Dualshock 4 game controller
From: Christopher Rosell @ 2013-12-20 15:43 UTC (permalink / raw)
  To: linux-bluetooth

Hello there,

I'm attempting to use a Sony Dualshock 4 game controller with bluez,
but have run into some problems. I'm running Linux kernel 3.12.4 on
Arch Linux with bluez 5.12.

The DS4 is a standard bluetooth device, unlike the DS3 and I was able
to pair and use it without any problems in Windows 7, but in Linux I
have not been able to get it working correctly. The controller can be
started in two modes, a pairing mode by pressing the buttons Share and
PS at the same time and a normal mode by pressing the PS button only.

The pairing appears to work correctly but once it connects I get these
errors and then the connection is dropped:

Dec 20 16:08:01 locutus bluetoothd[17163]: Refusing input device
connect: No such file or directory (2)
Dec 20 16:08:01 locutus bluetoothd[17163]: Refusing connection from
A4:15:66:73:03:F3: unknown device
Dec 20 16:08:01 locutus bluetoothd[17163]: A4:15:66:73:03:F3: error
updating services: Input/output error (5)


I used bluetoothctl to do the pairing, here is the log:

[root@locutus chrippa]# bluetoothctl -a
[NEW] Controller 00:26:83:30:B7:CB locutus [default]
Agent registered
[bluetooth]# power on
Changing power on succeeded
[bluetooth]# scan on
Discovery started
[CHG] Controller 00:26:83:30:B7:CB Discovering: yes
[NEW] Device 00:3C:7F:F0:F0:0A 00-3C-7F-F0-F0-0A
[NEW] Device A4:15:66:73:03:F3 Wireless Controller
[bluetooth]# pair A4:15:66:73:03:F3
Attempting to pair with A4:15:66:73:03:F3
[CHG] Device A4:15:66:73:03:F3 Connected: yes
[CHG] Device A4:15:66:73:03:F3 Paired: yes
Pairing successful
[CHG] Device A4:15:66:73:03:F3 Connected: no


Later, when attempting to start the controller in normal mode, I get
the first two errors only:

Dec 20 16:19:00 locutus bluetoothd[17163]: Refusing input device
connect: No such file or directory (2)
Dec 20 16:19:00 locutus bluetoothd[17163]: Refusing connection from
A4:15:66:73:03:F3: unknown device



Here is also the full debug log:


Dec 20 16:31:36 locutus bluetoothd[24893]: src/agent.c:agent_ref()
0x1d987a0: ref=1
Dec 20 16:31:36 locutus bluetoothd[24893]:
src/agent.c:register_agent() agent :1.33
Dec 20 16:31:39 locutus bluetoothd[24893]:
src/adapter.c:property_set_mode() sending Set Powered command for
index 0
Dec 20 16:31:40 locutus bluetoothd[24893]:
src/adapter.c:dev_class_changed_callback() Class: 0x0c0104
Dec 20 16:31:40 locutus bluetoothd[24893]:
src/attrib-server.c:attrib_db_update() handle=0x0008
Dec 20 16:31:40 locutus bluetoothd[24893]:
src/adapter.c:property_set_mode_complete() Success (0x00)
Dec 20 16:31:40 locutus bluetoothd[24893]:
src/adapter.c:new_settings_callback() Settings: 0x000000d3
Dec 20 16:31:40 locutus bluetoothd[24893]:
src/adapter.c:settings_changed() Changed settings: 0x00000001
Dec 20 16:31:40 locutus bluetoothd[24893]:
src/adapter.c:adapter_start() adapter /org/bluez/hci0 has been enabled
Dec 20 16:31:53 locutus bluetoothd[24893]:
src/adapter.c:start_discovery() sender :1.33
Dec 20 16:31:53 locutus bluetoothd[24893]:
src/adapter.c:trigger_start_discovery()
Dec 20 16:31:53 locutus bluetoothd[24893]:
src/adapter.c:start_discovery_timeout()
Dec 20 16:31:53 locutus bluetoothd[24893]:
src/adapter.c:start_discovery_complete() status 0x00
Dec 20 16:31:53 locutus bluetoothd[24893]:
src/adapter.c:discovering_callback() hci0 type 1 discovering 1
Dec 20 16:31:59 locutus bluetoothd[24893]:
src/adapter.c:device_found_callback() hci0 addr A4:15:66:73:03:F3,
rssi -75 flags 0x0000 eir_len 42
Dec 20 16:31:59 locutus bluetoothd[24893]:
src/device.c:device_create() dst A4:15:66:73:03:F3
Dec 20 16:31:59 locutus bluetoothd[24893]: src/device.c:device_new()
address A4:15:66:73:03:F3
Dec 20 16:31:59 locutus bluetoothd[24893]: src/device.c:device_new()
Creating device /org/bluez/hci0/dev_A4_15_66_73_03_F3
Dec 20 16:31:59 locutus bluetoothd[24893]:
src/device.c:btd_device_set_temporary() temporary 1
Dec 20 16:31:59 locutus bluetoothd[24893]:
src/adapter.c:adapter_connect_list_remove() device
/org/bluez/hci0/dev_A4_15_66_73_03_F3 is not on the list, ignoring
Dec 20 16:31:59 locutus bluetoothd[24893]:
src/device.c:device_set_legacy() legacy 0
Dec 20 16:31:59 locutus bluetoothd[24893]:
src/device.c:device_set_rssi() rssi -75
Dec 20 16:31:59 locutus bluetoothd[24893]:
src/device.c:device_set_class() /org/bluez/hci0/dev_A4_15_66_73_03_F3
0x002508
Dec 20 16:32:03 locutus bluetoothd[24893]:
src/adapter.c:device_found_callback() hci0 addr A4:15:66:73:03:F3,
rssi -73 flags 0x0000 eir_len 42
Dec 20 16:32:03 locutus bluetoothd[24893]:
src/device.c:device_set_legacy() legacy 0
Dec 20 16:32:03 locutus bluetoothd[24893]:
src/adapter.c:discovering_callback() hci0 type 1 discovering 0
Dec 20 16:32:03 locutus bluetoothd[24893]:
src/adapter.c:trigger_start_discovery()
Dec 20 16:32:07 locutus bluetoothd[24893]:
src/device.c:btd_device_set_temporary() temporary 0
Dec 20 16:32:07 locutus bluetoothd[24893]: src/agent.c:agent_ref()
0x1d987a0: ref=2
Dec 20 16:32:07 locutus bluetoothd[24893]:
src/device.c:bonding_request_new() Requesting bonding for
A4:15:66:73:03:F3
Dec 20 16:32:07 locutus bluetoothd[24893]: src/agent.c:agent_ref()
0x1d987a0: ref=3
Dec 20 16:32:07 locutus bluetoothd[24893]: src/agent.c:agent_unref()
0x1d987a0: ref=2
Dec 20 16:32:07 locutus bluetoothd[24893]: src/adapter.c:suspend_discovery()
Dec 20 16:32:07 locutus bluetoothd[24893]:
src/adapter.c:adapter_bonding_attempt() hci0 bdaddr A4:15:66:73:03:F3
type 0 io_cap 0x01
Dec 20 16:32:07 locutus bluetoothd[24893]:
src/adapter.c:connected_callback() hci0 device A4:15:66:73:03:F3
connected eir_len 26
Dec 20 16:32:10 locutus bluetoothd[24893]:
src/adapter.c:new_link_key_callback() hci0 new key for
A4:15:66:73:03:F3 type 5 pin_len 0
Dec 20 16:32:10 locutus bluetoothd[24893]:
src/device.c:device_set_bonded() bonded 1
Dec 20 16:32:10 locutus bluetoothd[24893]:
src/device.c:device_bonding_complete() bonding 0x1da0760 status 0x00
Dec 20 16:32:10 locutus bluetoothd[24893]:
src/device.c:device_bonding_complete() Proceeding with service
discovery
Dec 20 16:32:10 locutus bluetoothd[24893]: src/agent.c:agent_unref()
0x1d987a0: ref=1
Dec 20 16:32:10 locutus bluetoothd[24893]: src/adapter.c:resume_discovery()
Dec 20 16:32:10 locutus bluetoothd[24893]:
src/adapter.c:trigger_start_discovery()
Dec 20 16:32:10 locutus bluetoothd[24893]:
src/adapter.c:pair_device_complete() Success (0x00)
Dec 20 16:32:10 locutus bluetoothd[24893]:
src/adapter.c:bonding_attempt_complete() hci0 bdaddr A4:15:66:73:03:F3
type 0 status 0x0
Dec 20 16:32:10 locutus bluetoothd[24893]:
src/device.c:device_bonding_complete() bonding (nil) status 0x00
Dec 20 16:32:10 locutus bluetoothd[24893]: src/adapter.c:resume_discovery()
Dec 20 16:32:10 locutus bluetoothd[24893]:
src/adapter.c:trigger_start_discovery()
Dec 20 16:32:13 locutus bluetoothd[24893]:
profiles/input/server.c:connect_event_cb() Incoming connection from
A4:15:66:73:03:F3 on PSM 17
Dec 20 16:32:13 locutus bluetoothd[24893]:
profiles/input/device.c:input_device_set_channel() idev (nil) psm 17
Dec 20 16:32:13 locutus bluetoothd[24893]: Refusing input device
connect: No such file or directory (2)
Dec 20 16:32:13 locutus bluetoothd[24893]:
profiles/input/server.c:confirm_event_cb()
Dec 20 16:32:13 locutus bluetoothd[24893]: Refusing connection from
A4:15:66:73:03:F3: unknown device
Dec 20 16:32:13 locutus bluetoothd[24893]: A4:15:66:73:03:F3: error
updating services: Input/output error (5)
Dec 20 16:32:13 locutus bluetoothd[24893]:
src/device.c:device_svc_resolved()
/org/bluez/hci0/dev_A4_15_66_73_03_F3 err -5
Dec 20 16:32:14 locutus bluetoothd[24893]:
src/adapter.c:dev_disconnected() Device A4:15:66:73:03:F3
disconnected, reason 3
Dec 20 16:32:14 locutus bluetoothd[24893]:
src/adapter.c:adapter_remove_connection()
Dec 20 16:32:14 locutus bluetoothd[24893]:
src/adapter.c:bonding_attempt_complete() hci0 bdaddr A4:15:66:73:03:F3
type 0 status 0xe
Dec 20 16:32:14 locutus bluetoothd[24893]:
src/device.c:device_bonding_complete() bonding (nil) status 0x0e
Dec 20 16:32:14 locutus bluetoothd[24893]:
src/device.c:device_bonding_failed() status 14
Dec 20 16:32:14 locutus bluetoothd[24893]: src/adapter.c:resume_discovery()
Dec 20 16:32:14 locutus bluetoothd[24893]:
src/adapter.c:trigger_start_discovery()


--
Christopher Rosell

^ permalink raw reply

* [PATCH 1/4] android/bluetooth: Add support for timestamp device property
From: Szymon Janc @ 2013-12-20 18:30 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This allows to handle timestamp property request. Also this will be
usefull for devices info cache (clearing old devices).
---
 android/bluetooth.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index e534fc1..2e75864 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -80,6 +80,8 @@ struct device {
 	uint32_t class;
 	int32_t rssi;
 
+	uint32_t timestamp;
+
 	GSList *uuids;
 };
 
@@ -227,6 +229,8 @@ static void store_device_info(struct device *dev)
 	else
 		g_key_file_remove_key(key_file, addr, "Class", NULL);
 
+	g_key_file_set_integer(key_file, addr, "Timestamp", dev->timestamp);
+
 	if (dev->uuids) {
 		GSList *l;
 		int i;
@@ -291,6 +295,7 @@ static struct device *create_device(const bdaddr_t *bdaddr, uint8_t type)
 	bacpy(&dev->bdaddr, bdaddr);
 	dev->bdaddr_type = type;
 	dev->bond_state = HAL_BOND_STATE_NONE;
+	dev->timestamp = time(NULL);
 
 	/* use address for name, will be change if one is present
 	 * eg. in EIR or set by set_property. */
@@ -1004,6 +1009,8 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
 
 		ev->status = HAL_STATUS_SUCCESS;
 		bdaddr2android(bdaddr, ev->bdaddr);
+
+		dev->timestamp = time(NULL);
 	}
 
 	if (eir.class) {
@@ -1607,6 +1614,9 @@ static void create_device_from_info(GKeyFile *key_file, const char *peer)
 
 	dev->class = g_key_file_get_integer(key_file, peer, "Class", NULL);
 
+	dev->timestamp = g_key_file_get_integer(key_file, peer, "Timestamp",
+									NULL);
+
 	uuids = g_key_file_get_string_list(key_file, peer, "Services", NULL,
 									NULL);
 	if (uuids) {
@@ -2647,11 +2657,10 @@ static uint8_t get_device_version_info(struct device *dev)
 
 static uint8_t get_device_timestamp(struct device *dev)
 {
-	DBG("Not implemented");
-
-	/* TODO */
+	send_device_property(&dev->bdaddr, HAL_PROP_DEVICE_TIMESTAMP,
+				sizeof(dev->timestamp), &dev->timestamp);
 
-	return HAL_STATUS_FAILED;
+	return HAL_STATUS_SUCCESS;
 }
 
 static void handle_get_remote_device_props_cmd(const void *buf, uint16_t len)
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 2/4] android/bluetooth: Add support for DUT mode configure command
From: Szymon Janc @ 2013-12-20 18:30 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1387564210-6102-1-git-send-email-szymon.janc@tieto.com>

This allows to enable and disable DUT mode.
---
 android/bluetooth.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 2e75864..59f0810 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -50,6 +50,8 @@
 #include "utils.h"
 #include "bluetooth.h"
 
+#define DUT_MODE_FILE "/sys/kernel/debug/bluetooth/hci%u/dut_mode"
+
 #define DEVICE_ID_SOURCE	0x0002	/* USB */
 #define DEVICE_ID_VENDOR	0x1d6b	/* Linux Foundation */
 #define DEVICE_ID_PRODUCT	0x0247	/* BlueZ for Android */
@@ -2872,13 +2874,34 @@ failed:
 static void handle_dut_mode_conf_cmd(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_dut_mode_conf *cmd = buf;
+	char path[FILENAME_MAX];
+	uint8_t status;
+	int fd, ret;
 
 	DBG("enable %u", cmd->enable);
 
-	/* TODO */
+	snprintf(path, sizeof(path), DUT_MODE_FILE, adapter.index);
 
-	ipc_send_rsp(HAL_SERVICE_ID_BLUETOOTH, HAL_OP_DUT_MODE_CONF,
-							HAL_STATUS_FAILED);
+	fd = open(path, O_WRONLY);
+	if (fd < 0) {
+		status = HAL_STATUS_FAILED;
+		goto failed;
+	}
+
+	if (cmd->enable)
+		ret = write(fd, "1", sizeof("1"));
+	else
+		ret = write(fd, "0", sizeof("0"));
+
+	if (ret < 0)
+		status = HAL_STATUS_FAILED;
+	else
+		status = HAL_STATUS_SUCCESS;
+
+	close(fd);
+
+failed:
+	ipc_send_rsp(HAL_SERVICE_ID_BLUETOOTH, HAL_OP_DUT_MODE_CONF, status);
 }
 
 static void handle_dut_mode_send_cmd(const void *buf, uint16_t len)
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 3/4] android/bluetooth: Print error on unimplemented functions
From: Szymon Janc @ 2013-12-20 18:30 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1387564210-6102-1-git-send-email-szymon.janc@tieto.com>

Functions or callbacks that are not implemented due to being bogus or
not feasible now prints error messages instead of debugs.
---
 android/bluetooth.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 59f0810..5598ad8 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -2059,9 +2059,7 @@ static uint8_t get_adapter_type(void)
 
 static uint8_t get_adapter_service_rec(void)
 {
-	DBG("Not implemented");
-
-	/* TODO: Add implementation */
+	error("get adapter BT_PROPERTY_SERVICE_RECORD not supported");
 
 	return HAL_STATUS_FAILED;
 }
@@ -2619,9 +2617,7 @@ static uint8_t get_device_type(struct device *dev)
 
 static uint8_t get_device_service_rec(struct device *dev)
 {
-	DBG("Not implemented");
-
-	/* TODO */
+	error("get device BT_PROPERTY_SERVICE_RECORD not supported");
 
 	return HAL_STATUS_FAILED;
 }
@@ -2650,9 +2646,7 @@ static uint8_t get_device_rssi(struct device *dev)
 
 static uint8_t get_device_version_info(struct device *dev)
 {
-	DBG("Not implemented");
-
-	/* TODO */
+	error("get device BT_PROPERTY_REMOTE_VERSION_INFO not supported");
 
 	return HAL_STATUS_FAILED;
 }
@@ -2768,9 +2762,7 @@ static uint8_t set_device_friendly_name(struct device *dev, const uint8_t *val,
 
 static uint8_t set_device_version_info(struct device *dev)
 {
-	DBG("Not implemented");
-
-	/* TODO */
+	error("set device BT_PROPERTY_REMOTE_VERSION_INFO not supported");
 
 	return HAL_STATUS_FAILED;
 }
@@ -2816,7 +2808,7 @@ failed:
 
 static void handle_get_remote_service_rec_cmd(const void *buf, uint16_t len)
 {
-	/* TODO */
+	error("get_remote_service_record not supported");
 
 	ipc_send_rsp(HAL_SERVICE_ID_BLUETOOTH, HAL_OP_GET_REMOTE_SERVICE_REC,
 							HAL_STATUS_FAILED);
@@ -2914,7 +2906,7 @@ static void handle_dut_mode_send_cmd(const void *buf, uint16_t len)
 		return;
 	}
 
-	DBG("opcode %u", cmd->opcode);
+	error("dut_mode_send not supported");
 
 	/* TODO */
 
@@ -2932,7 +2924,7 @@ static void handle_le_test_mode_cmd(const void *buf, uint16_t len)
 		return;
 	}
 
-	DBG("opcode %u", cmd->opcode);
+	error("le_test_mode not supported");
 
 	/* TODO */
 
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 4/4] android: Add shortcommings section to README
From: Szymon Janc @ 2013-12-20 18:30 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1387564210-6102-1-git-send-email-szymon.janc@tieto.com>

This sections lists unimplemented methods, callbacks or properties
with few words of comments why feature is missing.
---
 android/README | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/android/README b/android/README
index 68c3e9f..6d0e242 100644
--- a/android/README
+++ b/android/README
@@ -91,3 +91,40 @@ use provided android/system-emulator, which takes care of launching daemon
 automatically on HAL library initialization. To deinitialize HAL library and
 stop daemon type 'bluetooth cleanup'. Type 'help' for more information. Tab
 completion is also supported.
+
+===========================
+Implementation shortcomings
+===========================
+
+It is possible that some of HAL functionality is missing implementation due to
+reasons like feature feasibility or necessity for latest Android Framework.
+This sections provides list of such deficiencies. Note that HAL library is
+always expected to fully implement HAL API so missing implementation might
+happen only in daemon.
+
+HAL Bluetooth
+=============
+methods:
+dut_mode_send                      never called from Android Framework
+le_test_mode                       never called from Android Framework
+get_remote_service_record          never called from Android Framework
+
+callbacks:
+dut_mode_recv_cb
+le_test_mode_cb
+
+properties:
+BT_PROPERTY_SERVICE_RECORD         not supported for adapter and device, for
+                                   device this property is to be returned as
+                                   response to get_remote_service_record,
+                                   not sure what to return on get_property
+                                   calls (records of all services?)
+
+BT_PROPERTY_REMOTE_VERSION_INFO    information required by this property (LMP
+                                   information) are not accessible from mgmt
+                                   interface, also marking this property as
+                                   settable is probably a typo in HAL header
+
+Socket HAL
+==========
+Support only for BTSOCK_RFCOMM socket type.
-- 
1.8.3.2


^ 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