2014-03-04 20:43:21

by Andrzej Kaczmarek

Subject: [PATCH 1/4] android/avrcp: Fix response for RegisterNotification

---
android/avrcp.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/android/avrcp.c b/android/avrcp.c
index 8eac4cd..f3427a3 100644
--- a/android/avrcp.c
+++ b/android/avrcp.c
@@ -255,6 +255,7 @@ static void handle_register_notification(const void *buf, uint16_t len)
struct avrcp_request *req;
uint8_t pdu[IPC_MTU];
size_t pdu_len;
+ uint8_t code;
int ret;

DBG("");
@@ -280,8 +281,20 @@ static void handle_register_notification(const void *buf, uint16_t len)
goto done;
}

+ switch (cmd->type) {
+ case HAL_AVRCP_EVENT_TYPE_INTERIM:
+ code = AVC_CTYPE_INTERIM;
+ break;
+ case HAL_AVRCP_EVENT_TYPE_CHANGED:
+ code = AVC_CTYPE_CHANGED;
+ break;
+ default:
+ status = HAL_STATUS_FAILED;
+ goto done;
+ }
+
ret = avrcp_register_notification_rsp(req->dev->session,
- req->transaction, cmd->type,
+ req->transaction, code,
pdu, pdu_len);
if (ret < 0) {
status = HAL_STATUS_FAILED;
--
1.9.0

2014-03-05 08:42:21

by Luiz Augusto von Dentz

Subject: Re: [PATCH 1/4] android/avrcp: Fix response for RegisterNotification

Hi Andrzej,

On Tue, Mar 4, 2014 at 10:43 PM, Andrzej Kaczmarek
<[email protected]> wrote:
> ---
> android/avrcp.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/android/avrcp.c b/android/avrcp.c
> index 8eac4cd..f3427a3 100644
> --- a/android/avrcp.c
> +++ b/android/avrcp.c
> @@ -255,6 +255,7 @@ static void handle_register_notification(const void *buf, uint16_t len)
> struct avrcp_request *req;
> uint8_t pdu[IPC_MTU];
> size_t pdu_len;
> + uint8_t code;
> int ret;
>
> DBG("");
> @@ -280,8 +281,20 @@ static void handle_register_notification(const void *buf, uint16_t len)
> goto done;
> }
>
> + switch (cmd->type) {
> + case HAL_AVRCP_EVENT_TYPE_INTERIM:
> + code = AVC_CTYPE_INTERIM;
> + break;
> + case HAL_AVRCP_EVENT_TYPE_CHANGED:
> + code = AVC_CTYPE_CHANGED;
> + break;
> + default:
> + status = HAL_STATUS_FAILED;
> + goto done;
> + }
> +
> ret = avrcp_register_notification_rsp(req->dev->session,
> - req->transaction, cmd->type,
> + req->transaction, code,
> pdu, pdu_len);
> if (ret < 0) {
> status = HAL_STATUS_FAILED;
> --
> 1.9.0

Applied, thanks.


--
Luiz Augusto von Dentz

2014-03-04 20:43:23

by Andrzej Kaczmarek

Subject: [PATCH 3/4] android/avrcp: Fix PDU length calculation

---
android/avrcp.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/android/avrcp.c b/android/avrcp.c
index a763b88..e428486 100644
--- a/android/avrcp.c
+++ b/android/avrcp.c
@@ -167,25 +167,28 @@ static void handle_get_player_values_text(const void *buf, uint16_t len)
HAL_OP_AVRCP_GET_PLAYER_VALUES_TEXT, HAL_STATUS_FAILED);
}

-static void write_element_text(uint8_t id, uint8_t text_len, uint8_t *text,
- uint8_t *pdu, size_t *len)
+static size_t write_element_text(uint8_t id, uint8_t text_len, uint8_t *text,
+ uint8_t *pdu)
{
uint16_t charset = 106;
+ size_t len = 0;

bt_put_be32(id, pdu);
pdu += 4;
- *len += 4;
+ len += 4;

bt_put_be16(charset, pdu);
pdu += 2;
- *len += 2;
+ len += 2;

bt_put_be16(text_len, pdu);
pdu += 2;
- *len += 2;
+ len += 2;

memcpy(pdu, text, text_len);
- *len += text_len;
+ len += text_len;
+
+ return len;
}

static void write_element_attrs(uint8_t *ptr, uint8_t number, uint8_t *pdu,
@@ -199,11 +202,13 @@ static void write_element_attrs(uint8_t *ptr, uint8_t number, uint8_t *pdu,

for (i = 0; i < number; i++) {
struct hal_avrcp_player_setting_text *text = (void *) ptr;
+ size_t ret;

- write_element_text(text->id, text->len, text->text, pdu, len);
+ ret = write_element_text(text->id, text->len, text->text, pdu);

ptr += sizeof(*text) + text->len;
- pdu += *len;
+ pdu += ret;
+ *len += ret;
}
}

--
1.9.0

2014-03-04 20:43:22

by Andrzej Kaczmarek

Subject: [PATCH 2/4] android/avrcp: Fix RegisterNotification response handling

For INTERIM RegisterNotification response we should keep request on
list since it will be needed for CHANGED response.

Requests on lists shall be matched by both pdu_id and event_id since we
can have multiple queued RegisterNotification requests and will need to
reply with proper transaction label.
---
android/avrcp.c | 67 ++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 40 insertions(+), 27 deletions(-)

diff --git a/android/avrcp.c b/android/avrcp.c
index f3427a3..a763b88 100644
--- a/android/avrcp.c
+++ b/android/avrcp.c
@@ -58,6 +58,7 @@ static struct ipc *hal_ipc = NULL;
struct avrcp_request {
struct avrcp_device *dev;
uint8_t pdu_id;
+ uint8_t event_id;
uint8_t transaction;
};

@@ -68,7 +69,8 @@ struct avrcp_device {
GQueue *queue;
};

-static struct avrcp_request *pop_request(uint8_t pdu_id)
+static struct avrcp_request *pop_request(uint8_t pdu_id, uint8_t event_id,
+ bool peek)
{
GSList *l;

@@ -80,10 +82,13 @@ static struct avrcp_request *pop_request(uint8_t pdu_id)
for (i = 0; reqs; reqs = g_list_next(reqs), i++) {
struct avrcp_request *req = reqs->data;

- if (req->pdu_id == pdu_id) {
+ if (req->pdu_id != pdu_id || req->event_id != event_id)
+ continue;
+
+ if (!peek)
g_queue_pop_nth(dev->queue, i);
- return req;
- }
+
+ return req;
}
}

@@ -99,7 +104,7 @@ static void handle_get_play_status(const void *buf, uint16_t len)

DBG("");

- req = pop_request(AVRCP_GET_PLAY_STATUS);
+ req = pop_request(AVRCP_GET_PLAY_STATUS, 0, false);
if (!req) {
status = HAL_STATUS_FAILED;
goto done;
@@ -214,7 +219,7 @@ static void handle_get_element_attrs_text(const void *buf, uint16_t len)

DBG("");

- req = pop_request(AVRCP_GET_ELEMENT_ATTRIBUTES);
+ req = pop_request(AVRCP_GET_ELEMENT_ATTRIBUTES, 0, false);
if (!req) {
status = HAL_STATUS_FAILED;
goto done;
@@ -256,11 +261,25 @@ static void handle_register_notification(const void *buf, uint16_t len)
uint8_t pdu[IPC_MTU];
size_t pdu_len;
uint8_t code;
+ bool peek = false;
int ret;

DBG("");

- req = pop_request(AVRCP_REGISTER_NOTIFICATION);
+ switch (cmd->type) {
+ case HAL_AVRCP_EVENT_TYPE_INTERIM:
+ code = AVC_CTYPE_INTERIM;
+ peek = true;
+ break;
+ case HAL_AVRCP_EVENT_TYPE_CHANGED:
+ code = AVC_CTYPE_CHANGED;
+ break;
+ default:
+ status = HAL_STATUS_FAILED;
+ goto done;
+ }
+
+ req = pop_request(AVRCP_REGISTER_NOTIFICATION, cmd->event, peek);
if (!req) {
status = HAL_STATUS_FAILED;
goto done;
@@ -281,29 +300,19 @@ static void handle_register_notification(const void *buf, uint16_t len)
goto done;
}

- switch (cmd->type) {
- case HAL_AVRCP_EVENT_TYPE_INTERIM:
- code = AVC_CTYPE_INTERIM;
- break;
- case HAL_AVRCP_EVENT_TYPE_CHANGED:
- code = AVC_CTYPE_CHANGED;
- break;
- default:
- status = HAL_STATUS_FAILED;
- goto done;
- }
-
ret = avrcp_register_notification_rsp(req->dev->session,
req->transaction, code,
pdu, pdu_len);
if (ret < 0) {
status = HAL_STATUS_FAILED;
- g_free(req);
+ if (!peek)
+ g_free(req);
goto done;
}

status = HAL_STATUS_SUCCESS;
- g_free(req);
+ if (!peek)
+ g_free(req);

done:
ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_AVRCP,
@@ -552,13 +561,14 @@ static ssize_t handle_get_capabilities_cmd(struct avrcp *session,
}

static void push_request(struct avrcp_device *dev, uint8_t pdu_id,
- uint8_t transaction)
+ uint8_t event_id, uint8_t transaction)
{
struct avrcp_request *req;

req = g_new0(struct avrcp_request, 1);
req->dev = dev;
req->pdu_id = pdu_id;
+ req->event_id = event_id;
req->transaction = transaction;

g_queue_push_tail(dev->queue, req);
@@ -580,7 +590,7 @@ static ssize_t handle_get_play_status_cmd(struct avrcp *session,
ipc_send_notif(hal_ipc, HAL_SERVICE_ID_AVRCP,
HAL_EV_AVRCP_GET_PLAY_STATUS, 0, NULL);

- push_request(dev, AVRCP_GET_PLAY_STATUS, transaction);
+ push_request(dev, AVRCP_GET_PLAY_STATUS, 0, transaction);

return -EAGAIN;
}
@@ -616,7 +626,7 @@ static ssize_t handle_get_element_attrs_cmd(struct avrcp *session,
HAL_EV_AVRCP_GET_ELEMENT_ATTRS,
sizeof(*ev) + ev->number, ev);

- push_request(dev, AVRCP_GET_ELEMENT_ATTRIBUTES, transaction);
+ push_request(dev, AVRCP_GET_ELEMENT_ATTRIBUTES, 0, transaction);

return -EAGAIN;

@@ -630,14 +640,17 @@ static ssize_t handle_register_notification_cmd(struct avrcp *session,
{
struct avrcp_device *dev = user_data;
struct hal_ev_avrcp_register_notification ev;
+ uint8_t event_id;

DBG("");

if (params_len != 5)
return -EINVAL;

+ event_id = params[0];
+
/* TODO: Add any missing events supported by Android */
- switch (params[0]) {
+ switch (event_id) {
case AVRCP_EVENT_STATUS_CHANGED:
case AVRCP_EVENT_TRACK_CHANGED:
case AVRCP_EVENT_PLAYBACK_POS_CHANGED:
@@ -646,14 +659,14 @@ static ssize_t handle_register_notification_cmd(struct avrcp *session,
return -EINVAL;
}

- ev.event = params[0];
+ ev.event = event_id;
ev.param = bt_get_be32(&params[1]);

ipc_send_notif(hal_ipc, HAL_SERVICE_ID_AVRCP,
HAL_EV_AVRCP_REGISTER_NOTIFICATION,
sizeof(ev), &ev);

- push_request(dev, AVRCP_REGISTER_NOTIFICATION, transaction);
+ push_request(dev, AVRCP_REGISTER_NOTIFICATION, event_id, transaction);

return -EAGAIN;
}
--
1.9.0

2014-03-04 20:43:24

by Andrzej Kaczmarek

Subject: [PATCH 4/4] android/hal-avrcp: Remove unused code

---
android/hal-avrcp.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/android/hal-avrcp.c b/android/hal-avrcp.c
index a720a1e..5f98f5b 100644
--- a/android/hal-avrcp.c
+++ b/android/hal-avrcp.c
@@ -351,7 +351,6 @@ static int write_text(uint8_t *ptr, uint8_t id, uint8_t *text, size_t *len)
value->len = strnlen((const char *) text, BTRC_MAX_ATTR_STR_LEN);

*len += attr_len;
- ptr += attr_len;

if (value->len + *len > IPC_MTU)
value->len = IPC_MTU - *len;
--
1.9.0