2014-03-05 14:32:23

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 01/10] unit/avrcp: Refactor check attributes code

From: Andrei Emeltchenko <[email protected]>

Make check_attributes() function which would handle attributes check.
---
unit/test-avrcp.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index 803dd6b..7ce2801 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -288,6 +288,20 @@ static const struct avrcp_passthrough_handler passthrough_handlers[] = {
{ },
};

+static bool check_attributes(const uint8_t *params)
+{
+ int i;
+
+ for (i = 1; i <= params[0]; i++) {
+ DBG("params[%d] = 0x%02x", i, params[i]);
+ if (params[i] > AVRCP_ATTRIBUTE_LAST ||
+ params[i] == AVRCP_ATTRIBUTE_ILEGAL)
+ return false;
+ }
+
+ return true;
+}
+
static ssize_t avrcp_handle_get_capabilities(struct avrcp *session,
uint8_t transaction,
uint16_t params_len,
@@ -326,16 +340,10 @@ static ssize_t avrcp_handle_get_player_attr_text(struct avrcp *session,
uint8_t *params,
void *user_data)
{
- int i;
-
DBG("params[0] %d params_len %d", params[0], params_len);

- for (i = 1; i <= params[0]; i++) {
- DBG("params[%d] = 0x%02x", i, params[i]);
- if (params[i] > AVRCP_ATTRIBUTE_LAST ||
- params[i] == AVRCP_ATTRIBUTE_ILEGAL)
- return -EINVAL;
- }
+ if (!check_attributes(params))
+ return -EINVAL;

params[0] = 0;

--
1.8.3.2



2014-03-06 15:45:26

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 01/10] unit/avrcp: Refactor check attributes code

Hi Andrei,

On Wed, Mar 5, 2014 at 4:32 PM, Andrei Emeltchenko
<[email protected]> wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> Make check_attributes() function which would handle attributes check.
> ---
> unit/test-avrcp.c | 24 ++++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
> index 803dd6b..7ce2801 100644
> --- a/unit/test-avrcp.c
> +++ b/unit/test-avrcp.c
> @@ -288,6 +288,20 @@ static const struct avrcp_passthrough_handler passthrough_handlers[] = {
> { },
> };
>
> +static bool check_attributes(const uint8_t *params)
> +{
> + int i;
> +
> + for (i = 1; i <= params[0]; i++) {
> + DBG("params[%d] = 0x%02x", i, params[i]);
> + if (params[i] > AVRCP_ATTRIBUTE_LAST ||
> + params[i] == AVRCP_ATTRIBUTE_ILEGAL)
> + return false;
> + }
> +
> + return true;
> +}
> +
> static ssize_t avrcp_handle_get_capabilities(struct avrcp *session,
> uint8_t transaction,
> uint16_t params_len,
> @@ -326,16 +340,10 @@ static ssize_t avrcp_handle_get_player_attr_text(struct avrcp *session,
> uint8_t *params,
> void *user_data)
> {
> - int i;
> -
> DBG("params[0] %d params_len %d", params[0], params_len);
>
> - for (i = 1; i <= params[0]; i++) {
> - DBG("params[%d] = 0x%02x", i, params[i]);
> - if (params[i] > AVRCP_ATTRIBUTE_LAST ||
> - params[i] == AVRCP_ATTRIBUTE_ILEGAL)
> - return -EINVAL;
> - }
> + if (!check_attributes(params))
> + return -EINVAL;
>
> params[0] = 0;
>
> --
> 1.8.3.2

Pushed after some code style fixes, please do not mix spaces and tabs.
Also the tests are starting to get logic outside of just testing the
handler but the actual PDU handling so perhaps we need to think if the
API of avrcp-lib.h is really a proper one or we should do create a
different one as it is done for passthrough where the callback are not
actually pure PDU handlers.


--
Luiz Augusto von Dentz

2014-03-05 14:32:29

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 07/10] unit/avrcp: Add /TP/PAS/BI-05-C test

From: Andrei Emeltchenko <[email protected]>

Test verifies that Set player application setting value returns error on
invalid attribute value.
---
unit/test-avrcp.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)

diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index 447f3af..b67b89c 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -431,6 +431,34 @@ static ssize_t avrcp_handle_get_current_player_value(struct avrcp *session,
return params[0] * 2 + 1;
}

+static ssize_t avrcp_handle_set_player_value(struct avrcp *session,
+ uint8_t transaction,
+ uint16_t params_len,
+ uint8_t *params,
+ void *user_data)
+{
+ int i;
+
+ DBG("params[0] %d params_len %d", params[0], params_len);
+
+ if (params_len != params[0] * 2 + 1)
+ return -EINVAL;
+
+ for (i = 0; i < params[0]; i++) {
+ uint8_t attr = params[i * 2 + 1];
+ uint8_t val = params[i * 2 + 2];
+
+ DBG("attr 0x%02x val 0x%02x", attr, val);
+ switch (attr) {
+ case AVRCP_ATTRIBUTE_REPEAT_MODE:
+ if (val < 0x01 || val > 0x05)
+ return -EINVAL;
+ }
+ }
+
+ return 1;
+}
+
static const struct avrcp_control_handler control_handlers[] = {
{ AVRCP_GET_CAPABILITIES,
AVC_CTYPE_STATUS, AVC_CTYPE_STABLE,
@@ -450,6 +478,9 @@ static const struct avrcp_control_handler control_handlers[] = {
{ AVRCP_GET_CURRENT_PLAYER_VALUE,
AVC_CTYPE_STATUS, AVC_CTYPE_STABLE,
avrcp_handle_get_current_player_value },
+ { AVRCP_SET_PLAYER_VALUE,
+ AVC_CTYPE_CONTROL, AVC_CTYPE_STABLE,
+ avrcp_handle_set_player_value },
{ },
};

@@ -728,5 +759,17 @@ int main(int argc, char *argv[])
AVRCP_GET_CURRENT_PLAYER_VALUE,
0x00, 0x00, 0x01, AVRCP_STATUS_INVALID_PARAM));

+ /* Set player application setting value invalid behavior - TG */
+ define_test("/TP/PAS/BI-05-C", test_server,
+ raw_pdu(0x00, 0x11, 0x0e, 0x00, 0x48, 0x00,
+ 0x00, 0x19, 0x58,
+ AVRCP_SET_PLAYER_VALUE,
+ 0x00, 0x00, 0x03, 0x01,
+ AVRCP_ATTRIBUTE_REPEAT_MODE, 0x7f),
+ raw_pdu(0x02, 0x11, 0x0e, AVC_CTYPE_REJECTED,
+ 0x48, 0x00, 0x00, 0x19, 0x58,
+ AVRCP_SET_PLAYER_VALUE,
+ 0x00, 0x00, 0x01, AVRCP_STATUS_INVALID_PARAM));
+
return g_test_run();
}
--
1.8.3.2


2014-03-05 14:32:28

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 06/10] unit/avrcp: Add /TP/PAS/BI-04-C test

From: Andrei Emeltchenko <[email protected]>

Test verifies that Get current player application setting value return
error when given invalid attribute id.
---
unit/test-avrcp.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index 3a26a06..447f3af 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -414,6 +414,9 @@ static ssize_t avrcp_handle_get_current_player_value(struct avrcp *session,

DBG("params[0] %d params_len %d", params[0], params_len);

+ if (!check_attributes(params))
+ return -EINVAL;
+
attributes = g_memdup(&params[1], params[0]);

for (i = 0; i < params[0]; i++) {
@@ -712,5 +715,18 @@ int main(int argc, char *argv[])
AVRCP_GET_PLAYER_VALUE_TEXT,
0x00, 0x00, 0x01, AVRCP_STATUS_INVALID_PARAM));

+ /* Get current player application setting value invalid behavior - TG */
+ define_test("/TP/PAS/BI-04-C", test_server,
+ raw_pdu(0x00, 0x11, 0x0e, 0x01, 0x48, 0x00,
+ 0x00, 0x19, 0x58,
+ AVRCP_GET_CURRENT_PLAYER_VALUE,
+ 0x00, 0x00, 0x02, 0x01,
+ /* Invalid attribute */
+ 0x7f),
+ raw_pdu(0x02, 0x11, 0x0e, AVC_CTYPE_REJECTED,
+ 0x48, 0x00, 0x00, 0x19, 0x58,
+ AVRCP_GET_CURRENT_PLAYER_VALUE,
+ 0x00, 0x00, 0x01, AVRCP_STATUS_INVALID_PARAM));
+
return g_test_run();
}
--
1.8.3.2


2014-03-05 14:32:31

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 09/10] unit/avrcp: Add /TP/MDI/BV-01-C test

From: Andrei Emeltchenko <[email protected]>

Test verifies Get play status command.
---
unit/test-avrcp.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index b67b89c..8682e63 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -530,6 +530,9 @@ static void test_client(gconstpointer data)
NULL, NULL);
}

+ if (g_str_equal(context->data->test_name, "/TP/MDI/BV-01-C"))
+ avrcp_get_play_status(context->session, NULL, NULL);
+
execute_context(context);
}

@@ -771,5 +774,13 @@ int main(int argc, char *argv[])
AVRCP_SET_PLAYER_VALUE,
0x00, 0x00, 0x01, AVRCP_STATUS_INVALID_PARAM));

+ /* Media Information Commands */
+
+ /* Get play status - CT */
+ define_test("/TP/MDI/BV-01-C", test_client,
+ raw_pdu(0x00, 0x11, 0x0e, 0x01, 0x48, 0x00,
+ 0x00, 0x19, 0x58, AVRCP_GET_PLAY_STATUS,
+ 0x00, 0x00, 0x00));
+
return g_test_run();
}
--
1.8.3.2


2014-03-05 14:32:27

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 05/10] unit/avrcp: Add /TP/PAS/BI-03-C test

From: Andrei Emeltchenko <[email protected]>

Test verifies thst Get player application setting value text returns
error for invalid attribute value.
---
unit/test-avrcp.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index f7550f7..3a26a06 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -698,5 +698,19 @@ int main(int argc, char *argv[])
AVRCP_LIST_PLAYER_VALUES,
0x00, 0x00, 0x01, AVRCP_STATUS_INVALID_PARAM));

+ /* Get player application setting value text invalid behavior - TG */
+ define_test("/TP/PAS/BI-03-C", test_server,
+ raw_pdu(0x00, 0x11, 0x0e, 0x01, 0x48, 0x00,
+ 0x00, 0x19, 0x58,
+ AVRCP_GET_PLAYER_VALUE_TEXT,
+ 0x00, 0x00, 0x03, AVRCP_ATTRIBUTE_EQUALIZER,
+ 0x01,
+ /* Invalid setting value */
+ 0x7f),
+ raw_pdu(0x02, 0x11, 0x0e, AVC_CTYPE_REJECTED,
+ 0x48, 0x00, 0x00, 0x19, 0x58,
+ AVRCP_GET_PLAYER_VALUE_TEXT,
+ 0x00, 0x00, 0x01, AVRCP_STATUS_INVALID_PARAM));
+
return g_test_run();
}
--
1.8.3.2


2014-03-05 14:32:24

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 02/10] unit/avrcp: Add attributes check in list_player_vals()

From: Andrei Emeltchenko <[email protected]>

Add check and fix test case since it was testing that response is
received but attribute 0 is illegal one.
---
unit/test-avrcp.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index 7ce2801..c3064da 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -356,11 +356,15 @@ static ssize_t avrcp_handle_list_player_values(struct avrcp *session,
uint8_t *params,
void *user_data)
{
- DBG("params[0] %d params_len %d", params[0], params_len);
+ DBG("params[0] 0x%02x params_len %d", params[0], params_len);

if (params_len != 1)
return -EINVAL;

+ if (params[0] > AVRCP_ATTRIBUTE_LAST ||
+ params[0] == AVRCP_ATTRIBUTE_ILEGAL)
+ return -EINVAL;
+
params[0] = 0;

return 1;
@@ -604,7 +608,7 @@ int main(int argc, char *argv[])
raw_pdu(0x00, 0x11, 0x0e, 0x01, 0x48, 0x00,
0x00, 0x19, 0x58,
AVRCP_LIST_PLAYER_VALUES,
- 0x00, 0x00, 0x01, 0x00),
+ 0x00, 0x00, 0x01, AVRCP_ATTRIBUTE_EQUALIZER),
raw_pdu(0x02, 0x11, 0x0e, 0x0c, 0x48, 0x00,
0x00, 0x19, 0x58,
AVRCP_LIST_PLAYER_VALUES,
--
1.8.3.2


2014-03-05 14:32:26

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 04/10] unit/avrcp: Add attributes and value check

From: Andrei Emeltchenko <[email protected]>

Adds check to get_player_value_text() and update test case.
---
unit/test-avrcp.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index 089ec21..f7550f7 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -376,11 +376,28 @@ static ssize_t avrcp_handle_get_player_value_text(struct avrcp *session,
uint8_t *params,
void *user_data)
{
- DBG("");
+ int i;

- if (params_len != 1)
+ DBG("attr_id %d num_vals %d len %d", params[0], params[1], params_len);
+
+ if (params_len != 2 + params[1])
return -EINVAL;

+ if (params[0] > AVRCP_ATTRIBUTE_LAST ||
+ params[0] == AVRCP_ATTRIBUTE_ILEGAL)
+ return -EINVAL;
+
+ for (i = 2; i < 2 + params[1]; i++) {
+ DBG("Value 0x%02x", params[i]);
+
+ /* Check for invalid value */
+ switch (params[0]) {
+ case AVRCP_ATTRIBUTE_EQUALIZER:
+ if (params[i] < 0x01 || params[i] > 0x02)
+ return -EINVAL;
+ }
+ }
+
params[0] = 0;

return 1;
@@ -618,7 +635,8 @@ int main(int argc, char *argv[])
raw_pdu(0x00, 0x11, 0x0e, 0x01, 0x48, 0x00,
0x00, 0x19, 0x58,
AVRCP_GET_PLAYER_VALUE_TEXT,
- 0x00, 0x00, 0x01, 0x00),
+ 0x00, 0x00, 0x03, AVRCP_ATTRIBUTE_EQUALIZER,
+ 0x01, 0x01),
raw_pdu(0x02, 0x11, 0x0e, 0x0c, 0x48, 0x00,
0x00, 0x19, 0x58,
AVRCP_GET_PLAYER_VALUE_TEXT,
--
1.8.3.2


2014-03-05 14:32:30

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 08/10] android/avrcp: Add avrcp_get_play_status() function

From: Andrei Emeltchenko <[email protected]>

---
android/avrcp-lib.c | 8 ++++++++
android/avrcp-lib.h | 2 ++
2 files changed, 10 insertions(+)

diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index 63a1f88..cd39071 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -395,6 +395,14 @@ int avrcp_set_player_value(struct avrcp *session, uint8_t *attributes,
func, user_data);
}

+int avrcp_get_play_status(struct avrcp *session, avctp_rsp_cb func,
+ void *user_data)
+{
+ return avrcp_send_req(session, AVC_CTYPE_STATUS, AVC_SUBUNIT_PANEL,
+ AVRCP_GET_PLAY_STATUS, NULL, 0, func,
+ user_data);
+}
+
int avrcp_get_play_status_rsp(struct avrcp *session, uint8_t transaction,
uint32_t position, uint32_t duration,
uint8_t status)
diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
index 6bfdff2..ba1d84a 100644
--- a/android/avrcp-lib.h
+++ b/android/avrcp-lib.h
@@ -143,6 +143,8 @@ int avrcp_set_player_value(struct avrcp *session, uint8_t *attributes,
int avrcp_get_current_player_value(struct avrcp *session, uint8_t *attrs,
uint8_t attr_count, avctp_rsp_cb func,
void *user_data);
+int avrcp_get_play_status(struct avrcp *session, avctp_rsp_cb func,
+ void *user_data);

int avrcp_get_play_status_rsp(struct avrcp *session, uint8_t transaction,
uint32_t position, uint32_t duration,
--
1.8.3.2


2014-03-05 14:32:25

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 03/10] unit/avrcp: Add /TP/PAS/BI-02-C test

From: Andrei Emeltchenko <[email protected]>

Test verifies that List player application setting values returns error
when invalid attribute id is provided.
---
unit/test-avrcp.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index c3064da..089ec21 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -667,5 +667,18 @@ int main(int argc, char *argv[])
AVRCP_GET_PLAYER_ATTRIBUTE_TEXT,
0x00, 0x00, 0x01, AVRCP_STATUS_INVALID_PARAM));

+ /* List player application setting values invalid behavior - TG */
+ define_test("/TP/PAS/BI-02-C", test_server,
+ raw_pdu(0x00, 0x11, 0x0e, 0x01, 0x48, 0x00,
+ 0x00, 0x19, 0x58,
+ AVRCP_LIST_PLAYER_VALUES,
+ 0x00, 0x00, 0x01,
+ /* Invalid attribute id */
+ 0x7f),
+ raw_pdu(0x02, 0x11, 0x0e, AVC_CTYPE_REJECTED,
+ 0x48, 0x00, 0x00, 0x19, 0x58,
+ AVRCP_LIST_PLAYER_VALUES,
+ 0x00, 0x00, 0x01, AVRCP_STATUS_INVALID_PARAM));
+
return g_test_run();
}
--
1.8.3.2


2014-03-05 14:32:32

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 10/10] doc: Update test coverage document

From: Andrei Emeltchenko <[email protected]>

---
doc/test-coverage.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/test-coverage.txt b/doc/test-coverage.txt
index 6568286..ab9b1e4 100644
--- a/doc/test-coverage.txt
+++ b/doc/test-coverage.txt
@@ -18,7 +18,7 @@ test-ringbuf 3 Ring buffer functionality
test-queue 1 Queue handling functionality
test-avdtp 60 AVDTP qualification test cases
test-avctp 9 AVCTP qualification test cases
-test-avrcp 24 AVRCP qualification test cases
+test-avrcp 29 AVRCP qualification test cases
test-gobex 31 Generic OBEX functionality
test-gobex-packet 9 OBEX packet handling
test-gobex-header 28 OBEX header handling
--
1.8.3.2