2013-12-19 12:42:21

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCH 1/6] android/tester: Fix for not checking for BT_STATUS_SUCCESS

For BT_STATUS_SUCCESS no checks were done as it is 0 in bt_status_t
enum. Valid status for no status expected should be STATUS_NOT_EXPECTED.

---
android/android-tester.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index c24f5a6..5549dcb 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -72,7 +72,7 @@ enum hal_bluetooth_callbacks_id {
};

struct generic_data {
- uint8_t expected_adapter_status;
+ int expected_adapter_status;
uint32_t expect_settings_set;
bt_property_t expected_property;
uint8_t expected_hal_callbacks[];
@@ -92,6 +92,8 @@ struct socket_data {
#define WAIT_FOR_SIGNAL_TIME 2 /* in seconds */
#define EMULATOR_SIGNAL "emulator_started"

+#define BT_STATUS_NOT_EXPECTED -1
+
struct test_data {
struct mgmt *mgmt;
uint16_t mgmt_index;
@@ -199,7 +201,7 @@ static void expected_status_init(struct test_data *data)
{
const struct generic_data *test_data = data->test_data;

- if (!(test_data->expected_adapter_status))
+ if (test_data->expected_adapter_status == BT_STATUS_NOT_EXPECTED)
data->status_checked = true;
}

--
1.8.5



2013-12-19 14:42:22

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 1/6] android/tester: Fix for not checking for BT_STATUS_SUCCESS

Hi,

On Thu, Dec 19, 2013, Johan Hedberg wrote:
> On Thu, Dec 19, 2013, Jakub Tyszkowski wrote:
> > For BT_STATUS_SUCCESS no checks were done as it is 0 in bt_status_t
> > enum. Valid status for no status expected should be STATUS_NOT_EXPECTED.
> >
> > ---
> > android/android-tester.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/android/android-tester.c b/android/android-tester.c
> > index c24f5a6..5549dcb 100644
> > --- a/android/android-tester.c
> > +++ b/android/android-tester.c
> > @@ -72,7 +72,7 @@ enum hal_bluetooth_callbacks_id {
> > };
> >
> > struct generic_data {
> > - uint8_t expected_adapter_status;
> > + int expected_adapter_status;
> > uint32_t expect_settings_set;
> > bt_property_t expected_property;
> > uint8_t expected_hal_callbacks[];
> > @@ -92,6 +92,8 @@ struct socket_data {
> > #define WAIT_FOR_SIGNAL_TIME 2 /* in seconds */
> > #define EMULATOR_SIGNAL "emulator_started"
> >
> > +#define BT_STATUS_NOT_EXPECTED -1
> > +
> > struct test_data {
> > struct mgmt *mgmt;
> > uint16_t mgmt_index;
> > @@ -199,7 +201,7 @@ static void expected_status_init(struct test_data *data)
> > {
> > const struct generic_data *test_data = data->test_data;
> >
> > - if (!(test_data->expected_adapter_status))
> > + if (test_data->expected_adapter_status == BT_STATUS_NOT_EXPECTED)
> > data->status_checked = true;
> > }
>
> I suppose it does make sense to have such a special value, but I can't
> see you use it anywhere in your patch set. So I'd postpone this patch
> until you've got some actual code that needs it.

Nevermind. I now see that even though you don't add any tests that would
use this new value the change of the if-statement that this patch
introduces is already enough of a justification.

All patches in the set have now been applied.

Johan

2013-12-19 14:41:57

by Jakub Tyszkowski

[permalink] [raw]
Subject: Re: [PATCH 1/6] android/tester: Fix for not checking for BT_STATUS_SUCCESS

On 12/19/2013 02:42 PM, Johan Hedberg wrote:
> Hi Jakub,
>
> On Thu, Dec 19, 2013, Jakub Tyszkowski wrote:
>> For BT_STATUS_SUCCESS no checks were done as it is 0 in bt_status_t
>> enum. Valid status for no status expected should be STATUS_NOT_EXPECTED.
>>
>> ---
>> android/android-tester.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/android/android-tester.c b/android/android-tester.c
>> index c24f5a6..5549dcb 100644
>> --- a/android/android-tester.c
>> +++ b/android/android-tester.c
>> @@ -72,7 +72,7 @@ enum hal_bluetooth_callbacks_id {
>> };
>>
>> struct generic_data {
>> - uint8_t expected_adapter_status;
>> + int expected_adapter_status;
>> uint32_t expect_settings_set;
>> bt_property_t expected_property;
>> uint8_t expected_hal_callbacks[];
>> @@ -92,6 +92,8 @@ struct socket_data {
>> #define WAIT_FOR_SIGNAL_TIME 2 /* in seconds */
>> #define EMULATOR_SIGNAL "emulator_started"
>>
>> +#define BT_STATUS_NOT_EXPECTED -1
>> +
>> struct test_data {
>> struct mgmt *mgmt;
>> uint16_t mgmt_index;
>> @@ -199,7 +201,7 @@ static void expected_status_init(struct test_data *data)
>> {
>> const struct generic_data *test_data = data->test_data;
>>
>> - if (!(test_data->expected_adapter_status))
>> + if (test_data->expected_adapter_status == BT_STATUS_NOT_EXPECTED)
>> data->status_checked = true;
>> }
>
> I suppose it does make sense to have such a special value, but I can't
> see you use it anywhere in your patch set. So I'd postpone this patch
> until you've got some actual code that needs it.
>
> Johan
>
But this will actually leave the tester not checking for
BT_STATUS_SUCCESS (0 in enum) at all during real tests, as it is already
being set as checked during the expected_status_init() call.

2013-12-19 13:42:24

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 1/6] android/tester: Fix for not checking for BT_STATUS_SUCCESS

Hi Jakub,

On Thu, Dec 19, 2013, Jakub Tyszkowski wrote:
> For BT_STATUS_SUCCESS no checks were done as it is 0 in bt_status_t
> enum. Valid status for no status expected should be STATUS_NOT_EXPECTED.
>
> ---
> android/android-tester.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/android/android-tester.c b/android/android-tester.c
> index c24f5a6..5549dcb 100644
> --- a/android/android-tester.c
> +++ b/android/android-tester.c
> @@ -72,7 +72,7 @@ enum hal_bluetooth_callbacks_id {
> };
>
> struct generic_data {
> - uint8_t expected_adapter_status;
> + int expected_adapter_status;
> uint32_t expect_settings_set;
> bt_property_t expected_property;
> uint8_t expected_hal_callbacks[];
> @@ -92,6 +92,8 @@ struct socket_data {
> #define WAIT_FOR_SIGNAL_TIME 2 /* in seconds */
> #define EMULATOR_SIGNAL "emulator_started"
>
> +#define BT_STATUS_NOT_EXPECTED -1
> +
> struct test_data {
> struct mgmt *mgmt;
> uint16_t mgmt_index;
> @@ -199,7 +201,7 @@ static void expected_status_init(struct test_data *data)
> {
> const struct generic_data *test_data = data->test_data;
>
> - if (!(test_data->expected_adapter_status))
> + if (test_data->expected_adapter_status == BT_STATUS_NOT_EXPECTED)
> data->status_checked = true;
> }

I suppose it does make sense to have such a special value, but I can't
see you use it anywhere in your patch set. So I'd postpone this patch
until you've got some actual code that needs it.

Johan

2013-12-19 12:42:23

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCH 3/6] android/tester: Add start device discovery success test case

Add test case for starting device discovery with success.

---
android/android-tester.c | 42 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 68614a2..1880cf1 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -68,7 +68,9 @@ enum hal_bluetooth_callbacks_id {
ADAPTER_PROP_SCAN_MODE,
ADAPTER_PROP_DISC_TIMEOUT,
ADAPTER_PROP_SERVICE_RECORD,
- ADAPTER_PROP_BONDED_DEVICES
+ ADAPTER_PROP_BONDED_DEVICES,
+ ADAPTER_DISCOVERY_STATE_ON,
+ ADAPTER_DISCOVERY_STATE_OFF
};

struct generic_data {
@@ -527,6 +529,20 @@ static void adapter_state_changed_cb(bt_state_t state)
}
}

+static void discovery_state_changed_cb(bt_discovery_state_t state)
+{
+ switch (state) {
+ case BT_DISCOVERY_STARTED:
+ update_hal_cb_list(ADAPTER_DISCOVERY_STATE_ON);
+ break;
+ case BT_DISCOVERY_STOPPED:
+ update_hal_cb_list(ADAPTER_DISCOVERY_STATE_OFF);
+ break;
+ default:
+ break;
+ }
+}
+
static void adapter_properties_cb(bt_status_t status, int num_properties,
bt_property_t *properties)
{
@@ -702,13 +718,19 @@ static const struct generic_data
.expected_property.len = sizeof(setprop_remote_service)
};

+static const struct generic_data bluetooth_discovery_start_success_test = {
+ .expected_hal_callbacks = { ADAPTER_DISCOVERY_STATE_ON,
+ ADAPTER_TEST_END },
+ .expected_adapter_status = BT_STATUS_SUCCESS
+};
+
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 = NULL,
.device_found_cb = NULL,
- .discovery_state_changed_cb = NULL,
+ .discovery_state_changed_cb = discovery_state_changed_cb,
.pin_request_cb = NULL,
.ssp_request_cb = NULL,
.bond_state_changed_cb = NULL,
@@ -1230,6 +1252,17 @@ clean:
close(sock_fd);
}

+static void test_discovery_start_success(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ bt_status_t status;
+
+ init_test_conditions(data);
+
+ status = data->if_bluetooth->start_discovery();
+ check_expected_status(status);
+}
+
static gboolean socket_chan_cb(GIOChannel *io, GIOCondition cond,
gpointer user_data)
{
@@ -1401,6 +1434,11 @@ int main(int argc, char *argv[])
setup_enabled_adapter,
test_setprop_service_record_invalid, teardown);

+ test_bredrle("Bluetooth BREDR Discovery Start - Success",
+ &bluetooth_discovery_start_success_test,
+ setup_enabled_adapter,
+ test_discovery_start_success, teardown);
+
test_bredrle("Socket Init", NULL, setup_socket_interface,
test_dummy, teardown);

--
1.8.5


2013-12-19 12:42:22

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCH 2/6] android/tester: Fix bluetooth disable success test case

This fixes not checking for proper status on bluetooth disable.

---
android/android-tester.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 5549dcb..68614a2 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -596,7 +596,8 @@ static const struct generic_data bluetooth_enable_done_test = {

static const struct generic_data bluetooth_disable_success_test = {
.expected_hal_callbacks = { ADAPTER_STATE_CHANGED_OFF,
- ADAPTER_TEST_END }
+ ADAPTER_TEST_END },
+ .expected_adapter_status = BT_STATUS_SUCCESS
};

static const struct generic_data bluetooth_setprop_bdname_success_test = {
@@ -858,10 +859,12 @@ static void test_enable_done(const void *test_data)
static void test_disable(const void *test_data)
{
struct test_data *data = tester_get_data();
+ bt_status_t adapter_status;

init_test_conditions(data);

- data->if_bluetooth->disable();
+ adapter_status = data->if_bluetooth->disable();
+ check_expected_status(adapter_status);
}

static void test_setprop_bdname_success(const void *test_data)
--
1.8.5


2013-12-19 12:42:26

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCH 6/6] android/tester: Add start device discovery done test case

This tests device discovery start being reported as done because of
ongoing discovery.

---
android/android-tester.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index 737aeb3..aa0c155 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -98,6 +98,7 @@ struct socket_data {

/* User flags for Device Discovery */
#define DEVICE_DISCOVERY_CANCEL_ON_START 0x01
+#define DEVICE_DISCOVERY_START_ON_START 0x02

struct test_data {
struct mgmt *mgmt;
@@ -543,6 +544,11 @@ static void post_discovery_started_cb(bt_discovery_state_t state)
status = data->if_bluetooth->cancel_discovery();
check_expected_status(status);
}
+
+ if (data->userflag & DEVICE_DISCOVERY_START_ON_START) {
+ status = data->if_bluetooth->start_discovery();
+ check_expected_status(status);
+ }
}

static void discovery_state_changed_cb(bt_discovery_state_t state)
@@ -741,6 +747,12 @@ static const struct generic_data bluetooth_discovery_start_success_test = {
.expected_adapter_status = BT_STATUS_SUCCESS
};

+static const struct generic_data bluetooth_discovery_start_done_test = {
+ .expected_hal_callbacks = { ADAPTER_DISCOVERY_STATE_ON,
+ ADAPTER_TEST_END },
+ .expected_adapter_status = BT_STATUS_DONE
+};
+
static const struct generic_data bluetooth_discovery_stop_done_test = {
.expected_hal_callbacks = { ADAPTER_TEST_END },
.expected_adapter_status = BT_STATUS_DONE
@@ -1324,6 +1336,20 @@ static void test_discovery_stop_success(const void *test_data)
check_expected_status(status);
}

+static void test_discovery_start_done(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+
+ data->userflag = DEVICE_DISCOVERY_START_ON_START;
+
+ 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();
+}
+
static gboolean socket_chan_cb(GIOChannel *io, GIOCondition cond,
gpointer user_data)
{
@@ -1495,6 +1521,11 @@ int main(int argc, char *argv[])
setup_enabled_adapter,
test_setprop_service_record_invalid, teardown);

+ test_bredrle("Bluetooth BREDR Discovery Start - Done",
+ &bluetooth_discovery_start_done_test,
+ setup_enabled_adapter,
+ test_discovery_start_done, teardown);
+
test_bredrle("Bluetooth BREDR Discovery Start - Success",
&bluetooth_discovery_start_success_test,
setup_enabled_adapter,
--
1.8.5


2013-12-19 12:42:25

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCH 5/6] android/tester: Add device discovery cancel success test case

Test for successfull discovery canceling after it was started.

---
android/android-tester.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index 0dae111..737aeb3 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -96,6 +96,9 @@ struct socket_data {

#define BT_STATUS_NOT_EXPECTED -1

+/* User flags for Device Discovery */
+#define DEVICE_DISCOVERY_CANCEL_ON_START 0x01
+
struct test_data {
struct mgmt *mgmt;
uint16_t mgmt_index;
@@ -115,6 +118,8 @@ struct test_data {

bt_property_t test_property;
GSList *expected_callbacks;
+
+ uint32_t userflag;
};

static char exec_dir[PATH_MAX + 1];
@@ -529,11 +534,23 @@ static void adapter_state_changed_cb(bt_state_t state)
}
}

+static void post_discovery_started_cb(bt_discovery_state_t state)
+{
+ struct test_data *data = tester_get_data();
+ bt_status_t status;
+
+ if (data->userflag & DEVICE_DISCOVERY_CANCEL_ON_START) {
+ status = data->if_bluetooth->cancel_discovery();
+ check_expected_status(status);
+ }
+}
+
static void discovery_state_changed_cb(bt_discovery_state_t state)
{
switch (state) {
case BT_DISCOVERY_STARTED:
update_hal_cb_list(ADAPTER_DISCOVERY_STATE_ON);
+ post_discovery_started_cb(state);
break;
case BT_DISCOVERY_STOPPED:
update_hal_cb_list(ADAPTER_DISCOVERY_STATE_OFF);
@@ -729,6 +746,12 @@ static const struct generic_data bluetooth_discovery_stop_done_test = {
.expected_adapter_status = BT_STATUS_DONE
};

+static const struct generic_data bluetooth_discovery_stop_success_test = {
+ .expected_hal_callbacks = { ADAPTER_DISCOVERY_STATE_ON,
+ ADAPTER_DISCOVERY_STATE_OFF, ADAPTER_TEST_END },
+ .expected_adapter_status = BT_STATUS_SUCCESS
+};
+
static bt_callbacks_t bt_callbacks = {
.size = sizeof(bt_callbacks),
.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -1279,6 +1302,28 @@ static void test_discovery_stop_done(const void *test_data)
check_expected_status(status);
}

+static bool pre_inq_compl_hook(const void *data, uint16_t len, void *user_data)
+{
+ /* Make sure Inquiry Command Complete is not called */
+ return false;
+}
+
+static void test_discovery_stop_success(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ bt_status_t status;
+
+ data->userflag = DEVICE_DISCOVERY_CANCEL_ON_START;
+
+ init_test_conditions(data);
+
+ hciemu_add_hook(data->hciemu, HCIEMU_HOOK_PRE_EVT, BT_HCI_CMD_INQUIRY,
+ pre_inq_compl_hook, data);
+
+ status = data->if_bluetooth->start_discovery();
+ check_expected_status(status);
+}
+
static gboolean socket_chan_cb(GIOChannel *io, GIOCondition cond,
gpointer user_data)
{
@@ -1460,6 +1505,11 @@ int main(int argc, char *argv[])
setup_enabled_adapter,
test_discovery_stop_done, teardown);

+ test_bredrle("Bluetooth BREDR Discovery Stop - Success",
+ &bluetooth_discovery_stop_success_test,
+ setup_enabled_adapter,
+ test_discovery_stop_success, teardown);
+
test_bredrle("Socket Init", NULL, setup_socket_interface,
test_dummy, teardown);

--
1.8.5


2013-12-19 12:42:24

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCH 4/6] android/tester: Add stop device discovery done test case

Add test case for stopping device discovery when it wasn't started.

---
android/android-tester.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/android/android-tester.c b/android/android-tester.c
index 1880cf1..0dae111 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -724,6 +724,11 @@ static const struct generic_data bluetooth_discovery_start_success_test = {
.expected_adapter_status = BT_STATUS_SUCCESS
};

+static const struct generic_data bluetooth_discovery_stop_done_test = {
+ .expected_hal_callbacks = { ADAPTER_TEST_END },
+ .expected_adapter_status = BT_STATUS_DONE
+};
+
static bt_callbacks_t bt_callbacks = {
.size = sizeof(bt_callbacks),
.adapter_state_changed_cb = adapter_state_changed_cb,
@@ -1263,6 +1268,17 @@ static void test_discovery_start_success(const void *test_data)
check_expected_status(status);
}

+static void test_discovery_stop_done(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ bt_status_t status;
+
+ init_test_conditions(data);
+
+ status = data->if_bluetooth->cancel_discovery();
+ check_expected_status(status);
+}
+
static gboolean socket_chan_cb(GIOChannel *io, GIOCondition cond,
gpointer user_data)
{
@@ -1439,6 +1455,11 @@ int main(int argc, char *argv[])
setup_enabled_adapter,
test_discovery_start_success, teardown);

+ test_bredrle("Bluetooth BREDR Discovery Stop - Done",
+ &bluetooth_discovery_stop_done_test,
+ setup_enabled_adapter,
+ test_discovery_stop_done, teardown);
+
test_bredrle("Socket Init", NULL, setup_socket_interface,
test_dummy, teardown);

--
1.8.5