LinuxLists.cc - [PATCHv3 0/9] Improve logging for Android

2013-10-29 12:57:30

Subject: [PATCHv3 0/9] Improve logging for Android

From: Andrei Emeltchenko <[email protected]>

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

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

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

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

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

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

--
1.7.10.4

2013-10-30 12:16:43

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv4 5/8] android/hal: Print adapter state

From: Andrei Emeltchenko <[email protected]>

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

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 93490f3..0908f06 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -33,6 +33,8 @@ static void handle_adapter_state_changed(void *buf)
{
struct hal_ev_adapter_state_changed *ev = buf;

+ DBG("state: %s", bt_state_t2str(ev->state));
+
if (bt_hal_cbacks->adapter_state_changed_cb)
bt_hal_cbacks->adapter_state_changed_cb(ev->state);
}
--
1.7.10.4

2013-10-30 12:16:44

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv4 6/8] android/hal: Print adapter property in callback

From: Andrei Emeltchenko <[email protected]>

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

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 0908f06..1d5b04f 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -59,6 +59,8 @@ static void repack_properties(bt_property_t *send_props,

p += sizeof(*hal_prop) + hal_prop->len;
hal_prop = p;
+
+ DBG("prop[%d]: %s", i, btproperty2str(&send_props[i]));
}
}

--
1.7.10.4

2013-10-30 12:16:45

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv4 7/8] android: Add thread-safe helpers

From: Andrei Emeltchenko <[email protected]>

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

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

2013-10-30 12:16:39

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv4 1/8] android/haltest: Export print property

From: Andrei Emeltchenko <[email protected]>

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

diff --git a/android/client/if-bt.c b/android/client/if-bt.c
index a20a7c6..e9edec5 100644
--- a/android/client/if-bt.c
+++ b/android/client/if-bt.c
@@ -29,20 +29,6 @@ const bt_interface_t *if_bluetooth;
} \
} while (0)

-static char *bdaddr2str(const bt_bdaddr_t *bd_addr)
-{
- static char buf[MAX_ADDR_STR_LEN];
-
- return bt_bdaddr_t2str(bd_addr, buf);
-}
-
-static char *btuuid2str(const bt_uuid_t *uuid)
-{
- static char buf[MAX_UUID_STR_LEN];
-
- return bt_uuid_t2str(uuid, buf);
-}
-
static bt_scan_mode_t str2btscanmode(const char *str)
{
bt_scan_mode_t v = str2bt_scan_mode_t(str);
@@ -76,102 +62,6 @@ static bt_property_type_t str2btpropertytype(const char *str)
return (bt_property_type_t) atoi(str);
}

-static char *btproperty2str(bt_property_t property)
-{
- static char buf[4096];
- char *p;
-
- p = buf + sprintf(buf, "type=%s len=%d val=",
- bt_property_type_t2str(property.type),
- property.len);
-
- switch (property.type) {
- case BT_PROPERTY_BDNAME:
- case BT_PROPERTY_REMOTE_FRIENDLY_NAME:
- sprintf(p, "%*s", property.len,
- ((bt_bdname_t *) property.val)->name);
- break;
-
- case BT_PROPERTY_BDADDR:
- sprintf(p, "%s", bdaddr2str((bt_bdaddr_t *) property.val));
- break;
-
- case BT_PROPERTY_CLASS_OF_DEVICE:
- sprintf(p, "%06x", *((int *) property.val));
- break;
-
- case BT_PROPERTY_TYPE_OF_DEVICE:
- sprintf(p, "%s", bt_device_type_t2str(
- *((bt_device_type_t *) property.val)));
- break;
-
- case BT_PROPERTY_REMOTE_RSSI:
- sprintf(p, "%d", *((char *) property.val));
- break;
-
- case BT_PROPERTY_ADAPTER_SCAN_MODE:
- sprintf(p, "%s",
- bt_scan_mode_t2str(*((bt_scan_mode_t *) property.val)));
- break;
-
- case BT_PROPERTY_ADAPTER_DISCOVERY_TIMEOUT:
- sprintf(p, "%d", *((int *) property.val));
- break;
-
- case BT_PROPERTY_ADAPTER_BONDED_DEVICES:
- {
- int count = property.len / sizeof(bt_bdaddr_t);
- char *ptr = property.val;
-
- strcat(p, "{");
-
- while (count--) {
- strcat(p, bdaddr2str((bt_bdaddr_t *) ptr));
- if (count)
- strcat(p, ", ");
- ptr += sizeof(bt_bdaddr_t);
- }
-
- strcat(p, "}");
-
- }
- break;
-
- case BT_PROPERTY_UUIDS:
- {
- int count = property.len / sizeof(bt_uuid_t);
- char *ptr = property.val;
-
- strcat(p, "{");
-
- while (count--) {
- strcat(p, btuuid2str((bt_uuid_t *) ptr));
- if (count)
- strcat(p, ", ");
- ptr += sizeof(bt_uuid_t);
- }
-
- strcat(p, "}");
-
- }
- break;
-
- case BT_PROPERTY_SERVICE_RECORD:
- {
- bt_service_record_t *rec = property.val;
-
- sprintf(p, "{%s, %d, %s}", btuuid2str(&rec->uuid),
- rec->channel, rec->name);
- }
- break;
-
- default:
- sprintf(p, "%p", property.val);
- }
-
- return buf;
-}
-
static void dump_properties(int num_properties, bt_property_t *properties)
{
int i;
diff --git a/android/client/textconv.c b/android/client/textconv.c
index 32b1cab..8f27948 100644
--- a/android/client/textconv.c
+++ b/android/client/textconv.c
@@ -226,3 +226,113 @@ const char *enum_one_string(void *v, int i)

return (i == 0) && (m[0] != 0) ? m : NULL;
}
+
+char *bdaddr2str(const bt_bdaddr_t *bd_addr)
+{
+ static char buf[MAX_ADDR_STR_LEN];
+
+ return bt_bdaddr_t2str(bd_addr, buf);
+}
+
+static char *btuuid2str(const bt_uuid_t *uuid)
+{
+ static char buf[MAX_UUID_STR_LEN];
+
+ return bt_uuid_t2str(uuid, buf);
+}
+
+char *btproperty2str(bt_property_t property)
+{
+ static char buf[4096];
+ char *p;
+
+ p = buf + sprintf(buf, "type=%s len=%d val=",
+ bt_property_type_t2str(property.type),
+ property.len);
+
+ switch (property.type) {
+ case BT_PROPERTY_BDNAME:
+ case BT_PROPERTY_REMOTE_FRIENDLY_NAME:
+ sprintf(p, "%*s", property.len,
+ ((bt_bdname_t *) property.val)->name);
+ break;
+
+ case BT_PROPERTY_BDADDR:
+ sprintf(p, "%s", bdaddr2str((bt_bdaddr_t *) property.val));
+ break;
+
+ case BT_PROPERTY_CLASS_OF_DEVICE:
+ sprintf(p, "%06x", *((int *) property.val));
+ break;
+
+ case BT_PROPERTY_TYPE_OF_DEVICE:
+ sprintf(p, "%s", bt_device_type_t2str(
+ *((bt_device_type_t *) property.val)));
+ break;
+
+ case BT_PROPERTY_REMOTE_RSSI:
+ sprintf(p, "%d", *((char *) property.val));
+ break;
+
+ case BT_PROPERTY_ADAPTER_SCAN_MODE:
+ sprintf(p, "%s",
+ bt_scan_mode_t2str(*((bt_scan_mode_t *) property.val)));
+ break;
+
+ case BT_PROPERTY_ADAPTER_DISCOVERY_TIMEOUT:
+ sprintf(p, "%d", *((int *) property.val));
+ break;
+
+ case BT_PROPERTY_ADAPTER_BONDED_DEVICES:
+ {
+ int count = property.len / sizeof(bt_bdaddr_t);
+ char *ptr = property.val;
+
+ strcat(p, "{");
+
+ while (count--) {
+ strcat(p, bdaddr2str((bt_bdaddr_t *) ptr));
+ if (count)
+ strcat(p, ", ");
+ ptr += sizeof(bt_bdaddr_t);
+ }
+
+ strcat(p, "}");
+
+ }
+ break;
+
+ case BT_PROPERTY_UUIDS:
+ {
+ int count = property.len / sizeof(bt_uuid_t);
+ char *ptr = property.val;
+
+ strcat(p, "{");
+
+ while (count--) {
+ strcat(p, btuuid2str((bt_uuid_t *) ptr));
+ if (count)
+ strcat(p, ", ");
+ ptr += sizeof(bt_uuid_t);
+ }
+
+ strcat(p, "}");
+
+ }
+ break;
+
+ case BT_PROPERTY_SERVICE_RECORD:
+ {
+ bt_service_record_t *rec = property.val;
+
+ sprintf(p, "{%s, %d, %s}", btuuid2str(&rec->uuid),
+ rec->channel, rec->name);
+ }
+ break;
+
+ default:
+ sprintf(p, "%p", property.val);
+ }
+
+ return buf;
+}
diff --git a/android/client/textconv.h b/android/client/textconv.h
index 085b141..89b29c6 100644
--- a/android/client/textconv.h
+++ b/android/client/textconv.h
@@ -107,6 +107,9 @@ void str2bt_bdaddr_t(const char *str, bt_bdaddr_t *bd_addr);
char *bt_uuid_t2str(const bt_uuid_t *uuid, char *buf);
void str2bt_uuid_t(const char *str, bt_uuid_t *uuid);

+char *btproperty2str(bt_property_t property);
+char *bdaddr2str(const bt_bdaddr_t *bd_addr);
+
DECINTMAP(bt_status_t);
DECINTMAP(bt_state_t);
DECINTMAP(bt_device_type_t);
--
1.7.10.4

2013-10-30 12:16:42

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv4 4/8] android/hal: Add extra logs

From: Andrei Emeltchenko <[email protected]>

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

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 89535ed..93490f3 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -346,7 +346,7 @@ static int set_adapter_property(const bt_property_t *property)

static int get_remote_device_properties(bt_bdaddr_t *remote_addr)
{
- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(remote_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -357,7 +357,8 @@ static int get_remote_device_properties(bt_bdaddr_t *remote_addr)
static int get_remote_device_property(bt_bdaddr_t *remote_addr,
bt_property_type_t type)
{
- DBG("");
+ DBG("bdaddr: %s prop: %s", bdaddr2str(remote_addr),
+ bt_property_type_t2str(type));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -368,7 +369,8 @@ static int get_remote_device_property(bt_bdaddr_t *remote_addr,
static int set_remote_device_property(bt_bdaddr_t *remote_addr,
const bt_property_t *property)
{
- DBG("");
+ DBG("bdaddr: %s prop: %s", bdaddr2str(remote_addr),
+ btproperty2str(property));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -378,7 +380,7 @@ static int set_remote_device_property(bt_bdaddr_t *remote_addr,

static int get_remote_service_record(bt_bdaddr_t *remote_addr, bt_uuid_t *uuid)
{
- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(remote_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -388,7 +390,7 @@ static int get_remote_service_record(bt_bdaddr_t *remote_addr, bt_uuid_t *uuid)

static int get_remote_services(bt_bdaddr_t *remote_addr)
{
- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(remote_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -424,7 +426,7 @@ static int create_bond(const bt_bdaddr_t *bd_addr)
{
struct hal_cmd_create_bond cmd;

- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(bd_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -439,7 +441,7 @@ static int cancel_bond(const bt_bdaddr_t *bd_addr)
{
struct hal_cmd_cancel_bond cmd;

- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(bd_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -454,7 +456,7 @@ static int remove_bond(const bt_bdaddr_t *bd_addr)
{
struct hal_cmd_remove_bond cmd;

- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(bd_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -470,7 +472,7 @@ static int pin_reply(const bt_bdaddr_t *bd_addr, uint8_t accept,
{
struct hal_cmd_pin_reply cmd;

- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(bd_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -489,7 +491,7 @@ static int ssp_reply(const bt_bdaddr_t *bd_addr, bt_ssp_variant_t variant,
{
struct hal_cmd_ssp_reply cmd;

- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(bd_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
--
1.7.10.4

2013-10-30 12:16:40

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv4 2/8] android/haltest: Use pointer as parameter for debug

From: Andrei Emeltchenko <[email protected]>

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

diff --git a/android/client/if-bt.c b/android/client/if-bt.c
index e9edec5..cbb828b 100644
--- a/android/client/if-bt.c
+++ b/android/client/if-bt.c
@@ -74,7 +74,7 @@ static void dump_properties(int num_properties, bt_property_t *properties)
bt_property_t prop;
memcpy(&prop, properties + i, sizeof(prop));

- haltest_info("prop: %s\n", btproperty2str(prop));
+ haltest_info("prop: %s\n", btproperty2str(&prop));
}
}

diff --git a/android/client/textconv.c b/android/client/textconv.c
index 8f27948..1dc6ad0 100644
--- a/android/client/textconv.c
+++ b/android/client/textconv.c
@@ -241,52 +241,52 @@ static char *btuuid2str(const bt_uuid_t *uuid)
return bt_uuid_t2str(uuid, buf);
}

-char *btproperty2str(bt_property_t property)
+char *btproperty2str(const bt_property_t *property)
{
static char buf[4096];
char *p;

p = buf + sprintf(buf, "type=%s len=%d val=",
- bt_property_type_t2str(property.type),
- property.len);
+ bt_property_type_t2str(property->type),
+ property->len);

- switch (property.type) {
+ switch (property->type) {
case BT_PROPERTY_BDNAME:
case BT_PROPERTY_REMOTE_FRIENDLY_NAME:
- sprintf(p, "%*s", property.len,
- ((bt_bdname_t *) property.val)->name);
+ sprintf(p, "%*s", property->len,
+ ((bt_bdname_t *) property->val)->name);
break;

case BT_PROPERTY_BDADDR:
- sprintf(p, "%s", bdaddr2str((bt_bdaddr_t *) property.val));
+ sprintf(p, "%s", bdaddr2str((bt_bdaddr_t *) property->val));
break;

case BT_PROPERTY_CLASS_OF_DEVICE:
- sprintf(p, "%06x", *((int *) property.val));
+ sprintf(p, "%06x", *((int *) property->val));
break;

case BT_PROPERTY_TYPE_OF_DEVICE:
sprintf(p, "%s", bt_device_type_t2str(
- *((bt_device_type_t *) property.val)));
+ *((bt_device_type_t *) property->val)));
break;

case BT_PROPERTY_REMOTE_RSSI:
- sprintf(p, "%d", *((char *) property.val));
+ sprintf(p, "%d", *((char *) property->val));
break;

case BT_PROPERTY_ADAPTER_SCAN_MODE:
sprintf(p, "%s",
- bt_scan_mode_t2str(*((bt_scan_mode_t *) property.val)));
+ bt_scan_mode_t2str(*((bt_scan_mode_t *) property->val)));
break;

case BT_PROPERTY_ADAPTER_DISCOVERY_TIMEOUT:
- sprintf(p, "%d", *((int *) property.val));
+ sprintf(p, "%d", *((int *) property->val));
break;

case BT_PROPERTY_ADAPTER_BONDED_DEVICES:
{
- int count = property.len / sizeof(bt_bdaddr_t);
- char *ptr = property.val;
+ int count = property->len / sizeof(bt_bdaddr_t);
+ char *ptr = property->val;

strcat(p, "{");

@@ -304,8 +304,8 @@ char *btproperty2str(bt_property_t property)

case BT_PROPERTY_UUIDS:
{
- int count = property.len / sizeof(bt_uuid_t);
- char *ptr = property.val;
+ int count = property->len / sizeof(bt_uuid_t);
+ char *ptr = property->val;

strcat(p, "{");

@@ -323,7 +323,7 @@ char *btproperty2str(bt_property_t property)

case BT_PROPERTY_SERVICE_RECORD:
{
- bt_service_record_t *rec = property.val;
+ bt_service_record_t *rec = property->val;

sprintf(p, "{%s, %d, %s}", btuuid2str(&rec->uuid),
rec->channel, rec->name);
@@ -331,7 +331,7 @@ char *btproperty2str(bt_property_t property)
break;

default:
- sprintf(p, "%p", property.val);
+ sprintf(p, "%p", property->val);
}

return buf;
diff --git a/android/client/textconv.h b/android/client/textconv.h
index 89b29c6..1c848ef 100644
--- a/android/client/textconv.h
+++ b/android/client/textconv.h
@@ -107,7 +107,7 @@ void str2bt_bdaddr_t(const char *str, bt_bdaddr_t *bd_addr);
char *bt_uuid_t2str(const bt_uuid_t *uuid, char *buf);
void str2bt_uuid_t(const char *str, bt_uuid_t *uuid);

-char *btproperty2str(bt_property_t property);
+char *btproperty2str(const bt_property_t *property);
char *bdaddr2str(const bt_bdaddr_t *bd_addr);

DECINTMAP(bt_status_t);
--
1.7.10.4

2013-10-30 12:16:41

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv4 3/8] android/hal: Print full property in debug

From: Andrei Emeltchenko <[email protected]>

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

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 2237063..89535ed 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -321,7 +321,7 @@ static int set_adapter_property(const bt_property_t *property)
char buf[sizeof(struct hal_cmd_set_adapter_prop) + property->len];
struct hal_cmd_set_adapter_prop *cmd = (void *) buf;

- DBG("prop: %s", bt_property_type_t2str(property->type));
+ DBG("prop: %s", btproperty2str(property));

if (!interface_ready())
return BT_STATUS_NOT_READY;
--
1.7.10.4

2013-10-30 12:16:46

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv4 8/8] android: Use thread-safe helpers

From: Andrei Emeltchenko <[email protected]>

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

diff --git a/android/client/textconv.c b/android/client/textconv.c
index 1dc6ad0..effd1b3 100644
--- a/android/client/textconv.c
+++ b/android/client/textconv.c
@@ -19,6 +19,8 @@
#include <stdio.h>
#include <hardware/bluetooth.h>

+#include "../pthread-local.h"
+
#include "textconv.h"

/*
@@ -227,11 +229,12 @@ const char *enum_one_string(void *v, int i)
return (i == 0) && (m[0] != 0) ? m : NULL;
}

+GLOBAL_INIT_THREAD_LOCAL_BUFFER(bdaddr);
char *bdaddr2str(const bt_bdaddr_t *bd_addr)
{
- static char buf[MAX_ADDR_STR_LEN];
+ LOCAL_INIT_THREAD_LOCAL_BUFFER(char*, bdaddr, MAX_ADDR_STR_LEN);

- return bt_bdaddr_t2str(bd_addr, buf);
+ return bt_bdaddr_t2str(bd_addr, bdaddr_tls_buffer);
}

static char *btuuid2str(const bt_uuid_t *uuid)
@@ -241,12 +244,14 @@ static char *btuuid2str(const bt_uuid_t *uuid)
return bt_uuid_t2str(uuid, buf);
}

+GLOBAL_INIT_THREAD_LOCAL_BUFFER(property);
char *btproperty2str(const bt_property_t *property)
{
- static char buf[4096];
char *p;
+ LOCAL_INIT_THREAD_LOCAL_BUFFER(char*, property, 4096);

- p = buf + sprintf(buf, "type=%s len=%d val=",
+ p = property_tls_buffer + sprintf(property_tls_buffer,
+ "type=%s len=%d val=",
bt_property_type_t2str(property->type),
property->len);

@@ -334,5 +339,5 @@ char *btproperty2str(const bt_property_t *property)
sprintf(p, "%p", property->val);
}

- return buf;
+ return property_tls_buffer;
}
--
1.7.10.4

2013-10-30 12:16:38

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv4 0/8] Improve logging for Android

From: Andrei Emeltchenko <[email protected]>

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

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

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

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

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

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

--
1.7.10.4

2013-10-29 12:57:38

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv3 8/9] android: Use thread-safe helpers

2013-10-29 12:57:34

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv3 4/9] android/hal: Add extra logs

From: Andrei Emeltchenko <[email protected]>

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

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 0b46465..85ccded 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -266,7 +266,7 @@ static int set_adapter_property(const bt_property_t *property)

static int get_remote_device_properties(bt_bdaddr_t *remote_addr)
{
- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(remote_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -277,7 +277,8 @@ static int get_remote_device_properties(bt_bdaddr_t *remote_addr)
static int get_remote_device_property(bt_bdaddr_t *remote_addr,
bt_property_type_t type)
{
- DBG("");
+ DBG("bdaddr: %s prop: %s", bdaddr2str(remote_addr),
+ bt_property_type_t2str(type));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -288,7 +289,8 @@ static int get_remote_device_property(bt_bdaddr_t *remote_addr,
static int set_remote_device_property(bt_bdaddr_t *remote_addr,
const bt_property_t *property)
{
- DBG("");
+ DBG("bdaddr: %s prop: %s", bdaddr2str(remote_addr),
+ btproperty2str(property));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -298,7 +300,7 @@ static int set_remote_device_property(bt_bdaddr_t *remote_addr,

static int get_remote_service_record(bt_bdaddr_t *remote_addr, bt_uuid_t *uuid)
{
- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(remote_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -308,7 +310,7 @@ static int get_remote_service_record(bt_bdaddr_t *remote_addr, bt_uuid_t *uuid)

static int get_remote_services(bt_bdaddr_t *remote_addr)
{
- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(remote_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -340,7 +342,7 @@ static int create_bond(const bt_bdaddr_t *bd_addr)
{
struct hal_cmd_create_bond cmd;

- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(bd_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -355,7 +357,7 @@ static int cancel_bond(const bt_bdaddr_t *bd_addr)
{
struct hal_cmd_cancel_bond cmd;

- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(bd_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -370,7 +372,7 @@ static int remove_bond(const bt_bdaddr_t *bd_addr)
{
struct hal_cmd_remove_bond cmd;

- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(bd_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -386,7 +388,7 @@ static int pin_reply(const bt_bdaddr_t *bd_addr, uint8_t accept,
{
struct hal_cmd_pin_reply cmd;

- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(bd_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -405,7 +407,7 @@ static int ssp_reply(const bt_bdaddr_t *bd_addr, bt_ssp_variant_t variant,
{
struct hal_cmd_ssp_reply cmd;

- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(bd_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
--
1.7.10.4

2013-10-29 12:57:35

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv3 5/9] android/hal: Print adapter state

From: Andrei Emeltchenko <[email protected]>

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

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 85ccded..4d467ee 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -33,6 +33,8 @@ static void handle_adapter_state_changed(void *buf)
{
struct hal_ev_adapter_state_changed *ev = buf;

+ DBG("state: %s", bt_state_t2str(ev->state));
+
if (bt_hal_cbacks->adapter_state_changed_cb)
bt_hal_cbacks->adapter_state_changed_cb(ev->state);
}
--
1.7.10.4

2013-10-29 12:57:37

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv3 7/9] android: Add thread-safe helpers

2013-10-29 12:57:32

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv3 2/9] android/haltest: Use pointer as parameter for debug

2013-10-29 12:57:31

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv3 1/9] android/haltest: Export print property

2013-10-29 12:57:36

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv3 6/9] android/hal: Print adapter property in callback

From: Andrei Emeltchenko <[email protected]>

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

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 4d467ee..cc63bd9 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -65,6 +65,8 @@ static void handle_adapter_props_changed(void *buf, uint16_t len)

p += sizeof(*hal_prop) + hal_prop->len;
hal_prop = p;
+
+ DBG("prop[%d]: %s", i, btproperty2str(&props[i]));
}

bt_hal_cbacks->adapter_properties_cb(ev->status, ev->num_props, props);
--
1.7.10.4

2013-10-29 12:57:39

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv3 9/9] android: Fix build errors

From: Andrei Emeltchenko <[email protected]>

The patch fixes following issues when building:

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

diff --git a/android/Android.mk b/android/Android.mk
index 28ec465..d8f9d0d 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -52,6 +52,8 @@ lib_headers := \
l2cap.h \
sdp_lib.h \
sdp.h \
+ rfcomm.h \
+ sco.h \

$(shell mkdir -p $(LOCAL_PATH)/../lib/bluetooth)

--
1.7.10.4

2013-10-29 12:57:33

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv3 3/9] android/hal: Print full property in debug

From: Andrei Emeltchenko <[email protected]>

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

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index a3f5038..0b46465 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -241,7 +241,7 @@ static int set_adapter_property(const bt_property_t *property)
char buf[sizeof(struct hal_cmd_set_adapter_prop) + property->len];
struct hal_cmd_set_adapter_prop *cmd = (void *) buf;

- DBG("prop: %s", bt_property_type_t2str(property->type));
+ DBG("prop: %s", btproperty2str(property));

if (!interface_ready())
return BT_STATUS_NOT_READY;
--
1.7.10.4

2013-11-04 07:34:26

by Johan Hedberg

[permalink] [raw]

Subject: Re: [PATCHv5 0/7] Improve logging for Android

Hi Andrei,

On Fri, Nov 01, 2013, Andrei Emeltchenko wrote:
> Logging on Android is really important since we do not have specification
> how and in which order hal functions are called, sometimes the only way is
> to examine logs or read Java Bluetooth code.
>
> Changes:
> * v5: Rebased against latest upstream and merged some patches, added several logs.
> * v4: Rebased against latest upstream
> * v3: Merged first two patches.
> * v2: Added thread-safe helpers for printing properties, bdaddr, etc
> after comments that simple printing is not thread-safe. The idea is to
> use TLS (thread local storage) like bionic is doing for strerror for
> example. More info can be found on manpage for pthread_key_create.
>
> This patch series uses debug functions defined already for haltest and
> allows to print very helpful logs on Android target like shown below:
>
> ...
> hal-bluetooth.c:set_adapter_property() prop: type=BT_PROPERTY_ADAPTER_SCAN_MODE len=4 val=BT_SCAN_MODE_NONE
> ...
>
> Andrei Emeltchenko (7):
> android/haltest: Export print property
> android/haltest: Use pointer as parameter for debug
> android/haltest: Fix print device name
> android/hal: Add extra logs to HAL
> android: Add thread-safe helpers
> android: Use thread-safe helpers
> android/daemon: Add logs to trace failed cmd
>
> android/adapter.c | 2 +
> android/client/if-bt.c | 111 +------------------------------------------
> android/client/textconv.c | 114 +++++++++++++++++++++++++++++++++++++++++++++
> android/client/textconv.h | 3 ++
> android/hal-bluetooth.c | 34 +++++++++-----
> android/pthread-local.h | 58 +++++++++++++++++++++++
> 6 files changed, 200 insertions(+), 122 deletions(-)
> create mode 100644 android/pthread-local.h

Patches 1-4 and patch 7 (with a minor modification) have been applied.
Thanks.

Johan

2013-11-01 15:19:15

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv5 3/7] android/haltest: Fix print device name

From: Andrei Emeltchenko <[email protected]>

---
android/client/textconv.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/android/client/textconv.c b/android/client/textconv.c
index 1dc6ad0..18f60ee 100644
--- a/android/client/textconv.c
+++ b/android/client/textconv.c
@@ -253,8 +253,7 @@ char *btproperty2str(const bt_property_t *property)
switch (property->type) {
case BT_PROPERTY_BDNAME:
case BT_PROPERTY_REMOTE_FRIENDLY_NAME:
- sprintf(p, "%*s", property->len,
- ((bt_bdname_t *) property->val)->name);
+ sprintf(p, "%s", ((bt_bdname_t *) property->val)->name);
break;

case BT_PROPERTY_BDADDR:
--
1.7.10.4

2013-11-01 15:19:18

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv5 6/7] android: Use thread-safe helpers

From: Andrei Emeltchenko <[email protected]>

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

diff --git a/android/client/textconv.c b/android/client/textconv.c
index 18f60ee..8e6bdb9 100644
--- a/android/client/textconv.c
+++ b/android/client/textconv.c
@@ -19,6 +19,8 @@
#include <stdio.h>
#include <hardware/bluetooth.h>

+#include "../pthread-local.h"
+
#include "textconv.h"

/*
@@ -227,11 +229,12 @@ const char *enum_one_string(void *v, int i)
return (i == 0) && (m[0] != 0) ? m : NULL;
}

+GLOBAL_INIT_THREAD_LOCAL_BUFFER(bdaddr);
char *bdaddr2str(const bt_bdaddr_t *bd_addr)
{
- static char buf[MAX_ADDR_STR_LEN];
+ LOCAL_INIT_THREAD_LOCAL_BUFFER(char*, bdaddr, MAX_ADDR_STR_LEN);

- return bt_bdaddr_t2str(bd_addr, buf);
+ return bt_bdaddr_t2str(bd_addr, bdaddr_tls_buffer);
}

static char *btuuid2str(const bt_uuid_t *uuid)
@@ -241,12 +244,14 @@ static char *btuuid2str(const bt_uuid_t *uuid)
return bt_uuid_t2str(uuid, buf);
}

+GLOBAL_INIT_THREAD_LOCAL_BUFFER(property);
char *btproperty2str(const bt_property_t *property)
{
- static char buf[4096];
char *p;
+ LOCAL_INIT_THREAD_LOCAL_BUFFER(char*, property, 4096);

- p = buf + sprintf(buf, "type=%s len=%d val=",
+ p = property_tls_buffer + sprintf(property_tls_buffer,
+ "type=%s len=%d val=",
bt_property_type_t2str(property->type),
property->len);

@@ -333,5 +338,5 @@ char *btproperty2str(const bt_property_t *property)
sprintf(p, "%p", property->val);
}

- return buf;
+ return property_tls_buffer;
}
--
1.7.10.4

2013-11-01 15:19:13

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv5 1/7] android/haltest: Export print property

From: Andrei Emeltchenko <[email protected]>

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

diff --git a/android/client/if-bt.c b/android/client/if-bt.c
index 3d97458..e7d0659 100644
--- a/android/client/if-bt.c
+++ b/android/client/if-bt.c
@@ -29,20 +29,6 @@ const bt_interface_t *if_bluetooth;
} \
} while (0)

-static char *bdaddr2str(const bt_bdaddr_t *bd_addr)
-{
- static char buf[MAX_ADDR_STR_LEN];
-
- return bt_bdaddr_t2str(bd_addr, buf);
-}
-
-static char *btuuid2str(const bt_uuid_t *uuid)
-{
- static char buf[MAX_UUID_STR_LEN];
-
- return bt_uuid_t2str(uuid, buf);
-}
-
static bt_scan_mode_t str2btscanmode(const char *str)
{
bt_scan_mode_t v = str2bt_scan_mode_t(str);
@@ -76,101 +62,6 @@ static bt_property_type_t str2btpropertytype(const char *str)
return (bt_property_type_t) atoi(str);
}

-static char *btproperty2str(bt_property_t property)
-{
- static char buf[4096];
- char *p;
-
- p = buf + sprintf(buf, "type=%s len=%d val=",
- bt_property_type_t2str(property.type),
- property.len);
-
- switch (property.type) {
- case BT_PROPERTY_BDNAME:
- case BT_PROPERTY_REMOTE_FRIENDLY_NAME:
- sprintf(p, "%s", ((bt_bdname_t *) property.val)->name);
- break;
-
- case BT_PROPERTY_BDADDR:
- sprintf(p, "%s", bdaddr2str((bt_bdaddr_t *) property.val));
- break;
-
- case BT_PROPERTY_CLASS_OF_DEVICE:
- sprintf(p, "%06x", *((int *) property.val));
- break;
-
- case BT_PROPERTY_TYPE_OF_DEVICE:
- sprintf(p, "%s", bt_device_type_t2str(
- *((bt_device_type_t *) property.val)));
- break;
-
- case BT_PROPERTY_REMOTE_RSSI:
- sprintf(p, "%d", *((char *) property.val));
- break;
-
- case BT_PROPERTY_ADAPTER_SCAN_MODE:
- sprintf(p, "%s",
- bt_scan_mode_t2str(*((bt_scan_mode_t *) property.val)));
- break;
-
- case BT_PROPERTY_ADAPTER_DISCOVERY_TIMEOUT:
- sprintf(p, "%d", *((int *) property.val));
- break;
-
- case BT_PROPERTY_ADAPTER_BONDED_DEVICES:
- {
- int count = property.len / sizeof(bt_bdaddr_t);
- char *ptr = property.val;
-
- strcat(p, "{");
-
- while (count--) {
- strcat(p, bdaddr2str((bt_bdaddr_t *) ptr));
- if (count)
- strcat(p, ", ");
- ptr += sizeof(bt_bdaddr_t);
- }
-
- strcat(p, "}");
-
- }
- break;
-
- case BT_PROPERTY_UUIDS:
- {
- int count = property.len / sizeof(bt_uuid_t);
- char *ptr = property.val;
-
- strcat(p, "{");
-
- while (count--) {
- strcat(p, btuuid2str((bt_uuid_t *) ptr));
- if (count)
- strcat(p, ", ");
- ptr += sizeof(bt_uuid_t);
- }
-
- strcat(p, "}");
-
- }
- break;
-
- case BT_PROPERTY_SERVICE_RECORD:
- {
- bt_service_record_t *rec = property.val;
-
- sprintf(p, "{%s, %d, %s}", btuuid2str(&rec->uuid),
- rec->channel, rec->name);
- }
- break;
-
- default:
- sprintf(p, "%p", property.val);
- }
-
- return buf;
-}
-
static void dump_properties(int num_properties, bt_property_t *properties)
{
int i;
diff --git a/android/client/textconv.c b/android/client/textconv.c
index 32b1cab..8f27948 100644
--- a/android/client/textconv.c
+++ b/android/client/textconv.c
@@ -226,3 +226,113 @@ const char *enum_one_string(void *v, int i)

return (i == 0) && (m[0] != 0) ? m : NULL;
}
+
+char *bdaddr2str(const bt_bdaddr_t *bd_addr)
+{
+ static char buf[MAX_ADDR_STR_LEN];
+
+ return bt_bdaddr_t2str(bd_addr, buf);
+}
+
+static char *btuuid2str(const bt_uuid_t *uuid)
+{
+ static char buf[MAX_UUID_STR_LEN];
+
+ return bt_uuid_t2str(uuid, buf);
+}
+
+char *btproperty2str(bt_property_t property)
+{
+ static char buf[4096];
+ char *p;
+
+ p = buf + sprintf(buf, "type=%s len=%d val=",
+ bt_property_type_t2str(property.type),
+ property.len);
+
+ switch (property.type) {
+ case BT_PROPERTY_BDNAME:
+ case BT_PROPERTY_REMOTE_FRIENDLY_NAME:
+ sprintf(p, "%*s", property.len,
+ ((bt_bdname_t *) property.val)->name);
+ break;
+
+ case BT_PROPERTY_BDADDR:
+ sprintf(p, "%s", bdaddr2str((bt_bdaddr_t *) property.val));
+ break;
+
+ case BT_PROPERTY_CLASS_OF_DEVICE:
+ sprintf(p, "%06x", *((int *) property.val));
+ break;
+
+ case BT_PROPERTY_TYPE_OF_DEVICE:
+ sprintf(p, "%s", bt_device_type_t2str(
+ *((bt_device_type_t *) property.val)));
+ break;
+
+ case BT_PROPERTY_REMOTE_RSSI:
+ sprintf(p, "%d", *((char *) property.val));
+ break;
+
+ case BT_PROPERTY_ADAPTER_SCAN_MODE:
+ sprintf(p, "%s",
+ bt_scan_mode_t2str(*((bt_scan_mode_t *) property.val)));
+ break;
+
+ case BT_PROPERTY_ADAPTER_DISCOVERY_TIMEOUT:
+ sprintf(p, "%d", *((int *) property.val));
+ break;
+
+ case BT_PROPERTY_ADAPTER_BONDED_DEVICES:
+ {
+ int count = property.len / sizeof(bt_bdaddr_t);
+ char *ptr = property.val;
+
+ strcat(p, "{");
+
+ while (count--) {
+ strcat(p, bdaddr2str((bt_bdaddr_t *) ptr));
+ if (count)
+ strcat(p, ", ");
+ ptr += sizeof(bt_bdaddr_t);
+ }
+
+ strcat(p, "}");
+
+ }
+ break;
+
+ case BT_PROPERTY_UUIDS:
+ {
+ int count = property.len / sizeof(bt_uuid_t);
+ char *ptr = property.val;
+
+ strcat(p, "{");
+
+ while (count--) {
+ strcat(p, btuuid2str((bt_uuid_t *) ptr));
+ if (count)
+ strcat(p, ", ");
+ ptr += sizeof(bt_uuid_t);
+ }
+
+ strcat(p, "}");
+
+ }
+ break;
+
+ case BT_PROPERTY_SERVICE_RECORD:
+ {
+ bt_service_record_t *rec = property.val;
+
+ sprintf(p, "{%s, %d, %s}", btuuid2str(&rec->uuid),
+ rec->channel, rec->name);
+ }
+ break;
+
+ default:
+ sprintf(p, "%p", property.val);
+ }
+
+ return buf;
+}
diff --git a/android/client/textconv.h b/android/client/textconv.h
index 085b141..89b29c6 100644
--- a/android/client/textconv.h
+++ b/android/client/textconv.h
@@ -107,6 +107,9 @@ void str2bt_bdaddr_t(const char *str, bt_bdaddr_t *bd_addr);
char *bt_uuid_t2str(const bt_uuid_t *uuid, char *buf);
void str2bt_uuid_t(const char *str, bt_uuid_t *uuid);

+char *btproperty2str(bt_property_t property);
+char *bdaddr2str(const bt_bdaddr_t *bd_addr);
+
DECINTMAP(bt_status_t);
DECINTMAP(bt_state_t);
DECINTMAP(bt_device_type_t);
--
1.7.10.4

2013-11-01 15:19:16

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv5 4/7] android/hal: Add extra logs to HAL

From: Andrei Emeltchenko <[email protected]>

Use exported functions from hal test tool to print properties.
---
android/hal-bluetooth.c | 34 ++++++++++++++++++++++------------
1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 5fb8846..7334d24 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -40,6 +40,8 @@ static void handle_adapter_state_changed(void *buf)
{
struct hal_ev_adapter_state_changed *ev = buf;

+ DBG("state: %s", bt_state_t2str(ev->state));
+
if (bt_hal_cbacks->adapter_state_changed_cb)
bt_hal_cbacks->adapter_state_changed_cb(ev->state);
}
@@ -74,6 +76,8 @@ static void adapter_props_to_hal(bt_property_t *send_props,
send_props[i].val = hal_prop->val;
break;
}
+
+ DBG("prop[%d]: %s", i, btproperty2str(&send_props[i]));
}
}

@@ -123,6 +127,8 @@ static void device_props_to_hal(bt_property_t *send_props,

p += sizeof(*hal_prop) + hal_prop->len;
hal_prop = p;
+
+ DBG("prop[%d]: %s", i, btproperty2str(&send_props[i]));
}
}

@@ -280,6 +286,8 @@ void bt_notify_adapter(uint16_t opcode, void *buf, uint16_t len)
if (!interface_ready())
return;

+ DBG("opcode 0x%x", opcode);
+
switch (opcode) {
case HAL_EV_ADAPTER_STATE_CHANGED:
handle_adapter_state_changed(buf);
@@ -406,7 +414,7 @@ static int get_adapter_property(bt_property_type_t type)
{
struct hal_cmd_get_adapter_prop cmd;

- DBG("prop: %s", bt_property_type_t2str(type));
+ DBG("prop: %s (%d)", bt_property_type_t2str(type), type);

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -438,7 +446,7 @@ static int set_adapter_property(const bt_property_t *property)
char buf[sizeof(struct hal_cmd_set_adapter_prop) + property->len];
struct hal_cmd_set_adapter_prop *cmd = (void *) buf;

- DBG("prop: %s", bt_property_type_t2str(property->type));
+ DBG("prop: %s", btproperty2str(property));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -463,7 +471,7 @@ static int set_adapter_property(const bt_property_t *property)

static int get_remote_device_properties(bt_bdaddr_t *remote_addr)
{
- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(remote_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -474,7 +482,8 @@ static int get_remote_device_properties(bt_bdaddr_t *remote_addr)
static int get_remote_device_property(bt_bdaddr_t *remote_addr,
bt_property_type_t type)
{
- DBG("");
+ DBG("bdaddr: %s prop: %s", bdaddr2str(remote_addr),
+ bt_property_type_t2str(type));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -485,7 +494,8 @@ static int get_remote_device_property(bt_bdaddr_t *remote_addr,
static int set_remote_device_property(bt_bdaddr_t *remote_addr,
const bt_property_t *property)
{
- DBG("");
+ DBG("bdaddr: %s prop: %s", bdaddr2str(remote_addr),
+ btproperty2str(property));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -495,7 +505,7 @@ static int set_remote_device_property(bt_bdaddr_t *remote_addr,

static int get_remote_service_record(bt_bdaddr_t *remote_addr, bt_uuid_t *uuid)
{
- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(remote_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -505,7 +515,7 @@ static int get_remote_service_record(bt_bdaddr_t *remote_addr, bt_uuid_t *uuid)

static int get_remote_services(bt_bdaddr_t *remote_addr)
{
- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(remote_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -541,7 +551,7 @@ static int create_bond(const bt_bdaddr_t *bd_addr)
{
struct hal_cmd_create_bond cmd;

- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(bd_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -556,7 +566,7 @@ static int cancel_bond(const bt_bdaddr_t *bd_addr)
{
struct hal_cmd_cancel_bond cmd;

- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(bd_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -571,7 +581,7 @@ static int remove_bond(const bt_bdaddr_t *bd_addr)
{
struct hal_cmd_remove_bond cmd;

- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(bd_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -587,7 +597,7 @@ static int pin_reply(const bt_bdaddr_t *bd_addr, uint8_t accept,
{
struct hal_cmd_pin_reply cmd;

- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(bd_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
@@ -606,7 +616,7 @@ static int ssp_reply(const bt_bdaddr_t *bd_addr, bt_ssp_variant_t variant,
{
struct hal_cmd_ssp_reply cmd;

- DBG("");
+ DBG("bdaddr: %s", bdaddr2str(bd_addr));

if (!interface_ready())
return BT_STATUS_NOT_READY;
--
1.7.10.4

2013-11-01 15:19:17

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv5 5/7] android: Add thread-safe helpers

2013-11-01 15:19:19

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv5 7/7] android/daemon: Add logs to trace failed cmd

From: Andrei Emeltchenko <[email protected]>

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

diff --git a/android/adapter.c b/android/adapter.c
index 0797aa0..5552517 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -1572,6 +1572,8 @@ void bt_adapter_handle_cmd(GIOChannel *io, uint8_t opcode, void *buf,
return;

error:
+ DBG("Error handling command 0x%x status %u", opcode, status);
+
ipc_send_rsp(io, HAL_SERVICE_ID_BLUETOOTH, status);
}

--
1.7.10.4

2013-11-01 15:19:12

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv5 0/7] Improve logging for Android

From: Andrei Emeltchenko <[email protected]>

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

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

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

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

Andrei Emeltchenko (7):
android/haltest: Export print property
android/haltest: Use pointer as parameter for debug
android/haltest: Fix print device name
android/hal: Add extra logs to HAL
android: Add thread-safe helpers
android: Use thread-safe helpers
android/daemon: Add logs to trace failed cmd

android/adapter.c | 2 +
android/client/if-bt.c | 111 +------------------------------------------
android/client/textconv.c | 114 +++++++++++++++++++++++++++++++++++++++++++++
android/client/textconv.h | 3 ++
android/hal-bluetooth.c | 34 +++++++++-----
android/pthread-local.h | 58 +++++++++++++++++++++++
6 files changed, 200 insertions(+), 122 deletions(-)
create mode 100644 android/pthread-local.h

--
1.7.10.4

2013-11-01 15:19:14

by Andrei Emeltchenko

[permalink] [raw]

Subject: [PATCHv5 2/7] android/haltest: Use pointer as parameter for debug

From: Andrei Emeltchenko <[email protected]>

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

diff --git a/android/client/if-bt.c b/android/client/if-bt.c
index e7d0659..43e8788 100644
--- a/android/client/if-bt.c
+++ b/android/client/if-bt.c
@@ -74,7 +74,7 @@ static void dump_properties(int num_properties, bt_property_t *properties)
bt_property_t prop;
memcpy(&prop, properties + i, sizeof(prop));

- haltest_info("prop: %s\n", btproperty2str(prop));
+ haltest_info("prop: %s\n", btproperty2str(&prop));
}
}

diff --git a/android/client/textconv.c b/android/client/textconv.c
index 8f27948..1dc6ad0 100644
--- a/android/client/textconv.c
+++ b/android/client/textconv.c
@@ -241,52 +241,52 @@ static char *btuuid2str(const bt_uuid_t *uuid)
return bt_uuid_t2str(uuid, buf);
}

-char *btproperty2str(bt_property_t property)
+char *btproperty2str(const bt_property_t *property)
{
static char buf[4096];
char *p;

p = buf + sprintf(buf, "type=%s len=%d val=",
- bt_property_type_t2str(property.type),
- property.len);
+ bt_property_type_t2str(property->type),
+ property->len);

- switch (property.type) {
+ switch (property->type) {
case BT_PROPERTY_BDNAME:
case BT_PROPERTY_REMOTE_FRIENDLY_NAME:
- sprintf(p, "%*s", property.len,
- ((bt_bdname_t *) property.val)->name);
+ sprintf(p, "%*s", property->len,
+ ((bt_bdname_t *) property->val)->name);
break;

case BT_PROPERTY_BDADDR:
- sprintf(p, "%s", bdaddr2str((bt_bdaddr_t *) property.val));
+ sprintf(p, "%s", bdaddr2str((bt_bdaddr_t *) property->val));
break;

case BT_PROPERTY_CLASS_OF_DEVICE:
- sprintf(p, "%06x", *((int *) property.val));
+ sprintf(p, "%06x", *((int *) property->val));
break;

case BT_PROPERTY_TYPE_OF_DEVICE:
sprintf(p, "%s", bt_device_type_t2str(
- *((bt_device_type_t *) property.val)));
+ *((bt_device_type_t *) property->val)));
break;

case BT_PROPERTY_REMOTE_RSSI:
- sprintf(p, "%d", *((char *) property.val));
+ sprintf(p, "%d", *((char *) property->val));
break;

case BT_PROPERTY_ADAPTER_SCAN_MODE:
sprintf(p, "%s",
- bt_scan_mode_t2str(*((bt_scan_mode_t *) property.val)));
+ bt_scan_mode_t2str(*((bt_scan_mode_t *) property->val)));
break;

case BT_PROPERTY_ADAPTER_DISCOVERY_TIMEOUT:
- sprintf(p, "%d", *((int *) property.val));
+ sprintf(p, "%d", *((int *) property->val));
break;

case BT_PROPERTY_ADAPTER_BONDED_DEVICES:
{
- int count = property.len / sizeof(bt_bdaddr_t);
- char *ptr = property.val;
+ int count = property->len / sizeof(bt_bdaddr_t);
+ char *ptr = property->val;

strcat(p, "{");

@@ -304,8 +304,8 @@ char *btproperty2str(bt_property_t property)

case BT_PROPERTY_UUIDS:
{
- int count = property.len / sizeof(bt_uuid_t);
- char *ptr = property.val;
+ int count = property->len / sizeof(bt_uuid_t);
+ char *ptr = property->val;

strcat(p, "{");

@@ -323,7 +323,7 @@ char *btproperty2str(bt_property_t property)

case BT_PROPERTY_SERVICE_RECORD:
{
- bt_service_record_t *rec = property.val;
+ bt_service_record_t *rec = property->val;

sprintf(p, "{%s, %d, %s}", btuuid2str(&rec->uuid),
rec->channel, rec->name);
@@ -331,7 +331,7 @@ char *btproperty2str(bt_property_t property)
break;

default:
- sprintf(p, "%p", property.val);
+ sprintf(p, "%p", property->val);
}

return buf;
diff --git a/android/client/textconv.h b/android/client/textconv.h
index 89b29c6..1c848ef 100644
--- a/android/client/textconv.h
+++ b/android/client/textconv.h
@@ -107,7 +107,7 @@ void str2bt_bdaddr_t(const char *str, bt_bdaddr_t *bd_addr);
char *bt_uuid_t2str(const bt_uuid_t *uuid, char *buf);
void str2bt_uuid_t(const char *str, bt_uuid_t *uuid);

-char *btproperty2str(bt_property_t property);
+char *btproperty2str(const bt_property_t *property);
char *bdaddr2str(const bt_bdaddr_t *bd_addr);

DECINTMAP(bt_status_t);
--
1.7.10.4