2014-02-19 13:54:51

by Jakub Tyszkowski

[permalink] [raw]
Subject: [RFCv2 00/11] Make android-tester callbacks thread safe

This patch set makes all callbacks executions in HAL's notification thread being
transfered to tester's context. As test-specific callbacks are called by generic
callbacks, making the later executed in tester's main loop makes all custom
callbacks (and newly added) automatically executed in the right contex.

Forking out the emulator to be controlled over IPC makes finer controll harder
and requires IPC extensions when new functionality is required. It also adds
extra complexity.

Our initial 'IPC' solution creates additional ~800 lines of code to maintain in
comparison to ~260 added by this simplier 'main loop' solution.

Please state your's opinions on these options.

v2 changes:
* Replaced generic callback data structure with per HAL structures for clarity
and to avoid type casting of reused data fields
* Fixed passing report data in HID callback

Best regards,

Jakub Tyszkowski (11):
android/tester: Execute device found cbacks in main loop
android/tester: Execute discovery state cbacks in main loop
android/tester: Execute device properties cbacks in main loop
android/tester: Execute adapter props cbacks in main loop
android/tester: Execute adapter state changed cbacks in main loop
android/tester: Execute socket cbacks in main loop
android/tester: Execute hh connection state cbacks in main loop
android/tester: Execute hh info cbacks in main loop
android/tester: Execute hh protocol mode cbacks in main loop
android/tester: Execute hh report cbacks in main loop
android/tester: Execute hh virtual unplug cbacks in main loop

android/android-tester.c | 303 ++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 258 insertions(+), 45 deletions(-)

--
1.8.5.2



2014-02-19 13:55:01

by Jakub Tyszkowski

[permalink] [raw]
Subject: [RFCv2 10/11] android/tester: Execute hh report cbacks in main loop

Execute HIDHost generic get_report_cb in tester's main loop.
---
android/android-tester.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 4789844..feddd8f 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -3319,17 +3319,34 @@ static void hidhost_protocol_mode_cb(bt_bdaddr_t *bd_addr,
g_idle_add(hidhost_protocol_mode, cb_data);
}

-static void hidhost_get_report_cb(bt_bdaddr_t *bd_addr, bthh_status_t status,
- uint8_t *report, int size)
+static gboolean hidhost_get_report(gpointer user_data)
{
struct test_data *data = tester_get_data();
const struct hidhost_generic_data *test = data->test_data;
+ struct hh_cb_data *cb_data = user_data;

data->cb_count++;

if (test && test->expected_hal_cb.get_report_cb)
- test->expected_hal_cb.get_report_cb(bd_addr, status, report,
- size);
+ test->expected_hal_cb.get_report_cb(&cb_data->bdaddr,
+ cb_data->status, cb_data->report, cb_data->size);
+
+ g_free(cb_data->report);
+ g_free(cb_data);
+ return FALSE;
+}
+
+static void hidhost_get_report_cb(bt_bdaddr_t *bd_addr, bthh_status_t status,
+ uint8_t *report, int size)
+{
+ struct hh_cb_data *cb_data = g_new0(struct hh_cb_data, 1);
+
+ cb_data->bdaddr = *bd_addr;
+ cb_data->status = status;
+ cb_data->report = g_memdup(report, size);
+ cb_data->size = size;
+
+ g_idle_add(hidhost_get_report, cb_data);
}

static bthh_callbacks_t bthh_callbacks = {
--
1.8.5.2


2014-02-19 13:55:02

by Jakub Tyszkowski

[permalink] [raw]
Subject: [RFCv2 11/11] android/tester: Execute hh virtual unplug cbacks in main loop

Execute generic HIDHost virtual_unplug_cb in tester's main loop.
---
android/android-tester.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index feddd8f..0cd130d 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -3253,15 +3253,30 @@ static void hidhost_connection_state_cb(bt_bdaddr_t *bd_addr,
g_idle_add(hidhost_connection_state, cb_data);
}

-static void hidhost_virual_unplug_cb(bt_bdaddr_t *bd_addr, bthh_status_t status)
+static gboolean hidhost_virual_unplug(gpointer user_data)
{
struct test_data *data = tester_get_data();
const struct hidhost_generic_data *test = data->test_data;
+ struct hh_cb_data *cb_data = user_data;

data->cb_count++;

if (test && test->expected_hal_cb.virtual_unplug_cb)
- test->expected_hal_cb.virtual_unplug_cb(bd_addr, status);
+ test->expected_hal_cb.virtual_unplug_cb(&cb_data->bdaddr,
+ cb_data->status);
+
+ g_free(cb_data);
+ return FALSE;
+}
+
+static void hidhost_virual_unplug_cb(bt_bdaddr_t *bd_addr, bthh_status_t status)
+{
+ struct hh_cb_data *cb_data = g_new0(struct hh_cb_data, 1);
+
+ cb_data->bdaddr = *bd_addr;
+ cb_data->status = status;
+
+ g_idle_add(hidhost_virual_unplug, cb_data);
}

static gboolean hidhost_hid_info(gpointer user_data)
--
1.8.5.2


2014-02-19 13:55:00

by Jakub Tyszkowski

[permalink] [raw]
Subject: [RFCv2 09/11] android/tester: Execute hh protocol mode cbacks in main loop

Execute generic HIDHost protocol_mode_cb in tester's main loop.
---
android/android-tester.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 116593d..4789844 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -3290,17 +3290,33 @@ static void hidhost_hid_info_cb(bt_bdaddr_t *bd_addr, bthh_hid_info_t hid)
g_idle_add(hidhost_hid_info, cb_data);
}

-static void hidhost_protocol_mode_cb(bt_bdaddr_t *bd_addr,
- bthh_status_t status,
- bthh_protocol_mode_t mode)
+static gboolean hidhost_protocol_mode(gpointer user_data)
{
struct test_data *data = tester_get_data();
const struct hidhost_generic_data *test = data->test_data;
+ struct hh_cb_data *cb_data = user_data;

data->cb_count++;

if (test && test->expected_hal_cb.protocol_mode_cb)
- test->expected_hal_cb.protocol_mode_cb(bd_addr, status, mode);
+ test->expected_hal_cb.protocol_mode_cb(&cb_data->bdaddr,
+ cb_data->status, cb_data->mode);
+
+ g_free(cb_data);
+ return FALSE;
+}
+
+static void hidhost_protocol_mode_cb(bt_bdaddr_t *bd_addr,
+ bthh_status_t status,
+ bthh_protocol_mode_t mode)
+{
+ struct hh_cb_data *cb_data = g_new0(struct hh_cb_data, 1);
+
+ cb_data->bdaddr = *bd_addr;
+ cb_data->status = status;
+ cb_data->mode = mode;
+
+ g_idle_add(hidhost_protocol_mode, cb_data);
}

static void hidhost_get_report_cb(bt_bdaddr_t *bd_addr, bthh_status_t status,
--
1.8.5.2


2014-02-19 13:54:58

by Jakub Tyszkowski

[permalink] [raw]
Subject: [RFCv2 07/11] android/tester: Execute hh connection state cbacks in main loop

Execute generic HIDHost connection_state_cb in tester's main loop.
---
android/android-tester.c | 35 +++++++++++++++++++++++++++++++----
1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 71957ec..73b25a5 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -127,6 +127,18 @@ struct bt_cb_data {
bt_property_t *props;
};

+struct hh_cb_data {
+ bt_bdaddr_t bdaddr;
+
+ bthh_status_t status;
+ bthh_hid_info_t hid_info;
+ bthh_protocol_mode_t mode;
+ bthh_connection_state_t state;
+
+ uint8_t *report;
+ int size;
+};
+
static char exec_dir[PATH_MAX + 1];

static void mgmt_debug(const char *str, void *user_data)
@@ -3211,19 +3223,34 @@ clean:
close(sock_fd);
}

-static void hidhost_connection_state_cb(bt_bdaddr_t *bd_addr,
- bthh_connection_state_t state)
+static gboolean hidhost_connection_state(gpointer user_data)
{
struct test_data *data = tester_get_data();
const struct hidhost_generic_data *test = data->test_data;
+ struct hh_cb_data *cb_data = user_data;

data->cb_count++;

- if (state == BTHH_CONN_STATE_CONNECTED)
+ if (cb_data->state == BTHH_CONN_STATE_CONNECTED)
tester_setup_complete();

if (test && test->expected_hal_cb.connection_state_cb)
- test->expected_hal_cb.connection_state_cb(bd_addr, state);
+ test->expected_hal_cb.connection_state_cb(&cb_data->bdaddr,
+ cb_data->state);
+
+ g_free(cb_data);
+ return FALSE;
+}
+
+static void hidhost_connection_state_cb(bt_bdaddr_t *bd_addr,
+ bthh_connection_state_t state)
+{
+ struct hh_cb_data *cb_data = g_new0(struct hh_cb_data, 1);
+
+ cb_data->state = state;
+ cb_data->bdaddr = *bd_addr;
+
+ g_idle_add(hidhost_connection_state, cb_data);
}

static void hidhost_virual_unplug_cb(bt_bdaddr_t *bd_addr, bthh_status_t status)
--
1.8.5.2


2014-02-19 13:54:59

by Jakub Tyszkowski

[permalink] [raw]
Subject: [RFCv2 08/11] android/tester: Execute hh info cbacks in main loop

Execute generic HIDHost hid_info_cb in tester's main loop.
---
android/android-tester.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 73b25a5..116593d 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -3264,15 +3264,30 @@ static void hidhost_virual_unplug_cb(bt_bdaddr_t *bd_addr, bthh_status_t status)
test->expected_hal_cb.virtual_unplug_cb(bd_addr, status);
}

-static void hidhost_hid_info_cb(bt_bdaddr_t *bd_addr, bthh_hid_info_t hid)
+static gboolean hidhost_hid_info(gpointer user_data)
{
struct test_data *data = tester_get_data();
const struct hidhost_generic_data *test = data->test_data;
+ struct hh_cb_data *cb_data = user_data;

data->cb_count++;

if (test && test->expected_hal_cb.hid_info_cb)
- test->expected_hal_cb.hid_info_cb(bd_addr, hid);
+ test->expected_hal_cb.hid_info_cb(&cb_data->bdaddr,
+ cb_data->hid_info);
+
+ g_free(cb_data);
+ return FALSE;
+}
+
+static void hidhost_hid_info_cb(bt_bdaddr_t *bd_addr, bthh_hid_info_t hid)
+{
+ struct hh_cb_data *cb_data = g_new0(struct hh_cb_data, 1);
+
+ cb_data->bdaddr = *bd_addr;
+ cb_data->hid_info = hid;
+
+ g_idle_add(hidhost_hid_info, cb_data);
}

static void hidhost_protocol_mode_cb(bt_bdaddr_t *bd_addr,
--
1.8.5.2


2014-02-19 13:54:56

by Jakub Tyszkowski

[permalink] [raw]
Subject: [RFCv2 05/11] android/tester: Execute adapter state changed cbacks in main loop

Execute generic adapter_state_changed_cb in tester's main loop.
---
android/android-tester.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index b542a1f..0425b86 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -599,19 +599,33 @@ static void disable_success_cb(bt_state_t state)
}
}

-static void adapter_state_changed_cb(bt_state_t state)
+static gboolean adapter_state_changed(gpointer user_data)
{
struct test_data *data = tester_get_data();
const struct generic_data *test = data->test_data;
+ struct bt_cb_data *cb_data = user_data;

if (data->test_init_done &&
test->expected_hal_cb.adapter_state_changed_cb) {
- test->expected_hal_cb.adapter_state_changed_cb(state);
- return;
+ test->expected_hal_cb.adapter_state_changed_cb(cb_data->state);
+ goto cleanup;
}

- if (!data->test_init_done && state == BT_STATE_ON)
+ if (!data->test_init_done && cb_data->state == BT_STATE_ON)
setup_powered_emulated_remote();
+
+cleanup:
+ g_free(cb_data);
+ return FALSE;
+}
+
+static void adapter_state_changed_cb(bt_state_t state)
+{
+ struct bt_cb_data *cb_data = g_new0(struct bt_cb_data, 1);
+
+ cb_data->state = state;
+
+ g_idle_add(adapter_state_changed, cb_data);
}

static void discovery_start_success_cb(bt_discovery_state_t state)
--
1.8.5.2


2014-02-19 13:54:57

by Jakub Tyszkowski

[permalink] [raw]
Subject: [RFCv2 06/11] android/tester: Execute socket cbacks in main loop

Execute socket test's callbacks in tester's main loop.
---
android/android-tester.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 0425b86..71957ec 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -2779,9 +2779,11 @@ static void test_dev_setprop_disctimeout_fail(const void *test_data)
}
/* Test Socket HAL */

-static void adapter_socket_state_changed_cb(bt_state_t state)
+static gboolean adapter_socket_state_changed(gpointer user_data)
{
- switch (state) {
+ struct bt_cb_data *cb_data = user_data;
+
+ switch (cb_data->state) {
case BT_STATE_ON:
setup_powered_emulated_remote();
break;
@@ -2791,6 +2793,19 @@ static void adapter_socket_state_changed_cb(bt_state_t state)
default:
break;
}
+
+ g_free(cb_data);
+
+ return FALSE;
+}
+
+static void adapter_socket_state_changed_cb(bt_state_t state)
+{
+ struct bt_cb_data *cb_data = g_new0(struct bt_cb_data, 1);
+
+ cb_data->state = state;
+
+ g_idle_add(adapter_socket_state_changed, cb_data);
}

const bt_bdaddr_t bdaddr_dummy = {
--
1.8.5.2


2014-02-19 13:54:54

by Jakub Tyszkowski

[permalink] [raw]
Subject: [RFCv2 03/11] android/tester: Execute device properties cbacks in main loop

Execute generic remote_device_properties_cb in tester's main loop.
---
android/android-tester.c | 32 +++++++++++++++++++++++++-------
1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index a3b7ead..d909390 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -963,18 +963,36 @@ static void remote_setprop_device_properties_cb(bt_status_t status,
}
}

-static void remote_device_properties_cb(bt_status_t status,
- bt_bdaddr_t *bd_addr, int num_properties,
- bt_property_t *properties)
+static gboolean remote_device_properties(gpointer user_data)
{
struct test_data *data = tester_get_data();
const struct generic_data *test = data->test_data;
+ struct bt_cb_data *cb_data = user_data;

if (data->test_init_done &&
- test->expected_hal_cb.remote_device_properties_cb) {
- test->expected_hal_cb.remote_device_properties_cb(status,
- bd_addr, num_properties, properties);
- }
+ test->expected_hal_cb.remote_device_properties_cb)
+ test->expected_hal_cb.remote_device_properties_cb(
+ cb_data->status, &cb_data->bdaddr,
+ cb_data->num, cb_data->props);
+
+ free_properties(cb_data->num, cb_data->props);
+ g_free(cb_data);
+
+ return FALSE;
+}
+
+static void remote_device_properties_cb(bt_status_t status,
+ bt_bdaddr_t *bd_addr, int num_properties,
+ bt_property_t *properties)
+{
+ struct bt_cb_data *cb_data = g_new0(struct bt_cb_data, 1);
+
+ cb_data->status = status;
+ cb_data->bdaddr = *bd_addr;
+ cb_data->num = num_properties;
+ cb_data->props = copy_properties(num_properties, properties);
+
+ g_idle_add(remote_device_properties, cb_data);
}

static bt_bdaddr_t enable_done_bdaddr_val = { {0x00} };
--
1.8.5.2


2014-02-19 13:54:53

by Jakub Tyszkowski

[permalink] [raw]
Subject: [RFCv2 02/11] android/tester: Execute discovery state cbacks in main loop

Execute generic discovery_state_changed_cb in tester's main loop.
---
android/android-tester.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 9966a6f..a3b7ead 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -695,14 +695,27 @@ static void remote_setprop_disc_state_changed_cb(bt_discovery_state_t state)
}
}

-static void discovery_state_changed_cb(bt_discovery_state_t state)
+static gboolean discovery_state_changed(gpointer user_data)
{
struct test_data *data = tester_get_data();
const struct generic_data *test = data->test_data;
+ struct bt_cb_data *cb_data = user_data;

- if (test && test->expected_hal_cb.discovery_state_changed_cb) {
- test->expected_hal_cb.discovery_state_changed_cb(state);
- }
+ if (test && test->expected_hal_cb.discovery_state_changed_cb)
+ test->expected_hal_cb.discovery_state_changed_cb(
+ cb_data->state);
+
+ g_free(cb_data);
+
+ return FALSE;
+}
+
+static void discovery_state_changed_cb(bt_discovery_state_t state)
+{
+ struct bt_cb_data *cb_data = g_new0(struct bt_cb_data, 1);
+
+ cb_data->state = state;
+ g_idle_add(discovery_state_changed, cb_data);
}

static bt_property_t *copy_properties(int num_properties,
--
1.8.5.2


2014-02-19 13:54:55

by Jakub Tyszkowski

[permalink] [raw]
Subject: [RFCv2 04/11] android/tester: Execute adapter props cbacks in main loop

Execute generic adapter_properties_cb in tester's main loop.
---
android/android-tester.c | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index d909390..b542a1f 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -915,19 +915,32 @@ static void check_count_properties_cb(bt_status_t status, int num_properties,
check_expected_property(properties[i]);
}

+static gboolean adapter_properties(gpointer user_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ struct bt_cb_data *cb_data = user_data;
+
+ if (data->test_init_done && test->expected_hal_cb.adapter_properties_cb)
+ test->expected_hal_cb.adapter_properties_cb(cb_data->status,
+ cb_data->num, cb_data->props);
+
+ free_properties(cb_data->num, cb_data->props);
+ g_free(cb_data);
+
+ return FALSE;
+}

static void adapter_properties_cb(bt_status_t status, int num_properties,
bt_property_t *properties)
{
- struct test_data *data = tester_get_data();
- const struct generic_data *test = data->test_data;
+ struct bt_cb_data *cb_data = g_new0(struct bt_cb_data, 1);

- if (data->test_init_done &&
- test->expected_hal_cb.adapter_properties_cb) {
- test->expected_hal_cb.adapter_properties_cb(
- status, num_properties,
- properties);
- }
+ cb_data->status = status;
+ cb_data->num = num_properties;
+ cb_data->props = copy_properties(num_properties, properties);
+
+ g_idle_add(adapter_properties, cb_data);
}

static void remote_test_device_properties_cb(bt_status_t status,
--
1.8.5.2


2014-02-19 13:54:52

by Jakub Tyszkowski

[permalink] [raw]
Subject: [RFCv2 01/11] android/tester: Execute device found cbacks in main loop

Execute generic device_found_cb in tester's main loop.
---
android/android-tester.c | 60 ++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 55 insertions(+), 5 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 9605c4d..9966a6f 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -117,6 +117,16 @@ struct test_data {
uint16_t intr_cid;
};

+struct bt_cb_data {
+ bt_state_t state;
+ bt_status_t status;
+
+ bt_bdaddr_t bdaddr;
+
+ int num;
+ bt_property_t *props;
+};
+
static char exec_dir[PATH_MAX + 1];

static void mgmt_debug(const char *str, void *user_data)
@@ -695,6 +705,31 @@ static void discovery_state_changed_cb(bt_discovery_state_t state)
}
}

+static bt_property_t *copy_properties(int num_properties,
+ bt_property_t *properties)
+{
+ int i;
+ bt_property_t *props = g_new0(bt_property_t, num_properties);
+
+ for (i = 0; i < num_properties; i++) {
+ props[i].type = properties[i].type;
+ props[i].len = properties[i].len;
+ props[i].val = g_memdup(properties[i].val, properties[i].len);
+ }
+
+ return props;
+}
+
+static void free_properties(int num_properties, bt_property_t *properties)
+{
+ int i;
+
+ for (i = 0; i < num_properties; i++)
+ g_free(properties[i].val);
+
+ g_free(properties);
+}
+
static void discovery_device_found_cb(int num_properties,
bt_property_t *properties)
{
@@ -832,15 +867,30 @@ static void remote_setprop_fail_device_found_cb(int num_properties,
check_expected_status(status);
}

-static void device_found_cb(int num_properties, bt_property_t *properties)
+static gboolean device_found(gpointer user_data)
{
struct test_data *data = tester_get_data();
const struct generic_data *test = data->test_data;
+ struct bt_cb_data *cb_data = user_data;

- if (data->test_init_done && test->expected_hal_cb.device_found_cb) {
- test->expected_hal_cb.device_found_cb(num_properties,
- properties);
- }
+ if (data->test_init_done && test->expected_hal_cb.device_found_cb)
+ test->expected_hal_cb.device_found_cb(cb_data->num,
+ cb_data->props);
+
+ free_properties(cb_data->num, cb_data->props);
+ g_free(cb_data);
+
+ return FALSE;
+}
+
+static void device_found_cb(int num_properties, bt_property_t *properties)
+{
+ struct bt_cb_data *cb_data = g_new0(struct bt_cb_data, 1);
+
+ cb_data->num = num_properties;
+ cb_data->props = copy_properties(num_properties, properties);
+
+ g_idle_add(device_found, cb_data);
}

static void check_count_properties_cb(bt_status_t status, int num_properties,
--
1.8.5.2