From: Szymon Janc <szymon.janc@tieto.com>
To: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: Marcin Kraglak <marcin.kraglak@tieto.com>,
"linux-bluetooth@vger.kernel.org"
<linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCHv3 1/2] android/tester: Simplify setup procedure
Date: Tue, 09 Sep 2014 14:16:30 +0200 [thread overview]
Message-ID: <2497968.NSXm3y7leF@leonov> (raw)
In-Reply-To: <CABBYNZLGUqXOO9xyTMc=kzo2oiWPk_1GaC8B-dg6JFrh_6cMdA@mail.gmail.com>
Hi Luiz, Marcin,
On Tuesday 09 of September 2014 13:03:25 Luiz Augusto von Dentz wrote:
> Hi Marcin,
>
> On Fri, Sep 5, 2014 at 4:00 PM, Marcin Kraglak <marcin.kraglak@tieto.com>
wrote:
> > Every profile test case must initialize bluetooth profile and start
> > bluetoothd, and it is now done in setup_generic(). Additionally
> > each profile can define its own init routine, which will be
> > called after initializing stack.
> > ---
> >
> > android/tester-main.c | 220
> > ++++++++++++-------------------------------------- android/tester-main.h
> > | 1 +
> > 2 files changed, 53 insertions(+), 168 deletions(-)
> >
> > diff --git a/android/tester-main.c b/android/tester-main.c
> > index 297aadb..7e8621a 100644
> > --- a/android/tester-main.c
> > +++ b/android/tester-main.c
> > @@ -1336,7 +1336,7 @@ static bool setup_base(struct test_data *data)
> >
> > return true;
> >
> > }
> >
> > -static void setup(const void *test_data)
> > +static void setup_generic(const void *test_data)
> >
> > {
> >
> > struct test_data *data = tester_get_data();
> > bt_status_t status;
> >
> > @@ -1353,210 +1353,117 @@ static void setup(const void *test_data)
> >
> > return;
> >
> > }
> >
> > - tester_setup_complete();
> > + if (!data->init_routine || data->init_routine() ==
> > BT_STATUS_SUCCESS) + tester_setup_complete();
> > + else
> > + tester_setup_failed();
> >
> > }
> >
> > -static void setup_socket(const void *test_data)
> > +static int init_socket(void)
> >
> > {
> >
> > struct test_data *data = tester_get_data();
> >
> > - bt_status_t status;
> >
> > const void *sock;
> >
> > - if (!setup_base(data)) {
> > - tester_setup_failed();
> > - return;
> > - }
> > -
> > - status = data->if_bluetooth->init(&bt_callbacks);
> > - if (status != BT_STATUS_SUCCESS) {
> > - data->if_bluetooth = NULL;
> > - tester_setup_failed();
> > - return;
> > - }
> > -
> >
> > sock =
> > data->if_bluetooth->get_profile_interface(BT_PROFILE_SOCKETS_ID);
> >
> > - if (!sock) {
> > - tester_setup_failed();
> > - return;
> > - }
> > + if (!sock)
> > + return BT_STATUS_FAIL;
> >
> > data->if_sock = sock;
> >
> > - tester_setup_complete();
> > + return BT_STATUS_SUCCESS;
> >
> > }
> >
> > -static void setup_hidhost(const void *test_data)
> > +static int init_hidhost(void)
> >
> > {
> >
> > struct test_data *data = tester_get_data();
> > bt_status_t status;
> > const void *hid;
> >
> > - if (!setup_base(data)) {
> > - tester_setup_failed();
> > - return;
> > - }
> > -
> > - status = data->if_bluetooth->init(&bt_callbacks);
> > - if (status != BT_STATUS_SUCCESS) {
> > - data->if_bluetooth = NULL;
> > - tester_setup_failed();
> > - return;
> > - }
> > -
> >
> > hid =
> > data->if_bluetooth->get_profile_interface(BT_PROFILE_HIDHOST_ID);
> >
> > - if (!hid) {
> > - tester_setup_failed();
> > - return;
> > - }
> > + if (!hid)
> > + return BT_STATUS_FAIL;
> >
> > data->if_hid = hid;
> >
> > status = data->if_hid->init(&bthh_callbacks);
> >
> > - if (status != BT_STATUS_SUCCESS) {
> > + if (status != BT_STATUS_SUCCESS)
> >
> > data->if_hid = NULL;
> >
> > - tester_setup_failed();
> > - return;
> > - }
> >
> > - tester_setup_complete();
> > + return status;
> >
> > }
> >
> > -static void setup_pan(const void *test_data)
> > +static int init_pan(void)
> >
> > {
> >
> > struct test_data *data = tester_get_data();
> > bt_status_t status;
> > const void *pan;
> >
> > - if (!setup_base(data)) {
> > - tester_setup_failed();
> > - return;
> > - }
> > -
> > - status = data->if_bluetooth->init(&bt_callbacks);
> > - if (status != BT_STATUS_SUCCESS) {
> > - data->if_bluetooth = NULL;
> > - tester_setup_failed();
> > - return;
> > - }
> > -
> >
> > pan =
> > data->if_bluetooth->get_profile_interface(BT_PROFILE_PAN_ID);
> >
> > - if (!pan) {
> > - tester_setup_failed();
> > - return;
> > - }
> > + if (!pan)
> > + return BT_STATUS_FAIL;
> >
> > data->if_pan = pan;
> >
> > status = data->if_pan->init(&btpan_callbacks);
> >
> > - if (status != BT_STATUS_SUCCESS) {
> > + if (status != BT_STATUS_SUCCESS)
> >
> > data->if_pan = NULL;
> >
> > - tester_setup_failed();
> > - return;
> > - }
> >
> > - tester_setup_complete();
> > + return status;
> >
> > }
> >
> > -static void setup_hdp(const void *test_data)
> > +static int init_hdp(void)
> >
> > {
> >
> > struct test_data *data = tester_get_data();
> > bt_status_t status;
> > const void *hdp;
> >
> > - if (!setup_base(data)) {
> > - tester_setup_failed();
> > - return;
> > - }
> > -
> > - status = data->if_bluetooth->init(&bt_callbacks);
> > - if (status != BT_STATUS_SUCCESS) {
> > - data->if_bluetooth = NULL;
> > - tester_setup_failed();
> > - return;
> > - }
> > -
> >
> > hdp =
> > data->if_bluetooth->get_profile_interface(BT_PROFILE_HEALTH_ID);
> >
> > - if (!hdp) {
> > - tester_setup_failed();
> > - return;
> > - }
> > + if (!hdp)
> > + return BT_STATUS_FAIL;
> >
> > data->if_hdp = hdp;
> >
> > status = data->if_hdp->init(&bthl_callbacks);
> >
> > - if (status != BT_STATUS_SUCCESS) {
> > + if (status != BT_STATUS_SUCCESS)
> >
> > data->if_hdp = NULL;
> >
> > - tester_setup_failed();
> > - return;
> > - }
> >
> > - tester_setup_complete();
> > + return status;
> >
> > }
> >
> > -static void setup_a2dp(const void *test_data)
> > +static int init_a2dp(void)
> >
> > {
> >
> > struct test_data *data = tester_get_data();
> > const bt_interface_t *if_bt;
> > bt_status_t status;
> > const void *a2dp;
> >
> > - if (!setup_base(data)) {
> > - tester_setup_failed();
> > - return;
> > - }
> > -
> >
> > if_bt = data->if_bluetooth;
> >
> > - status = if_bt->init(&bt_callbacks);
> > - if (status != BT_STATUS_SUCCESS) {
> > - data->if_bluetooth = NULL;
> > - tester_setup_failed();
> > - return;
> > - }
> > -
> >
> > a2dp = if_bt->get_profile_interface(BT_PROFILE_ADVANCED_AUDIO_ID);
> >
> > - if (!a2dp) {
> > - tester_setup_failed();
> > - return;
> > - }
> > + if (!a2dp)
> > + return BT_STATUS_FAIL;
> >
> > data->if_a2dp = a2dp;
> >
> > status = data->if_a2dp->init(&bta2dp_callbacks);
> >
> > - if (status != BT_STATUS_SUCCESS) {
> > + if (status != BT_STATUS_SUCCESS)
> >
> > data->if_a2dp = NULL;
> >
> > - tester_setup_failed();
> > - return;
> > - }
> >
> > - tester_setup_complete();
> > + return status;
> >
> > }
> >
> > -static void setup_avrcp(const void *test_data)
> > +static int init_avrcp(void)
> >
> > {
> >
> > struct test_data *data = tester_get_data();
> > const bt_interface_t *if_bt;
> > bt_status_t status;
> > const void *a2dp, *avrcp;
> >
> > - if (!setup_base(data)) {
> > - tester_setup_failed();
> > - return;
> > - }
> > -
> >
> > if_bt = data->if_bluetooth;
> >
> > - status = if_bt->init(&bt_callbacks);
> > - if (status != BT_STATUS_SUCCESS) {
> > - data->if_bluetooth = NULL;
> > - tester_setup_failed();
> > - return;
> > - }
> > -
> >
> > a2dp = if_bt->get_profile_interface(BT_PROFILE_ADVANCED_AUDIO_ID);
> > if (!a2dp) {
> >
> > - tester_setup_failed();
> > - return;
> > + return BT_STATUS_FAIL;
> >
> > }
> >
> > data->if_a2dp = a2dp;
> >
> > @@ -1564,63 +1471,39 @@ static void setup_avrcp(const void *test_data)
> >
> > status = data->if_a2dp->init(&bta2dp_callbacks);
> > if (status != BT_STATUS_SUCCESS) {
> >
> > data->if_a2dp = NULL;
> >
> > - tester_setup_failed();
> > - return;
> > + return status;
> >
> > }
> >
> > avrcp = if_bt->get_profile_interface(BT_PROFILE_AV_RC_ID);
> >
> > - if (!a2dp) {
> > - tester_setup_failed();
> > - return;
> > - }
> > + if (!a2dp)
> > + return BT_STATUS_FAIL;
> >
> > data->if_avrcp = avrcp;
> >
> > status = data->if_avrcp->init(&btavrcp_callbacks);
> >
> > - if (status != BT_STATUS_SUCCESS) {
> > + if (status != BT_STATUS_SUCCESS)
> >
> > data->if_avrcp = NULL;
> >
> > - tester_setup_failed();
> > - return;
> > - }
> >
> > - tester_setup_complete();
> > + return status;
> >
> > }
> >
> > -static void setup_gatt(const void *test_data)
> > +static int init_gatt(void)
> >
> > {
> >
> > struct test_data *data = tester_get_data();
> > bt_status_t status;
> > const void *gatt;
> >
> > - if (!setup_base(data)) {
> > - tester_setup_failed();
> > - return;
> > - }
> > -
> > - status = data->if_bluetooth->init(&bt_callbacks);
> > - if (status != BT_STATUS_SUCCESS) {
> > - data->if_bluetooth = NULL;
> > - tester_setup_failed();
> > - return;
> > - }
> > -
> >
> > gatt =
> > data->if_bluetooth->get_profile_interface(BT_PROFILE_GATT_ID);
> >
> > - if (!gatt) {
> > - tester_setup_failed();
> > - return;
> > - }
> > + if (!gatt)
> > + return BT_STATUS_FAIL;
> >
> > data->if_gatt = gatt;
> >
> > status = data->if_gatt->init(&btgatt_callbacks);
> >
> > - if (status != BT_STATUS_SUCCESS) {
> > + if (status != BT_STATUS_SUCCESS)
> >
> > data->if_gatt = NULL;
> >
> > - tester_setup_failed();
> > - return;
> > - }
> > -
> > - tester_setup_complete();
> > + return status;
> >
> > }
> >
> > static void teardown(const void *test_data)
> >
> > @@ -2174,7 +2057,7 @@ static void generic_test_function(const void
> > *test_data)>
> > first_step->action();
> >
> > }
> >
> > -#define test(data, test_setup, test, test_teardown) \
> > +#define test(data, init, test, test_teardown) \
> >
> > do { \
> >
> > struct test_data *user; \
> > user = g_malloc0(sizeof(struct test_data)); \
> >
> > @@ -2182,8 +2065,9 @@ static void generic_test_function(const void
> > *test_data)>
> > break; \
> >
> > user->hciemu_type = data->emu_type; \
> > user->test_data = data; \
> >
> > + user->init_routine = init; \
> >
> > tester_add_full(data->title, data, test_pre_setup, \
> >
> > - test_setup, test, test_teardown, \
> > + setup_generic, test,
> > test_teardown, \>
> > test_post_teardown, 3, user,
> > g_free); \
> >
> > } while (0)
> >
> > @@ -2203,56 +2087,56 @@ static void add_bluetooth_tests(void *data, void
> > *user_data)>
> > {
> >
> > struct test_case *tc = data;
> >
> > - test(tc, setup, generic_test_function, teardown);
> > + test(tc, NULL, generic_test_function, teardown);
> >
> > }
> >
> > static void add_socket_tests(void *data, void *user_data)
> > {
> >
> > struct test_case *tc = data;
> >
> > - test(tc, setup_socket, generic_test_function, teardown);
> > + test(tc, init_socket, generic_test_function, teardown);
> >
> > }
> >
> > static void add_hidhost_tests(void *data, void *user_data)
> > {
> >
> > struct test_case *tc = data;
> >
> > - test(tc, setup_hidhost, generic_test_function, teardown);
> > + test(tc, init_hidhost, generic_test_function, teardown);
> >
> > }
> >
> > static void add_pan_tests(void *data, void *user_data)
> > {
> >
> > struct test_case *tc = data;
> >
> > - test(tc, setup_pan, generic_test_function, teardown);
> > + test(tc, init_pan, generic_test_function, teardown);
> >
> > }
> >
> > static void add_hdp_tests(void *data, void *user_data)
> > {
> >
> > struct test_case *tc = data;
> >
> > - test(tc, setup_hdp, generic_test_function, teardown);
> > + test(tc, init_hdp, generic_test_function, teardown);
> >
> > }
> >
> > static void add_a2dp_tests(void *data, void *user_data)
> > {
> >
> > struct test_case *tc = data;
> >
> > - test(tc, setup_a2dp, generic_test_function, teardown);
> > + test(tc, init_a2dp, generic_test_function, teardown);
> >
> > }
> >
> > static void add_avrcp_tests(void *data, void *user_data)
> > {
> >
> > struct test_case *tc = data;
> >
> > - test(tc, setup_avrcp, generic_test_function, teardown);
> > + test(tc, init_avrcp, generic_test_function, teardown);
> >
> > }
> >
> > static void add_gatt_tests(void *data, void *user_data)
> > {
> >
> > struct test_case *tc = data;
> >
> > - test(tc, setup_gatt, generic_test_function, teardown);
> > + test(tc, init_gatt, generic_test_function, teardown);
> >
> > }
> >
> > int main(int argc, char *argv[])
> >
> > diff --git a/android/tester-main.h b/android/tester-main.h
> > index 6cad803..8247c5a 100644
> > --- a/android/tester-main.h
> > +++ b/android/tester-main.h
> > @@ -314,6 +314,7 @@ struct test_data {
> >
> > struct hw_device_t *device;
> > struct hciemu *hciemu;
> > enum hciemu_type hciemu_type;
> >
> > + int (*init_routine)(void);
> >
> > const bt_interface_t *if_bluetooth;
> > const btsock_interface_t *if_sock;
> >
> > --
> > 1.9.3
>
> I still think the initialization is not a valid end to end test it is
> a unit test since it does not communicate, therefore it does not
> belong here.
I believe this was done for convenience of not having to wrap struct test_data
inside tester_main.c. We already have few other members there which are not
used outside of main.
I don't mind leaving this as is.
--
BR
Szymon Janc
prev parent reply other threads:[~2014-09-09 12:16 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-05 13:00 [PATCHv3 1/2] android/tester: Simplify setup procedure Marcin Kraglak
2014-09-05 13:00 ` [PATCHv3 2/2] android/tester: Init profiles in separate thread Marcin Kraglak
2014-09-09 10:03 ` [PATCHv3 1/2] android/tester: Simplify setup procedure Luiz Augusto von Dentz
2014-09-09 12:16 ` Szymon Janc [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2497968.NSXm3y7leF@leonov \
--to=szymon.janc@tieto.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=luiz.dentz@gmail.com \
--cc=marcin.kraglak@tieto.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).