Hi Sean,
Did you have a chance to repeat my findings with a remote control unit
that uses the RC5 protocol?
I tried to understand the code changes. A few observations:
In rtl2831u_get_rc_config(), rc->interval is set to 400 and rc->timeout
is not set at all. Maybe it is OK, similar chips supported by the same
driver.
In rtl2832u_rc_query(), the idle_length is being computed from the last
two bytes of the IR_RX_BUF buffer. The threshold is 0xc0 and thus
it can only be exceeded if both last 7-bit bytes are included in the
sum. Side note: the & 0x7f is redundant, because the most significant
bit was already tested to be clear:
idle_length = 0;
if (len > 2) {
if (!(buf[len - 1] & 0x80))
idle_length += buf[len - 1] & 0x7f;
if (!(buf[len - 2] & 0x80))
idle_length += buf[len - 2] & 0x7f;
}
dev_dbg(&d->intf->dev, "idle_length=%x\n", idle_length);
if (idle_length < 0xbf)
return 0;
However, I spotted a potential problem here. I may of course be mistaken
because I do not know how the IR_RX_BUF is supposed to work. Could it
happen that buf[] contains some IR events, then 2 or more consecutive
bytes of no pulses (with the total time exceeding 0xbf*51µs), and then
again some IR events until the very end of the buffer, so that in the
above check, we would return 0? Could we in that case discard some IR
events? Could that explain the glitch that I observed with the NEC
protocol?
Best regards,
Marko
Fri, Jan 14, 2022 at 08:31:37AM +0200, Marko Mäkelä wrote:
Thu, Jan 13, 2022 at 04:55:52PM +0000, Sean Young wrote:
So I had to dig around for a while, but I have the same device here.
After some experimenting I've written a patch. Please could test it
out for me, a `Tested-by:` would be appreciated (if it works of
course!).
It is significantly better for the bundled remote control unit (using
the NEC protocol). But it can still lose events. I tested by holding
down the VOL+ and VOL- keys. Also the GNOME Desktop was listening to
those events, so I got some additional visual feedback.
Here is a trace from ir-keytable -t, using the NEC protocol:
Testing events. Please, press CTRL-C to abort.
261.125938: lirc protocol(nec): scancode = 0x1e
261.125959: event type EV_MSC(0x04): scancode = 0x1e
261.125959: event type EV_KEY(0x01) key_down: KEY_VOLUMEUP(0x0073)
261.125959: event type EV_SYN(0x00).
261.189926: lirc protocol(nec): scancode = 0x1e repeat
261.189939: event type EV_MSC(0x04): scancode = 0x1e
261.189939: event type EV_SYN(0x00).
261.309891: lirc protocol(nec): scancode = 0x1e repeat
261.309905: event type EV_MSC(0x04): scancode = 0x1e
261.309905: event type EV_SYN(0x00).
261.429960: lirc protocol(nec): scancode = 0x1e repeat
261.429973: event type EV_MSC(0x04): scancode = 0x1e
261.429973: event type EV_SYN(0x00).
261.553908: lirc protocol(nec): scancode = 0x1e repeat
261.553923: event type EV_MSC(0x04): scancode = 0x1e
261.553923: event type EV_SYN(0x00).
261.617911: lirc protocol(nec): scancode = 0x1e repeat
261.617924: event type EV_MSC(0x04): scancode = 0x1e
261.617924: event type EV_SYN(0x00).
261.628041: event type EV_KEY(0x01) key_down: KEY_VOLUMEUP(0x0073)
261.628041: event type EV_SYN(0x00).
261.737970: lirc protocol(nec): scancode = 0x1e repeat
261.628041: event type EV_MSC(0x04): scancode = 0x1e
261.628041: event type EV_SYN(0x00).
261.760048: event type EV_KEY(0x01) key_down: KEY_VOLUMEUP(0x0073)
261.760048: event type EV_SYN(0x00).
261.857958: lirc protocol(nec): scancode = 0x1e repeat
261.760048: event type EV_MSC(0x04): scancode = 0x1e
261.760048: event type EV_SYN(0x00).
261.892056: event type EV_KEY(0x01) key_down: KEY_VOLUMEUP(0x0073)
261.892056: event type EV_SYN(0x00).
So far, so good. The initial delay between the key_down events is
500ms. But, keep reading:
262.024055: event type EV_KEY(0x01) key_down: KEY_VOLUMEUP(0x0073)
262.024055: event type EV_SYN(0x00).
262.045960: lirc protocol(nec): scancode = 0x1e repeat
262.024055: event type EV_MSC(0x04): scancode = 0x1e
262.024055: event type EV_SYN(0x00).
262.156052: event type EV_KEY(0x01) key_down: KEY_VOLUMEUP(0x0073)
262.156052: event type EV_SYN(0x00).
262.156052: event type EV_KEY(0x01) key_up: KEY_VOLUMEUP(0x0073)
262.156052: event type EV_SYN(0x00).
264.074505: lirc protocol(nec): scancode = 0x1e repeat
264.074524: event type EV_MSC(0x04): scancode = 0x1e
264.074524: event type EV_SYN(0x00).
264.137961: lirc protocol(nec): scancode = 0xa
264.137979: event type EV_MSC(0x04): scancode = 0x0a
264.137979: event type EV_KEY(0x01) key_down: KEY_VOLUMEDOWN(0x0072)
264.137979: event type EV_SYN(0x00).
264.324062: event type EV_KEY(0x01) key_up: KEY_VOLUMEDOWN(0x0072)
264.324062: event type EV_SYN(0x00).
264.325944: lirc protocol(nec): scancode = 0xa repeat
264.325960: event type EV_MSC(0x04): scancode = 0x0a
264.325960: event type EV_SYN(0x00).
264.445972: lirc protocol(nec): scancode = 0xa repeat
264.445988: event type EV_MSC(0x04): scancode = 0x0a
264.445988: event type EV_SYN(0x00).
264.565937: lirc protocol(nec): scancode = 0xa repeat
264.565952: event type EV_MSC(0x04): scancode = 0x0a
[snip]
266.585974: lirc protocol(nec): scancode = 0xa repeat
266.585989: event type EV_MSC(0x04): scancode = 0x0a
266.585989: event type EV_SYN(0x00).
267.270556: lirc protocol(nec): scancode = 0xa repeat
267.270571: event type EV_MSC(0x04): scancode = 0x0a
267.270571: event type EV_SYN(0x00).
267.389958: lirc protocol(nec): scancode = 0xa
267.389976: event type EV_MSC(0x04): scancode = 0x0a
267.389976: event type EV_KEY(0x01) key_down: KEY_VOLUMEDOWN(0x0072)
267.389976: event type EV_SYN(0x00).
267.509941: lirc protocol(nec): scancode = 0xa repeat
267.509957: event type EV_MSC(0x04): scancode = 0x0a
267.509957: event type EV_SYN(0x00).
267.629971: lirc protocol(nec): scancode = 0xa repeat
267.629986: event type EV_MSC(0x04): scancode = 0x0a
267.629986: event type EV_SYN(0x00).
267.749948: lirc protocol(nec): scancode = 0xa repeat
267.749963: event type EV_MSC(0x04): scancode = 0x0a
267.749963: event type EV_SYN(0x00).
267.873963: lirc protocol(nec): scancode = 0xa repeat
267.873978: event type EV_MSC(0x04): scancode = 0x0a
267.873978: event type EV_SYN(0x00).
267.900054: event type EV_KEY(0x01) key_down: KEY_VOLUMEDOWN(0x0072)
267.900054: event type EV_SYN(0x00).
267.937934: lirc protocol(nec): scancode = 0xa repeat
267.900054: event type EV_MSC(0x04): scancode = 0x0a
267.900054: event type EV_SYN(0x00).
268.032044: event type EV_KEY(0x01) key_down: KEY_VOLUMEDOWN(0x0072)
268.032044: event type EV_SYN(0x00).
Why was there a key_up event at 264.324062 even though I continued to
hold down that button? Maybe shortly before another key_down was
finally generated, I had let the button go and pressed it again.
All in all, I long-pressed VOL+, VOL-, VOL+, VOL-, and only for the
second press (VOL-) I failed to get proper repeat events. So, it
almost works. For comparison, I used the stock kernel module, which
never generated repeat events for the NEC protocol.
Then, I loaded the hauppauge.toml and tested with RC5. It was fairly
OK, except for an anomaly: When I pressed Vol- or Vol+ after just
having long-pressed the other key, I got one more event for the
previous key! Here is a trace of that:
1471.642128: event type EV_MSC(0x04): scancode = 0x1e10
1471.642128: event type EV_SYN(0x00).
1471.676071: event type EV_KEY(0x01) key_down: KEY_VOLUMEUP(0x0073)
1471.676071: event type EV_SYN(0x00).
1471.808059: event type EV_KEY(0x01) key_down: KEY_VOLUMEUP(0x0073)
1471.808059: event type EV_SYN(0x00).
1471.808059: event type EV_KEY(0x01) key_up: KEY_VOLUMEUP(0x0073)
1471.808059: event type EV_SYN(0x00).
1472.266096: lirc protocol(rc5): scancode = 0x1e10 toggle=1
1472.266113: event type EV_MSC(0x04): scancode = 0x1e10
1472.266113: event type EV_KEY(0x01) key_down: KEY_VOLUMEUP(0x0073)
1472.266113: event type EV_SYN(0x00).
1472.386062: lirc protocol(rc5): scancode = 0x1e11
1472.386079: event type EV_KEY(0x01) key_up: KEY_VOLUMEUP(0x0073)
1472.386079: event type EV_MSC(0x04): scancode = 0x1e11
1472.386079: event type EV_KEY(0x01) key_down: KEY_VOLUMEDOWN(0x0072)
1472.386079: event type EV_SYN(0x00).
1472.450104: lirc protocol(rc5): scancode = 0x1e11
1472.450118: event type EV_MSC(0x04): scancode = 0x1e11
1472.450118: event type EV_SYN(0x00).
1472.570090: lirc protocol(rc5): scancode = 0x1e11
1472.570105: event type EV_MSC(0x04): scancode = 0x1e11
1472.570105: event type EV_SYN(0x00).
1472.760061: event type EV_KEY(0x01) key_up: KEY_VOLUMEDOWN(0x0072)
1472.760061: event type EV_SYN(0x00).
The events between 1472.266096 and 1472.386079 are fake. I only
pressed and released Vol+ once. Once I started to press Vol- I got
fake key_down and key_up for KEY_VOLUMEUP instead of getting the
initial message for KEY_VOLUMEDOWN.
I tried this also with short presses, pressing Vol- and Vol+
alternatively. Almost all of the time, the GNOME volume control widget
reacted by getting exactly the wrong event (from the previous key
press).
From f860a05c35a7b0e7b331e61e1b61674c2a9279f0 Mon Sep 17 00:00:00 2001
From: Sean Young <sean@xxxxxxxx>
Date: Thu, 13 Jan 2022 16:33:13 +0000
Subject: [PATCH] media: rtl28xxu: improve IR receiver
In my opinion, this patch is a significant improvement for decoding
the NEC protocol, as well as for the RC5 protocol, because for that
one it no longer sends key_up events before each repeated key_down
event for a held-down button.
But, for the RC5 decoder, there appears to be a serious regression
that instead of sending the event for the last received message, it
will repeat an event for the previously pressed button.
Best regards,
Marko
