Re: [PATCH] media: rc: Always report LIRC repeat flag

["Marko Mäkelä" <marko.makela@iki.fi>]

Hi Sean,

Thu, Jul 07, 2022 at 09:57:10AM +0100, Sean Young wrote:
>Hi Marko,
>
>On Wed, Jul 06, 2022 at 07:39:17PM +0300, Marko Mäkelä wrote:
>> Tue, Jul 05, 2022 at 06:43:55PM +0100, Sean Young wrote:
>> > On Tue, Jul 05, 2022 at 11:53:58AM +0300, Marko Mäkelä wrote:
>> > > The flag LIRC_SCANCODE_FLAG_REPEAT was never set by rc_keydown().
>> > > Previously it was only set by rc_repeat(), but not all protocol
>> > > decoders invoke that function.
>> >
>> > This should say _why_ you are making this change, not _what_ the change
>> > is.
>>
>> How would you find the following?
>>
>> ---
>> media: lirc: ensure lirc device receives repeats
>>
>> Commit de142c32410649e64d44928505ffad2176a96a9e ("media: lirc: implement
>> reading scancode") would never set the LIRC_SCANCODE_FLAG_REPEAT flag in the
>> LIRC messages.
>>
>> Commit b66218fddfd29f315a103db811152ab0c95fb054
>> ("media: lirc: ensure lirc device receives nec repeats") fixed it up for
>> those protocol drivers that may call rc_repeat().
>> ---
>
>That's no good. See:
>
>https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
>
>The heading is called "Describe your changes".

I see. A quick read of "git log --oneline drivers/media/rc" suggests 
that the first line of the commit message is expected to be a summary 
_what_ the change is, not _why_ it was made. Would the commit message be 
acceptable after adding a "why" part right after the heading line, like 
this? If not, I would appreciate specific suggestions.

---
media: lirc: ensure lirc device receives repeats

For remote controls using RC5 and similar protocols that include a
"toggle" flag, the LIRC device never set the "repeat" flag to distinguish
repeated messages that were sent several times per second due to a
long keypress, and messages sent due to repeated short keypresses.

While a user-space program may implement logic around the "toggle" flag
to distinguish long keypresses, it would be simpler to be able to rely 
on the "repeat" flag for any type of protocol.

Commit de142c32410649e64d44928505ffad2176a96a9e ("media: lirc: implement
reading scancode") would never set the LIRC_SCANCODE_FLAG_REPEAT flag in
the LIRC messages.

Commit b66218fddfd29f315a103db811152ab0c95fb054
("media: lirc: ensure lirc device receives nec repeats") fixed it up for
those protocol drivers that may call rc_repeat().
---

Best regards,

	Marko