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;
}
