[PATCH v5 00/10] Change inquiry handling in btdev

[PATCH v5 00/10] Change inquiry handling in btdev

[Lukasz Rymanowski]

This patch set introduce new way of handling inquiry in btdev.

v2:
* Changes in handling inquiry lenght
* Changes in handling inquiry cancel
* Updates in mgmt-tester

v3:
* fixes on Johan comments
* Fix in timeout-mainloop.c for destroy callback

v4:
* rebase on top fixed timeout.h
* minor cleanups

v5:
* minor cleanups

Lukasz Rymanowski (10):
  emulator: Use timeout for sending inquiry results
  emulator: Add inquiry cancel
  emulator: Add handling inquiry number of responses
  emulator: Add handling inquiry_lenght from inquiry command
  android/tester: Fix handling inquiry by android tester
  tools/mgmt-tester: Fix for Stop discovery test
  tools/mgmt-tester: Update Stop Discovery-BR/EDR (Inquiry) Success 1
  tools/mgmt-tester: Remove not used condition
  tools/mgmt-tester: Refactor setup_start_discovery function
  shared: Fix missing destroy callback in timeout_add

 Makefile.tools                |  18 +++--
 android/Makefile.am           |   2 +
 android/android-tester.c      |  24 ++-----
 emulator/btdev.c              | 149 ++++++++++++++++++++++++++++++++++++++++--
 src/shared/timeout-mainloop.c |   1 +
 tools/mgmt-tester.c           |  70 +++++---------------
 6 files changed, 181 insertions(+), 83 deletions(-)

-- 
1.8.4

[PATCH v5 01/10] emulator: Use timeout for sending inquiry results

[Lukasz Rymanowski]

With this patch btdev uses timeout to schedule inquiry results
It also allows btdev to receive hci commands during inquiry.
Previously btdev was blocked since all the inquiry result were sent in
single loop
---
 Makefile.tools      | 18 ++++++++----
 android/Makefile.am |  2 ++
 emulator/btdev.c    | 84 +++++++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 92 insertions(+), 12 deletions(-)

diff --git a/Makefile.tools b/Makefile.tools
index 0583f3b..a5491ac 100644
--- a/Makefile.tools
+++ b/Makefile.tools
@@ -88,7 +88,8 @@ tools_mgmt_tester_SOURCES = tools/mgmt-tester.c monitor/bt.h \
 				src/shared/util.h src/shared/util.c \
 				src/shared/mgmt.h src/shared/mgmt.c \
 				src/shared/hciemu.h src/shared/hciemu.c \
-				src/shared/tester.h src/shared/tester.c
+				src/shared/tester.h src/shared/tester.c \
+				src/shared/timeout.h src/shared/timeout-glib.c
 tools_mgmt_tester_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
 
 tools_l2cap_tester_SOURCES = tools/l2cap-tester.c monitor/bt.h \
@@ -101,7 +102,8 @@ tools_l2cap_tester_SOURCES = tools/l2cap-tester.c monitor/bt.h \
 				src/shared/util.h src/shared/util.c \
 				src/shared/mgmt.h src/shared/mgmt.c \
 				src/shared/hciemu.h src/shared/hciemu.c \
-				src/shared/tester.h src/shared/tester.c
+				src/shared/tester.h src/shared/tester.c \
+				src/shared/timeout.h src/shared/timeout-glib.c
 tools_l2cap_tester_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
 
 tools_rfcomm_tester_SOURCES = tools/rfcomm-tester.c monitor/bt.h \
@@ -114,7 +116,8 @@ tools_rfcomm_tester_SOURCES = tools/rfcomm-tester.c monitor/bt.h \
 				src/shared/util.h src/shared/util.c \
 				src/shared/mgmt.h src/shared/mgmt.c \
 				src/shared/hciemu.h src/shared/hciemu.c \
-				src/shared/tester.h src/shared/tester.c
+				src/shared/tester.h src/shared/tester.c \
+				src/shared/timeout.h src/shared/timeout-glib.c
 tools_rfcomm_tester_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
 
 tools_smp_tester_SOURCES = tools/smp-tester.c monitor/bt.h \
@@ -127,7 +130,8 @@ tools_smp_tester_SOURCES = tools/smp-tester.c monitor/bt.h \
 				src/shared/util.h src/shared/util.c \
 				src/shared/mgmt.h src/shared/mgmt.c \
 				src/shared/hciemu.h src/shared/hciemu.c \
-				src/shared/tester.h src/shared/tester.c
+				src/shared/tester.h src/shared/tester.c \
+				src/shared/timeout.h src/shared/timeout-glib.c
 tools_smp_tester_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
 
 tools_gap_tester_SOURCES = tools/gap-tester.c monitor/bt.h \
@@ -138,7 +142,8 @@ tools_gap_tester_SOURCES = tools/gap-tester.c monitor/bt.h \
 				src/shared/util.h src/shared/util.c \
 				src/shared/queue.h src/shared/queue.c \
 				src/shared/hciemu.h src/shared/hciemu.c \
-				src/shared/tester.h src/shared/tester.c
+				src/shared/tester.h src/shared/tester.c \
+				src/shared/timeout.h src/shared/timeout-glib.c
 tools_gap_tester_LDADD =  lib/libbluetooth-internal.la \
 				gdbus/libgdbus-internal.la \
 				@GLIB_LIBS@ @DBUS_LIBS@
@@ -153,7 +158,8 @@ tools_sco_tester_SOURCES = tools/sco-tester.c monitor/bt.h \
 				src/shared/util.h src/shared/util.c \
 				src/shared/mgmt.h src/shared/mgmt.c \
 				src/shared/hciemu.h src/shared/hciemu.c \
-				src/shared/tester.h src/shared/tester.c
+				src/shared/tester.h src/shared/tester.c \
+				src/shared/timeout.h src/shared/timeout-glib.c
 tools_sco_tester_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
 
 tools_hci_tester_SOURCES = tools/hci-tester.c monitor/bt.h \
diff --git a/android/Makefile.am b/android/Makefile.am
index 3c503da..58ee3c9 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -126,6 +126,7 @@ android_android_tester_SOURCES = emulator/btdev.h emulator/btdev.c \
 				src/shared/mgmt.h src/shared/mgmt.c \
 				src/shared/hciemu.h src/shared/hciemu.c \
 				src/shared/tester.h src/shared/tester.c \
+				src/shared/timeout.h src/shared/timeout-glib.c \
 				monitor/rfcomm.h \
 				android/hardware/hardware.c \
 				android/android-tester.c
@@ -149,6 +150,7 @@ android_ipc_tester_SOURCES = emulator/btdev.h emulator/btdev.c \
 				src/shared/mgmt.h src/shared/mgmt.c \
 				src/shared/hciemu.h src/shared/hciemu.c \
 				src/shared/tester.h src/shared/tester.c \
+				src/shared/timeout.h src/shared/timeout-glib.c \
 				android/hal-utils.h android/hal-utils.c \
 				android/ipc-common.h android/ipc-tester.c
 
diff --git a/emulator/btdev.c b/emulator/btdev.c
index 6a3b163..f959570 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -33,6 +33,7 @@
 #include <alloca.h>
 
 #include "src/shared/util.h"
+#include "src/shared/timeout.h"
 #include "monitor/bt.h"
 #include "btdev.h"
 
@@ -68,6 +69,8 @@ struct btdev {
 	btdev_send_func send_handler;
 	void *send_data;
 
+	unsigned int inquiry_id;
+
 	struct hook *hook_list[MAX_HOOK_ENTRIES];
 
         uint16_t manufacturer;
@@ -131,6 +134,14 @@ struct btdev {
 	uint8_t  sync_train_service_data;
 };
 
+struct inquiry_data {
+	struct btdev *btdev;
+	int sent_count;
+	int iter;
+};
+
+#define DEFAULT_INQUIRY_INTERVAL 100 /* 100 miliseconds */
+
 #define MAX_BTDEV_ENTRIES 16
 
 static const uint8_t LINK_KEY_NONE[16] = { 0 };
@@ -536,6 +547,9 @@ void btdev_destroy(struct btdev *btdev)
 	if (!btdev)
 		return;
 
+	if (btdev->inquiry_id > 0)
+		timeout_remove(btdev->inquiry_id);
+
 	del_btdev(btdev);
 
 	free(btdev);
@@ -692,12 +706,18 @@ static void num_completed_packets(struct btdev *btdev)
 	}
 }
 
-static void inquiry_complete(struct btdev *btdev, uint8_t status)
+static bool inquiry_callback(void *user_data)
 {
-	struct bt_hci_evt_inquiry_complete ic;
+	struct inquiry_data *data = user_data;
+	struct btdev *btdev = data->btdev;
+	int sent = data->sent_count;
 	int i;
 
-	for (i = 0; i < MAX_BTDEV_ENTRIES; i++) {
+	for (i = data->iter; i < MAX_BTDEV_ENTRIES; i++) {
+		/*Lets sent 10 inquiry results at once */
+		if (sent + 10 == data->sent_count)
+			break;
+
 		if (!btdev_list[i] || btdev_list[i] == btdev)
 			continue;
 
@@ -719,6 +739,7 @@ static void inquiry_complete(struct btdev *btdev, uint8_t status)
 
 			send_event(btdev, BT_HCI_EVT_EXT_INQUIRY_RESULT,
 							&ir, sizeof(ir));
+			data->sent_count++;
 			continue;
 		}
 
@@ -735,6 +756,7 @@ static void inquiry_complete(struct btdev *btdev, uint8_t status)
 
 			send_event(btdev, BT_HCI_EVT_INQUIRY_RESULT_WITH_RSSI,
 							&ir, sizeof(ir));
+			data->sent_count++;
 		} else {
 			struct bt_hci_evt_inquiry_result ir;
 
@@ -748,11 +770,61 @@ static void inquiry_complete(struct btdev *btdev, uint8_t status)
 
 			send_event(btdev, BT_HCI_EVT_INQUIRY_RESULT,
 							&ir, sizeof(ir));
+			data->sent_count++;
 		}
-        }
+	}
 
-	ic.status = status;
+	data->iter = i;
+
+	if (i == MAX_BTDEV_ENTRIES) {
+		struct bt_hci_evt_inquiry_complete ic;
+
+		ic.status = BT_HCI_ERR_SUCCESS;
+		send_event(btdev, BT_HCI_EVT_INQUIRY_COMPLETE, &ic, sizeof(ic));
+
+		return false;
+	}
 
+	return true;
+}
+
+static void inquiry_destroy(void *user_data)
+{
+	struct inquiry_data *data = user_data;
+
+	if (data->btdev)
+		data->btdev->inquiry_id = 0;
+
+	free(data);
+}
+
+static void inquiry_cmd(struct btdev *btdev, const void *cmd)
+{
+	struct inquiry_data *data;
+	struct bt_hci_evt_inquiry_complete ic;
+	int status = BT_HCI_ERR_HARDWARE_FAILURE;
+
+	if (btdev->inquiry_id > 0) {
+		status = BT_HCI_ERR_COMMAND_DISALLOWED;
+		goto failed;
+	}
+
+	data = malloc(sizeof(*data));
+	if (!data)
+		goto failed;
+
+	memset(data, 0, sizeof(*data));
+	data->btdev = btdev;
+
+	btdev->inquiry_id = timeout_add(DEFAULT_INQUIRY_INTERVAL,
+							inquiry_callback, data,
+							inquiry_destroy);
+	/* Return if success */
+	if (btdev->inquiry_id > 0)
+		return;
+
+failed:
+	ic.status = status;
 	send_event(btdev, BT_HCI_EVT_INQUIRY_COMPLETE, &ic, sizeof(ic));
 }
 
@@ -2647,7 +2719,7 @@ static void default_cmd_completion(struct btdev *btdev, uint16_t opcode,
 	case BT_HCI_CMD_INQUIRY:
 		if (btdev->type == BTDEV_TYPE_LE)
 			return;
-		inquiry_complete(btdev, BT_HCI_ERR_SUCCESS);
+		inquiry_cmd(btdev, data);
 		break;
 
 	case BT_HCI_CMD_CREATE_CONN:
-- 
1.8.4

[PATCH v5 02/10] emulator: Add inquiry cancel

[Lukasz Rymanowski]

With this patch, scheduled inquiry session in btdev can be canceled
---
 emulator/btdev.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/emulator/btdev.c b/emulator/btdev.c
index f959570..5b8cc2f 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -791,10 +791,14 @@ static bool inquiry_callback(void *user_data)
 static void inquiry_destroy(void *user_data)
 {
 	struct inquiry_data *data = user_data;
+	struct btdev *btdev = data->btdev;
+
+	if (!btdev)
+		goto finish;
 
-	if (data->btdev)
-		data->btdev->inquiry_id = 0;
+	btdev->inquiry_id = 0;
 
+finish:
 	free(data);
 }
 
@@ -828,6 +832,24 @@ failed:
 	send_event(btdev, BT_HCI_EVT_INQUIRY_COMPLETE, &ic, sizeof(ic));
 }
 
+static void inquiry_cancel(struct btdev *btdev)
+{
+	uint8_t status = BT_HCI_ERR_COMMAND_DISALLOWED;
+
+	if (!btdev->inquiry_id) {
+		cmd_complete(btdev, BT_HCI_CMD_INQUIRY_CANCEL, &status,
+							sizeof(status));
+		return;
+	}
+
+	timeout_remove(btdev->inquiry_id);
+	btdev->inquiry_id = 0;
+
+	status = BT_HCI_ERR_SUCCESS;
+	cmd_complete(btdev, BT_HCI_CMD_INQUIRY_CANCEL, &status,
+							sizeof(status));
+}
+
 static void conn_complete(struct btdev *btdev,
 					const uint8_t *bdaddr, uint8_t status)
 {
@@ -1760,8 +1782,7 @@ static void default_cmd(struct btdev *btdev, uint16_t opcode,
 	case BT_HCI_CMD_INQUIRY_CANCEL:
 		if (btdev->type == BTDEV_TYPE_LE)
 			goto unsupported;
-		status = BT_HCI_ERR_SUCCESS;
-		cmd_complete(btdev, opcode, &status, sizeof(status));
+		inquiry_cancel(btdev);
 		break;
 
 	case BT_HCI_CMD_CREATE_CONN:
-- 
1.8.4

[PATCH v5 03/10] emulator: Add handling inquiry number of responses

[Lukasz Rymanowski]

---
 emulator/btdev.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/emulator/btdev.c b/emulator/btdev.c
index 5b8cc2f..ea08da6 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -136,6 +136,8 @@ struct btdev {
 
 struct inquiry_data {
 	struct btdev *btdev;
+	int num_resp;
+
 	int sent_count;
 	int iter;
 };
@@ -710,6 +712,7 @@ static bool inquiry_callback(void *user_data)
 {
 	struct inquiry_data *data = user_data;
 	struct btdev *btdev = data->btdev;
+	struct bt_hci_evt_inquiry_complete ic;
 	int sent = data->sent_count;
 	int i;
 
@@ -776,16 +779,21 @@ static bool inquiry_callback(void *user_data)
 
 	data->iter = i;
 
-	if (i == MAX_BTDEV_ENTRIES) {
-		struct bt_hci_evt_inquiry_complete ic;
-
-		ic.status = BT_HCI_ERR_SUCCESS;
-		send_event(btdev, BT_HCI_EVT_INQUIRY_COMPLETE, &ic, sizeof(ic));
+	/* Check if we sent already required amount of responses*/
+	if (data->num_resp && data->sent_count == data->num_resp)
+		goto finish;
 
-		return false;
-	}
+	if (i == MAX_BTDEV_ENTRIES)
+		goto finish;
 
 	return true;
+
+finish:
+	/* Note that destroy will be called */
+	ic.status = BT_HCI_ERR_SUCCESS;
+	send_event(btdev, BT_HCI_EVT_INQUIRY_COMPLETE, &ic, sizeof(ic));
+
+	return false;
 }
 
 static void inquiry_destroy(void *user_data)
@@ -804,6 +812,7 @@ finish:
 
 static void inquiry_cmd(struct btdev *btdev, const void *cmd)
 {
+	const struct bt_hci_cmd_inquiry *inq_cmd = cmd;
 	struct inquiry_data *data;
 	struct bt_hci_evt_inquiry_complete ic;
 	int status = BT_HCI_ERR_HARDWARE_FAILURE;
@@ -819,6 +828,7 @@ static void inquiry_cmd(struct btdev *btdev, const void *cmd)
 
 	memset(data, 0, sizeof(*data));
 	data->btdev = btdev;
+	data->num_resp = inq_cmd->num_resp;
 
 	btdev->inquiry_id = timeout_add(DEFAULT_INQUIRY_INTERVAL,
 							inquiry_callback, data,
-- 
1.8.4

[PATCH v5 04/10] emulator: Add handling inquiry_lenght from inquiry command

[Lukasz Rymanowski]

With this patch, btdev is taking into accoung inquiry_lenght from hci
inquiry command.
Inquiry session will last that long unless number of devices parameter
has been provided different then 0
---
 emulator/btdev.c | 40 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/emulator/btdev.c b/emulator/btdev.c
index ea08da6..9e48a51 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -70,6 +70,7 @@ struct btdev {
 	void *send_data;
 
 	unsigned int inquiry_id;
+	unsigned int inquiry_timeout_id;
 
 	struct hook *hook_list[MAX_HOOK_ENTRIES];
 
@@ -716,6 +717,10 @@ static bool inquiry_callback(void *user_data)
 	int sent = data->sent_count;
 	int i;
 
+	/*Report devices only once and wait for inquiry timeout*/
+	if (data->iter == MAX_BTDEV_ENTRIES)
+		return true;
+
 	for (i = data->iter; i < MAX_BTDEV_ENTRIES; i++) {
 		/*Lets sent 10 inquiry results at once */
 		if (sent + 10 == data->sent_count)
@@ -776,16 +781,12 @@ static bool inquiry_callback(void *user_data)
 			data->sent_count++;
 		}
 	}
-
 	data->iter = i;
 
 	/* Check if we sent already required amount of responses*/
 	if (data->num_resp && data->sent_count == data->num_resp)
 		goto finish;
 
-	if (i == MAX_BTDEV_ENTRIES)
-		goto finish;
-
 	return true;
 
 finish:
@@ -806,16 +807,38 @@ static void inquiry_destroy(void *user_data)
 
 	btdev->inquiry_id = 0;
 
+	if (btdev->inquiry_timeout_id > 0) {
+		timeout_remove(btdev->inquiry_timeout_id);
+		btdev->inquiry_timeout_id = 0;
+	}
+
 finish:
 	free(data);
 }
 
+static bool inquiry_timeout(void *user_data)
+{
+	struct inquiry_data *data = user_data;
+	struct btdev *btdev = data->btdev;
+	struct bt_hci_evt_inquiry_complete ic;
+
+	timeout_remove(btdev->inquiry_id);
+	btdev->inquiry_timeout_id = 0;
+
+	/* Inquiry is stopped, send Inquiry complete event. */
+	ic.status = BT_HCI_ERR_SUCCESS;
+	send_event(btdev, BT_HCI_EVT_INQUIRY_COMPLETE, &ic, sizeof(ic));
+
+	return false;
+}
+
 static void inquiry_cmd(struct btdev *btdev, const void *cmd)
 {
 	const struct bt_hci_cmd_inquiry *inq_cmd = cmd;
 	struct inquiry_data *data;
 	struct bt_hci_evt_inquiry_complete ic;
 	int status = BT_HCI_ERR_HARDWARE_FAILURE;
+	unsigned int inquiry_len_ms;
 
 	if (btdev->inquiry_id > 0) {
 		status = BT_HCI_ERR_COMMAND_DISALLOWED;
@@ -830,6 +853,13 @@ static void inquiry_cmd(struct btdev *btdev, const void *cmd)
 	data->btdev = btdev;
 	data->num_resp = inq_cmd->num_resp;
 
+	/* Add timeout to cancel inquiry */
+	inquiry_len_ms = 1280 * inq_cmd->length;
+	if (inquiry_len_ms)
+		btdev->inquiry_timeout_id = timeout_add(inquiry_len_ms,
+							inquiry_timeout,
+								data, NULL);
+
 	btdev->inquiry_id = timeout_add(DEFAULT_INQUIRY_INTERVAL,
 							inquiry_callback, data,
 							inquiry_destroy);
@@ -852,6 +882,8 @@ static void inquiry_cancel(struct btdev *btdev)
 		return;
 	}
 
+	timeout_remove(btdev->inquiry_timeout_id);
+	btdev->inquiry_timeout_id = 0;
 	timeout_remove(btdev->inquiry_id);
 	btdev->inquiry_id = 0;
 
-- 
1.8.4

[PATCH v5 05/10] android/tester: Fix handling inquiry by android tester

[Lukasz Rymanowski]

Previously btdev sent inquiry_complete in one mainloop iteration. Due to
new way of handling it we need to make sure cancel_discovery is called
if start_discovery has been used.
There is also no need to hook before inquiry_complete_event in case of
test for cancel_discovery.

Conflicts:
	android/android-tester.c
---
 android/android-tester.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index bab92a1..375f30d 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -941,10 +941,13 @@ static void discovery_device_found_cb(int num_properties,
 		}
 
 		if (!check_test_property(received_prop, expected_prop)) {
+			data->if_bluetooth->cancel_discovery();
 			tester_test_failed();
 			return;
 		}
 	}
+
+	data->if_bluetooth->cancel_discovery();
 }
 
 static void remote_getprops_device_found_cb(int num_properties,
@@ -959,6 +962,7 @@ static void remote_getprops_device_found_cb(int num_properties,
 	if (data->cb_count == 2)
 		data->cb_count--;
 
+	data->if_bluetooth->cancel_discovery();
 	data->if_bluetooth->get_remote_device_properties(&remote_addr);
 }
 
@@ -978,6 +982,7 @@ static void remote_get_property_device_found_cb(int num_properties,
 	if (data->cb_count == 2)
 		data->cb_count--;
 
+	data->if_bluetooth->cancel_discovery();
 	status = data->if_bluetooth->get_remote_device_property(&remote_addr,
 								prop.type);
 	check_expected_status(status);
@@ -999,6 +1004,7 @@ static void remote_setprop_device_found_cb(int num_properties,
 	if (data->cb_count == 3)
 		data->cb_count--;
 
+	data->if_bluetooth->cancel_discovery();
 	status = data->if_bluetooth->set_remote_device_property(&remote_addr,
 									&prop);
 	check_expected_status(status);
@@ -1020,6 +1026,7 @@ static void remote_setprop_fail_device_found_cb(int num_properties,
 	if (data->cb_count == 2)
 		data->cb_count--;
 
+	data->if_bluetooth->cancel_discovery();
 	status = data->if_bluetooth->set_remote_device_property(&remote_addr,
 									&prop);
 	check_expected_status(status);
@@ -2970,26 +2977,12 @@ static void test_discovery_stop_done(const void *test_data)
 	check_expected_status(status);
 }
 
-static bool pre_inq_compl_hook(const void *dummy, uint16_t len, void *user_data)
-{
-	struct test_data *data = tester_get_data();
-
-	/* Make sure Inquiry Command Complete is not called */
-
-	hciemu_del_hook(data->hciemu, HCIEMU_HOOK_PRE_EVT, BT_HCI_CMD_INQUIRY);
-
-	return false;
-}
-
 static void test_discovery_stop_success(const void *test_data)
 {
 	struct test_data *data = tester_get_data();
 
 	init_test_conditions(data);
 
-	hciemu_add_hook(data->hciemu, HCIEMU_HOOK_PRE_EVT, BT_HCI_CMD_INQUIRY,
-					pre_inq_compl_hook, data);
-
 	data->if_bluetooth->start_discovery();
 }
 
@@ -2999,9 +2992,6 @@ static void test_discovery_start_done(const void *test_data)
 
 	init_test_conditions(data);
 
-	hciemu_add_hook(data->hciemu, HCIEMU_HOOK_PRE_EVT, BT_HCI_CMD_INQUIRY,
-					pre_inq_compl_hook, data);
-
 	data->if_bluetooth->start_discovery();
 }
 
-- 
1.8.4

[PATCH v5 06/10] tools/mgmt-tester: Fix for Stop discovery test

[Lukasz Rymanowski]

Test "Stop Discovery - BR/EDR (Inquiry) Success 1" uses hciemu_type
BREDR but uses power settings to enable LE and tries to set LE:

  mgmt: [0x0002] command 0x000d
  mgmt: < 0d 00 02 00 01 00 01                             .......
  mgmt: > 02 00 02 00 03 00 0d 00 0c                       .........
  mgmt: [0x0002] command 0x0d status: 0x0c

This patch fixes this minor issue
---
 tools/mgmt-tester.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/mgmt-tester.c b/tools/mgmt-tester.c
index a3c1c72..9f71743 100644
--- a/tools/mgmt-tester.c
+++ b/tools/mgmt-tester.c
@@ -1733,7 +1733,7 @@ static const struct generic_data stop_discovery_success_test_1 = {
 };
 
 static const struct generic_data stop_discovery_bredr_success_test_1 = {
-	.setup_settings = settings_powered_le,
+	.setup_settings = settings_powered,
 	.setup_expect_hci_command = BT_HCI_CMD_INQUIRY,
 	.setup_expect_hci_param = stop_discovery_inq_param,
 	.setup_expect_hci_len = sizeof(stop_discovery_inq_param),
-- 
1.8.4

[PATCH v5 07/10] tools/mgmt-tester: Update Stop Discovery-BR/EDR (Inquiry) Success 1

[Lukasz Rymanowski]

With new way of handling inquiry in btdev there is no need to register
inquiry event hook
---
 tools/mgmt-tester.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tools/mgmt-tester.c b/tools/mgmt-tester.c
index 9f71743..17d2bd2 100644
--- a/tools/mgmt-tester.c
+++ b/tools/mgmt-tester.c
@@ -1734,9 +1734,6 @@ static const struct generic_data stop_discovery_success_test_1 = {
 
 static const struct generic_data stop_discovery_bredr_success_test_1 = {
 	.setup_settings = settings_powered,
-	.setup_expect_hci_command = BT_HCI_CMD_INQUIRY,
-	.setup_expect_hci_param = stop_discovery_inq_param,
-	.setup_expect_hci_len = sizeof(stop_discovery_inq_param),
 	.send_opcode = MGMT_OP_STOP_DISCOVERY,
 	.send_param = stop_discovery_bredr_param,
 	.send_len = sizeof(stop_discovery_bredr_param),
-- 
1.8.4

[PATCH v5 08/10] tools/mgmt-tester: Remove not used condition

[Lukasz Rymanowski]

---
 tools/mgmt-tester.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tools/mgmt-tester.c b/tools/mgmt-tester.c
index 17d2bd2..a711d0d 100644
--- a/tools/mgmt-tester.c
+++ b/tools/mgmt-tester.c
@@ -2868,9 +2868,6 @@ static void setup_start_discovery(const void *test_data)
 				setup_command_hci_callback,
 				NULL);
 
-		if (test->send_func)
-			send_param = test->send_func(&send_len);
-
 		mgmt_send(data->mgmt, MGMT_OP_START_DISCOVERY, data->mgmt_index,
 				send_len, send_param, NULL, NULL, NULL);
 	} else {
-- 
1.8.4

[PATCH v5 09/10] tools/mgmt-tester: Refactor setup_start_discovery function

[Lukasz Rymanowski]

This patch removes handling hook register from this
function and its callback as this is no longer necessary for any stop
discovery tests.

This patch also adds setup_send_param and setup_send_len  parameters to
test data so it is easy to control start discovery command parameters.
It is useful for tests:
Stop Discovery - Invalid parameters 1
Stop Discovery - BR/EDR (Inquiry) Success 1
---
 tools/mgmt-tester.c | 62 ++++++++++++++---------------------------------------
 1 file changed, 16 insertions(+), 46 deletions(-)

diff --git a/tools/mgmt-tester.c b/tools/mgmt-tester.c
index a711d0d..64188f1 100644
--- a/tools/mgmt-tester.c
+++ b/tools/mgmt-tester.c
@@ -362,6 +362,9 @@ struct generic_data {
 	uint16_t setup_expect_hci_command;
 	const void *setup_expect_hci_param;
 	uint8_t setup_expect_hci_len;
+	uint16_t setup_send_opcode;
+	const void *setup_send_param;
+	uint16_t setup_send_len;
 	bool send_index_none;
 	uint16_t send_opcode;
 	const void *send_param;
@@ -1734,6 +1737,8 @@ static const struct generic_data stop_discovery_success_test_1 = {
 
 static const struct generic_data stop_discovery_bredr_success_test_1 = {
 	.setup_settings = settings_powered,
+	.setup_send_param = start_discovery_bredr_param,
+	.setup_send_len = sizeof(start_discovery_bredr_param),
 	.send_opcode = MGMT_OP_STOP_DISCOVERY,
 	.send_param = stop_discovery_bredr_param,
 	.send_len = sizeof(stop_discovery_bredr_param),
@@ -1758,6 +1763,8 @@ static const struct generic_data stop_discovery_rejected_test_1 = {
 
 static const struct generic_data stop_discovery_invalid_param_test_1 = {
 	.setup_settings = settings_powered_le,
+	.setup_send_param = start_discovery_bredrle_param,
+	.setup_send_len = sizeof(start_discovery_bredrle_param),
 	.send_opcode = MGMT_OP_STOP_DISCOVERY,
 	.send_param = stop_discovery_bredrle_invalid_param,
 	.send_len = sizeof(stop_discovery_bredrle_invalid_param),
@@ -2824,59 +2831,22 @@ static void setup_discovery_callback(uint8_t status, uint16_t length,
 	tester_setup_complete();
 }
 
-static bool setup_command_hci_callback(const void *data, uint16_t len,
-								void *user_data)
-{
-	struct test_data *tdata = tester_get_data();
-	const struct generic_data *test = tdata->test_data;
-
-	tester_print("HCI Command 0x%04x length %u (setup)",
-					test->setup_expect_hci_command, len);
-
-	if (len != test->setup_expect_hci_len) {
-		tester_warn("Invalid parameter size for HCI command (setup)");
-		tester_setup_failed();
-		goto done;
-	}
-
-	if (memcmp(data, test->setup_expect_hci_param, len) != 0) {
-		tester_warn("Unexpected HCI command parameter value (setup)");
-		tester_setup_failed();
-		goto done;
-	}
-
-	tester_setup_complete();
-
-done:
-	hciemu_del_hook(tdata->hciemu, HCIEMU_HOOK_PRE_EVT,
-			test->setup_expect_hci_command);
-
-	return false;
-}
-
 static void setup_start_discovery(const void *test_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct generic_data *test = data->test_data;
-	const void *send_param = test->send_param;
-	uint16_t send_len = test->send_len;
-
-	if (test->setup_expect_hci_command) {
-		tester_print("Registering HCI command callback (setup)");
-		hciemu_add_hook(data->hciemu, HCIEMU_HOOK_PRE_EVT,
-				test->setup_expect_hci_command,
-				setup_command_hci_callback,
-				NULL);
+	const void *send_param = test->setup_send_param;
+	uint16_t send_len = test->setup_send_len;
+	unsigned char disc_param[] = { 0x07 };
 
-		mgmt_send(data->mgmt, MGMT_OP_START_DISCOVERY, data->mgmt_index,
-				send_len, send_param, NULL, NULL, NULL);
-	} else {
-		unsigned char disc_param[] = { 0x07 };
+	if (!send_param) {
+		send_param = disc_param;
+		send_len = 1;
+	}
 
-		mgmt_send(data->mgmt, MGMT_OP_START_DISCOVERY, data->mgmt_index,
-					sizeof(disc_param), disc_param,
+	mgmt_send(data->mgmt, MGMT_OP_START_DISCOVERY, data->mgmt_index,
+					send_len, send_param,
 					setup_discovery_callback, NULL, NULL);
-	}
 }
 
 static void setup_multi_uuid32(const void *test_data)
-- 
1.8.4

[PATCH v5 10/10] shared: Fix missing destroy callback in timeout_add

[Lukasz Rymanowski]

---
 src/shared/timeout-mainloop.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/shared/timeout-mainloop.c b/src/shared/timeout-mainloop.c
index 37bb51c..77aaa63 100644
--- a/src/shared/timeout-mainloop.c
+++ b/src/shared/timeout-mainloop.c
@@ -65,6 +65,7 @@ unsigned int timeout_add(unsigned int timeout, timeout_func_t func,
 	data->func = func;
 	data->user_data = user_data;
 	data->timeout = timeout;
+	data->destroy = destroy;
 
 	data->id = mainloop_add_timeout(timeout, timeout_callback, data,
 							timeout_destroy);
-- 
1.8.4

Re: [PATCH v5 09/10] tools/mgmt-tester: Refactor setup_start_discovery function

[Johan Hedberg]

Hi Lukasz,

On Thu, Mar 20, 2014, Lukasz Rymanowski wrote:
> This patch removes handling hook register from this
> function and its callback as this is no longer necessary for any stop
> discovery tests.
> 
> This patch also adds setup_send_param and setup_send_len  parameters to
> test data so it is easy to control start discovery command parameters.
> It is useful for tests:
> Stop Discovery - Invalid parameters 1
> Stop Discovery - BR/EDR (Inquiry) Success 1
> ---
>  tools/mgmt-tester.c | 62 ++++++++++++++---------------------------------------
>  1 file changed, 16 insertions(+), 46 deletions(-)

I've applied all other patches in the set besides this one.

> --- a/tools/mgmt-tester.c
> +++ b/tools/mgmt-tester.c
> @@ -362,6 +362,9 @@ struct generic_data {
>  	uint16_t setup_expect_hci_command;
>  	const void *setup_expect_hci_param;
>  	uint8_t setup_expect_hci_len;
> +	uint16_t setup_send_opcode;
> +	const void *setup_send_param;
> +	uint16_t setup_send_len;
>  	bool send_index_none;
>  	uint16_t send_opcode;
>  	const void *send_param;
> @@ -1734,6 +1737,8 @@ static const struct generic_data stop_discovery_success_test_1 = {
>  
>  static const struct generic_data stop_discovery_bredr_success_test_1 = {
>  	.setup_settings = settings_powered,
> +	.setup_send_param = start_discovery_bredr_param,
> +	.setup_send_len = sizeof(start_discovery_bredr_param),
>  	.send_opcode = MGMT_OP_STOP_DISCOVERY,
>  	.send_param = stop_discovery_bredr_param,
>  	.send_len = sizeof(stop_discovery_bredr_param),

Where's the setup_send_opcode which you added to the struct? Since
you're calling these by generic names you really should be setting all
of them.

> @@ -1758,6 +1763,8 @@ static const struct generic_data stop_discovery_rejected_test_1 = {
>  
>  static const struct generic_data stop_discovery_invalid_param_test_1 = {
>  	.setup_settings = settings_powered_le,
> +	.setup_send_param = start_discovery_bredrle_param,
> +	.setup_send_len = sizeof(start_discovery_bredrle_param),
>  	.send_opcode = MGMT_OP_STOP_DISCOVERY,
>  	.send_param = stop_discovery_bredrle_invalid_param,
>  	.send_len = sizeof(stop_discovery_bredrle_invalid_param),

Same here.

>  static void setup_start_discovery(const void *test_data)
>  {
>  	struct test_data *data = tester_get_data();
>  	const struct generic_data *test = data->test_data;
> -	const void *send_param = test->send_param;
> -	uint16_t send_len = test->send_len;
> -
> -	if (test->setup_expect_hci_command) {
> -		tester_print("Registering HCI command callback (setup)");
> -		hciemu_add_hook(data->hciemu, HCIEMU_HOOK_PRE_EVT,
> -				test->setup_expect_hci_command,
> -				setup_command_hci_callback,
> -				NULL);
> +	const void *send_param = test->setup_send_param;
> +	uint16_t send_len = test->setup_send_len;
> +	unsigned char disc_param[] = { 0x07 };
>  
> -		mgmt_send(data->mgmt, MGMT_OP_START_DISCOVERY, data->mgmt_index,
> -				send_len, send_param, NULL, NULL, NULL);
> -	} else {
> -		unsigned char disc_param[] = { 0x07 };
> +	if (!send_param) {
> +		send_param = disc_param;
> +		send_len = 1;
> +	}
>  
> -		mgmt_send(data->mgmt, MGMT_OP_START_DISCOVERY, data->mgmt_index,
> -					sizeof(disc_param), disc_param,
> +	mgmt_send(data->mgmt, MGMT_OP_START_DISCOVERY, data->mgmt_index,
> +					send_len, send_param,
>  					setup_discovery_callback, NULL, NULL);

And here you shouldn't have a hard-coded MGMT_OP_START_DISCOVERY but use
setup_send_opcode instead.

Johan