2012-03-19 17:41:31

by Luiz Augusto von Dentz

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

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

---
parser/avrcp.c | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/parser/avrcp.c b/parser/avrcp.c
index 643e494..d746937 100644
--- a/parser/avrcp.c
+++ b/parser/avrcp.c
@@ -1201,6 +1201,23 @@ response:
}
}

+static void avrcp_set_absolut_volume_dump(int level, struct frame *frm,
+ uint8_t ctype, uint16_t len)
+{
+ uint8_t value;
+
+ p_indent(level, frm);
+
+ if (len < 1) {
+ printf("PDU Malformed\n");
+ raw_dump(level, frm);
+ return;
+ }
+
+ value = get_u8(frm);
+ printf("Value: 0x%02x\n", value);
+}
+
static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
{
uint8_t pduid, pt;
@@ -1267,6 +1284,9 @@ static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
case AVRCP_REGISTER_NOTIFICATION:
avrcp_register_notification_dump(level + 1, frm, ctype, len);
break;
+ case AVRCP_SET_ABSOLUTE_VOLUME:
+ avrcp_set_absolut_volume_dump(level + 1, frm, ctype, len);
+ break;
default:
raw_dump(level, frm);
}
--
1.7.7.6

2012-03-22 01:22:21

by Lucas De Marchi

Subject: Re: [PATCH hcidump 2/2] AVRCP: Add parsing support for Volume Change notification

On Mon, Mar 19, 2012 at 2:41 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> ---
> ?parser/avrcp.c | ? ?4 ++++
> ?1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/parser/avrcp.c b/parser/avrcp.c
> index d746937..9698934 100644
> --- a/parser/avrcp.c
> +++ b/parser/avrcp.c
> @@ -1198,6 +1198,10 @@ response:
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?value2str(attr, value));
> ? ? ? ? ? ? ? ?}
> ? ? ? ? ? ? ? ?break;
> + ? ? ? case AVRCP_EVENT_VOLUME_CHANGED:
> + ? ? ? ? ? ? ? status = get_u8(frm);
> + ? ? ? ? ? ? ? printf("Volume: 0x%02x\n", status);
> + ? ? ? ? ? ? ? break;

Ack.


Lucas De Marchi

2012-03-20 11:22:08

by Luiz Augusto von Dentz

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

Hi Michal,

On Tue, Mar 20, 2012 at 6:15 AM, Michal Labedzki
<[email protected]> wrote:
> Hi Luiz,
>
> On Mon, Mar 19, 2012 at 07:41:31PM +0200, Luiz Augusto von Dentz wrote:
>> From: Luiz Augusto von Dentz <[email protected]>
>>
>> ---
>> ?parser/avrcp.c | ? 20 ++++++++++++++++++++
>> ?1 files changed, 20 insertions(+), 0 deletions(-)
>>
>> diff --git a/parser/avrcp.c b/parser/avrcp.c
>> index 643e494..d746937 100644
>> --- a/parser/avrcp.c
>> +++ b/parser/avrcp.c
>> @@ -1201,6 +1201,23 @@ response:
>> ? ? ? }
>> ?}
>>
>> +static void avrcp_set_absolut_volume_dump(int level, struct frame *frm,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? uint8_t ctype, uint16_t len)
>
> Typo? "absolut" --> "absolute"

Yep, gonna fix that.

>> +{
>> + ? ? uint8_t value;
>> +
>> + ? ? p_indent(level, frm);
>> +
>> + ? ? if (len < 1) {
>> + ? ? ? ? ? ? printf("PDU Malformed\n");
>> + ? ? ? ? ? ? raw_dump(level, frm);
>> + ? ? ? ? ? ? return;
>> + ? ? }
>> +
>> + ? ? value = get_u8(frm);
>
> "value" is only lower seven bits. You should use mask 0x80.
> The same for Patch 2/2.

Actually the mask is 0x7F, although as hcidump is not actually
implementing this but dumping letting it print the full value would
catch bugs when reserved bit is used.

>> + ? ? printf("Value: 0x%02x\n", value);
>
> "Value"? "Volume" seems to be better, and why hex value? I propose
> decimal and percent value, for example: "Volume: 38% (48/127)"

Sounds good


--
Luiz Augusto von Dentz

2012-03-20 09:15:02

by Michal Labedzki

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

Hi Luiz,

On Mon, Mar 19, 2012 at 07:41:31PM +0200, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> ---
> parser/avrcp.c | 20 ++++++++++++++++++++
> 1 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/parser/avrcp.c b/parser/avrcp.c
> index 643e494..d746937 100644
> --- a/parser/avrcp.c
> +++ b/parser/avrcp.c
> @@ -1201,6 +1201,23 @@ response:
> }
> }
>
> +static void avrcp_set_absolut_volume_dump(int level, struct frame *frm,
> + uint8_t ctype, uint16_t len)

Typo? "absolut" --> "absolute"

> +{
> + uint8_t value;
> +
> + p_indent(level, frm);
> +
> + if (len < 1) {
> + printf("PDU Malformed\n");
> + raw_dump(level, frm);
> + return;
> + }
> +
> + value = get_u8(frm);

"value" is only lower seven bits. You should use mask 0x80.
The same for Patch 2/2.

> + printf("Value: 0x%02x\n", value);

"Value"? "Volume" seems to be better, and why hex value? I propose
decimal and percent value, for example: "Volume: 38% (48/127)"


> +}
> +
> static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
> {
> uint8_t pduid, pt;
> @@ -1267,6 +1284,9 @@ static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
> case AVRCP_REGISTER_NOTIFICATION:
> avrcp_register_notification_dump(level + 1, frm, ctype, len);
> break;
> + case AVRCP_SET_ABSOLUTE_VOLUME:
> + avrcp_set_absolut_volume_dump(level + 1, frm, ctype, len);
> + break;
> default:
> raw_dump(level, frm);
> }
> --
> 1.7.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Regards
Michal Labedzki

2012-03-19 17:41:32

by Luiz Augusto von Dentz

Subject: [PATCH hcidump 2/2] AVRCP: Add parsing support for Volume Change notification

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

---
parser/avrcp.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/parser/avrcp.c b/parser/avrcp.c
index d746937..9698934 100644
--- a/parser/avrcp.c
+++ b/parser/avrcp.c
@@ -1198,6 +1198,10 @@ response:
value2str(attr, value));
}
break;
+ case AVRCP_EVENT_VOLUME_CHANGED:
+ status = get_u8(frm);
+ printf("Volume: 0x%02x\n", status);
+ break;
}
}

--
1.7.7.6