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;
}
--
1.8.5.2 (Apple Git-48)
[ 587.936293] ir_nec_decode: NEC decode started at state 0 (9078us pulse)
[ 587.936298] ir_nec_decode: NEC decode started at state 1 (4441us space)
[ 587.952299] ir_nec_decode: NEC decode started at state 2 (627us pulse)
[ 587.952304] ir_nec_decode: NEC decode started at state 3 (504us space)
... more states 2||3 ...
[ 588.000351] ir_nec_decode: NEC decode started at state 2 (625us pulse)
[ 588.000353] ir_nec_decode: NEC decode started at state 3 (503us space)
[ 588.000355] ir_nec_decode: NEC decode started at state 4 (623us pulse)
[ 588.044379] ir_nec_decode: NEC decode started at state 5 (39684us space)
[ 588.044383] ir_nec_decode: NEC scancode 0x0088
[ 588.044386] rc_g_keycode_from_table: saa716x IR (TurboSight TBS 6281): scancode 0x0088 keycode 0x6c
[ 588.044394] ir_do_keydown: saa716x IR (TurboSight TBS 6281): key down event, key 0x006c, scancode 0x0088
[ 588.044420] ir_nec_decode: NEC decode started at state 0 (9080us pulse)
[ 588.044422] ir_nec_decode: NEC decode started at state 1 (2184us space)
[ 588.044426] ir_nec_decode: Repeat last key
[ 588.044428] ir_nec_decode: NEC decode started at state 4 (625us pulse)
[ 588.152556] ir_nec_decode: NEC decode started at state 5 (96421us space)
[ 588.152559] ir_nec_decode: NEC scancode 0x0088
[ 588.152561] rc_g_keycode_from_table: saa716x IR (TurboSight TBS 6281): scancode 0x0088 keycode 0x6c
[ 588.260571] ir_nec_decode: NEC decode started at state 0 (9078us pulse)
[ 588.276577] ir_nec_decode: NEC decode started at state 1 (4440us space)
[ 588.276583] ir_nec_decode: NEC decode started at state 2 (627us pulse)
[ 588.276587] ir_nec_decode: NEC decode started at state 3 (504us space)
... more states 2||3 ...
[ 588.324632] ir_nec_decode: NEC decode started at state 2 (625us pulse)
[ 588.324634] ir_nec_decode: NEC decode started at state 3 (505us space)
[ 588.324636] ir_nec_decode: NEC decode started at state 4 (624us pulse)
[ 588.404656] ir_do_keyup: keyup key 0x006c
[ 588.568833] ir_nec_decode: NEC decode started at state 5 (240550us space)
[ 588.568837] ir_nec_decode: NEC scancode 0x0088
[ 588.568840] rc_g_keycode_from_table: saa716x IR (TurboSight TBS 6281): scancode 0x0088 keycode 0x6c
[ 588.568843] ir_do_keydown: saa716x IR (TurboSight TBS 6281): key down event, key 0x006c, scancode 0x0088
[ 588.568863] ir_nec_decode: NEC decode started at state 0 (9076us pulse)
[ 588.584848] ir_nec_decode: NEC decode started at state 1 (4445us space)
[ 588.584853] ir_nec_decode: NEC decode started at state 2 (621us pulse)
[ 588.584855] ir_nec_decode: NEC decode started at state 3 (507us space)
... more states 2||3 ...
[ 588.632927] ir_nec_decode: NEC decode started at state 2 (621us pulse)
[ 588.632928] ir_nec_decode: NEC decode started at state 3 (509us space)
[ 588.632930] ir_nec_decode: NEC decode started at state 4 (620us pulse)
[ 588.676927] ir_nec_decode: NEC decode started at state 5 (39683us space)
[ 588.676931] ir_nec_decode: NEC scancode 0x0088
[ 588.676938] rc_g_keycode_from_table: saa716x IR (TurboSight TBS 6281): scancode 0x0088 keycode 0x6c
[ 588.676963] ir_nec_decode: NEC decode started at state 0 (9048us pulse)
[ 588.676966] ir_nec_decode: NEC decode started at state 1 (2212us space)
[ 588.676970] ir_nec_decode: Repeat last key
[ 588.676972] ir_nec_decode: NEC decode started at state 4 (597us pulse)
[ 588.929112] ir_do_keyup: keyup key 0x006c
[ 588.949162] ir_nec_decode: NEC decode started at state 5 (260338us space)
[ 588.949166] ir_nec_decode: NEC scancode 0x0088
[ 588.949169] rc_g_keycode_from_table: saa716x IR (TurboSight TBS 6281): scancode 0x0088 keycode 0x6c
[ 588.949172] ir_do_keydown: saa716x IR (TurboSight TBS 6281): key down event, key 0x006c, scancode 0x0088
[ 588.949192] ir_nec_decode: NEC decode started at state 0 (9014us pulse)
[ 588.965177] ir_nec_decode: NEC decode started at state 1 (4509us space)
[ 588.965183] ir_nec_decode: NEC decode started at state 2 (582us pulse)
[ 588.965187] ir_nec_decode: NEC decode started at state 3 (571us space)
... more states 2||3 ...
[ 589.013261] ir_nec_decode: NEC decode started at state 2 (594us pulse)
[ 589.013263] ir_nec_decode: NEC decode started at state 3 (534us space)
[ 589.013265] ir_nec_decode: NEC decode started at state 4 (596us pulse)
[ 589.201348] ir_do_keyup: keyup key 0x006c
