I have not received any response on this email... so I hope to bump this thread back to the more active region in most people's in-boxes. On 2014-05-31 10:37, Niels Laukens wrote: > Hi, > > I believe I've found a bug in the NEC decoder for InfraRed remote > controls. The problem manifests itself as an extra keypress that happens > when pushing different buttons in "rapid" succession. > > I can reproduce the problem easily (but not always) by pushing DOWN, DOWN, > UP in "rapid" succession. I put "rapid" in quotes, because I don't have to > hurry in any way, it happens when I use it normally. Depending on the > duration of the presses, I get a number of repeats of DOWN. The bug is > that an additional DOWN keypress happens at the time that I press the UP > key (or so it seams). > > Attached is kernel-debug.log, which contains the redacted and annotated > dmesg output, illustrating the problem described above. Especially note > lines 31-36 and 54-59, where more than 200ms pass between the end of the > IR-code and the actual emit of the keydown event. > > > I've debugged this issue, and believe I've found the cause: The keypress > is only emitted in state 5 (STATE_TRAILER_SPACE). This state is only > executed when the space after the message is received, i.e. when the > next pulse (of the next message) starts. It is only then that the length > of the space is known, and that ir_raw_event will fire an event. > > The patch below addresses this issue. This is my first kernel patch. > I've tried to follow all guidelines that I could find, but might have > missed a few. > > I've tested this patch with the out-of-tree TBS drivers (which seem to > be based on 3.13), and it solves the bug. > I've compared this TBS-version with the current master (1487385). There > are 8 (non-comment) lines that differ, none affect this patch. This > patch applies cleanly to the current master. > > Regards, > Niels > > > > > From 071c316e9315f22a055d6713cc8cdcdc73642193 Mon Sep 17 00:00:00 2001 > From: Niels Laukens <niels@xxxxxxxxxxxxxxx> > Date: Sat, 31 May 2014 10:30:18 +0200 > Subject: [PATCH] drivers/media/rc/ir-nec-decode: fix phantom detect > > The IR NEC decoder waited until the TRAILER_SPACE state to emit a > keypress. Since the triggering 'space' event will only be sent at the > beginning of the *next* IR-code, this is way to late. > > This patch moves the processing to the TRAILER_PULSE state. Since we > arrived here with a 'pulse' event, we know that the pulse has ended and > thus that the space is there (as of yet with unknown length). > > Signed-off-by: Niels Laukens <niels@xxxxxxxxxxxxxxx> > --- > drivers/media/rc/ir-nec-decoder.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/media/rc/ir-nec-decoder.c b/drivers/media/rc/ir-nec-decoder.c > index 35c42e5..955f99d 100644 > --- a/drivers/media/rc/ir-nec-decoder.c > +++ b/drivers/media/rc/ir-nec-decoder.c > @@ -148,16 +148,6 @@ static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev) > if (!eq_margin(ev.duration, NEC_TRAILER_PULSE, NEC_UNIT / 2)) > break; > > - data->state = STATE_TRAILER_SPACE; > - return 0; > - > - case STATE_TRAILER_SPACE: > - if (ev.pulse) > - break; > - > - if (!geq_margin(ev.duration, NEC_TRAILER_SPACE, NEC_UNIT / 2)) > - break; > - > address = bitrev8((data->bits >> 24) & 0xff); > not_address = bitrev8((data->bits >> 16) & 0xff); > command = bitrev8((data->bits >> 8) & 0xff); > @@ -190,6 +180,16 @@ static int ir_nec_decode(struct rc_dev *dev, struct ir_raw_event ev) > data->necx_repeat = true; > > rc_keydown(dev, scancode, 0); > + data->state = STATE_TRAILER_SPACE; > + return 0; > + > + case STATE_TRAILER_SPACE: > + if (ev.pulse) > + break; > + > + if (!geq_margin(ev.duration, NEC_TRAILER_SPACE, NEC_UNIT / 2)) > + break; > + > data->state = STATE_INACTIVE; > return 0; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
