Re: [PATCH v2 01/14] AVRCP: Update constants

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Lucas,

On Wed, Jun 27, 2012 at 7:22 PM, Lucas De Marchi
<lucas.demarchi@xxxxxxxxxxxxxx> wrote:
>> No. According to Lucas I removed unused constants.
>> What wrong with enum? Look into sap.h, vhci.h, reporter.h, parser.h, cups.h and... avrcp.c ("enum battery_status" is here)
>
> I strongly NACKed this before. Not only because of the unused constants.
>
>>
>> I have idea to make enum new convention in BlueZ. See sap.h - it looks very clearly for me. Pros for used enum is that enum can be thread as group of constants, mostly constants defined in specification. But "magic constants" should stay define - mostly defined by developer.
>> Disadvantage for "define" is that as you can see in BlueZ AVRCP - you must use additional comment to describe group of constants.
>
> I don't see any advantage in being able to group them. You still have
> to define the name by prefixing it with the namespace. If it was
> already enum, fine - changing for the sake of change only is not good.
>
>>
>> Luiz, why "nak"? Could you present your point of view?
>
> IMO this change doesn't give us anything

Exactly, enum gives us nothing, no type checking or anything, so why bother.

@Michal: As for the rest of the patches, I will apply the typo and
code style fixes that are straightforward, the rest will probably need
rebasing. I specially liked the idea of avrcp_session, it will be
really useful, I guess we can even store the avctp_session on it. As
for the dummy player, my initial take was that it was necessary, but
you are actually not using it as a place holder, instead it just
return errors so in that I would just check if there is no player
available send the proper errors.

-- 
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux