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

[PATCH v2 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

Lukasz Rymanowski (10):
  emulator: Minor style fix
  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

 Makefile.tools           |  19 ++++--
 android/Makefile.am      |   2 +
 android/android-tester.c |  24 ++-----
 emulator/btdev.c         | 166 +++++++++++++++++++++++++++++++++++++++++++----
 tools/mgmt-tester.c      |  70 +++++---------------
 5 files changed, 194 insertions(+), 87 deletions(-)

-- 
1.8.4

[PATCH v2 01/10] emulator: Minor style fix

[Lukasz Rymanowski]

---
 emulator/btdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/emulator/btdev.c b/emulator/btdev.c
index b54f91e..2cf8f89 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -709,8 +709,8 @@ static void inquiry_complete(struct btdev *btdev, uint8_t status)
 			ir.rssi = -60;
 			memcpy(ir.data, btdev_list[i]->ext_inquiry_rsp, 240);
 
-			send_event(btdev, BT_HCI_EVT_EXT_INQUIRY_RESULT,
-							&ir, sizeof(ir));
+			send_event(btdev, BT_HCI_EVT_EXT_INQUIRY_RESULT, &ir,
+								sizeof(ir));
 			continue;
 		}
 
@@ -738,8 +738,8 @@ static void inquiry_complete(struct btdev *btdev, uint8_t status)
 			memcpy(ir.dev_class, btdev_list[i]->dev_class, 3);
 			ir.clock_offset = 0x0000;
 
-			send_event(btdev, BT_HCI_EVT_INQUIRY_RESULT,
-							&ir, sizeof(ir));
+			send_event(btdev, BT_HCI_EVT_INQUIRY_RESULT, &ir,
+								sizeof(ir));
 		}
         }
 
-- 
1.8.4

Re: [PATCH v2 01/10] emulator: Minor style fix

[Johan Hedberg]

Hi Lukasz,

On Mon, Mar 10, 2014, Lukasz Rymanowski wrote:
> ---
>  emulator/btdev.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/emulator/btdev.c b/emulator/btdev.c
> index b54f91e..2cf8f89 100644
> --- a/emulator/btdev.c
> +++ b/emulator/btdev.c
> @@ -709,8 +709,8 @@ static void inquiry_complete(struct btdev *btdev, uint8_t status)
>  			ir.rssi = -60;
>  			memcpy(ir.data, btdev_list[i]->ext_inquiry_rsp, 240);
>  
> -			send_event(btdev, BT_HCI_EVT_EXT_INQUIRY_RESULT,
> -							&ir, sizeof(ir));
> +			send_event(btdev, BT_HCI_EVT_EXT_INQUIRY_RESULT, &ir,
> +								sizeof(ir));
>  			continue;
>  		}
>  
> @@ -738,8 +738,8 @@ static void inquiry_complete(struct btdev *btdev, uint8_t status)
>  			memcpy(ir.dev_class, btdev_list[i]->dev_class, 3);
>  			ir.clock_offset = 0x0000;
>  
> -			send_event(btdev, BT_HCI_EVT_INQUIRY_RESULT,
> -							&ir, sizeof(ir));
> +			send_event(btdev, BT_HCI_EVT_INQUIRY_RESULT, &ir,
> +								sizeof(ir));
>  		}
>          }

I wouldn't bother with this one. The reason these two variables have
been put on the same line is that they are tightly related to each
other.

Johan

[PATCH v2 02/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      | 19 ++++++++----
 android/Makefile.am |  2 ++
 emulator/btdev.c    | 84 +++++++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 93 insertions(+), 12 deletions(-)

diff --git a/Makefile.tools b/Makefile.tools
index 7ffdbc6..88a15ee 100644
--- a/Makefile.tools
+++ b/Makefile.tools
@@ -48,6 +48,7 @@ emulator_btvirt_SOURCES = emulator/main.c monitor/bt.h \
 				monitor/mainloop.h monitor/mainloop.c \
 				src/shared/util.h src/shared/util.c \
 				src/shared/crypto.h src/shared/crypto.c \
+				src/sharead/timeout.h src/shared/timeout-mainloop.c \
 				emulator/server.h emulator/server.c \
 				emulator/vhci.h emulator/vhci.c \
 				emulator/btdev.h emulator/btdev.c \
@@ -84,7 +85,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 \
@@ -97,7 +99,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 \
@@ -110,7 +113,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 \
@@ -123,7 +127,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 \
@@ -134,7 +139,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@
@@ -149,7 +155,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 bbd3d2f..b3a8cc8 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -119,6 +119,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
@@ -142,6 +143,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-tester.c
 
diff --git a/emulator/btdev.c b/emulator/btdev.c
index 2cf8f89..8b3a2d8 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;
 
+	int inquiry_id;
+
 	struct hook *hook_list[MAX_HOOK_ENTRIES];
 
         uint16_t manufacturer;
@@ -123,8 +126,17 @@ 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 };
 static const uint8_t LINK_KEY_DUMMY[16] = {	0, 1, 2, 3, 4, 5, 6, 7,
 						8, 9, 0, 1, 2, 3, 4, 5 };
@@ -528,9 +540,13 @@ void btdev_destroy(struct btdev *btdev)
 	if (!btdev)
 		return;
 
+	if (btdev->inquiry_id)
+		timeout_remove(btdev->inquiry_id);
+
 	del_btdev(btdev);
 
 	free(btdev);
+	btdev = NULL;
 }
 
 const uint8_t *btdev_get_bdaddr(struct btdev *btdev)
@@ -684,12 +700,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++, data->iter++) {
+		/*Lets sent 10 inquiry results at once */
+		if (sent + 10 == data->sent_count)
+			break;
+
 		if (!btdev_list[i] || btdev_list[i] == btdev)
 			continue;
 
@@ -711,6 +733,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;
 		}
 
@@ -727,6 +750,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;
 
@@ -740,11 +764,59 @@ 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;
+	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));
+
+		btdev->inquiry_id = 0;
+		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) {
+		status = BT_HCI_ERR_COMMAND_DISALLOWED;
+		goto error;
+	}
+
+	data = malloc(sizeof(*data));
+	if (!data)
+		goto error;
+
+	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)
+		return;
+
+error:
+	ic.status = status;
 	send_event(btdev, BT_HCI_EVT_INQUIRY_COMPLETE, &ic, sizeof(ic));
 }
 
@@ -2491,7 +2563,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

Re: [PATCH v2 02/10] emulator: Use timeout for sending inquiry results

[Johan Hedberg]

Hi Lukasz,

On Mon, Mar 10, 2014, Lukasz Rymanowski wrote:
> +#define DEFAULT_INQUIRY_INTERVAL 100 /* 100 miliseconds */
> +
>  #define MAX_BTDEV_ENTRIES 16
>  
> +
>  static const uint8_t LINK_KEY_NONE[16] = { 0 };

Seems like you're introducing an unnecessary empty line above.

>  static const uint8_t LINK_KEY_DUMMY[16] = {	0, 1, 2, 3, 4, 5, 6, 7,
>  						8, 9, 0, 1, 2, 3, 4, 5 };
> @@ -528,9 +540,13 @@ void btdev_destroy(struct btdev *btdev)
>  	if (!btdev)
>  		return;
>  
> +	if (btdev->inquiry_id)
> +		timeout_remove(btdev->inquiry_id);

We usually check for valid timeout or GSource IDs with id > 0.

> +
>  	del_btdev(btdev);
>  
>  	free(btdev);
> +	btdev = NULL;
>  }

This last btdev = NULL is pointless since you're leaving the function
and the variable goes out of scope.

>  
> -	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++, data->iter++) {

Iterating data->iter and i++ throughout the entire loop seems overkill
I'd just assign the resulting value of i to data->iter once you've
exited to loop.

> +		/*Lets sent 10 inquiry results at once */
> +		if (sent + 10 == data->sent_count)
> +			break;

I'd prefer if we can make this so that you instead have the timeout
called as many times as necessary and send just one event each time.
I.e. upon reception of HCI_Inquiry you check the duration of the
inquiry, the maximum amount of responses, MAX_BTDEV_ENTRIES and
determine how frequently the timeout should be called in order to send
all events by the time the inquiry should complete.

> +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) {

if (btdev->inquiry_id > 0)

> +	data = malloc(sizeof(*data));
> +	if (!data)
> +		goto error;

We usually use "failed" for this kind of labels.

> +	btdev->inquiry_id = timeout_add(DEFAULT_INQUIRY_INTERVAL,
> +				inquiry_callback, data, inquiry_destroy);

This looks like possibly incorrect indentation. The indentation should
go past the opening ( and then have more lines if necessary.

Johan

Re: [PATCH v2 02/10] emulator: Use timeout for sending inquiry results

[Lukasz Rymanowski]

Hi Johan,

On 12 March 2014 12:28, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Lukasz,
>
> On Mon, Mar 10, 2014, Lukasz Rymanowski wrote:
>> +#define DEFAULT_INQUIRY_INTERVAL 100 /* 100 miliseconds */
>> +
>>  #define MAX_BTDEV_ENTRIES 16
>>
>> +
>>  static const uint8_t LINK_KEY_NONE[16] = { 0 };
>
> Seems like you're introducing an unnecessary empty line above.

will fix

>
>>  static const uint8_t LINK_KEY_DUMMY[16] = {  0, 1, 2, 3, 4, 5, 6, 7,
>>                                               8, 9, 0, 1, 2, 3, 4, 5 };
>> @@ -528,9 +540,13 @@ void btdev_destroy(struct btdev *btdev)
>>       if (!btdev)
>>               return;
>>
>> +     if (btdev->inquiry_id)
>> +             timeout_remove(btdev->inquiry_id);
>
> We usually check for valid timeout or GSource IDs with id > 0.
>
>> +
>>       del_btdev(btdev);
>>
>>       free(btdev);
>> +     btdev = NULL;
>>  }
>
> This last btdev = NULL is pointless since you're leaving the function
> and the variable goes out of scope.

some trash - wll fix
>
>>
>> -     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++, data->iter++) {
>
> Iterating data->iter and i++ throughout the entire loop seems overkill
> I'd just assign the resulting value of i to data->iter once you've
> exited to loop.

sure
>
>> +             /*Lets sent 10 inquiry results at once */
>> +             if (sent + 10 == data->sent_count)
>> +                     break;
>
> I'd prefer if we can make this so that you instead have the timeout
> called as many times as necessary and send just one event each time.
> I.e. upon reception of HCI_Inquiry you check the duration of the
> inquiry, the maximum amount of responses, MAX_BTDEV_ENTRIES and
> determine how frequently the timeout should be called in order to send
> all events by the time the inquiry should complete.
>
This approach would cause an issue at least in android-tester when we
have usually 1 remote device emulated and 10sec of inquiry duration. I
would expect timeouts in the tester.
Maybe I could do configurable timeout on how often the inquiry result
shall be sent ? It will be also easier to create stress scenarios.

BTW. I put here 10 device per timeout, because I wanted to stress
daemon when running android-tester (daemon and btdev runs in one
mainloop)
Actually I think there is no sense to create test for xxx-tester with
scenario of tons of device so I will change it to send one inquiry
result per timeout
For the  test of inquiry of tons of devices I think it is better to
run btvirt and deamon on vhci

>> +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) {
>
> if (btdev->inquiry_id > 0)
>
>> +     data = malloc(sizeof(*data));
>> +     if (!data)
>> +             goto error;
>
> We usually use "failed" for this kind of labels.
>
>> +     btdev->inquiry_id = timeout_add(DEFAULT_INQUIRY_INTERVAL,
>> +                             inquiry_callback, data, inquiry_destroy);
>
> This looks like possibly incorrect indentation. The indentation should
> go past the opening ( and then have more lines if necessary.
>
Will fix

> Johan

\Lukasz

[PATCH v2 03/10] emulator: Add inquiry cancel

[Lukasz Rymanowski]

With this patch, scheduled inquiry session in btdev can be canceled

Conflicts:
	emulator/btdev.c
---
 emulator/btdev.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/emulator/btdev.c b/emulator/btdev.c
index 8b3a2d8..3c0b0ad 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -70,6 +70,7 @@ struct btdev {
 	void *send_data;
 
 	int inquiry_id;
+	bool inquiry_cancel;
 
 	struct hook *hook_list[MAX_HOOK_ENTRIES];
 
@@ -784,10 +785,20 @@ 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;
+	uint8_t status = BT_HCI_ERR_SUCCESS;
+
+	if (!btdev)
+		goto finish;
 
-	if (data->btdev)
-		data->btdev->inquiry_id = 0;
+	if (btdev->inquiry_cancel)
+		cmd_complete(btdev, BT_HCI_CMD_INQUIRY_CANCEL, &status,
+							sizeof(status));
 
+	btdev->inquiry_cancel = false;
+	btdev->inquiry_id = 0;
+
+finish:
 	free(data);
 }
 
@@ -820,6 +831,20 @@ error:
 	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;
+	}
+
+	btdev->inquiry_cancel = true;
+	timeout_remove(btdev->inquiry_id);
+	btdev->inquiry_id = 0;
+}
 static void conn_complete(struct btdev *btdev,
 					const uint8_t *bdaddr, uint8_t status)
 {
@@ -1637,8 +1662,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

Re: [PATCH v2 03/10] emulator: Add inquiry cancel

[Johan Hedberg]

Hi Lukasz,

On Mon, Mar 10, 2014, Lukasz Rymanowski wrote:
> With this patch, scheduled inquiry session in btdev can be canceled
> 
> Conflicts:
> 	emulator/btdev.c

Seems like you forgot to clean up the commit message here.

> @@ -70,6 +70,7 @@ struct btdev {
>  	void *send_data;
>  
>  	int inquiry_id;
> +	bool inquiry_cancel;
>  
>  	struct hook *hook_list[MAX_HOOK_ENTRIES];
>  
> @@ -784,10 +785,20 @@ 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;
> +	uint8_t status = BT_HCI_ERR_SUCCESS;
> +
> +	if (!btdev)
> +		goto finish;
>  
> -	if (data->btdev)
> -		data->btdev->inquiry_id = 0;
> +	if (btdev->inquiry_cancel)
> +		cmd_complete(btdev, BT_HCI_CMD_INQUIRY_CANCEL, &status,
> +							sizeof(status));
>  
> +	btdev->inquiry_cancel = false;
> +	btdev->inquiry_id = 0;
> +
> +finish:
>  	free(data);
>  }
>  
> @@ -820,6 +831,20 @@ error:
>  	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;
> +	}
> +
> +	btdev->inquiry_cancel = true;
> +	timeout_remove(btdev->inquiry_id);

This whole btdev->inquiry_cancel variable seems pointless to me. Why
don't you simply send the cmd_complete for it after calling
timeout_remove here in the inquiry_cancel function?

Johan

Re: [PATCH v2 03/10] emulator: Add inquiry cancel

[Lukasz Rymanowski]

Hi Johan,

On 12 March 2014 12:39, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Lukasz,
>
> On Mon, Mar 10, 2014, Lukasz Rymanowski wrote:
>> With this patch, scheduled inquiry session in btdev can be canceled
>>
>> Conflicts:
>>       emulator/btdev.c
>
> Seems like you forgot to clean up the commit message here.
Rather for sure :)
>
>> @@ -70,6 +70,7 @@ struct btdev {
>>       void *send_data;
>>
>>       int inquiry_id;
>> +     bool inquiry_cancel;
>>
>>       struct hook *hook_list[MAX_HOOK_ENTRIES];
>>
>> @@ -784,10 +785,20 @@ 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;
>> +     uint8_t status = BT_HCI_ERR_SUCCESS;
>> +
>> +     if (!btdev)
>> +             goto finish;
>>
>> -     if (data->btdev)
>> -             data->btdev->inquiry_id = 0;
>> +     if (btdev->inquiry_cancel)
>> +             cmd_complete(btdev, BT_HCI_CMD_INQUIRY_CANCEL, &status,
>> +                                                     sizeof(status));
>>
>> +     btdev->inquiry_cancel = false;
>> +     btdev->inquiry_id = 0;
>> +
>> +finish:
>>       free(data);
>>  }
>>
>> @@ -820,6 +831,20 @@ error:
>>       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;
>> +     }
>> +
>> +     btdev->inquiry_cancel = true;
>> +     timeout_remove(btdev->inquiry_id);
>
> This whole btdev->inquiry_cancel variable seems pointless to me. Why
> don't you simply send the cmd_complete for it after calling
> timeout_remove here in the inquiry_cancel function?

Well you are actually right here. I will get destroy in the same
mainloop iteration anywa. Will fix it.

>
> Johan

Lukasz

[PATCH v2 04/10] emulator: Add handling inquiry number of responses

[Lukasz Rymanowski]

---
 emulator/btdev.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/emulator/btdev.c b/emulator/btdev.c
index 3c0b0ad..c932a18 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -129,6 +129,8 @@ struct btdev {
 
 struct inquiry_data {
 	struct btdev *btdev;
+	int num_resp;
+
 	int sent_count;
 	int iter;
 };
@@ -705,6 +707,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;
 
@@ -769,17 +772,22 @@ static bool inquiry_callback(void *user_data)
 		}
 	}
 
-	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;
 
-		btdev->inquiry_id = 0;
-		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));
+
+	btdev->inquiry_id = 0;
+	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, inquiry_destroy);
-- 
1.8.4

Re: [PATCH v2 04/10] emulator: Add handling inquiry number of responses

[Johan Hedberg]

Hi Lukasz,

On Mon, Mar 10, 2014, Lukasz Rymanowski wrote:
> @@ -769,17 +772,22 @@ static bool inquiry_callback(void *user_data)
>  		}
>  	}
>  
> -	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;
>  
> -		btdev->inquiry_id = 0;
> -		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));
> +
> +	btdev->inquiry_id = 0;
> +	return false;
>  }

I know you're just moving it around from the first patch but isn't the
btdev->inquiry_id = unnecessary here since you're doing the same in the
destroy function which gets called as soon as you return false?

Johan

Re: [PATCH v2 04/10] emulator: Add handling inquiry number of responses

[Lukasz Rymanowski]

Hi Johan,

On 12 March 2014 12:42, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Lukasz,
>
> On Mon, Mar 10, 2014, Lukasz Rymanowski wrote:
>> @@ -769,17 +772,22 @@ static bool inquiry_callback(void *user_data)
>>               }
>>       }
>>
>> -     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;
>>
>> -             btdev->inquiry_id = 0;
>> -             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));
>> +
>> +     btdev->inquiry_id = 0;
>> +     return false;
>>  }
>
> I know you're just moving it around from the first patch but isn't the
> btdev->inquiry_id = unnecessary here since you're doing the same in the
> destroy function which gets called as soon as you return false?
>
Good catch, will remove it.

> Johan

Lukasz

[PATCH v2 05/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 | 46 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 42 insertions(+), 4 deletions(-)

diff --git a/emulator/btdev.c b/emulator/btdev.c
index c932a18..27bbb08 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -70,6 +70,7 @@ struct btdev {
 	void *send_data;
 
 	int inquiry_id;
+	int inquiry_timeout_id;
 	bool inquiry_cancel;
 
 	struct hook *hook_list[MAX_HOOK_ENTRIES];
@@ -133,6 +134,8 @@ struct inquiry_data {
 
 	int sent_count;
 	int iter;
+
+	bool timeout;
 };
 
 #define DEFAULT_INQUIRY_INTERVAL 100 /* 100 miliseconds */
@@ -711,6 +714,14 @@ static bool inquiry_callback(void *user_data)
 	int sent = data->sent_count;
 	int i;
 
+	/* Check if inquiry timeout fired. */
+	if (data->timeout)
+		goto finish;
+
+	/*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++, data->iter++) {
 		/*Lets sent 10 inquiry results at once */
 		if (sent + 10 == data->sent_count)
@@ -771,14 +782,10 @@ static bool inquiry_callback(void *user_data)
 			data->sent_count++;
 		}
 	}
-
 	/* 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 +813,33 @@ static void inquiry_destroy(void *user_data)
 	btdev->inquiry_cancel = false;
 	btdev->inquiry_id = 0;
 
+	if (btdev->inquiry_timeout_id)
+		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;
+
+	data->timeout = true;
+	btdev->inquiry_timeout_id = 0;
+
+	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) {
 		status = BT_HCI_ERR_COMMAND_DISALLOWED;
@@ -830,6 +854,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);
 	/* Return if success */
@@ -852,9 +883,16 @@ static void inquiry_cancel(struct btdev *btdev)
 	}
 
 	btdev->inquiry_cancel = true;
+
+	if (btdev->inquiry_timeout_id)
+		timeout_remove(btdev->inquiry_timeout_id);
+
+	btdev->inquiry_timeout_id = 0;
+
 	timeout_remove(btdev->inquiry_id);
 	btdev->inquiry_id = 0;
 }
+
 static void conn_complete(struct btdev *btdev,
 					const uint8_t *bdaddr, uint8_t status)
 {
-- 
1.8.4

Re: [PATCH v2 05/10] emulator: Add handling inquiry_lenght from inquiry command

[Johan Hedberg]

Hi Lukasz,

On Mon, Mar 10, 2014, Lukasz Rymanowski wrote:
> --- a/emulator/btdev.c
> +++ b/emulator/btdev.c
> @@ -70,6 +70,7 @@ struct btdev {
>  	void *send_data;
>  
>  	int inquiry_id;
> +	int inquiry_timeout_id;
>  	bool inquiry_cancel;
>  
>  	struct hook *hook_list[MAX_HOOK_ENTRIES];
> @@ -133,6 +134,8 @@ struct inquiry_data {
>  
>  	int sent_count;
>  	int iter;
> +
> +	bool timeout;
>  };
>  
>  #define DEFAULT_INQUIRY_INTERVAL 100 /* 100 miliseconds */
> @@ -711,6 +714,14 @@ static bool inquiry_callback(void *user_data)
>  	int sent = data->sent_count;
>  	int i;
>  
> +	/* Check if inquiry timeout fired. */
> +	if (data->timeout)
> +		goto finish;
> +
> +	/*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++, data->iter++) {
>  		/*Lets sent 10 inquiry results at once */
>  		if (sent + 10 == data->sent_count)
> @@ -771,14 +782,10 @@ static bool inquiry_callback(void *user_data)
>  			data->sent_count++;
>  		}
>  	}
> -
>  	/* 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 +813,33 @@ static void inquiry_destroy(void *user_data)
>  	btdev->inquiry_cancel = false;
>  	btdev->inquiry_id = 0;
>  
> +	if (btdev->inquiry_timeout_id)
> +		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;
> +
> +	data->timeout = true;
> +	btdev->inquiry_timeout_id = 0;
> +
> +	return false;
> +}

Instead of adding the new data->timeout variable why don't you simply
send the inquiry_complete from within the inquiry_timeout callback?

Johan

Re: [PATCH v2 05/10] emulator: Add handling inquiry_lenght from inquiry command

[Lukasz Rymanowski]

Hi Johan,

On 12 March 2014 12:44, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Lukasz,
>
> On Mon, Mar 10, 2014, Lukasz Rymanowski wrote:
>> --- a/emulator/btdev.c
>> +++ b/emulator/btdev.c
>> @@ -70,6 +70,7 @@ struct btdev {
>>       void *send_data;
>>
>>       int inquiry_id;
>> +     int inquiry_timeout_id;
>>       bool inquiry_cancel;
>>
>>       struct hook *hook_list[MAX_HOOK_ENTRIES];
>> @@ -133,6 +134,8 @@ struct inquiry_data {
>>
>>       int sent_count;
>>       int iter;
>> +
>> +     bool timeout;
>>  };
>>
>>  #define DEFAULT_INQUIRY_INTERVAL 100 /* 100 miliseconds */
>> @@ -711,6 +714,14 @@ static bool inquiry_callback(void *user_data)
>>       int sent = data->sent_count;
>>       int i;
>>
>> +     /* Check if inquiry timeout fired. */
>> +     if (data->timeout)
>> +             goto finish;
>> +
>> +     /*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++, data->iter++) {
>>               /*Lets sent 10 inquiry results at once */
>>               if (sent + 10 == data->sent_count)
>> @@ -771,14 +782,10 @@ static bool inquiry_callback(void *user_data)
>>                       data->sent_count++;
>>               }
>>       }
>> -
>>       /* 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 +813,33 @@ static void inquiry_destroy(void *user_data)
>>       btdev->inquiry_cancel = false;
>>       btdev->inquiry_id = 0;
>>
>> +     if (btdev->inquiry_timeout_id)
>> +             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;
>> +
>> +     data->timeout = true;
>> +     btdev->inquiry_timeout_id = 0;
>> +
>> +     return false;
>> +}
>
> Instead of adding the new data->timeout variable why don't you simply
> send the inquiry_complete from within the inquiry_timeout callback?
>

True, will fix.
> Johan

\Lukasz

[PATCH v2 06/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 b762a2d..7cab79c 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -940,10 +940,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,
@@ -958,6 +961,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);
 }
 
@@ -977,6 +981,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);
@@ -998,6 +1003,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);
@@ -1019,6 +1025,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);
@@ -2965,26 +2972,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();
 }
 
@@ -2994,9 +2987,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 v2 07/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 v2 08/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 v2 09/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 v2 10/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