2011-09-08 23:32:50

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH hcidump] avrcp: fix name of metadata field

Metadata field number 0x7 should be the track duration and not the
progress of the track playback. Thus rename it to a better description.
---
parser/avrcp.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/parser/avrcp.c b/parser/avrcp.c
index f8e4443..0b98a3e 100644
--- a/parser/avrcp.c
+++ b/parser/avrcp.c
@@ -166,7 +166,7 @@
#define AVRCP_MEDIA_ATTRIBUTE_TRACK 0x4
#define AVRCP_MEDIA_ATTRIBUTE_TOTAL 0x5
#define AVRCP_MEDIA_ATTRIBUTE_GENRE 0x6
-#define AVRCP_MEDIA_ATTRIBUTE_PROGRESS 0x7
+#define AVRCP_MEDIA_ATTRIBUTE_DURATION 0x7

/* play status */
#define AVRCP_PLAY_STATUS_STOPPED 0x00
@@ -909,8 +909,8 @@ static const char *mediattr2str(uint32_t attr)
return "Track Total";
case AVRCP_MEDIA_ATTRIBUTE_GENRE:
return "Genre";
- case AVRCP_MEDIA_ATTRIBUTE_PROGRESS:
- return "Progress";
+ case AVRCP_MEDIA_ATTRIBUTE_DURATION:
+ return "Track duration";
default:
return "Reserved";
}
--
1.7.6.1



2011-09-27 09:57:53

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH hcidump] avrcp: fix name of metadata field

Hi Lucas,

On Thu, Sep 08, 2011, Lucas De Marchi wrote:
> Metadata field number 0x7 should be the track duration and not the
> progress of the track playback. Thus rename it to a better description.
> ---
> parser/avrcp.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)

Applied. Thanks.

Johan

2011-09-12 15:05:01

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH hcidump] avrcp: fix name of metadata field

Hi Lucas,

On Fri, Sep 9, 2011 at 2:32 AM, Lucas De Marchi
<[email protected]> wrote:
> Metadata field number 0x7 should be the track duration and not the
> progress of the track playback. Thus rename it to a better description.
> ---
> ?parser/avrcp.c | ? ?6 +++---
> ?1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/parser/avrcp.c b/parser/avrcp.c
> index f8e4443..0b98a3e 100644
> --- a/parser/avrcp.c
> +++ b/parser/avrcp.c
> @@ -166,7 +166,7 @@
> ?#define AVRCP_MEDIA_ATTRIBUTE_TRACK ? ?0x4
> ?#define AVRCP_MEDIA_ATTRIBUTE_TOTAL ? ?0x5
> ?#define AVRCP_MEDIA_ATTRIBUTE_GENRE ? ?0x6
> -#define AVRCP_MEDIA_ATTRIBUTE_PROGRESS 0x7
> +#define AVRCP_MEDIA_ATTRIBUTE_DURATION 0x7
>
> ?/* play status */
> ?#define AVRCP_PLAY_STATUS_STOPPED ? ? ?0x00
> @@ -909,8 +909,8 @@ static const char *mediattr2str(uint32_t attr)
> ? ? ? ? ? ? ? ?return "Track Total";
> ? ? ? ?case AVRCP_MEDIA_ATTRIBUTE_GENRE:
> ? ? ? ? ? ? ? ?return "Genre";
> - ? ? ? case AVRCP_MEDIA_ATTRIBUTE_PROGRESS:
> - ? ? ? ? ? ? ? return "Progress";
> + ? ? ? case AVRCP_MEDIA_ATTRIBUTE_DURATION:
> + ? ? ? ? ? ? ? return "Track duration";
> ? ? ? ?default:
> ? ? ? ? ? ? ? ?return "Reserved";
> ? ? ? ?}
> --
> 1.7.6.1

Ack

@David: Lets keep hcidump simple, if we start doing enums for
everything in hcidump we would just introduce more code for almost
nothing back for a tracing tool.

--
Luiz Augusto von Dentz

2011-09-09 23:16:27

by David Stockwell

[permalink] [raw]
Subject: Re: [PATCH hcidump] avrcp: fix name of metadata field

Hi Lucas,

-----Original Message-----
From: Lucas De Marchi
Sent: Friday, September 09, 2011 8:29 AM
To: David Stockwell
Cc: [email protected]
Subject: Re: [PATCH hcidump] avrcp: fix name of metadata field

Hi David,

On Fri, Sep 9, 2011 at 9:22 AM, David Stockwell
<[email protected]> wrote:
> That's fair. For what it's worth, would this be better as an enum?

Please, don't top-post.

++++ I realized I did it afterwards, sorry. I then decided it would be
wasteful to send yet another e-mail (to you, Johan, the devel list, etc.)
apologizing.
For most of my e-mail, I am on Windows 7, and I just forgot.

The avrcp parser in hcidump was done entirely with defines... I don't
see any point in changing that.

+++++ At this point, I agree with you, and in fact coded my version the same
way. In the Windows/.Net world, they strongly recommend enums because the
compilers can provide somewhat better error checking. No matter either way,
just something to think about.

Lucas De Marchi

2011-09-09 13:29:47

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH hcidump] avrcp: fix name of metadata field

Hi David,

On Fri, Sep 9, 2011 at 9:22 AM, David Stockwell
<[email protected]> wrote:
> That's fair. ?For what it's worth, would this be better as an enum?

Please, don't top-post.

The avrcp parser in hcidump was done entirely with defines... I don't
see any point in changing that.

Lucas De Marchi

2011-09-09 12:22:50

by David Stockwell

[permalink] [raw]
Subject: Re: [PATCH hcidump] avrcp: fix name of metadata field

That's fair. For what it's worth, would this be better as an enum?

DS

-----Original Message-----
From: Lucas De Marchi
Sent: Thursday, September 08, 2011 6:32 PM
To: [email protected]
Cc: Lucas De Marchi
Subject: [PATCH hcidump] avrcp: fix name of metadata field

Metadata field number 0x7 should be the track duration and not the
progress of the track playback. Thus rename it to a better description.
---
parser/avrcp.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/parser/avrcp.c b/parser/avrcp.c
index f8e4443..0b98a3e 100644
--- a/parser/avrcp.c
+++ b/parser/avrcp.c
@@ -166,7 +166,7 @@
#define AVRCP_MEDIA_ATTRIBUTE_TRACK 0x4
#define AVRCP_MEDIA_ATTRIBUTE_TOTAL 0x5
#define AVRCP_MEDIA_ATTRIBUTE_GENRE 0x6
-#define AVRCP_MEDIA_ATTRIBUTE_PROGRESS 0x7
+#define AVRCP_MEDIA_ATTRIBUTE_DURATION 0x7

/* play status */
#define AVRCP_PLAY_STATUS_STOPPED 0x00
@@ -909,8 +909,8 @@ static const char *mediattr2str(uint32_t attr)
return "Track Total";
case AVRCP_MEDIA_ATTRIBUTE_GENRE:
return "Genre";
- case AVRCP_MEDIA_ATTRIBUTE_PROGRESS:
- return "Progress";
+ case AVRCP_MEDIA_ATTRIBUTE_DURATION:
+ return "Track duration";
default:
return "Reserved";
}
--
1.7.6.1