2012-06-18 14:09:22

by Luiz Augusto von Dentz

Subject: [PATCH hcidump 1/2] AVRCP: Add parsing for SetAddressedPlayer PDU

From: Luiz Augusto von Dentz <[email protected]>

---
parser/avrcp.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)

diff --git a/parser/avrcp.c b/parser/avrcp.c
index 850741b..0f2f1e6 100644
--- a/parser/avrcp.c
+++ b/parser/avrcp.c
@@ -1222,6 +1222,38 @@ static void avrcp_set_absolute_volume_dump(int level, struct frame *frm,
printf("Volume: %.2f%% (%d/127)\n", value/1.27, value);
}

+static void avrcp_set_addressed_player(int level, struct frame *frm,
+ uint8_t ctype, uint16_t len)
+{
+ uint16_t id;
+ uint8_t status;
+
+ p_indent(level, frm);
+
+ if (ctype > AVC_CTYPE_GENERAL_INQUIRY)
+ goto response;
+
+ if (len < 2) {
+ printf("PDU Malformed\n");
+ raw_dump(level, frm);
+ return;
+ }
+
+ id = get_u16(frm);
+ printf("PlayerID: 0x%04x", id);
+ return;
+
+response:
+ if (len < 1) {
+ printf("PDU Malformed\n");
+ raw_dump(level, frm);
+ return;
+ }
+
+ status = get_u8(frm);
+ printf("Status: 0x%02x (%s)\n", status, error2str(status));
+}
+
static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
{
uint8_t pduid, pt;
@@ -1291,6 +1323,9 @@ static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
case AVRCP_SET_ABSOLUTE_VOLUME:
avrcp_set_absolute_volume_dump(level + 1, frm, ctype, len);
break;
+ case AVRCP_SET_ADDRESSED_PLAYER:
+ avrcp_set_addressed_player(level + 1, frm, ctype, len);
+ break;
default:
raw_dump(level, frm);
}
--
1.7.10.2

2012-06-19 07:04:21

by Luiz Augusto von Dentz

Subject: Re: [PATCH hcidump 1/2] AVRCP: Add parsing for SetAddressedPlayer PDU

Hi Michal,

On Tue, Jun 19, 2012 at 9:39 AM, <[email protected]> wrote:
> Hi Luiz,
>
> Some cosmetic comments.
>
>> From: Luiz Augusto von Dentz <[email protected]>
>>
>> ---
>> ?parser/avrcp.c | ? 35 +++++++++++++++++++++++++++++++++++
>> ?1 file changed, 35 insertions(+)
>>
>> diff --git a/parser/avrcp.c b/parser/avrcp.c
>> index 850741b..0f2f1e6 100644
>> --- a/parser/avrcp.c
>> +++ b/parser/avrcp.c
>> @@ -1222,6 +1222,38 @@ static void avrcp_set_absolute_volume_dump(int level, struct frame *frm,
>> ? ? ? ? printf("Volume: %.2f%% (%d/127)\n", value/1.27, value);
>> ?}
>>
>> +static void avrcp_set_addressed_player(int level, struct frame *frm,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? uint8_t ctype, uint16_t len)
>> +{
>> + ? ? ? uint16_t id;
>> + ? ? ? uint8_t status;
>> +
>> + ? ? ? p_indent(level, frm);
>> +
>> + ? ? ? if (ctype > AVC_CTYPE_GENERAL_INQUIRY)
>> + ? ? ? ? ? ? ? goto response;
>
> This command type is only AVC_CTYPE_CONTROL with AVC_CTYPE_NOT_IMPLEMENTED, AVC_CTYPE_ACCEPTED, AVC_CTYPE_REJECTED responses, so maybe all other should be tread as malformed.

This is a dumping tool, so the purpose to print as much as possible.

>
>> +
>> + ? ? ? if (len < 2) {
>
> What do you think about replace "<" by "!="? Large packet probably is not correct. But this can be what it is.

Again the purpose is to print as much as possible.

>
>> + ? ? ? ? ? ? ? printf("PDU Malformed\n");
>> + ? ? ? ? ? ? ? raw_dump(level, frm);
>> + ? ? ? ? ? ? ? return;
>> + ? ? ? }
>> +
>> + ? ? ? id = get_u16(frm);
>> + ? ? ? printf("PlayerID: 0x%04x", id);
>> + ? ? ? return;
>
> ID in hex? Decimal seems to be more natural.

This was taken from the spec, but either way is fine.

--
Luiz Augusto von Dentz

2012-06-19 06:52:51

by Michal Labedzki

Subject: RE: [PATCH hcidump 2/2] AVRCP: Add support for Addressed Player Changed event

Hi Luiz,

One important comment and one cosmetic.

> From: Luiz Augusto von Dentz <[email protected]>
>
> ---
> parser/avrcp.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/parser/avrcp.c b/parser/avrcp.c
> index 0f2f1e6..6fa3742 100644
> --- a/parser/avrcp.c
> +++ b/parser/avrcp.c
> @@ -1107,6 +1107,7 @@ static void avrcp_register_notification_dump(int level, struct frame *frm,
> uint8_t ctype, uint16_t len)
> {
> uint8_t event, status;
> + uint16_t uid;
> uint32_t interval;
> uint64_t id;
>
> @@ -1202,6 +1203,15 @@ response:
> status = get_u8(frm) & 0x7F;
> printf("Volume: %.2f%% (%d/127)\n", status/1.27, status);
> break;
> + case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
> + uid = get_u16(frm);
> + printf("PlayerID: 0x%04x", uid);

uid? You can use "id" to replace "uid" in this place.
ID can be decimal.

> +
> + p_indent(level, frm);
> +
> + uid = get_u16(frm);
> + printf("UID: 0x%04x", uid);
> + break;

This is not "UID". This is "UID Counter".

> }
> }
>
> --
> 1.7.10.2

Regards / Pozdrawiam
-------------------------------------------------------------------------------------------------------------
Micha? ?ab?dzki
ASCII: Michal Labedzki
e-mail: [email protected]
office communicator: [email protected]
location: Poland, Wroc?aw, Legnicka 55F
room: 315
phone: +48 717 740 340
---
Tieto Corporation / Tieto Poland
http://www.tieto.com / http://www.tieto.pl
---
Tieto Poland sp??ka z ograniczon? odpowiedzialno?ci? z siedzib? w Szczecinie, ul. Malczewskiego 26. Zarejestrowana w S?dzie Rejonowym Szczecin-Centrum w Szczecinie, XIII Wydzia? Gospodarczy Krajowego Rejestru S?dowego pod numerem 0000124858. NIP: 8542085557. REGON: 812023656. Kapita? zak?adowy: 4 271500 PLN

2012-06-19 06:39:42

by Michal Labedzki

Subject: RE: [PATCH hcidump 1/2] AVRCP: Add parsing for SetAddressedPlayer PDU

Hi Luiz,

Some cosmetic comments.

> From: Luiz Augusto von Dentz <[email protected]>
>
> ---
> parser/avrcp.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/parser/avrcp.c b/parser/avrcp.c
> index 850741b..0f2f1e6 100644
> --- a/parser/avrcp.c
> +++ b/parser/avrcp.c
> @@ -1222,6 +1222,38 @@ static void avrcp_set_absolute_volume_dump(int level, struct frame *frm,
> printf("Volume: %.2f%% (%d/127)\n", value/1.27, value);
> }
>
> +static void avrcp_set_addressed_player(int level, struct frame *frm,
> + uint8_t ctype, uint16_t len)
> +{
> + uint16_t id;
> + uint8_t status;
> +
> + p_indent(level, frm);
> +
> + if (ctype > AVC_CTYPE_GENERAL_INQUIRY)
> + goto response;

This command type is only AVC_CTYPE_CONTROL with AVC_CTYPE_NOT_IMPLEMENTED, AVC_CTYPE_ACCEPTED, AVC_CTYPE_REJECTED responses, so maybe all other should be tread as malformed.

> +
> + if (len < 2) {

What do you think about replace "<" by "!="? Large packet probably is not correct. But this can be what it is.

> + printf("PDU Malformed\n");
> + raw_dump(level, frm);
> + return;
> + }
> +
> + id = get_u16(frm);
> + printf("PlayerID: 0x%04x", id);
> + return;

ID in hex? Decimal seems to be more natural.

> +
> +response:
> + if (len < 1) {

The same. What do you think about replace "<" by "!="?

> + printf("PDU Malformed\n");
> + raw_dump(level, frm);
> + return;
> + }
> +
> + status = get_u8(frm);
> + printf("Status: 0x%02x (%s)\n", status, error2str(status));
> +}
> +
> static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
> {
> uint8_t pduid, pt;
> @@ -1291,6 +1323,9 @@ static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
> case AVRCP_SET_ABSOLUTE_VOLUME:
> avrcp_set_absolute_volume_dump(level + 1, frm, ctype, len);
> break;
> + case AVRCP_SET_ADDRESSED_PLAYER:
> + avrcp_set_addressed_player(level + 1, frm, ctype, len);
> + break;
> default:
> raw_dump(level, frm);
> }
> --
> 1.7.10.2
>



Regards / Pozdrawiam
-------------------------------------------------------------------------------------------------------------
Micha? ?ab?dzki
ASCII: Michal Labedzki
e-mail: [email protected]
office communicator: [email protected]
location: Poland, Wroc?aw, Legnicka 55F
room: 315
phone: +48 717 740 340
---
Tieto Corporation / Tieto Poland
http://www.tieto.com / http://www.tieto.pl
---
Tieto Poland sp??ka z ograniczon? odpowiedzialno?ci? z siedzib? w Szczecinie, ul. Malczewskiego 26. Zarejestrowana w S?dzie Rejonowym Szczecin-Centrum w Szczecinie, XIII Wydzia? Gospodarczy Krajowego Rejestru S?dowego pod numerem 0000124858. NIP: 8542085557. REGON: 812023656. Kapita? zak?adowy: 4 271500 PLN

2012-06-18 14:09:23

by Luiz Augusto von Dentz

Subject: [PATCH hcidump 2/2] AVRCP: Add support for Addressed Player Changed event

From: Luiz Augusto von Dentz <[email protected]>

---
parser/avrcp.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/parser/avrcp.c b/parser/avrcp.c
index 0f2f1e6..6fa3742 100644
--- a/parser/avrcp.c
+++ b/parser/avrcp.c
@@ -1107,6 +1107,7 @@ static void avrcp_register_notification_dump(int level, struct frame *frm,
uint8_t ctype, uint16_t len)
{
uint8_t event, status;
+ uint16_t uid;
uint32_t interval;
uint64_t id;

@@ -1202,6 +1203,15 @@ response:
status = get_u8(frm) & 0x7F;
printf("Volume: %.2f%% (%d/127)\n", status/1.27, status);
break;
+ case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
+ uid = get_u16(frm);
+ printf("PlayerID: 0x%04x", uid);
+
+ p_indent(level, frm);
+
+ uid = get_u16(frm);
+ printf("UID: 0x%04x", uid);
+ break;
}
}

--
1.7.10.2