All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] unit/avrcp: Add /TP/CFG/BV-01-C test
@ 2014-02-25 13:56 Andrei Emeltchenko
  2014-02-25 13:56 ` [PATCH 2/3] android/avrcp: Add control handlers to avrcp-lib Andrei Emeltchenko
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Andrei Emeltchenko @ 2014-02-25 13:56 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Test verifies that the get capabilities command issued from the
Controller. For that we add the command to avrcp-lib and it will be used
in AVRCP later.
---
 android/avrcp-lib.c | 40 ++++++++++++++++++++++++++++++++++++++++
 android/avrcp-lib.h |  2 ++
 android/avrcp.c     |  1 +
 unit/test-avrcp.c   | 17 +++++++++++++++++
 4 files changed, 60 insertions(+)

diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index c280cf8..136801e 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -50,6 +50,12 @@
 #define AVRCP_STATUS_NO_AVAILABLE_PLAYERS	0x15
 #define AVRCP_STATUS_ADDRESSED_PLAYER_CHANGED	0x16
 
+/* Packet types */
+#define AVRCP_PACKET_TYPE_SINGLE	0x00
+#define AVRCP_PACKET_TYPE_START		0x01
+#define AVRCP_PACKET_TYPE_CONTINUING	0x02
+#define AVRCP_PACKET_TYPE_END		0x03
+
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 
 struct avrcp_header {
@@ -202,6 +208,18 @@ static bool handle_passthrough_pdu(struct avctp *conn, uint8_t op,
 	return handler->func(session);
 }
 
+/*
+ * set_company_id:
+ *
+ * Set three-byte Company_ID into outgoing AVRCP message
+ */
+static void set_company_id(uint8_t cid[3], const uint32_t cid_in)
+{
+	cid[0] = cid_in >> 16;
+	cid[1] = cid_in >> 8;
+	cid[2] = cid_in;
+}
+
 struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 {
 	struct avrcp *session;
@@ -253,3 +271,25 @@ int avrcp_init_uinput(struct avrcp *session, const char *name,
 {
 	return avctp_init_uinput(session->conn, name, address);
 }
+
+#define AVRCP_GET_CAPABILITIES_PARAM_LENGTH 1
+void avrcp_get_capabilities(struct avrcp *session, avctp_rsp_cb func)
+{
+	uint8_t buf[AVRCP_HEADER_LENGTH + AVRCP_GET_CAPABILITIES_PARAM_LENGTH];
+	struct avrcp_header *pdu = (void *) buf;
+	uint8_t length;
+
+	memset(buf, 0, sizeof(buf));
+
+	set_company_id(pdu->company_id, IEEEID_BTSIG);
+	pdu->pdu_id = AVRCP_GET_CAPABILITIES;
+	pdu->packet_type = AVRCP_PACKET_TYPE_SINGLE;
+	pdu->params[0] = CAP_EVENTS_SUPPORTED;
+	pdu->params_len = htons(AVRCP_GET_CAPABILITIES_PARAM_LENGTH);
+
+	length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
+
+	avctp_send_vendordep_req(session->conn, AVC_CTYPE_STATUS,
+					AVC_SUBUNIT_PANEL, buf, length,
+					func, session);
+}
diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
index 2337429..4f3a632 100644
--- a/android/avrcp-lib.h
+++ b/android/avrcp-lib.h
@@ -74,3 +74,5 @@ void avrcp_set_passthrough_handlers(struct avrcp *session,
 			void *user_data);
 int avrcp_init_uinput(struct avrcp *session, const char *name,
 							const char *address);
+
+void avrcp_get_capabilities(struct avrcp *session, avctp_rsp_cb func);
diff --git a/android/avrcp.c b/android/avrcp.c
index 48444a4..3d39d91 100644
--- a/android/avrcp.c
+++ b/android/avrcp.c
@@ -36,6 +36,7 @@
 #include "bluetooth.h"
 #include "hal-msg.h"
 #include "ipc.h"
+#include "avctp.h"
 #include "avrcp-lib.h"
 #include "avrcp.h"
 
diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index 53e9237..4a92860 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -297,6 +297,16 @@ static void test_server(gconstpointer data)
 	execute_context(context);
 }
 
+static void test_client(gconstpointer data)
+{
+	struct context *context = create_context(0x0100, data);
+
+	if (g_str_equal(context->data->test_name, "/TP/CFG/BV-01-C"))
+		avrcp_get_capabilities(context->session, NULL);
+
+	execute_context(context);
+}
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
@@ -364,5 +374,12 @@ int main(int argc, char *argv[])
 			raw_pdu(0x02, 0x11, 0x0e, 0x09, 0x48, 0x7c,
 				AVC_PLAY | 0x80, 0x00));
 
+	/* Metadata transfer tests */
+
+	define_test("/TP/CFG/BV-01-C", test_client,
+			raw_pdu(0x00, 0x11, 0x0e, 0x01, 0x48, 0x00,
+				0x00, 0x19, 0x58, 0x10, 0x00, 0x00,
+				0x01, 0x03));
+
 	return g_test_run();
 }
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH 2/3] android/avrcp: Add control handlers to avrcp-lib
  2014-02-25 13:56 [PATCH 1/3] unit/avrcp: Add /TP/CFG/BV-01-C test Andrei Emeltchenko
@ 2014-02-25 13:56 ` Andrei Emeltchenko
  2014-02-25 14:19   ` Luiz Augusto von Dentz
  2014-02-25 13:56 ` [PATCH 3/3] unit/avrcp: Add /TP/CFG/BV-02-C test Andrei Emeltchenko
  2014-02-26 11:59 ` [PATCH 1/3] unit/avrcp: Add /TP/CFG/BV-01-C test Luiz Augusto von Dentz
  2 siblings, 1 reply; 11+ messages in thread
From: Andrei Emeltchenko @ 2014-02-25 13:56 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/avrcp-lib.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 android/avrcp-lib.h | 12 +++++++++++
 2 files changed, 72 insertions(+)

diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index 136801e..95e10f2 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -56,6 +56,15 @@
 #define AVRCP_PACKET_TYPE_CONTINUING	0x02
 #define AVRCP_PACKET_TYPE_END		0x03
 
+/* Capabilities for AVRCP_GET_CAPABILITIES pdu */
+#define CAP_COMPANY_ID		0x02
+#define CAP_EVENTS_SUPPORTED	0x03
+
+/* Company IDs supported by this device */
+static uint32_t company_ids[] = {
+	IEEEID_BTSIG,
+};
+
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 
 struct avrcp_header {
@@ -108,6 +117,8 @@ struct avrcp {
 	const struct avrcp_passthrough_handler *passthrough_handlers;
 	void *passthrough_data;
 	unsigned int passthrough_id;
+
+	uint16_t supported_events;
 };
 
 void avrcp_shutdown(struct avrcp *session)
@@ -220,6 +231,53 @@ static void set_company_id(uint8_t cid[3], const uint32_t cid_in)
 	cid[2] = cid_in;
 }
 
+static uint8_t avrcp_handle_get_capabilities(struct avrcp *session,
+				uint8_t transaction, uint16_t *params_len,
+				uint8_t *params, void *user_data)
+{
+	unsigned int i;
+
+	DBG("id %d params_len %d", params[0], *params_len);
+
+	if (*params_len != 1)
+		goto fail;
+
+	switch (params[0]) {
+	case CAP_COMPANY_ID:
+		for (i = 0; i < G_N_ELEMENTS(company_ids); i++)
+			set_company_id(&params[2 + i * 3], company_ids[i]);
+
+		*params_len = 2 + (3 * G_N_ELEMENTS(company_ids));
+		params[1] = G_N_ELEMENTS(company_ids);
+
+		return AVC_CTYPE_STABLE;
+	case CAP_EVENTS_SUPPORTED:
+		params[1] = 0;
+		for (i = 1; i <= AVRCP_EVENT_LAST; i++) {
+			if (session->supported_events & (1 << i)) {
+				params[1]++;
+				params[params[1] + 1] = i;
+			}
+		}
+
+		*params_len = 2 + params[1];
+
+		return AVC_CTYPE_STABLE;
+	}
+
+fail:
+	*params_len = htons(1);
+	params[0] = AVRCP_STATUS_INVALID_PARAM;
+
+	return AVC_CTYPE_REJECTED;
+}
+
+static const struct avrcp_control_handler control_handlers[] = {
+		{ AVRCP_GET_CAPABILITIES, AVC_CTYPE_STATUS,
+					avrcp_handle_get_capabilities },
+		{ },
+};
+
 struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 {
 	struct avrcp *session;
@@ -241,6 +299,8 @@ struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 							handle_vendordep_pdu,
 							session);
 
+	avrcp_set_control_handlers(session, control_handlers, NULL);
+
 	return session;
 }
 
diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
index 4f3a632..0821287 100644
--- a/android/avrcp-lib.h
+++ b/android/avrcp-lib.h
@@ -46,6 +46,18 @@
 #define AVRCP_ADD_TO_NOW_PLAYING	0x90
 #define AVRCP_GENERAL_REJECT		0xA0
 
+/* Notification events */
+#define AVRCP_EVENT_STATUS_CHANGED		0x01
+#define AVRCP_EVENT_TRACK_CHANGED		0x02
+#define AVRCP_EVENT_TRACK_REACHED_END		0x03
+#define AVRCP_EVENT_TRACK_REACHED_START		0x04
+#define AVRCP_EVENT_SETTINGS_CHANGED		0x08
+#define AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED	0x0a
+#define AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED	0x0b
+#define AVRCP_EVENT_UIDS_CHANGED		0x0c
+#define AVRCP_EVENT_VOLUME_CHANGED		0x0d
+#define AVRCP_EVENT_LAST			AVRCP_EVENT_VOLUME_CHANGED
+
 struct avrcp;
 
 struct avrcp_control_handler {
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH 3/3] unit/avrcp: Add /TP/CFG/BV-02-C test
  2014-02-25 13:56 [PATCH 1/3] unit/avrcp: Add /TP/CFG/BV-01-C test Andrei Emeltchenko
  2014-02-25 13:56 ` [PATCH 2/3] android/avrcp: Add control handlers to avrcp-lib Andrei Emeltchenko
@ 2014-02-25 13:56 ` Andrei Emeltchenko
  2014-02-26 11:59 ` [PATCH 1/3] unit/avrcp: Add /TP/CFG/BV-01-C test Luiz Augusto von Dentz
  2 siblings, 0 replies; 11+ messages in thread
From: Andrei Emeltchenko @ 2014-02-25 13:56 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Test verifies that the Target responds to Get Capability request.
---
 unit/test-avrcp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index 4a92860..71a4d0e 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -381,5 +381,13 @@ int main(int argc, char *argv[])
 				0x00, 0x19, 0x58, 0x10, 0x00, 0x00,
 				0x01, 0x03));
 
+	define_test("/TP/CFG/BV-02-C", test_server,
+			raw_pdu(0x00, 0x11, 0x0e, 0x01, 0x48, 0x00,
+				0x00, 0x19, 0x58, 0x10, 0x00, 0x00,
+				0x01, 0x02),
+			raw_pdu(0x02, 0x11, 0x0e, 0x0c, 0x48, 0x00,
+				0x00, 0x19, 0x58, 0x10, 0x00, 0x00,
+				0x01, 0x02, 0x01, 0x00, 0x19, 0x58));
+
 	return g_test_run();
 }
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] android/avrcp: Add control handlers to avrcp-lib
  2014-02-25 13:56 ` [PATCH 2/3] android/avrcp: Add control handlers to avrcp-lib Andrei Emeltchenko
@ 2014-02-25 14:19   ` Luiz Augusto von Dentz
  2014-02-26  7:56     ` Andrei Emeltchenko
  0 siblings, 1 reply; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2014-02-25 14:19 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth

Hi Andrei,

On Tue, Feb 25, 2014 at 3:56 PM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> ---
>  android/avrcp-lib.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  android/avrcp-lib.h | 12 +++++++++++
>  2 files changed, 72 insertions(+)
>
> diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
> index 136801e..95e10f2 100644
> --- a/android/avrcp-lib.c
> +++ b/android/avrcp-lib.c
> @@ -56,6 +56,15 @@
>  #define AVRCP_PACKET_TYPE_CONTINUING   0x02
>  #define AVRCP_PACKET_TYPE_END          0x03
>
> +/* Capabilities for AVRCP_GET_CAPABILITIES pdu */
> +#define CAP_COMPANY_ID         0x02
> +#define CAP_EVENTS_SUPPORTED   0x03
> +
> +/* Company IDs supported by this device */
> +static uint32_t company_ids[] = {
> +       IEEEID_BTSIG,
> +};
> +
>  #if __BYTE_ORDER == __LITTLE_ENDIAN
>
>  struct avrcp_header {
> @@ -108,6 +117,8 @@ struct avrcp {
>         const struct avrcp_passthrough_handler *passthrough_handlers;
>         void *passthrough_data;
>         unsigned int passthrough_id;
> +
> +       uint16_t supported_events;
>  };
>
>  void avrcp_shutdown(struct avrcp *session)
> @@ -220,6 +231,53 @@ static void set_company_id(uint8_t cid[3], const uint32_t cid_in)
>         cid[2] = cid_in;
>  }
>
> +static uint8_t avrcp_handle_get_capabilities(struct avrcp *session,
> +                               uint8_t transaction, uint16_t *params_len,
> +                               uint8_t *params, void *user_data)
> +{
> +       unsigned int i;
> +
> +       DBG("id %d params_len %d", params[0], *params_len);
> +
> +       if (*params_len != 1)
> +               goto fail;
> +
> +       switch (params[0]) {
> +       case CAP_COMPANY_ID:
> +               for (i = 0; i < G_N_ELEMENTS(company_ids); i++)
> +                       set_company_id(&params[2 + i * 3], company_ids[i]);
> +
> +               *params_len = 2 + (3 * G_N_ELEMENTS(company_ids));
> +               params[1] = G_N_ELEMENTS(company_ids);
> +
> +               return AVC_CTYPE_STABLE;
> +       case CAP_EVENTS_SUPPORTED:
> +               params[1] = 0;
> +               for (i = 1; i <= AVRCP_EVENT_LAST; i++) {
> +                       if (session->supported_events & (1 << i)) {
> +                               params[1]++;
> +                               params[params[1] + 1] = i;
> +                       }
> +               }
> +
> +               *params_len = 2 + params[1];
> +
> +               return AVC_CTYPE_STABLE;
> +       }
> +
> +fail:
> +       *params_len = htons(1);
> +       params[0] = AVRCP_STATUS_INVALID_PARAM;
> +
> +       return AVC_CTYPE_REJECTED;
> +}
> +
> +static const struct avrcp_control_handler control_handlers[] = {
> +               { AVRCP_GET_CAPABILITIES, AVC_CTYPE_STATUS,
> +                                       avrcp_handle_get_capabilities },
> +               { },
> +};
> +
>  struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
>  {
>         struct avrcp *session;
> @@ -241,6 +299,8 @@ struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
>                                                         handle_vendordep_pdu,
>                                                         session);
>
> +       avrcp_set_control_handlers(session, control_handlers, NULL);
> +
>         return session;
>  }
>
> diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
> index 4f3a632..0821287 100644
> --- a/android/avrcp-lib.h
> +++ b/android/avrcp-lib.h
> @@ -46,6 +46,18 @@
>  #define AVRCP_ADD_TO_NOW_PLAYING       0x90
>  #define AVRCP_GENERAL_REJECT           0xA0
>
> +/* Notification events */
> +#define AVRCP_EVENT_STATUS_CHANGED             0x01
> +#define AVRCP_EVENT_TRACK_CHANGED              0x02
> +#define AVRCP_EVENT_TRACK_REACHED_END          0x03
> +#define AVRCP_EVENT_TRACK_REACHED_START                0x04
> +#define AVRCP_EVENT_SETTINGS_CHANGED           0x08
> +#define AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED  0x0a
> +#define AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED   0x0b
> +#define AVRCP_EVENT_UIDS_CHANGED               0x0c
> +#define AVRCP_EVENT_VOLUME_CHANGED             0x0d
> +#define AVRCP_EVENT_LAST                       AVRCP_EVENT_VOLUME_CHANGED
> +
>  struct avrcp;
>
>  struct avrcp_control_handler {
> --
> 1.8.3.2

That is the actual AVRCP implementation not the library, the library
only offer means to handle the commands but don't parse it there since
we can't do anything with it. For unit tests you can implement dummy
handlers as we did for passthrough.

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] android/avrcp: Add control handlers to avrcp-lib
  2014-02-25 14:19   ` Luiz Augusto von Dentz
@ 2014-02-26  7:56     ` Andrei Emeltchenko
  2014-02-26  8:16       ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 11+ messages in thread
From: Andrei Emeltchenko @ 2014-02-26  7:56 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

On Tue, Feb 25, 2014 at 04:19:53PM +0200, Luiz Augusto von Dentz wrote:
> Hi Andrei,
> 
> On Tue, Feb 25, 2014 at 3:56 PM, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >
> > ---
> >  android/avrcp-lib.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  android/avrcp-lib.h | 12 +++++++++++
> >  2 files changed, 72 insertions(+)
> >
> > diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
> > index 136801e..95e10f2 100644
> > --- a/android/avrcp-lib.c
> > +++ b/android/avrcp-lib.c
> > @@ -56,6 +56,15 @@
> >  #define AVRCP_PACKET_TYPE_CONTINUING   0x02
> >  #define AVRCP_PACKET_TYPE_END          0x03
> >
> > +/* Capabilities for AVRCP_GET_CAPABILITIES pdu */
> > +#define CAP_COMPANY_ID         0x02
> > +#define CAP_EVENTS_SUPPORTED   0x03
> > +
> > +/* Company IDs supported by this device */
> > +static uint32_t company_ids[] = {
> > +       IEEEID_BTSIG,
> > +};
> > +
> >  #if __BYTE_ORDER == __LITTLE_ENDIAN
> >
> >  struct avrcp_header {
> > @@ -108,6 +117,8 @@ struct avrcp {
> >         const struct avrcp_passthrough_handler *passthrough_handlers;
> >         void *passthrough_data;
> >         unsigned int passthrough_id;
> > +
> > +       uint16_t supported_events;
> >  };
> >
> >  void avrcp_shutdown(struct avrcp *session)
> > @@ -220,6 +231,53 @@ static void set_company_id(uint8_t cid[3], const uint32_t cid_in)
> >         cid[2] = cid_in;
> >  }
> >
> > +static uint8_t avrcp_handle_get_capabilities(struct avrcp *session,
> > +                               uint8_t transaction, uint16_t *params_len,
> > +                               uint8_t *params, void *user_data)
> > +{
> > +       unsigned int i;
> > +
> > +       DBG("id %d params_len %d", params[0], *params_len);
> > +
> > +       if (*params_len != 1)
> > +               goto fail;
> > +
> > +       switch (params[0]) {
> > +       case CAP_COMPANY_ID:
> > +               for (i = 0; i < G_N_ELEMENTS(company_ids); i++)
> > +                       set_company_id(&params[2 + i * 3], company_ids[i]);
> > +
> > +               *params_len = 2 + (3 * G_N_ELEMENTS(company_ids));
> > +               params[1] = G_N_ELEMENTS(company_ids);
> > +
> > +               return AVC_CTYPE_STABLE;
> > +       case CAP_EVENTS_SUPPORTED:
> > +               params[1] = 0;
> > +               for (i = 1; i <= AVRCP_EVENT_LAST; i++) {
> > +                       if (session->supported_events & (1 << i)) {
> > +                               params[1]++;
> > +                               params[params[1] + 1] = i;
> > +                       }
> > +               }
> > +
> > +               *params_len = 2 + params[1];
> > +
> > +               return AVC_CTYPE_STABLE;
> > +       }
> > +
> > +fail:
> > +       *params_len = htons(1);
> > +       params[0] = AVRCP_STATUS_INVALID_PARAM;
> > +
> > +       return AVC_CTYPE_REJECTED;
> > +}
> > +
> > +static const struct avrcp_control_handler control_handlers[] = {
> > +               { AVRCP_GET_CAPABILITIES, AVC_CTYPE_STATUS,
> > +                                       avrcp_handle_get_capabilities },
> > +               { },
> > +};
> > +
> >  struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
> >  {
> >         struct avrcp *session;
> > @@ -241,6 +299,8 @@ struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
> >                                                         handle_vendordep_pdu,
> >                                                         session);
> >
> > +       avrcp_set_control_handlers(session, control_handlers, NULL);
> > +
> >         return session;
> >  }
> >
> > diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
> > index 4f3a632..0821287 100644
> > --- a/android/avrcp-lib.h
> > +++ b/android/avrcp-lib.h
> > @@ -46,6 +46,18 @@
> >  #define AVRCP_ADD_TO_NOW_PLAYING       0x90
> >  #define AVRCP_GENERAL_REJECT           0xA0
> >
> > +/* Notification events */
> > +#define AVRCP_EVENT_STATUS_CHANGED             0x01
> > +#define AVRCP_EVENT_TRACK_CHANGED              0x02
> > +#define AVRCP_EVENT_TRACK_REACHED_END          0x03
> > +#define AVRCP_EVENT_TRACK_REACHED_START                0x04
> > +#define AVRCP_EVENT_SETTINGS_CHANGED           0x08
> > +#define AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED  0x0a
> > +#define AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED   0x0b
> > +#define AVRCP_EVENT_UIDS_CHANGED               0x0c
> > +#define AVRCP_EVENT_VOLUME_CHANGED             0x0d
> > +#define AVRCP_EVENT_LAST                       AVRCP_EVENT_VOLUME_CHANGED
> > +
> >  struct avrcp;
> >
> >  struct avrcp_control_handler {
> > --
> > 1.8.3.2
> 
> That is the actual AVRCP implementation not the library, the library
> only offer means to handle the commands but don't parse it there since
> we can't do anything with it. For unit tests you can implement dummy
> handlers as we did for passthrough.

I can implement dummy handlers but this would mean that we are testing
that dummy handlers. Is the idea to test actual production code?

Best regards 
Andrei Emeltchenko 

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] android/avrcp: Add control handlers to avrcp-lib
  2014-02-26  7:56     ` Andrei Emeltchenko
@ 2014-02-26  8:16       ` Luiz Augusto von Dentz
  2014-02-26 10:28         ` Andrei Emeltchenko
  0 siblings, 1 reply; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2014-02-26  8:16 UTC (permalink / raw)
  To: Andrei Emeltchenko, Luiz Augusto von Dentz, linux-bluetooth

Hi Andrei,

On Wed, Feb 26, 2014 at 9:56 AM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> Hi Luiz,
>
> On Tue, Feb 25, 2014 at 04:19:53PM +0200, Luiz Augusto von Dentz wrote:
>> Hi Andrei,
>>
>> On Tue, Feb 25, 2014 at 3:56 PM, Andrei Emeltchenko
>> <Andrei.Emeltchenko.news@gmail.com> wrote:
>> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>> >
>> > ---
>> >  android/avrcp-lib.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >  android/avrcp-lib.h | 12 +++++++++++
>> >  2 files changed, 72 insertions(+)
>> >
>> > diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
>> > index 136801e..95e10f2 100644
>> > --- a/android/avrcp-lib.c
>> > +++ b/android/avrcp-lib.c
>> > @@ -56,6 +56,15 @@
>> >  #define AVRCP_PACKET_TYPE_CONTINUING   0x02
>> >  #define AVRCP_PACKET_TYPE_END          0x03
>> >
>> > +/* Capabilities for AVRCP_GET_CAPABILITIES pdu */
>> > +#define CAP_COMPANY_ID         0x02
>> > +#define CAP_EVENTS_SUPPORTED   0x03
>> > +
>> > +/* Company IDs supported by this device */
>> > +static uint32_t company_ids[] = {
>> > +       IEEEID_BTSIG,
>> > +};
>> > +
>> >  #if __BYTE_ORDER == __LITTLE_ENDIAN
>> >
>> >  struct avrcp_header {
>> > @@ -108,6 +117,8 @@ struct avrcp {
>> >         const struct avrcp_passthrough_handler *passthrough_handlers;
>> >         void *passthrough_data;
>> >         unsigned int passthrough_id;
>> > +
>> > +       uint16_t supported_events;
>> >  };
>> >
>> >  void avrcp_shutdown(struct avrcp *session)
>> > @@ -220,6 +231,53 @@ static void set_company_id(uint8_t cid[3], const uint32_t cid_in)
>> >         cid[2] = cid_in;
>> >  }
>> >
>> > +static uint8_t avrcp_handle_get_capabilities(struct avrcp *session,
>> > +                               uint8_t transaction, uint16_t *params_len,
>> > +                               uint8_t *params, void *user_data)
>> > +{
>> > +       unsigned int i;
>> > +
>> > +       DBG("id %d params_len %d", params[0], *params_len);
>> > +
>> > +       if (*params_len != 1)
>> > +               goto fail;
>> > +
>> > +       switch (params[0]) {
>> > +       case CAP_COMPANY_ID:
>> > +               for (i = 0; i < G_N_ELEMENTS(company_ids); i++)
>> > +                       set_company_id(&params[2 + i * 3], company_ids[i]);
>> > +
>> > +               *params_len = 2 + (3 * G_N_ELEMENTS(company_ids));
>> > +               params[1] = G_N_ELEMENTS(company_ids);
>> > +
>> > +               return AVC_CTYPE_STABLE;
>> > +       case CAP_EVENTS_SUPPORTED:
>> > +               params[1] = 0;
>> > +               for (i = 1; i <= AVRCP_EVENT_LAST; i++) {
>> > +                       if (session->supported_events & (1 << i)) {
>> > +                               params[1]++;
>> > +                               params[params[1] + 1] = i;
>> > +                       }
>> > +               }
>> > +
>> > +               *params_len = 2 + params[1];
>> > +
>> > +               return AVC_CTYPE_STABLE;
>> > +       }
>> > +
>> > +fail:
>> > +       *params_len = htons(1);
>> > +       params[0] = AVRCP_STATUS_INVALID_PARAM;
>> > +
>> > +       return AVC_CTYPE_REJECTED;
>> > +}
>> > +
>> > +static const struct avrcp_control_handler control_handlers[] = {
>> > +               { AVRCP_GET_CAPABILITIES, AVC_CTYPE_STATUS,
>> > +                                       avrcp_handle_get_capabilities },
>> > +               { },
>> > +};
>> > +
>> >  struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
>> >  {
>> >         struct avrcp *session;
>> > @@ -241,6 +299,8 @@ struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
>> >                                                         handle_vendordep_pdu,
>> >                                                         session);
>> >
>> > +       avrcp_set_control_handlers(session, control_handlers, NULL);
>> > +
>> >         return session;
>> >  }
>> >
>> > diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
>> > index 4f3a632..0821287 100644
>> > --- a/android/avrcp-lib.h
>> > +++ b/android/avrcp-lib.h
>> > @@ -46,6 +46,18 @@
>> >  #define AVRCP_ADD_TO_NOW_PLAYING       0x90
>> >  #define AVRCP_GENERAL_REJECT           0xA0
>> >
>> > +/* Notification events */
>> > +#define AVRCP_EVENT_STATUS_CHANGED             0x01
>> > +#define AVRCP_EVENT_TRACK_CHANGED              0x02
>> > +#define AVRCP_EVENT_TRACK_REACHED_END          0x03
>> > +#define AVRCP_EVENT_TRACK_REACHED_START                0x04
>> > +#define AVRCP_EVENT_SETTINGS_CHANGED           0x08
>> > +#define AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED  0x0a
>> > +#define AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED   0x0b
>> > +#define AVRCP_EVENT_UIDS_CHANGED               0x0c
>> > +#define AVRCP_EVENT_VOLUME_CHANGED             0x0d
>> > +#define AVRCP_EVENT_LAST                       AVRCP_EVENT_VOLUME_CHANGED
>> > +
>> >  struct avrcp;
>> >
>> >  struct avrcp_control_handler {
>> > --
>> > 1.8.3.2
>>
>> That is the actual AVRCP implementation not the library, the library
>> only offer means to handle the commands but don't parse it there since
>> we can't do anything with it. For unit tests you can implement dummy
>> handlers as we did for passthrough.
>
> I can implement dummy handlers but this would mean that we are testing
> that dummy handlers. Is the idea to test actual production code?

Which is fine, the capabilities will anyway depend on the underline
player implementation so a dummy one is fine here, in fact the unit
tests should not care what is the capabilities just that the PDU is
formed correctly.


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] android/avrcp: Add control handlers to avrcp-lib
  2014-02-26  8:16       ` Luiz Augusto von Dentz
@ 2014-02-26 10:28         ` Andrei Emeltchenko
  0 siblings, 0 replies; 11+ messages in thread
From: Andrei Emeltchenko @ 2014-02-26 10:28 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

On Wed, Feb 26, 2014 at 10:16:15AM +0200, Luiz Augusto von Dentz wrote:
> Hi Andrei,
> 
> On Wed, Feb 26, 2014 at 9:56 AM, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > Hi Luiz,
> >
> > On Tue, Feb 25, 2014 at 04:19:53PM +0200, Luiz Augusto von Dentz wrote:
> >> Hi Andrei,
> >>
> >> On Tue, Feb 25, 2014 at 3:56 PM, Andrei Emeltchenko
> >> <Andrei.Emeltchenko.news@gmail.com> wrote:
> >> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >> >
> >> > ---
> >> >  android/avrcp-lib.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> >  android/avrcp-lib.h | 12 +++++++++++
> >> >  2 files changed, 72 insertions(+)
> >> >
> >> > diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
> >> > index 136801e..95e10f2 100644
> >> > --- a/android/avrcp-lib.c
> >> > +++ b/android/avrcp-lib.c
> >> > @@ -56,6 +56,15 @@
> >> >  #define AVRCP_PACKET_TYPE_CONTINUING   0x02
> >> >  #define AVRCP_PACKET_TYPE_END          0x03
> >> >
> >> > +/* Capabilities for AVRCP_GET_CAPABILITIES pdu */
> >> > +#define CAP_COMPANY_ID         0x02
> >> > +#define CAP_EVENTS_SUPPORTED   0x03
> >> > +
> >> > +/* Company IDs supported by this device */
> >> > +static uint32_t company_ids[] = {
> >> > +       IEEEID_BTSIG,
> >> > +};
> >> > +
> >> >  #if __BYTE_ORDER == __LITTLE_ENDIAN
> >> >
> >> >  struct avrcp_header {
> >> > @@ -108,6 +117,8 @@ struct avrcp {
> >> >         const struct avrcp_passthrough_handler *passthrough_handlers;
> >> >         void *passthrough_data;
> >> >         unsigned int passthrough_id;
> >> > +
> >> > +       uint16_t supported_events;
> >> >  };
> >> >
> >> >  void avrcp_shutdown(struct avrcp *session)
> >> > @@ -220,6 +231,53 @@ static void set_company_id(uint8_t cid[3], const uint32_t cid_in)
> >> >         cid[2] = cid_in;
> >> >  }
> >> >
> >> > +static uint8_t avrcp_handle_get_capabilities(struct avrcp *session,
> >> > +                               uint8_t transaction, uint16_t *params_len,
> >> > +                               uint8_t *params, void *user_data)
> >> > +{
> >> > +       unsigned int i;
> >> > +
> >> > +       DBG("id %d params_len %d", params[0], *params_len);
> >> > +
> >> > +       if (*params_len != 1)
> >> > +               goto fail;
> >> > +
> >> > +       switch (params[0]) {
> >> > +       case CAP_COMPANY_ID:
> >> > +               for (i = 0; i < G_N_ELEMENTS(company_ids); i++)
> >> > +                       set_company_id(&params[2 + i * 3], company_ids[i]);
> >> > +
> >> > +               *params_len = 2 + (3 * G_N_ELEMENTS(company_ids));
> >> > +               params[1] = G_N_ELEMENTS(company_ids);
> >> > +
> >> > +               return AVC_CTYPE_STABLE;
> >> > +       case CAP_EVENTS_SUPPORTED:
> >> > +               params[1] = 0;
> >> > +               for (i = 1; i <= AVRCP_EVENT_LAST; i++) {
> >> > +                       if (session->supported_events & (1 << i)) {
> >> > +                               params[1]++;
> >> > +                               params[params[1] + 1] = i;
> >> > +                       }
> >> > +               }
> >> > +
> >> > +               *params_len = 2 + params[1];
> >> > +
> >> > +               return AVC_CTYPE_STABLE;
> >> > +       }
> >> > +
> >> > +fail:
> >> > +       *params_len = htons(1);
> >> > +       params[0] = AVRCP_STATUS_INVALID_PARAM;
> >> > +
> >> > +       return AVC_CTYPE_REJECTED;
> >> > +}
> >> > +
> >> > +static const struct avrcp_control_handler control_handlers[] = {
> >> > +               { AVRCP_GET_CAPABILITIES, AVC_CTYPE_STATUS,
> >> > +                                       avrcp_handle_get_capabilities },
> >> > +               { },
> >> > +};
> >> > +
> >> >  struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
> >> >  {
> >> >         struct avrcp *session;
> >> > @@ -241,6 +299,8 @@ struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
> >> >                                                         handle_vendordep_pdu,
> >> >                                                         session);
> >> >
> >> > +       avrcp_set_control_handlers(session, control_handlers, NULL);
> >> > +
> >> >         return session;
> >> >  }
> >> >
> >> > diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
> >> > index 4f3a632..0821287 100644
> >> > --- a/android/avrcp-lib.h
> >> > +++ b/android/avrcp-lib.h
> >> > @@ -46,6 +46,18 @@
> >> >  #define AVRCP_ADD_TO_NOW_PLAYING       0x90
> >> >  #define AVRCP_GENERAL_REJECT           0xA0
> >> >
> >> > +/* Notification events */
> >> > +#define AVRCP_EVENT_STATUS_CHANGED             0x01
> >> > +#define AVRCP_EVENT_TRACK_CHANGED              0x02
> >> > +#define AVRCP_EVENT_TRACK_REACHED_END          0x03
> >> > +#define AVRCP_EVENT_TRACK_REACHED_START                0x04
> >> > +#define AVRCP_EVENT_SETTINGS_CHANGED           0x08
> >> > +#define AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED  0x0a
> >> > +#define AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED   0x0b
> >> > +#define AVRCP_EVENT_UIDS_CHANGED               0x0c
> >> > +#define AVRCP_EVENT_VOLUME_CHANGED             0x0d
> >> > +#define AVRCP_EVENT_LAST                       AVRCP_EVENT_VOLUME_CHANGED
> >> > +
> >> >  struct avrcp;
> >> >
> >> >  struct avrcp_control_handler {
> >> > --
> >> > 1.8.3.2
> >>
> >> That is the actual AVRCP implementation not the library, the library
> >> only offer means to handle the commands but don't parse it there since
> >> we can't do anything with it. For unit tests you can implement dummy
> >> handlers as we did for passthrough.
> >
> > I can implement dummy handlers but this would mean that we are testing
> > that dummy handlers. Is the idea to test actual production code?
> 
> Which is fine, the capabilities will anyway depend on the underline
> player implementation so a dummy one is fine here, in fact the unit
> tests should not care what is the capabilities just that the PDU is
> formed correctly.

OK, what about tests which sends this request, test TP/CFG/BV-01-C.

Best regards 
Andrei Emeltchenko 

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] unit/avrcp: Add /TP/CFG/BV-01-C test
  2014-02-25 13:56 [PATCH 1/3] unit/avrcp: Add /TP/CFG/BV-01-C test Andrei Emeltchenko
  2014-02-25 13:56 ` [PATCH 2/3] android/avrcp: Add control handlers to avrcp-lib Andrei Emeltchenko
  2014-02-25 13:56 ` [PATCH 3/3] unit/avrcp: Add /TP/CFG/BV-02-C test Andrei Emeltchenko
@ 2014-02-26 11:59 ` Luiz Augusto von Dentz
  2014-02-26 12:22   ` Andrei Emeltchenko
  2 siblings, 1 reply; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2014-02-26 11:59 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth

Hi Andrei,

On Tue, Feb 25, 2014 at 3:56 PM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Test verifies that the get capabilities command issued from the
> Controller. For that we add the command to avrcp-lib and it will be used
> in AVRCP later.
> ---
>  android/avrcp-lib.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  android/avrcp-lib.h |  2 ++
>  android/avrcp.c     |  1 +
>  unit/test-avrcp.c   | 17 +++++++++++++++++
>  4 files changed, 60 insertions(+)
>
> diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
> index c280cf8..136801e 100644
> --- a/android/avrcp-lib.c
> +++ b/android/avrcp-lib.c
> @@ -50,6 +50,12 @@
>  #define AVRCP_STATUS_NO_AVAILABLE_PLAYERS      0x15
>  #define AVRCP_STATUS_ADDRESSED_PLAYER_CHANGED  0x16
>
> +/* Packet types */
> +#define AVRCP_PACKET_TYPE_SINGLE       0x00
> +#define AVRCP_PACKET_TYPE_START                0x01
> +#define AVRCP_PACKET_TYPE_CONTINUING   0x02
> +#define AVRCP_PACKET_TYPE_END          0x03
> +
>  #if __BYTE_ORDER == __LITTLE_ENDIAN
>
>  struct avrcp_header {
> @@ -202,6 +208,18 @@ static bool handle_passthrough_pdu(struct avctp *conn, uint8_t op,
>         return handler->func(session);
>  }
>
> +/*
> + * set_company_id:
> + *
> + * Set three-byte Company_ID into outgoing AVRCP message
> + */
> +static void set_company_id(uint8_t cid[3], const uint32_t cid_in)
> +{
> +       cid[0] = cid_in >> 16;
> +       cid[1] = cid_in >> 8;
> +       cid[2] = cid_in;
> +}

Check out what I did with ntoh24, you need to check the byte order as
well and create hton24.

>  struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
>  {
>         struct avrcp *session;
> @@ -253,3 +271,25 @@ int avrcp_init_uinput(struct avrcp *session, const char *name,
>  {
>         return avctp_init_uinput(session->conn, name, address);
>  }
> +
> +#define AVRCP_GET_CAPABILITIES_PARAM_LENGTH 1
> +void avrcp_get_capabilities(struct avrcp *session, avctp_rsp_cb func)
> +{
> +       uint8_t buf[AVRCP_HEADER_LENGTH + AVRCP_GET_CAPABILITIES_PARAM_LENGTH];
> +       struct avrcp_header *pdu = (void *) buf;
> +       uint8_t length;
> +
> +       memset(buf, 0, sizeof(buf));
> +
> +       set_company_id(pdu->company_id, IEEEID_BTSIG);
> +       pdu->pdu_id = AVRCP_GET_CAPABILITIES;
> +       pdu->packet_type = AVRCP_PACKET_TYPE_SINGLE;
> +       pdu->params[0] = CAP_EVENTS_SUPPORTED;
> +       pdu->params_len = htons(AVRCP_GET_CAPABILITIES_PARAM_LENGTH);
> +
> +       length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
> +
> +       avctp_send_vendordep_req(session->conn, AVC_CTYPE_STATUS,
> +                                       AVC_SUBUNIT_PANEL, buf, length,
> +                                       func, session);
> +}

I would create a helper function called avrc_send_req which takes the
callback + params and params_len, also Im not sure if it is a good
idea to reuse avctp_rsp_cb or have a different callback so we can
treat AVCTP internally.

> diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
> index 2337429..4f3a632 100644
> --- a/android/avrcp-lib.h
> +++ b/android/avrcp-lib.h
> @@ -74,3 +74,5 @@ void avrcp_set_passthrough_handlers(struct avrcp *session,
>                         void *user_data);
>  int avrcp_init_uinput(struct avrcp *session, const char *name,
>                                                         const char *address);
> +
> +void avrcp_get_capabilities(struct avrcp *session, avctp_rsp_cb func);
> diff --git a/android/avrcp.c b/android/avrcp.c
> index 48444a4..3d39d91 100644
> --- a/android/avrcp.c
> +++ b/android/avrcp.c
> @@ -36,6 +36,7 @@
>  #include "bluetooth.h"
>  #include "hal-msg.h"
>  #include "ipc.h"
> +#include "avctp.h"
>  #include "avrcp-lib.h"
>  #include "avrcp.h"
>
> diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
> index 53e9237..4a92860 100644
> --- a/unit/test-avrcp.c
> +++ b/unit/test-avrcp.c
> @@ -297,6 +297,16 @@ static void test_server(gconstpointer data)
>         execute_context(context);
>  }
>
> +static void test_client(gconstpointer data)
> +{
> +       struct context *context = create_context(0x0100, data);
> +
> +       if (g_str_equal(context->data->test_name, "/TP/CFG/BV-01-C"))
> +               avrcp_get_capabilities(context->session, NULL);
> +
> +       execute_context(context);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>         g_test_init(&argc, &argv, NULL);
> @@ -364,5 +374,12 @@ int main(int argc, char *argv[])
>                         raw_pdu(0x02, 0x11, 0x0e, 0x09, 0x48, 0x7c,
>                                 AVC_PLAY | 0x80, 0x00));
>
> +       /* Metadata transfer tests */
> +
> +       define_test("/TP/CFG/BV-01-C", test_client,
> +                       raw_pdu(0x00, 0x11, 0x0e, 0x01, 0x48, 0x00,
> +                               0x00, 0x19, 0x58, 0x10, 0x00, 0x00,
> +                               0x01, 0x03));
> +
>         return g_test_run();
>  }
> --
> 1.8.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] unit/avrcp: Add /TP/CFG/BV-01-C test
  2014-02-26 11:59 ` [PATCH 1/3] unit/avrcp: Add /TP/CFG/BV-01-C test Luiz Augusto von Dentz
@ 2014-02-26 12:22   ` Andrei Emeltchenko
  0 siblings, 0 replies; 11+ messages in thread
From: Andrei Emeltchenko @ 2014-02-26 12:22 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

On Wed, Feb 26, 2014 at 01:59:30PM +0200, Luiz Augusto von Dentz wrote:
> Hi Andrei,
> 
> On Tue, Feb 25, 2014 at 3:56 PM, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >
> > Test verifies that the get capabilities command issued from the
> > Controller. For that we add the command to avrcp-lib and it will be used
> > in AVRCP later.
> > ---
> >  android/avrcp-lib.c | 40 ++++++++++++++++++++++++++++++++++++++++
> >  android/avrcp-lib.h |  2 ++
> >  android/avrcp.c     |  1 +
> >  unit/test-avrcp.c   | 17 +++++++++++++++++
> >  4 files changed, 60 insertions(+)
> >
> > diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
> > index c280cf8..136801e 100644
> > --- a/android/avrcp-lib.c
> > +++ b/android/avrcp-lib.c
> > @@ -50,6 +50,12 @@
> >  #define AVRCP_STATUS_NO_AVAILABLE_PLAYERS      0x15
> >  #define AVRCP_STATUS_ADDRESSED_PLAYER_CHANGED  0x16
> >
> > +/* Packet types */
> > +#define AVRCP_PACKET_TYPE_SINGLE       0x00
> > +#define AVRCP_PACKET_TYPE_START                0x01
> > +#define AVRCP_PACKET_TYPE_CONTINUING   0x02
> > +#define AVRCP_PACKET_TYPE_END          0x03
> > +
> >  #if __BYTE_ORDER == __LITTLE_ENDIAN
> >
> >  struct avrcp_header {
> > @@ -202,6 +208,18 @@ static bool handle_passthrough_pdu(struct avctp *conn, uint8_t op,
> >         return handler->func(session);
> >  }
> >
> > +/*
> > + * set_company_id:
> > + *
> > + * Set three-byte Company_ID into outgoing AVRCP message
> > + */
> > +static void set_company_id(uint8_t cid[3], const uint32_t cid_in)
> > +{
> > +       cid[0] = cid_in >> 16;
> > +       cid[1] = cid_in >> 8;
> > +       cid[2] = cid_in;
> > +}
> 
> Check out what I did with ntoh24, you need to check the byte order as
> well and create hton24.
> 
> >  struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
> >  {
> >         struct avrcp *session;
> > @@ -253,3 +271,25 @@ int avrcp_init_uinput(struct avrcp *session, const char *name,
> >  {
> >         return avctp_init_uinput(session->conn, name, address);
> >  }
> > +
> > +#define AVRCP_GET_CAPABILITIES_PARAM_LENGTH 1
> > +void avrcp_get_capabilities(struct avrcp *session, avctp_rsp_cb func)
> > +{
> > +       uint8_t buf[AVRCP_HEADER_LENGTH + AVRCP_GET_CAPABILITIES_PARAM_LENGTH];
> > +       struct avrcp_header *pdu = (void *) buf;
> > +       uint8_t length;
> > +
> > +       memset(buf, 0, sizeof(buf));
> > +
> > +       set_company_id(pdu->company_id, IEEEID_BTSIG);
> > +       pdu->pdu_id = AVRCP_GET_CAPABILITIES;
> > +       pdu->packet_type = AVRCP_PACKET_TYPE_SINGLE;
> > +       pdu->params[0] = CAP_EVENTS_SUPPORTED;
> > +       pdu->params_len = htons(AVRCP_GET_CAPABILITIES_PARAM_LENGTH);
> > +
> > +       length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
> > +
> > +       avctp_send_vendordep_req(session->conn, AVC_CTYPE_STATUS,
> > +                                       AVC_SUBUNIT_PANEL, buf, length,
> > +                                       func, session);
> > +}
> 
> I would create a helper function called avrc_send_req which takes the
> callback + params and params_len, also Im not sure if it is a good
> idea to reuse avctp_rsp_cb or have a different callback so we can
> treat AVCTP internally.

So avrcp_send_req does not really help here since it would be just wrapper
for avctp_send functions. Maybe then this unit test is not needed at all?

Best regards 
Andrei Emeltchenko 


^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] unit/avrcp: Add /TP/CFG/BV-01-C test
  2014-02-26 15:14 Andrei Emeltchenko
@ 2014-02-26 17:30 ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2014-02-26 17:30 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth

Hi Andrei,

On Wed, Feb 26, 2014 at 4:14 PM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Test verifies that the get capabilities command issued from the
> Controller.
> ---
>  unit/test-avrcp.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
> index 53e9237..4a92860 100644
> --- a/unit/test-avrcp.c
> +++ b/unit/test-avrcp.c
> @@ -297,6 +297,16 @@ static void test_server(gconstpointer data)
>         execute_context(context);
>  }
>
> +static void test_client(gconstpointer data)
> +{
> +       struct context *context = create_context(0x0100, data);
> +
> +       if (g_str_equal(context->data->test_name, "/TP/CFG/BV-01-C"))
> +               avrcp_get_capabilities(context->session, NULL);
> +
> +       execute_context(context);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>         g_test_init(&argc, &argv, NULL);
> @@ -364,5 +374,12 @@ int main(int argc, char *argv[])
>                         raw_pdu(0x02, 0x11, 0x0e, 0x09, 0x48, 0x7c,
>                                 AVC_PLAY | 0x80, 0x00));
>
> +       /* Metadata transfer tests */
> +
> +       define_test("/TP/CFG/BV-01-C", test_client,
> +                       raw_pdu(0x00, 0x11, 0x0e, 0x01, 0x48, 0x00,
> +                               0x00, 0x19, 0x58, 0x10, 0x00, 0x00,
> +                               0x01, 0x03));
> +
>         return g_test_run();
>  }
> --
> 1.8.3.2

Applied after doing a lot of changes, please don't copy code from
audio the library will most likely work differently as we do have
different APIs, also add code that you are actually testing for
example you were only testing part of the GetCapabilities handler and
doing a lot more work than necessary.


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [PATCH 1/3] unit/avrcp: Add /TP/CFG/BV-01-C test
@ 2014-02-26 15:14 Andrei Emeltchenko
  2014-02-26 17:30 ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 11+ messages in thread
From: Andrei Emeltchenko @ 2014-02-26 15:14 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Test verifies that the get capabilities command issued from the
Controller.
---
 unit/test-avrcp.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
index 53e9237..4a92860 100644
--- a/unit/test-avrcp.c
+++ b/unit/test-avrcp.c
@@ -297,6 +297,16 @@ static void test_server(gconstpointer data)
 	execute_context(context);
 }
 
+static void test_client(gconstpointer data)
+{
+	struct context *context = create_context(0x0100, data);
+
+	if (g_str_equal(context->data->test_name, "/TP/CFG/BV-01-C"))
+		avrcp_get_capabilities(context->session, NULL);
+
+	execute_context(context);
+}
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
@@ -364,5 +374,12 @@ int main(int argc, char *argv[])
 			raw_pdu(0x02, 0x11, 0x0e, 0x09, 0x48, 0x7c,
 				AVC_PLAY | 0x80, 0x00));
 
+	/* Metadata transfer tests */
+
+	define_test("/TP/CFG/BV-01-C", test_client,
+			raw_pdu(0x00, 0x11, 0x0e, 0x01, 0x48, 0x00,
+				0x00, 0x19, 0x58, 0x10, 0x00, 0x00,
+				0x01, 0x03));
+
 	return g_test_run();
 }
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-02-26 17:30 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-25 13:56 [PATCH 1/3] unit/avrcp: Add /TP/CFG/BV-01-C test Andrei Emeltchenko
2014-02-25 13:56 ` [PATCH 2/3] android/avrcp: Add control handlers to avrcp-lib Andrei Emeltchenko
2014-02-25 14:19   ` Luiz Augusto von Dentz
2014-02-26  7:56     ` Andrei Emeltchenko
2014-02-26  8:16       ` Luiz Augusto von Dentz
2014-02-26 10:28         ` Andrei Emeltchenko
2014-02-25 13:56 ` [PATCH 3/3] unit/avrcp: Add /TP/CFG/BV-02-C test Andrei Emeltchenko
2014-02-26 11:59 ` [PATCH 1/3] unit/avrcp: Add /TP/CFG/BV-01-C test Luiz Augusto von Dentz
2014-02-26 12:22   ` Andrei Emeltchenko
2014-02-26 15:14 Andrei Emeltchenko
2014-02-26 17:30 ` Luiz Augusto von Dentz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.