[PATCH 00/11] Add bonding test cases

[PATCH 00/11] Add bonding test cases

[Andrzej Kaczmarek]

Hi,

A bit more explanation for last patch in this series:

We've encountered random timeouts for test cases which we found to be
because of missing "credit" in bthost which queued HCI command
indefinitely. The problem here was that previous HCI command in bthost
was sent from notification_handler thread in android-tester while reply
was handled in main thread. This caused race condition in bthost code
which is not thread-safe.

A quick fix for this would be to simply change order of operations in
send_command as follows:

if (bthost->ncmd) {
	bthost->ncmd--;
	send_packet(bthost, pkt_data, pkt_len);
} else {


So, first decrease number of "credits" and then send packet, so command
complete received in another thread cannot overwrite this value before
it's updated. But since this does not make code thread-safe and just
hides problem temporarily, we've decided to wrap all callbacks from
notification_thread to serialize their execution in main loop.


Andrzej Kaczmarek (1):
  android/tester: Make bt_callbacks thread-safe

Grzegorz Kolodziejczyk (10):
  android/tester: Coding style and syntax fix
  android/tester: Add create bond with PIN success test case
  android/tester: Add create bond with PIN fail test case
  android/tester: Add create bond with SSP sucess test case
  android/tester: Add create bond with SSP fail test case
  android/tester: Add create bond with no discovery test case
  android/tester: Add create bond with bad addr fail test case
  android/tester: Add cancel bond success test case
  android/tester: Add remove bond success test case
  android/tester: Add remove bond bad addr dev test case

 android/android-tester.c | 809 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 763 insertions(+), 46 deletions(-)

-- 
1.8.5.2

[PATCH 01/11] android/tester: Coding style and syntax fix

[Andrzej Kaczmarek]

From: Grzegorz Kolodziejczyk <grzegorz.kolodziejczyk@tieto.com>

Remove white spaces, break lines over 80 characters, redundand braces
for single statement blocks.
---
 android/android-tester.c | 50 +++++++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 870ad8d..8071e68 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -690,9 +690,8 @@ static void discovery_state_changed_cb(bt_discovery_state_t state)
 	struct test_data *data = tester_get_data();
 	const struct generic_data *test = data->test_data;
 
-	if (test && test->expected_hal_cb.discovery_state_changed_cb) {
+	if (test && test->expected_hal_cb.discovery_state_changed_cb)
 		test->expected_hal_cb.discovery_state_changed_cb(state);
-	}
 }
 
 static void discovery_device_found_cb(int num_properties,
@@ -837,10 +836,9 @@ static void device_found_cb(int num_properties, bt_property_t *properties)
 	struct test_data *data = tester_get_data();
 	const struct generic_data *test = data->test_data;
 
-	if (data->test_init_done && test->expected_hal_cb.device_found_cb) {
+	if (data->test_init_done && test->expected_hal_cb.device_found_cb)
 		test->expected_hal_cb.device_found_cb(num_properties,
 								properties);
-	}
 }
 
 static void check_count_properties_cb(bt_status_t status, int num_properties,
@@ -852,7 +850,6 @@ static void check_count_properties_cb(bt_status_t status, int num_properties,
 		check_expected_property(properties[i]);
 }
 
-
 static void adapter_properties_cb(bt_status_t status, int num_properties,
 						bt_property_t *properties)
 {
@@ -860,11 +857,9 @@ static void adapter_properties_cb(bt_status_t status, int num_properties,
 	const struct generic_data *test = data->test_data;
 
 	if (data->test_init_done &&
-				test->expected_hal_cb.adapter_properties_cb) {
-		test->expected_hal_cb.adapter_properties_cb(
-							status, num_properties,
-							properties);
-	}
+				test->expected_hal_cb.adapter_properties_cb)
+		test->expected_hal_cb.adapter_properties_cb(status,
+						num_properties, properties);
 }
 
 static void remote_test_device_properties_cb(bt_status_t status,
@@ -908,10 +903,9 @@ static void remote_device_properties_cb(bt_status_t status,
 	const struct generic_data *test = data->test_data;
 
 	if (data->test_init_done &&
-			test->expected_hal_cb.remote_device_properties_cb) {
+			test->expected_hal_cb.remote_device_properties_cb)
 		test->expected_hal_cb.remote_device_properties_cb(status,
 					bd_addr, num_properties, properties);
-	}
 }
 
 static bt_bdaddr_t enable_done_bdaddr_val = { {0x00} };
@@ -1324,7 +1318,8 @@ static struct priority_property setprop_scanmode_none_props[] = {
 	},
 };
 
-static const struct generic_data bluetooth_setprop_scanmode_none_success2_test = {
+static const struct generic_data
+			bluetooth_setprop_scanmode_none_success2_test = {
 	.expected_hal_cb.adapter_properties_cb = check_count_properties_cb,
 	.expected_properties_num = 1,
 	.expected_properties = setprop_scanmode_none_props,
@@ -2090,7 +2085,8 @@ static void test_enable(const void *test_data)
 
 	init_test_conditions(data);
 
-	bdaddr2android((const bdaddr_t *)bdaddr, &enable_done_bdaddr_val.address);
+	bdaddr2android((const bdaddr_t *)bdaddr,
+					&enable_done_bdaddr_val.address);
 
 	adapter_status = data->if_bluetooth->enable();
 	check_expected_status(adapter_status);
@@ -2105,7 +2101,8 @@ static void test_enable_done(const void *test_data)
 
 	init_test_conditions(data);
 
-	bdaddr2android((const bdaddr_t *)bdaddr, &enable_done_bdaddr_val.address);
+	bdaddr2android((const bdaddr_t *)bdaddr,
+					&enable_done_bdaddr_val.address);
 
 	adapter_status = data->if_bluetooth->enable();
 	check_expected_status(adapter_status);
@@ -2167,7 +2164,8 @@ static void test_getprop_bdaddr_success(const void *test_data)
 
 	init_test_conditions(data);
 
-	bdaddr2android((const bdaddr_t *)bdaddr, &test_getprop_bdaddr_val.address);
+	bdaddr2android((const bdaddr_t *)bdaddr,
+					&test_getprop_bdaddr_val.address);
 
 	adapter_status = data->if_bluetooth->get_adapter_property(prop.type);
 	check_expected_status(adapter_status);
@@ -3433,7 +3431,6 @@ static void emu_powered_complete(uint16_t opcode, uint8_t status,
 	bt_status = data->if_hid->connect(&bdaddr);
 	if (bt_status != BT_STATUS_SUCCESS)
 		tester_setup_failed();
-
 }
 
 static void setup_hidhost_connect(const void *test_data)
@@ -3632,15 +3629,20 @@ int main(int argc, char *argv[])
 
 	test_bredrle("Bluetooth Init", NULL, setup_base, test_dummy, teardown);
 
-	test_bredrle("Bluetooth Enable - Success", &bluetooth_enable_success_test,
-					setup_base, test_enable, teardown);
+	test_bredrle("Bluetooth Enable - Success",
+						&bluetooth_enable_success_test,
+						setup_base, test_enable,
+						teardown);
 
 	test_bredrle("Bluetooth Enable - Success 2",
-			&bluetooth_enable_success2_test, setup_enabled_adapter,
-			test_enable_done, teardown);
-
-	test_bredrle("Bluetooth Disable - Success", &bluetooth_disable_success_test,
-			setup_enabled_adapter, test_disable, teardown);
+						&bluetooth_enable_success2_test,
+						setup_enabled_adapter,
+						test_enable_done, teardown);
+
+	test_bredrle("Bluetooth Disable - Success",
+						&bluetooth_disable_success_test,
+						setup_enabled_adapter,
+						test_disable, teardown);
 
 	test_bredrle("Bluetooth Set BDNAME - Success",
 					&bluetooth_setprop_bdname_success_test,
-- 
1.8.5.2

[PATCH 02/11] android/tester: Add create bond with PIN success test case

[Andrzej Kaczmarek]

From: Grzegorz Kolodziejczyk <grzegorz.kolodziejczyk@tieto.com>

This adds create bond with PIN success test case.
---
 android/android-tester.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 116 insertions(+), 2 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 8071e68..761827e 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -831,6 +831,22 @@ static void remote_setprop_fail_device_found_cb(int num_properties,
 	check_expected_status(status);
 }
 
+static void bond_device_found_cb(int num_properties, bt_property_t *properties)
+{
+	struct test_data *data = tester_get_data();
+	uint8_t *bdaddr = (uint8_t *)hciemu_get_client_bdaddr(data->hciemu);
+	bt_bdaddr_t remote_addr;
+	bt_status_t status;
+
+	bdaddr2android((const bdaddr_t *)bdaddr, &remote_addr.address);
+
+	if (data->cb_count == 4) {
+		data->cb_count--;
+		status = data->if_bluetooth->create_bond(&remote_addr);
+		check_expected_status(status);
+	}
+}
+
 static void device_found_cb(int num_properties, bt_property_t *properties)
 {
 	struct test_data *data = tester_get_data();
@@ -908,6 +924,75 @@ static void remote_device_properties_cb(bt_status_t status,
 					bd_addr, num_properties, properties);
 }
 
+static void bond_test_state_changed_cb(bt_status_t status,
+			bt_bdaddr_t *remote_bd_addr, bt_bond_state_t state)
+{
+	struct test_data *data = tester_get_data();
+
+	switch (state) {
+	case BT_BOND_STATE_NONE:
+		data->cb_count--;
+		check_cb_count();
+		break;
+	case BT_BOND_STATE_BONDING:
+		data->cb_count--;
+		break;
+	case BT_BOND_STATE_BONDED:
+		data->cb_count--;
+		check_cb_count();
+		break;
+	default:
+		break;
+	}
+}
+
+static void bond_state_changed_cb(bt_status_t status,
+			bt_bdaddr_t *remote_bd_addr, bt_bond_state_t state)
+{
+	struct test_data *data = tester_get_data();
+	const struct generic_data *test = data->test_data;
+
+	if (data->test_init_done && test->expected_hal_cb.bond_state_changed_cb)
+			test->expected_hal_cb.bond_state_changed_cb(status,
+							remote_bd_addr, state);
+}
+
+static void bond_create_pin_success_request_cb(bt_bdaddr_t *remote_bd_addr,
+					bt_bdname_t *bd_name, uint32_t cod)
+{
+	struct test_data *data = tester_get_data();
+	struct mgmt_cp_pin_code_reply rp;
+	bdaddr_t remote_addr;
+	static uint8_t pair_device_pin[] = { 0x30, 0x30, 0x30, 0x30 };
+	const void *pin_code = pair_device_pin;
+	uint8_t pin_len = 4;
+
+	memset(&rp, 0, sizeof(rp));
+
+	data->cb_count--;
+
+	android2bdaddr(remote_bd_addr, &remote_addr);
+
+	bacpy(&rp.addr.bdaddr, &remote_addr);
+	rp.addr.type = BDADDR_BREDR;
+	rp.pin_len = pin_len;
+	memcpy(rp.pin_code, pin_code, rp.pin_len);
+
+	mgmt_reply(data->mgmt, MGMT_OP_PIN_CODE_REPLY, data->mgmt_index,
+					sizeof(rp), &rp, NULL, NULL, NULL);
+}
+
+static void pin_request_cb(bt_bdaddr_t *remote_bd_addr,
+					bt_bdname_t *bd_name, uint32_t cod)
+{
+	struct test_data *data = tester_get_data();
+	const struct generic_data *test = data->test_data;
+
+	if (data->test_init_done && test->expected_hal_cb.pin_request_cb)
+		test->expected_hal_cb.pin_request_cb(remote_bd_addr, bd_name,
+									cod);
+}
+
 static bt_bdaddr_t enable_done_bdaddr_val = { {0x00} };
 static char enable_done_bdname_val[] = "BlueZ for Android";
 static bt_uuid_t enable_done_uuids_val = {
@@ -1930,6 +2015,14 @@ static const struct generic_data bt_dev_setprop_disctimeout_fail_test = {
 	.expected_adapter_status = BT_STATUS_FAIL,
 };
 
+static const struct generic_data bt_bond_create_pin_success_test = {
+	.expected_hal_cb.device_found_cb = bond_device_found_cb,
+	.expected_hal_cb.bond_state_changed_cb = bond_test_state_changed_cb,
+	.expected_hal_cb.pin_request_cb = bond_create_pin_success_request_cb,
+	.expected_cb_count = 4,
+	.expected_adapter_status = BT_STATUS_SUCCESS,
+};
+
 static bt_callbacks_t bt_callbacks = {
 	.size = sizeof(bt_callbacks),
 	.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -1937,9 +2030,9 @@ static bt_callbacks_t bt_callbacks = {
 	.remote_device_properties_cb = remote_device_properties_cb,
 	.device_found_cb = device_found_cb,
 	.discovery_state_changed_cb = discovery_state_changed_cb,
-	.pin_request_cb = NULL,
 	.ssp_request_cb = NULL,
-	.bond_state_changed_cb = NULL,
+	.pin_request_cb = pin_request_cb,
+	.bond_state_changed_cb = bond_state_changed_cb,
 	.acl_state_changed_cb = NULL,
 	.thread_evt_cb = NULL,
 	.dut_mode_recv_cb = NULL,
@@ -2667,6 +2760,22 @@ static void test_dev_setprop_disctimeout_fail(const void *test_data)
 
 	data->if_bluetooth->start_discovery();
 }
+static void test_bond_create_pin_success(const void *test_data)
+{
+	struct test_data *data = tester_get_data();
+	struct bthost *bthost = hciemu_client_get_host(data->hciemu);
+
+	static uint8_t pair_device_pin[] = { 0x30, 0x30, 0x30, 0x30 };
+	const void *pin = pair_device_pin;
+	uint8_t pin_len = 4;
+
+	init_test_conditions(data);
+
+	bthost_set_pin_code(bthost, pin, pin_len);
+
+	data->if_bluetooth->start_discovery();
+}
+
 /* Test Socket HAL */
 
 static void adapter_socket_state_changed_cb(bt_state_t state)
@@ -3899,6 +4008,11 @@ int main(int argc, char *argv[])
 				setup_enabled_adapter,
 				test_dev_setprop_disctimeout_fail, teardown);
 
+	test_bredrle("Bluetooth Create Bond PIN - Success",
+					&bt_bond_create_pin_success_test,
+					setup_enabled_adapter,
+					test_bond_create_pin_success, teardown);
+
 	test_bredrle("Socket Init", NULL, setup_socket_interface,
 						test_dummy, teardown);
 
-- 
1.8.5.2

Re: [PATCH 02/11] android/tester: Add create bond with PIN success test case

[Anderson Lizardo]

Hi Andrzej,

On Sun, Feb 2, 2014 at 7:16 AM, Andrzej Kaczmarek
<andrzej.kaczmarek@tieto.com> wrote:
> +static void bond_create_pin_success_request_cb(bt_bdaddr_t *remote_bd_addr,
> +                                       bt_bdname_t *bd_name, uint32_t cod)
> +{
> +       struct test_data *data = tester_get_data();
> +       struct mgmt_cp_pin_code_reply rp;
> +       bdaddr_t remote_addr;
> +       static uint8_t pair_device_pin[] = { 0x30, 0x30, 0x30, 0x30 };
> +       const void *pin_code = pair_device_pin;
> +       uint8_t pin_len = 4;
> +
> +       memset(&rp, 0, sizeof(rp));
> +
> +       data->cb_count--;
> +
> +       android2bdaddr(remote_bd_addr, &remote_addr);
> +
> +       bacpy(&rp.addr.bdaddr, &remote_addr);
> +       rp.addr.type = BDADDR_BREDR;
> +       rp.pin_len = pin_len;
> +       memcpy(rp.pin_code, pin_code, rp.pin_len);
> +
> +       mgmt_reply(data->mgmt, MGMT_OP_PIN_CODE_REPLY, data->mgmt_index,
> +                                       sizeof(rp), &rp, NULL, NULL, NULL);

I wonder why you are sending mgmt commands directly instead of using
data->if_bluetooth->pin_reply()?

NOTE: I'm not very familiar with the HAL API so I may just be missing
the point of the test.

Best Regards,
-- 
Anderson Lizardo
http://www.indt.org/?lang=en
INdT - Manaus - Brazil

[PATCH 03/11] android/tester: Add create bond with PIN fail test case

[Andrzej Kaczmarek]

From: Grzegorz Kolodziejczyk <grzegorz.kolodziejczyk@tieto.com>

This adds create bond with PIN fail test case.
---
 android/android-tester.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index 761827e..d8f7d82 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -847,6 +847,21 @@ static void bond_device_found_cb(int num_properties, bt_property_t *properties)
 	}
 }
 
+static void bond_nostatus_device_found_cb(int num_properties,
+						bt_property_t *properties)
+{
+	struct test_data *data = tester_get_data();
+	uint8_t *bdaddr = (uint8_t *)hciemu_get_client_bdaddr(data->hciemu);
+	bt_bdaddr_t remote_addr;
+
+	bdaddr2android((const bdaddr_t *)bdaddr, &remote_addr.address);
+
+	if (data->cb_count == 4) {
+		data->cb_count--;
+		data->if_bluetooth->create_bond(&remote_addr);
+	}
+}
+
 static void device_found_cb(int num_properties, bt_property_t *properties)
 {
 	struct test_data *data = tester_get_data();
@@ -982,6 +997,31 @@ static void bond_create_pin_success_request_cb(bt_bdaddr_t *remote_bd_addr,
 					sizeof(rp), &rp, NULL, NULL, NULL);
 }
 
+static void bond_create_pin_fail_request_cb(bt_bdaddr_t *remote_bd_addr,
+					bt_bdname_t *bd_name, uint32_t cod)
+{
+	struct test_data *data = tester_get_data();
+	struct mgmt_cp_pin_code_reply rp;
+	bdaddr_t remote_addr;
+	static uint8_t bad_device_pin[] = { 0x31, 0x31, 0x31, 0x31 };
+	const void *pin_code = bad_device_pin;
+	uint8_t pin_len = 4;
+
+	memset(&rp, 0, sizeof(rp));
+
+	android2bdaddr(remote_bd_addr, &remote_addr);
+
+	data->cb_count--;
+
+	bacpy(&rp.addr.bdaddr, &remote_addr);
+	rp.addr.type = BDADDR_BREDR;
+	rp.pin_len = pin_len;
+	memcpy(rp.pin_code, pin_code, rp.pin_len);
+
+	mgmt_reply(data->mgmt, MGMT_OP_PIN_CODE_REPLY, data->mgmt_index,
+					sizeof(rp), &rp, NULL, NULL, NULL);
+}
+
 static void pin_request_cb(bt_bdaddr_t *remote_bd_addr,
 					bt_bdname_t *bd_name, uint32_t cod)
 {
@@ -2023,6 +2063,14 @@ static const struct generic_data bt_bond_create_pin_success_test = {
 	.expected_adapter_status = BT_STATUS_SUCCESS,
 };
 
+static const struct generic_data bt_bond_create_pin_fail_test = {
+	.expected_hal_cb.device_found_cb = bond_nostatus_device_found_cb,
+	.expected_hal_cb.bond_state_changed_cb = bond_test_state_changed_cb,
+	.expected_hal_cb.pin_request_cb = bond_create_pin_fail_request_cb,
+	.expected_cb_count = 4,
+	.expected_adapter_status = MGMT_STATUS_AUTH_FAILED,
+};
+
 static bt_callbacks_t bt_callbacks = {
 	.size = sizeof(bt_callbacks),
 	.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -2760,6 +2808,16 @@ static void test_dev_setprop_disctimeout_fail(const void *test_data)
 
 	data->if_bluetooth->start_discovery();
 }
+
+static void bond_device_auth_fail_callback(uint16_t index, uint16_t length,
+							const void *param,
+							void *user_data)
+{
+	const struct mgmt_ev_auth_failed *ev = param;
+
+	check_expected_status(ev->status);
+}
+
 static void test_bond_create_pin_success(const void *test_data)
 {
 	struct test_data *data = tester_get_data();
@@ -2776,6 +2834,26 @@ static void test_bond_create_pin_success(const void *test_data)
 	data->if_bluetooth->start_discovery();
 }
 
+static void test_bond_create_pin_fail(const void *test_data)
+{
+	struct test_data *data = tester_get_data();
+	struct bthost *bthost = hciemu_client_get_host(data->hciemu);
+
+	static uint8_t pair_device_pin[] = { 0x30, 0x30, 0x30, 0x30 };
+	const void *pin = pair_device_pin;
+	uint8_t pin_len = 4;
+
+	init_test_conditions(data);
+
+	mgmt_register(data->mgmt, MGMT_EV_AUTH_FAILED, data->mgmt_index,
+					bond_device_auth_fail_callback, data,
+					NULL);
+
+	bthost_set_pin_code(bthost, pin, pin_len);
+
+	data->if_bluetooth->start_discovery();
+}
+
 /* Test Socket HAL */
 
 static void adapter_socket_state_changed_cb(bt_state_t state)
@@ -4013,6 +4091,11 @@ int main(int argc, char *argv[])
 					setup_enabled_adapter,
 					test_bond_create_pin_success, teardown);
 
+	test_bredrle("Bluetooth Create Bond PIN - Bad PIN",
+					&bt_bond_create_pin_fail_test,
+					setup_enabled_adapter,
+					test_bond_create_pin_fail, teardown);
+
 	test_bredrle("Socket Init", NULL, setup_socket_interface,
 						test_dummy, teardown);
 
-- 
1.8.5.2

[PATCH 04/11] android/tester: Add create bond with SSP sucess test case

[Andrzej Kaczmarek]

From: Grzegorz Kolodziejczyk <grzegorz.kolodziejczyk@tieto.com>

This adds create bond with SSP succes test case. Confirm is set as SSP
pairing mode.
---
 android/android-tester.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 77 insertions(+), 1 deletion(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index d8f7d82..bfa23e9 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -1033,6 +1033,57 @@ static void pin_request_cb(bt_bdaddr_t *remote_bd_addr,
 									cod);
 }
 
+static void bond_create_ssp_request_cb(const bt_bdaddr_t *remote_bd_addr,
+					bt_ssp_variant_t pairing_variant,
+					bool accept)
+{
+	bdaddr_t remote_addr;
+	struct test_data *data = tester_get_data();
+	struct mgmt_addr_info cp;
+	uint16_t opcode;
+
+	android2bdaddr(remote_bd_addr, &remote_addr);
+
+	if (pairing_variant == 0) {	/* HAL_SSP_VARIANT_CONFIRM */
+		if (accept)
+			opcode = MGMT_OP_USER_CONFIRM_REPLY;
+		else
+			opcode = MGMT_OP_USER_CONFIRM_NEG_REPLY;
+
+		data->cb_count--;
+
+		bacpy(&cp.bdaddr, &remote_addr);
+		cp.type = BDADDR_BREDR;
+
+		mgmt_reply(data->mgmt, opcode, data->mgmt_index, sizeof(cp),
+							&cp, NULL, NULL, NULL);
+	} else
+		tester_test_failed();
+}
+
+static void bond_create_ssp_success_request_cb(bt_bdaddr_t *remote_bd_addr,
+					bt_bdname_t *bd_name, uint32_t cod,
+					bt_ssp_variant_t pairing_variant,
+					uint32_t pass_key)
+{
+	bool accept = true;
+
+	bond_create_ssp_request_cb(remote_bd_addr, pairing_variant, accept);
+}
+
+static void ssp_request_cb(bt_bdaddr_t *remote_bd_addr, bt_bdname_t *bd_name,
+				uint32_t cod, bt_ssp_variant_t pairing_variant,
+				uint32_t pass_key)
+{
+	struct test_data *data = tester_get_data();
+	const struct generic_data *test = data->test_data;
+
+	if (data->test_init_done &&
+				test->expected_hal_cb.ssp_request_cb)
+		test->expected_hal_cb.ssp_request_cb(remote_bd_addr, bd_name,
+					cod, pairing_variant, pass_key);
+}
+
 static bt_bdaddr_t enable_done_bdaddr_val = { {0x00} };
 static char enable_done_bdname_val[] = "BlueZ for Android";
 static bt_uuid_t enable_done_uuids_val = {
@@ -2071,6 +2122,14 @@ static const struct generic_data bt_bond_create_pin_fail_test = {
 	.expected_adapter_status = MGMT_STATUS_AUTH_FAILED,
 };
 
+static const struct generic_data bt_bond_create_ssp_success_test = {
+	.expected_hal_cb.device_found_cb = bond_device_found_cb,
+	.expected_hal_cb.bond_state_changed_cb = bond_test_state_changed_cb,
+	.expected_hal_cb.ssp_request_cb = bond_create_ssp_success_request_cb,
+	.expected_cb_count = 4,
+	.expected_adapter_status = BT_STATUS_SUCCESS,
+};
+
 static bt_callbacks_t bt_callbacks = {
 	.size = sizeof(bt_callbacks),
 	.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -2078,8 +2137,8 @@ static bt_callbacks_t bt_callbacks = {
 	.remote_device_properties_cb = remote_device_properties_cb,
 	.device_found_cb = device_found_cb,
 	.discovery_state_changed_cb = discovery_state_changed_cb,
-	.ssp_request_cb = NULL,
 	.pin_request_cb = pin_request_cb,
+	.ssp_request_cb = ssp_request_cb,
 	.bond_state_changed_cb = bond_state_changed_cb,
 	.acl_state_changed_cb = NULL,
 	.thread_evt_cb = NULL,
@@ -2854,6 +2913,18 @@ static void test_bond_create_pin_fail(const void *test_data)
 	data->if_bluetooth->start_discovery();
 }
 
+static void test_bond_create_ssp_success(const void *test_data)
+{
+	struct test_data *data = tester_get_data();
+	struct bthost *bthost = hciemu_client_get_host(data->hciemu);
+
+	init_test_conditions(data);
+
+	bthost_write_ssp_mode(bthost, 0x01);
+
+	data->if_bluetooth->start_discovery();
+}
+
 /* Test Socket HAL */
 
 static void adapter_socket_state_changed_cb(bt_state_t state)
@@ -4096,6 +4167,11 @@ int main(int argc, char *argv[])
 					setup_enabled_adapter,
 					test_bond_create_pin_fail, teardown);
 
+	test_bredrle("Bluetooth Create Bond SSP - Success",
+					&bt_bond_create_ssp_success_test,
+					setup_enabled_adapter,
+					test_bond_create_ssp_success, teardown);
+
 	test_bredrle("Socket Init", NULL, setup_socket_interface,
 						test_dummy, teardown);
 
-- 
1.8.5.2

[PATCH 05/11] android/tester: Add create bond with SSP fail test case

[Andrzej Kaczmarek]

From: Grzegorz Kolodziejczyk <grzegorz.kolodziejczyk@tieto.com>

This adds create bond with SSP fail test case. Confirm is set as SSP
pairing mode.
---
 android/android-tester.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index bfa23e9..ba3b812 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -1071,6 +1071,16 @@ static void bond_create_ssp_success_request_cb(bt_bdaddr_t *remote_bd_addr,
 	bond_create_ssp_request_cb(remote_bd_addr, pairing_variant, accept);
 }
 
+static void bond_create_ssp_fail_request_cb(bt_bdaddr_t *remote_bd_addr,
+					bt_bdname_t *bd_name, uint32_t cod,
+					bt_ssp_variant_t pairing_variant,
+					uint32_t pass_key)
+{
+	bool accept = false;
+
+	bond_create_ssp_request_cb(remote_bd_addr, pairing_variant, accept);
+}
+
 static void ssp_request_cb(bt_bdaddr_t *remote_bd_addr, bt_bdname_t *bd_name,
 				uint32_t cod, bt_ssp_variant_t pairing_variant,
 				uint32_t pass_key)
@@ -2130,6 +2140,14 @@ static const struct generic_data bt_bond_create_ssp_success_test = {
 	.expected_adapter_status = BT_STATUS_SUCCESS,
 };
 
+static const struct generic_data bt_bond_create_ssp_fail_test = {
+	.expected_hal_cb.device_found_cb = bond_nostatus_device_found_cb,
+	.expected_hal_cb.bond_state_changed_cb = bond_test_state_changed_cb,
+	.expected_hal_cb.ssp_request_cb = bond_create_ssp_fail_request_cb,
+	.expected_cb_count = 4,
+	.expected_adapter_status = MGMT_STATUS_AUTH_FAILED,
+};
+
 static bt_callbacks_t bt_callbacks = {
 	.size = sizeof(bt_callbacks),
 	.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -2925,6 +2943,22 @@ static void test_bond_create_ssp_success(const void *test_data)
 	data->if_bluetooth->start_discovery();
 }
 
+static void test_bond_create_ssp_fail(const void *test_data)
+{
+	struct test_data *data = tester_get_data();
+	struct bthost *bthost = hciemu_client_get_host(data->hciemu);
+
+	init_test_conditions(data);
+
+	mgmt_register(data->mgmt, MGMT_EV_AUTH_FAILED, data->mgmt_index,
+					bond_device_auth_fail_callback, data,
+					NULL);
+
+	bthost_write_ssp_mode(bthost, 0x01);
+
+	data->if_bluetooth->start_discovery();
+}
+
 /* Test Socket HAL */
 
 static void adapter_socket_state_changed_cb(bt_state_t state)
@@ -4172,6 +4206,11 @@ int main(int argc, char *argv[])
 					setup_enabled_adapter,
 					test_bond_create_ssp_success, teardown);
 
+	test_bredrle("Bluetooth Create Bond SSP - Negative reply",
+					&bt_bond_create_ssp_fail_test,
+					setup_enabled_adapter,
+					test_bond_create_ssp_fail, teardown);
+
 	test_bredrle("Socket Init", NULL, setup_socket_interface,
 						test_dummy, teardown);
 
-- 
1.8.5.2

[PATCH 06/11] android/tester: Add create bond with no discovery test case

[Andrzej Kaczmarek]

From: Grzegorz Kolodziejczyk <grzegorz.kolodziejczyk@tieto.com>

This adds create bond with no discovery before create bond as NFC does.
SSP with confirm as authentication mode.
---
 android/android-tester.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index ba3b812..c8c2a75 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -2148,6 +2148,13 @@ static const struct generic_data bt_bond_create_ssp_fail_test = {
 	.expected_adapter_status = MGMT_STATUS_AUTH_FAILED,
 };
 
+static const struct generic_data bt_bond_create_no_disc_success_test = {
+	.expected_hal_cb.bond_state_changed_cb = bond_test_state_changed_cb,
+	.expected_hal_cb.ssp_request_cb = bond_create_ssp_success_request_cb,
+	.expected_cb_count = 3,
+	.expected_adapter_status = BT_STATUS_SUCCESS,
+};
+
 static bt_callbacks_t bt_callbacks = {
 	.size = sizeof(bt_callbacks),
 	.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -2959,6 +2966,25 @@ static void test_bond_create_ssp_fail(const void *test_data)
 	data->if_bluetooth->start_discovery();
 }
 
+static void test_bond_create_no_disc_success(const void *test_data)
+{
+	struct test_data *data = tester_get_data();
+	struct bthost *bthost = hciemu_client_get_host(data->hciemu);
+
+	uint8_t *bdaddr = (uint8_t *)hciemu_get_client_bdaddr(data->hciemu);
+	bt_bdaddr_t remote_addr;
+	bt_status_t status;
+
+	init_test_conditions(data);
+
+	bdaddr2android((const bdaddr_t *)bdaddr, &remote_addr.address);
+
+	bthost_write_ssp_mode(bthost, 0x01);
+
+	status = data->if_bluetooth->create_bond(&remote_addr);
+	check_expected_status(status);
+}
+
 /* Test Socket HAL */
 
 static void adapter_socket_state_changed_cb(bt_state_t state)
@@ -4211,6 +4237,11 @@ int main(int argc, char *argv[])
 					setup_enabled_adapter,
 					test_bond_create_ssp_fail, teardown);
 
+	test_bredrle("Bluetooth Create Bond - No Discovery",
+				&bt_bond_create_no_disc_success_test,
+				setup_enabled_adapter,
+				test_bond_create_no_disc_success, teardown);
+
 	test_bredrle("Socket Init", NULL, setup_socket_interface,
 						test_dummy, teardown);
 
-- 
1.8.5.2

[PATCH 07/11] android/tester: Add create bond with bad addr fail test case

[Andrzej Kaczmarek]

From: Grzegorz Kolodziejczyk <grzegorz.kolodziejczyk@tieto.com>

This adds create bond with bad addr fail test case.
---
 android/android-tester.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index c8c2a75..3497f4a 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -2155,6 +2155,10 @@ static const struct generic_data bt_bond_create_no_disc_success_test = {
 	.expected_adapter_status = BT_STATUS_SUCCESS,
 };
 
+static const struct generic_data bt_bond_create_bad_addr_success_test = {
+	.expected_adapter_status = MGMT_STATUS_CONNECT_FAILED,
+};
+
 static bt_callbacks_t bt_callbacks = {
 	.size = sizeof(bt_callbacks),
 	.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -2985,6 +2989,22 @@ static void test_bond_create_no_disc_success(const void *test_data)
 	check_expected_status(status);
 }
 
+static void test_bond_create_bad_addr_success(const void *test_data)
+{
+	struct test_data *data = tester_get_data();
+	bt_bdaddr_t bad_addr = {
+		.address = { 0x12, 0x34, 0x56, 0x78, 0x90, 0x12 }
+	};
+
+	init_test_conditions(data);
+
+	mgmt_register(data->mgmt, MGMT_EV_CONNECT_FAILED, data->mgmt_index,
+					bond_device_auth_fail_callback, data,
+					NULL);
+
+	data->if_bluetooth->create_bond(&bad_addr);
+}
+
 /* Test Socket HAL */
 
 static void adapter_socket_state_changed_cb(bt_state_t state)
@@ -4242,6 +4262,11 @@ int main(int argc, char *argv[])
 				setup_enabled_adapter,
 				test_bond_create_no_disc_success, teardown);
 
+	test_bredrle("Bluetooth Create Bond - Bad Address",
+				&bt_bond_create_bad_addr_success_test,
+				setup_enabled_adapter,
+				test_bond_create_bad_addr_success, teardown);
+
 	test_bredrle("Socket Init", NULL, setup_socket_interface,
 						test_dummy, teardown);
 
-- 
1.8.5.2

[PATCH 08/11] android/tester: Add cancel bond success test case

[Andrzej Kaczmarek]

From: Grzegorz Kolodziejczyk <grzegorz.kolodziejczyk@tieto.com>

This adds cancel bond success test case. SSP as pairing mode.
---
 android/android-tester.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index 3497f4a..b7abc24 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -1081,6 +1081,24 @@ static void bond_create_ssp_fail_request_cb(bt_bdaddr_t *remote_bd_addr,
 	bond_create_ssp_request_cb(remote_bd_addr, pairing_variant, accept);
 }
 
+static void bond_cancel_success_ssp_request_cb(bt_bdaddr_t *remote_bd_addr,
+					bt_bdname_t *bd_name, uint32_t cod,
+					bt_ssp_variant_t pairing_variant,
+					uint32_t pass_key)
+{
+	struct test_data *data = tester_get_data();
+	uint8_t *bdaddr = (uint8_t *)hciemu_get_client_bdaddr(data->hciemu);
+	bt_bdaddr_t remote_addr;
+	bt_status_t status;
+
+	bdaddr2android((const bdaddr_t *)bdaddr, &remote_addr.address);
+
+	data->cb_count--;
+
+	status = data->if_bluetooth->cancel_bond(&remote_addr);
+	check_expected_status(status);
+}
+
 static void ssp_request_cb(bt_bdaddr_t *remote_bd_addr, bt_bdname_t *bd_name,
 				uint32_t cod, bt_ssp_variant_t pairing_variant,
 				uint32_t pass_key)
@@ -2159,6 +2177,14 @@ static const struct generic_data bt_bond_create_bad_addr_success_test = {
 	.expected_adapter_status = MGMT_STATUS_CONNECT_FAILED,
 };
 
+static const struct generic_data bt_bond_cancel_success_test = {
+	.expected_hal_cb.device_found_cb = bond_nostatus_device_found_cb,
+	.expected_hal_cb.bond_state_changed_cb = bond_test_state_changed_cb,
+	.expected_hal_cb.ssp_request_cb = bond_cancel_success_ssp_request_cb,
+	.expected_cb_count = 4,
+	.expected_adapter_status = BT_STATUS_SUCCESS,
+};
+
 static bt_callbacks_t bt_callbacks = {
 	.size = sizeof(bt_callbacks),
 	.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -3005,6 +3031,18 @@ static void test_bond_create_bad_addr_success(const void *test_data)
 	data->if_bluetooth->create_bond(&bad_addr);
 }
 
+static void test_bond_cancel_success(const void *test_data)
+{
+	struct test_data *data = tester_get_data();
+	struct bthost *bthost = hciemu_client_get_host(data->hciemu);
+
+	init_test_conditions(data);
+
+	bthost_write_ssp_mode(bthost, 0x01);
+
+	data->if_bluetooth->start_discovery();
+}
+
 /* Test Socket HAL */
 
 static void adapter_socket_state_changed_cb(bt_state_t state)
@@ -4267,6 +4305,11 @@ int main(int argc, char *argv[])
 				setup_enabled_adapter,
 				test_bond_create_bad_addr_success, teardown);
 
+	test_bredrle("Bluetooth Cancel Bonding - Success",
+					&bt_bond_cancel_success_test,
+					setup_enabled_adapter,
+					test_bond_cancel_success, teardown);
+
 	test_bredrle("Socket Init", NULL, setup_socket_interface,
 						test_dummy, teardown);
 
-- 
1.8.5.2

[PATCH 09/11] android/tester: Add remove bond success test case

[Andrzej Kaczmarek]

From: Grzegorz Kolodziejczyk <grzegorz.kolodziejczyk@tieto.com>

This adds remove bond success test case. SSP as pairing mode.
---
 android/android-tester.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index b7abc24..a6ec9a4 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -961,6 +961,29 @@ static void bond_test_state_changed_cb(bt_status_t status,
 	}
 }
 
+static void bond_remove_success_state_changed_cb(bt_status_t status,
+			bt_bdaddr_t *remote_bd_addr, bt_bond_state_t state)
+{
+	struct test_data *data = tester_get_data();
+	bt_status_t remove_status;
+	uint8_t *bdaddr = (uint8_t *)hciemu_get_client_bdaddr(data->hciemu);
+	bt_bdaddr_t remote_addr;
+
+	bdaddr2android((const bdaddr_t *)bdaddr, &remote_addr.address);
+
+	if (state == BT_BOND_STATE_BONDED) {
+		data->cb_count--;
+		remove_status = data->if_bluetooth->remove_bond(&remote_addr);
+		check_expected_status(remove_status);
+		return;
+	}
+
+	if (state == BT_BOND_STATE_NONE) {
+		data->cb_count--;
+		check_cb_count();
+	}
+}
+
 static void bond_state_changed_cb(bt_status_t status,
 			bt_bdaddr_t *remote_bd_addr, bt_bond_state_t state)
 {
@@ -2185,6 +2208,15 @@ static const struct generic_data bt_bond_cancel_success_test = {
 	.expected_adapter_status = BT_STATUS_SUCCESS,
 };
 
+static const struct generic_data bt_bond_remove_success_test = {
+	.expected_hal_cb.device_found_cb = bond_nostatus_device_found_cb,
+	.expected_hal_cb.bond_state_changed_cb =
+					bond_remove_success_state_changed_cb,
+	.expected_hal_cb.ssp_request_cb = bond_create_ssp_success_request_cb,
+	.expected_cb_count = 4,
+	.expected_adapter_status = BT_STATUS_SUCCESS,
+};
+
 static bt_callbacks_t bt_callbacks = {
 	.size = sizeof(bt_callbacks),
 	.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -3043,6 +3075,18 @@ static void test_bond_cancel_success(const void *test_data)
 	data->if_bluetooth->start_discovery();
 }
 
+static void test_bond_remove_success(const void *test_data)
+{
+	struct test_data *data = tester_get_data();
+	struct bthost *bthost = hciemu_client_get_host(data->hciemu);
+
+	init_test_conditions(data);
+
+	bthost_write_ssp_mode(bthost, 0x01);
+
+	data->if_bluetooth->start_discovery();
+}
+
 /* Test Socket HAL */
 
 static void adapter_socket_state_changed_cb(bt_state_t state)
@@ -4310,6 +4354,11 @@ int main(int argc, char *argv[])
 					setup_enabled_adapter,
 					test_bond_cancel_success, teardown);
 
+	test_bredrle("Bluetooth Remove Bond - Success",
+					&bt_bond_remove_success_test,
+					setup_enabled_adapter,
+					test_bond_remove_success, teardown);
+
 	test_bredrle("Socket Init", NULL, setup_socket_interface,
 						test_dummy, teardown);
 
-- 
1.8.5.2

[PATCH 10/11] android/tester: Add remove bond bad addr dev test case

[Andrzej Kaczmarek]

From: Grzegorz Kolodziejczyk <grzegorz.kolodziejczyk@tieto.com>

This adds remove bond fail test case. Bad addres is given as a parametr
for remove bond hal function.
---
 android/android-tester.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index a6ec9a4..417fd22 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -984,6 +984,15 @@ static void bond_remove_success_state_changed_cb(bt_status_t status,
 	}
 }
 
+static void bond_remove_bad_addr_state_changed_cb(bt_status_t status,
+			bt_bdaddr_t *remote_bd_addr, bt_bond_state_t state)
+{
+	struct test_data *data = tester_get_data();
+
+	if (state == BT_BOND_STATE_NONE)
+		data->cb_count--;
+}
+
 static void bond_state_changed_cb(bt_status_t status,
 			bt_bdaddr_t *remote_bd_addr, bt_bond_state_t state)
 {
@@ -2217,6 +2226,13 @@ static const struct generic_data bt_bond_remove_success_test = {
 	.expected_adapter_status = BT_STATUS_SUCCESS,
 };
 
+static const struct generic_data bt_bond_remove_bad_addr_test = {
+	.expected_hal_cb.bond_state_changed_cb =
+					bond_remove_bad_addr_state_changed_cb,
+	.expected_cb_count = 1,
+	.expected_adapter_status = BT_STATUS_SUCCESS,
+};
+
 static bt_callbacks_t bt_callbacks = {
 	.size = sizeof(bt_callbacks),
 	.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -3087,6 +3103,20 @@ static void test_bond_remove_success(const void *test_data)
 	data->if_bluetooth->start_discovery();
 }
 
+static void test_bond_remove_bad_addr(const void *test_data)
+{
+	struct test_data *data = tester_get_data();
+	bt_status_t status;
+	bt_bdaddr_t bad_addr = {
+		.address = { 0x12, 0x34, 0x56, 0x78, 0x90, 0x12 }
+	};
+
+	init_test_conditions(data);
+
+	status = data->if_bluetooth->remove_bond(&bad_addr);
+	check_expected_status(status);
+}
+
 /* Test Socket HAL */
 
 static void adapter_socket_state_changed_cb(bt_state_t state)
@@ -4359,6 +4389,11 @@ int main(int argc, char *argv[])
 					setup_enabled_adapter,
 					test_bond_remove_success, teardown);
 
+	test_bredrle("Bluetooth Remove Bond - Bad Address",
+					&bt_bond_remove_bad_addr_test,
+					setup_enabled_adapter,
+					test_bond_remove_bad_addr, teardown);
+
 	test_bredrle("Socket Init", NULL, setup_socket_interface,
 						test_dummy, teardown);
 
-- 
1.8.5.2

[PATCH 11/11] android/tester: Make bt_callbacks thread-safe

[Andrzej Kaczmarek]

This patch adds wrappers for BT HAL callback which execute them in main
thread instead of notification_handler() thread. Otherwise test
execution is prone to race conditions since we do not provide any
locking mechanism for tester.
---
 android/android-tester.c | 298 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 259 insertions(+), 39 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 417fd22..e4b5531 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -117,6 +117,52 @@ struct test_data {
 	uint16_t intr_cid;
 };
 
+struct adapter_state_changed_cb_param {
+	bt_state_t state;
+};
+
+struct discovery_state_changed_cb_param {
+	bt_discovery_state_t state;
+};
+
+struct device_found_cb_param {
+	int num_properties;
+	bt_property_t *properties;
+};
+
+struct adapter_properties_cb_param {
+	bt_status_t status;
+	int num_properties;
+	bt_property_t *properties;
+};
+
+struct remote_device_properties_cb_param {
+	bt_status_t status;
+	bt_bdaddr_t *bd_addr;
+	int num_properties;
+	bt_property_t *properties;
+};
+
+struct pin_request_cb_param {
+	bt_bdaddr_t *remote_bd_addr;
+	bt_bdname_t *bd_name;
+	uint32_t cod;
+};
+
+struct ssp_request_cb_param {
+	bt_bdaddr_t *remote_bd_addr;
+	bt_bdname_t *bd_name;
+	uint32_t cod;
+	bt_ssp_variant_t pairing_variant;
+	uint32_t pass_key;
+};
+
+struct bond_state_changed_cb_param {
+	bt_status_t status;
+	bt_bdaddr_t *remote_bd_addr;
+	bt_bond_state_t state;
+};
+
 static char exec_dir[PATH_MAX + 1];
 
 static void mgmt_debug(const char *str, void *user_data)
@@ -249,6 +295,31 @@ static void check_expected_status(uint8_t status)
 		tester_test_failed();
 }
 
+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 int locate_property(gconstpointer expected_data,
 						gconstpointer received_prop)
 {
@@ -589,19 +660,26 @@ static void disable_success_cb(bt_state_t state)
 	}
 }
 
-static void adapter_state_changed_cb(bt_state_t state)
+static gboolean adapter_state_changed_cb(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct generic_data *test = data->test_data;
+	struct adapter_state_changed_cb_param *param =
+			(struct adapter_state_changed_cb_param *) 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(param->state);
+		goto cleanup;
 	}
 
-	if (!data->test_init_done && state == BT_STATE_ON)
+	if (!data->test_init_done && param->state == BT_STATE_ON)
 		setup_powered_emulated_remote();
+
+cleanup:
+	g_free(user_data);
+
+	return FALSE;
 }
 
 static void discovery_start_success_cb(bt_discovery_state_t state)
@@ -685,13 +763,17 @@ 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_cb(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct generic_data *test = data->test_data;
+	struct discovery_state_changed_cb_param *param = (struct discovery_state_changed_cb_param *) user_data;
 
 	if (test && test->expected_hal_cb.discovery_state_changed_cb)
-		test->expected_hal_cb.discovery_state_changed_cb(state);
+		test->expected_hal_cb.discovery_state_changed_cb(param->state);
+
+	g_free(user_data);
+	return FALSE;
 }
 
 static void discovery_device_found_cb(int num_properties,
@@ -862,14 +944,19 @@ static void bond_nostatus_device_found_cb(int num_properties,
 	}
 }
 
-static void device_found_cb(int num_properties, bt_property_t *properties)
+static gboolean device_found_cb(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct generic_data *test = data->test_data;
+	struct device_found_cb_param *params = (struct device_found_cb_param *) user_data;
 
 	if (data->test_init_done && test->expected_hal_cb.device_found_cb)
-		test->expected_hal_cb.device_found_cb(num_properties,
-								properties);
+		test->expected_hal_cb.device_found_cb(params->num_properties,
+							params->properties);
+
+	free_properties(params->num_properties, params->properties);
+	g_free(params);
+	return FALSE;
 }
 
 static void check_count_properties_cb(bt_status_t status, int num_properties,
@@ -881,16 +968,21 @@ static void check_count_properties_cb(bt_status_t status, int num_properties,
 		check_expected_property(properties[i]);
 }
 
-static void adapter_properties_cb(bt_status_t status, int num_properties,
-						bt_property_t *properties)
+static gboolean adapter_properties_cb(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct generic_data *test = data->test_data;
+	struct adapter_properties_cb_param *params = (struct adapter_properties_cb_param *) user_data;
 
 	if (data->test_init_done &&
 				test->expected_hal_cb.adapter_properties_cb)
-		test->expected_hal_cb.adapter_properties_cb(status,
-						num_properties, properties);
+		test->expected_hal_cb.adapter_properties_cb(params->status,
+				params->num_properties, params->properties);
+
+	free_properties(params->num_properties, params->properties);
+	g_free(user_data);
+
+	return FALSE;
 }
 
 static void remote_test_device_properties_cb(bt_status_t status,
@@ -926,17 +1018,21 @@ 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_cb(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct generic_data *test = data->test_data;
+	struct remote_device_properties_cb_param *params = (struct remote_device_properties_cb_param *) 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(params->status,
+					params->bd_addr, params->num_properties, params->properties);
+
+	g_free(params->bd_addr);
+	free_properties(params->num_properties, params->properties);
+	g_free(params);
+	return FALSE;
 }
 
 static void bond_test_state_changed_cb(bt_status_t status,
@@ -993,15 +1089,22 @@ static void bond_remove_bad_addr_state_changed_cb(bt_status_t status,
 		data->cb_count--;
 }
 
-static void bond_state_changed_cb(bt_status_t status,
-			bt_bdaddr_t *remote_bd_addr, bt_bond_state_t state)
+static gboolean bond_state_changed_cb(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct generic_data *test = data->test_data;
+	struct bond_state_changed_cb_param *param =
+			(struct bond_state_changed_cb_param *) user_data;
 
 	if (data->test_init_done && test->expected_hal_cb.bond_state_changed_cb)
-			test->expected_hal_cb.bond_state_changed_cb(status,
-							remote_bd_addr, state);
+		test->expected_hal_cb.bond_state_changed_cb(param->status,
+							param->remote_bd_addr,
+							param->state);
+
+	g_free(param->remote_bd_addr);
+	g_free(param);
+
+	return FALSE;
 }
 
 static void bond_create_pin_success_request_cb(bt_bdaddr_t *remote_bd_addr,
@@ -1054,15 +1157,23 @@ static void bond_create_pin_fail_request_cb(bt_bdaddr_t *remote_bd_addr,
 					sizeof(rp), &rp, NULL, NULL, NULL);
 }
 
-static void pin_request_cb(bt_bdaddr_t *remote_bd_addr,
-					bt_bdname_t *bd_name, uint32_t cod)
+static gboolean pin_request_cb(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct generic_data *test = data->test_data;
+	struct pin_request_cb_param *param =
+				(struct pin_request_cb_param *) user_data;
 
 	if (data->test_init_done && test->expected_hal_cb.pin_request_cb)
-		test->expected_hal_cb.pin_request_cb(remote_bd_addr, bd_name,
-									cod);
+		test->expected_hal_cb.pin_request_cb(param->remote_bd_addr,
+							param->bd_name,
+							param->cod);
+
+	g_free(param->remote_bd_addr);
+	g_free(param->bd_name);
+	g_free(param);
+
+	return FALSE;
 }
 
 static void bond_create_ssp_request_cb(const bt_bdaddr_t *remote_bd_addr,
@@ -1131,17 +1242,26 @@ static void bond_cancel_success_ssp_request_cb(bt_bdaddr_t *remote_bd_addr,
 	check_expected_status(status);
 }
 
-static void ssp_request_cb(bt_bdaddr_t *remote_bd_addr, bt_bdname_t *bd_name,
-				uint32_t cod, bt_ssp_variant_t pairing_variant,
-				uint32_t pass_key)
+static gboolean ssp_request_cb(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct generic_data *test = data->test_data;
+	struct ssp_request_cb_param *param =
+				(struct ssp_request_cb_param *) user_data;
 
 	if (data->test_init_done &&
 				test->expected_hal_cb.ssp_request_cb)
-		test->expected_hal_cb.ssp_request_cb(remote_bd_addr, bd_name,
-					cod, pairing_variant, pass_key);
+		test->expected_hal_cb.ssp_request_cb(param->remote_bd_addr,
+							param->bd_name,
+							param->cod,
+							param->pairing_variant,
+							param->pass_key);
+
+	g_free(param->remote_bd_addr);
+	g_free(param->bd_name);
+	g_free(param);
+
+	return FALSE;
 }
 
 static bt_bdaddr_t enable_done_bdaddr_val = { {0x00} };
@@ -2233,16 +2353,116 @@ static const struct generic_data bt_bond_remove_bad_addr_test = {
 	.expected_adapter_status = BT_STATUS_SUCCESS,
 };
 
+static void adapter_state_changed_cb_wrap(bt_state_t state)
+{
+	struct adapter_state_changed_cb_param *param =
+						g_malloc(sizeof(*param));
+
+	param->state = state;
+
+	g_idle_add(adapter_state_changed_cb, param);
+}
+
+static void discovery_state_changed_cb_wrap(bt_discovery_state_t state)
+{
+	struct discovery_state_changed_cb_param *param =
+						g_malloc(sizeof(*param));
+
+	param->state = state;
+
+	g_idle_add(discovery_state_changed_cb, param);
+}
+
+static void device_found_cb_wrap(int num_properties, bt_property_t *properties)
+{
+	struct device_found_cb_param *param = g_malloc(sizeof(*param));
+
+	param->num_properties = num_properties;
+	param->properties = copy_properties(num_properties, properties);
+
+	g_idle_add(device_found_cb, param);
+}
+
+static void adapter_properties_cb_wrap(bt_status_t status, int num_properties,
+						bt_property_t *properties)
+{
+	struct adapter_properties_cb_param *param = g_malloc(sizeof(*param));
+
+	param->status = status;
+	param->num_properties = num_properties;
+	param->properties = copy_properties(num_properties, properties);
+
+	g_idle_add(adapter_properties_cb, param);
+}
+
+static void remote_device_properties_cb_wrap(bt_status_t status,
+				bt_bdaddr_t *bd_addr, int num_properties,
+				bt_property_t *properties)
+{
+	struct remote_device_properties_cb_param *param =
+						g_malloc(sizeof(*param));
+
+	param->status = status;
+	param->bd_addr = g_memdup(bd_addr, sizeof(*bd_addr));
+	param->num_properties = num_properties;
+	param->properties = copy_properties(num_properties, properties);
+
+	g_idle_add(remote_device_properties_cb, param);
+}
+
+static void pin_request_cb_wrap(bt_bdaddr_t *remote_bd_addr,
+					bt_bdname_t *bd_name, uint32_t cod)
+{
+	struct pin_request_cb_param *param = g_malloc(sizeof(*param));
+
+	param->remote_bd_addr = g_memdup(remote_bd_addr,
+						sizeof(*remote_bd_addr));
+	param->bd_name = g_memdup(bd_name, sizeof(*bd_name));
+
+	g_idle_add(pin_request_cb, param);
+}
+
+static void ssp_request_cb_wrap(bt_bdaddr_t *remote_bd_addr,
+					bt_bdname_t *bd_name, uint32_t cod,
+					bt_ssp_variant_t pairing_variant,
+					uint32_t pass_key)
+{
+	struct ssp_request_cb_param *param = g_malloc(sizeof(*param));
+
+	param->remote_bd_addr = g_memdup(remote_bd_addr,
+						sizeof(*remote_bd_addr));
+	param->bd_name = g_memdup(bd_name, sizeof(*bd_name));
+	param->cod = cod;
+	param->pairing_variant = pairing_variant;
+	param->pass_key = pass_key;
+
+	g_idle_add(ssp_request_cb, param);
+}
+
+static void bond_state_changed_cb_wrap(bt_status_t status,
+					bt_bdaddr_t *remote_bd_addr,
+					bt_bond_state_t state)
+{
+	struct bond_state_changed_cb_param *param = g_malloc(sizeof(*param));
+
+	param->status = status;
+	param->remote_bd_addr = g_memdup(remote_bd_addr,
+						sizeof(*remote_bd_addr));
+	param->state = state;
+
+	g_idle_add(bond_state_changed_cb, param);
+}
+
 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 = remote_device_properties_cb,
-	.device_found_cb = device_found_cb,
-	.discovery_state_changed_cb = discovery_state_changed_cb,
-	.pin_request_cb = pin_request_cb,
-	.ssp_request_cb = ssp_request_cb,
-	.bond_state_changed_cb = bond_state_changed_cb,
+	.adapter_state_changed_cb = adapter_state_changed_cb_wrap,
+	.adapter_properties_cb = adapter_properties_cb_wrap,
+	.remote_device_properties_cb = remote_device_properties_cb_wrap,
+	.device_found_cb = device_found_cb_wrap,
+	.discovery_state_changed_cb = discovery_state_changed_cb_wrap,
+	.pin_request_cb = pin_request_cb_wrap,
+	.ssp_request_cb = ssp_request_cb_wrap,
+	.bond_state_changed_cb = bond_state_changed_cb_wrap,
 	.acl_state_changed_cb = NULL,
 	.thread_evt_cb = NULL,
 	.dut_mode_recv_cb = NULL,
-- 
1.8.5.2

Re: [PATCH 11/11] android/tester: Make bt_callbacks thread-safe

[Anderson Lizardo]

Hi Andrzej,

On Sun, Feb 2, 2014 at 7:16 AM, Andrzej Kaczmarek
<andrzej.kaczmarek@tieto.com> wrote:
> This patch adds wrappers for BT HAL callback which execute them in main
> thread instead of notification_handler() thread. Otherwise test
> execution is prone to race conditions since we do not provide any
> locking mechanism for tester.

In my opinion, this is becoming too messy. I'm getting races even
inside the emulator code: sometimes bthost->ncmd becomes zero before a
HCI command is sent by the emulated host because the Command Status /
Command Complete comes after the command is written to the socket, but
before bthost->ncmd is decremented.

Also try running android-tester under valgrind. At least for me, I get
a few failures that I don't have when running without valgrind (at
least one in HIDHost apparently due to if_bluetooth->enable() not
being called on test setup and thus the tests rely on finishing before
the controller is powered off by the kernel after initialization).

IMHO, the best approach would be to keep all HAL API usage in a
separate process, and keep android-tester single-threaded. Of course,
this could extra complexity for the required IPC between
android-tester and this new process...

Again, I'm not familiar with how HAL API works, all this is just based
on my failed attempt to make android-tester run reliably under
valgrind.

Best Regards,
-- 
Anderson Lizardo
http://www.indt.org/?lang=en
INdT - Manaus - Brazil

Re: [PATCH 11/11] android/tester: Make bt_callbacks thread-safe

[Marcel Holtmann]

Hi Anderson,

>> This patch adds wrappers for BT HAL callback which execute them in main
>> thread instead of notification_handler() thread. Otherwise test
>> execution is prone to race conditions since we do not provide any
>> locking mechanism for tester.
> 
> In my opinion, this is becoming too messy. I'm getting races even
> inside the emulator code: sometimes bthost->ncmd becomes zero before a
> HCI command is sent by the emulated host because the Command Status /
> Command Complete comes after the command is written to the socket, but
> before bthost->ncmd is decremented.
> 
> Also try running android-tester under valgrind. At least for me, I get
> a few failures that I don't have when running without valgrind (at
> least one in HIDHost apparently due to if_bluetooth->enable() not
> being called on test setup and thus the tests rely on finishing before
> the controller is powered off by the kernel after initialization).
> 
> IMHO, the best approach would be to keep all HAL API usage in a
> separate process, and keep android-tester single-threaded. Of course,
> this could extra complexity for the required IPC between
> android-tester and this new process...
> 
> Again, I'm not familiar with how HAL API works, all this is just based
> on my failed attempt to make android-tester run reliably under
> valgrind.

I have to agree. We might better spawn processes for this. Our emulator code was never designed to be thread safe and never will be. We are just hiding the real problem here and it will break somewhere else later on.

Regards

Marcel

Re: [PATCH 11/11] android/tester: Make bt_callbacks thread-safe

[Anderson Lizardo]

Hi Marcel,

On Sun, Feb 2, 2014 at 12:55 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> I have to agree. We might better spawn processes for this. Our emulator code was never designed to be thread safe and never will be. We are just hiding the real problem here and it will break somewhere else later on.

One thing I've been experimenting with is to use android/haltest to
"script" the tests. It is not as fine grained as implementing your own
HAL interfaces, but I believe it will allow me to implement most tests
I want (I just started writing the tests).

(BTW, if anyone is doing something similar with haltest, let me know!)

Best Regards,
-- 
Anderson Lizardo
http://www.indt.org/?lang=en
INdT - Manaus - Brazil

Re: [PATCH 11/11] android/tester: Make bt_callbacks thread-safe

[Szymon Janc]

Hi Marcel and Anderson,

On Sunday 02 of February 2014 08:55:49 Marcel Holtmann wrote:
> Hi Anderson,
> 
> >> This patch adds wrappers for BT HAL callback which execute them in main
> >> thread instead of notification_handler() thread. Otherwise test
> >> execution is prone to race conditions since we do not provide any
> >> locking mechanism for tester.
> > 
> > In my opinion, this is becoming too messy. I'm getting races even
> > inside the emulator code: sometimes bthost->ncmd becomes zero before a
> > HCI command is sent by the emulated host because the Command Status /
> > Command Complete comes after the command is written to the socket, but
> > before bthost->ncmd is decremented.
> > 
> > Also try running android-tester under valgrind. At least for me, I get
> > a few failures that I don't have when running without valgrind (at
> > least one in HIDHost apparently due to if_bluetooth->enable() not
> > being called on test setup and thus the tests rely on finishing before
> > the controller is powered off by the kernel after initialization).
> > 
> > IMHO, the best approach would be to keep all HAL API usage in a
> > separate process, and keep android-tester single-threaded. Of course,
> > this could extra complexity for the required IPC between
> > android-tester and this new process...
> > 
> > Again, I'm not familiar with how HAL API works, all this is just based
> > on my failed attempt to make android-tester run reliably under
> > valgrind.
> 
> I have to agree. We might better spawn processes for this. Our emulator code
> was never designed to be thread safe and never will be. We are just hiding
> the real problem here and it will break somewhere else later on.

I think using g_idle_add to have checks executes in right thread context 
pretty clear and easy to follow. Spawning processes will require dedicated IPC 
that will wrap either HAL or emulator and that will be quite complicated 
comparing to using g_idle_add. Jakub did some initial implementation for this 
and it adds ~800 sloc (and will be more in future...).

So I would go with using g_idle_add but refactor propose patch to emphasize 
that those are tests checks that execute in mainloop context, not HAL 
callbacks.

-- 
BR
Szymon Janc