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

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

 



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



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux