search

plexinc / plex-media-player

Next generation Plex Desktop/Embedded Client
GNU General Public License v2.0
1.17k stars 170 forks source link

Patch for infinite repeating of remote key presses with LIRC mapping in Fedora 23 #326

Open truckershitch opened 8 years ago

truckershitch commented 8 years ago

This affects PMP from a 1.1.0rc git pull from April 23, 2016. In Fedora 23, Qt-5.6.0 final from RPMs, using LIRC 0.9.3a, PMP repeats the last key pressed until another key is pressed on the remote. When looking at the log, it seems that the up key press is not registered.

I changed this in /src/input/InputLIRC.cpp

if ((repeatCount % 3) == 0)
{
  bool up = (command.endsWith("_UP") && (command != "KEY_UP"));
  emit receivedInput("LIRC", command, !up);
}

changes to

if ((repeatCount % 3) == 0)
{
  emit receivedInput("LIRC", command, true);
  emit receivedInput("LIRC", command, false);
}

Perhaps the developers have a better way, but this fixes the issue for me.

LongChair commented 8 years ago

Hello,

This patch is partly right & partly wrong :)

first we need to check if the key ends with _UP to make sure if it's an up event. also there is a special case with KEY_UP which will send 'KEY_UP & KEY_UP_UP , so removing this check probably gives strange behavior on the up arrow.

We're refactoring the inputs currently, so will fix that so that it works properly :)

truckershitch commented 8 years ago

I'm sure you're right about the up event. However, on my system, no up events are registered at all. I am not seeing any strange behavior with the up arrow. In fact, my patch works perfectly with every key press I've tried.

Perhaps this is different on other systems. I'm not sure why the up event is not registering with PMP, because when viewing button presses with irw, I am seeing both the down and the up events.

I just wanted to share what works for me. I have a post in the forums, and it seems like the Intel NUC people are experiencing double presses, which is not what I'm seeing (infinite down presses, no up press). There is more going on here that I understand.

Thanks for responding! I'll leave my patch in on my system until you commit further fixes into the code.

LongChair commented 8 years ago

would you mind sending me a log of what happens in LIRC without the patch ?

truckershitch commented 8 years ago

PMP Log is here: http://pastebin.com/5yGCseGY

Below are a few key presses using the irw utility

[me@mypc ~]$ irw 000000037ff07be1 00 Up mceusb 000000037ff07be1 00 Up mceusb 000000037ff07be1 01 Up mceusb 000000037ff07be0 00 Down mceusb 000000037ff07be0 01 Down mceusb 000000037ff07bdf 00 Left mceusb 000000037ff07bdf 01 Left mceusb 000000037ff07bdd 00 OK mceusb 000000037ff07bdd 01 OK mceusb

LIRC seems to be recording both the up and the down key presses.

tumd commented 8 years ago

I have the same issue using a harmony one with inputlircd running under Ubuntu 16.04. The above patch by @truckershitch solves it for me in master-branch. But the 1.1.x-branch returns compile-errors with the same changes.

truckershitch commented 8 years ago

@tumd: The code was refactored. Below is an updated patch to match the current code in the 1.1.x branch:

if ((repeatCount % 3) == 0)
      {
        //bool up = (command.endsWith("_UP") && (command != "KEY_UP"));
        //emit receivedInput("LIRC", command, up ? InputBase::KeyUp : InputBase::KeyDown);

        emit receivedInput("LIRC", command, InputBase::KeyDown);
        emit receivedInput("LIRC", command, InputBase::KeyUp);
      }
LongChair commented 7 years ago

I still believe that your LIRC daemon is not configured to send the "UP" Key events as it is on our Linux distro. You should really check this before trying to change the code i think :)

truckershitch commented 7 years ago

I changed the settings on my LIRC daemon to send the "UP" key events to avoid having to patch PMP every time there was a new release. It hasn't caused any issues with Mythtv or Kodi or OpenPHT.

aerickson commented 7 years ago

@LongChair @truckershitch Is the option you're referring to --release? Thanks.

truckershitch commented 7 years ago

Yes. I use -r which is the same as --release. It generates the "UP" events. It hasn't raised any issues for me for a few months now. My lircd.service file now includes the line: ExecStart=/usr/sbin/lircd -r --nodaemon

aerickson commented 7 years ago

It looks like --release=_LIRCUP is required since https://github.com/plexinc/plex-media-player/commit/8764331207ff4a616ca3013f0fea047d2127edfd (-r alone will send '_UP', or '_EVUP' since Lirc 0.10.0rc1).

Kodi was less responsive to LIRC input for me with the option enabled. It seemed like button presses were getting missed. I've got Jenkins building PMP with the patch so it's not horrible.

Interestingly, the --release option has recently been deprecated.

0.10.0rc1 28/5/17
 ...
 * lircd: The --release option is deprecated.

from https://sourceforge.net/p/lirc/git/ci/master/tree/NEWS

truckershitch commented 6 years ago

This should be revisited. I'm having issues in Fedora 28 now using the --release option as it is causing problems in other programs such as MythTV. Also, as @aerickson has stated, this option will soon be deprecated. Please return to older code that does not rely on release events from LIRC. Remote keys will repeat if held down anyway. There is no need to look for the "UP" event.

LongChair commented 6 years ago

Recent linux kernel handle IR remotes at least most of them. You might not even need LIRC anymore, using it still will mostly result in doublepressed keys.

truckershitch commented 6 years ago

Actually I have no issues with button presses at all unless my remote isn't pointed at the receiver. LIRC is relatively painless unless a new version breaks something and I have to figure out what is different.

I'm using the kernel to handle the IR remote, but I need LIRC because I have four applications that depend on it. irexec is also essential in my setup. I cannot abandon LIRC at this point. It would be nice to just map the events to keyboard keys but with four applications it's very difficult to do so well. And then I'd also lose irexec. If there is an alternative to that program, or someone writes one, maybe I could figure something out.

I played around with the LIRC configuration again and managed to get the lircd release events to work. This will be deprecated in future versions of LIRC. There is an alternative called lircd-uinput which is supposed to handle the release events. I am having great difficulty getting this to work and have filed a bug report. I hope to hear something soon but I know that the LIRC project is understaffed.

I know that you want to distinguish between long and short button presses. I suppose that is good for remotes with only a few buttons. Other than that, I can't see a use for it. But I'm only one person. These release events are quite bothersome to me and I've never had to deal with them until PMP.

As long as there is a way for me (and others) to disable the release events, I'll be okay and will just grumble at compile time.

Thanks for the hard work on PMP and PLEX. I don't want to come off as ungrateful. I use PMP almost every single day and I look forward to its frequent updates.

mengalva commented 5 years ago

I found a possible source of the double key press, This is my configuration as a reference:

Intel NUC D54250WYK Ubuntu 18.04 Lirc 0.10.0 (with lircd-uinput.service enabled) lirc_options.conf file (lircd-uinput section enabled)

[lircd] nodaemon = False driver = default device = /dev/lirc0 output = /var/run/lirc/lircd pidfile = /var/run/lirc/lircd.pid plugindir = /usr/lib/x86_64-linux-gnu/lirc/plugins permission = 666 allow-simulate = No repeat-max = 600

[lircd-uinput] add-release-events = true release-timeout = 200 release-suffix = _EVUP

Plex Media Player (1. git clone the project 2. Remove keyboard.json file from /pmp/plex-media-player/resources/inputmaps/keyboard.json 3. Continue install)

Removing this file I get free of double key press.

I'm just a technology entusiastic but maybe you can review the keyboard.json file and the InputComponent.cpp. I'm attaching the log of three buttons with this fix for your reference:

2018-11-05 09:56:52 [ INFO ] InputLIRC.cpp @ 103 - LIRC Got Key : "KEY_NUMERIC_9" , repeat count: 0 , from remote "SamsungAA5900559A\n" 2018-11-05 09:56:52 [ DEBUG ] InputComponent.cpp @ 160 - Input received: source: "LIRC" keycode: "KEY_NUMERIC_9" : InputBase::InputkeyState(KeyDown) 2018-11-05 09:56:52 [ DEBUG ] CachedRegexMatcher.cpp @ 68 - No match for: "KEY_NUMERIC_9" 2018-11-05 09:56:52 [ DEBUG ] InputComponent.cpp @ 229 - Emit input action: ("9") 2018-11-05 09:56:52 [ DEBUG ] InputComponent.cpp @ 160 - Input received: source: "lircd-uinput" keycode: "KEY_BUTTON_233" : InputBase::InputkeyState(KeyDown) 2018-11-05 09:56:52 [ DEBUG ] CachedRegexMatcher.cpp @ 68 - No match for: "lircd-uinput" 2018-11-05 09:56:52 [ INFO ] InputLIRC.cpp @ 103 - LIRC Got Key : "KEY_NUMERIC_9" , repeat count: 1 , from remote "SamsungAA5900559A\n" 2018-11-05 09:56:52 [ DEBUG ] InputComponent.cpp @ 160 - Input received: source: "lircd-uinput" keycode: "KEY_BUTTON_233" : InputBase::InputkeyState(KeyUp)

2018-11-05 09:56:58 [ INFO ] InputLIRC.cpp @ 103 - LIRC Got Key : "KEY_NUMERIC_7" , repeat count: 0 , from remote "SamsungAA5900559A\n" 2018-11-05 09:56:58 [ DEBUG ] InputComponent.cpp @ 160 - Input received: source: "LIRC" keycode: "KEY_NUMERIC_7" : InputBase::InputkeyState(KeyDown) 2018-11-05 09:56:58 [ DEBUG ] CachedRegexMatcher.cpp @ 68 - No match for: "KEY_NUMERIC_7" 2018-11-05 09:56:58 [ DEBUG ] InputComponent.cpp @ 229 - Emit input action: ("7") 2018-11-05 09:56:58 [ DEBUG ] InputComponent.cpp @ 160 - Input received: source: "lircd-uinput" keycode: "KEY_BUTTON_231" : InputBase::InputkeyState(KeyDown) 2018-11-05 09:56:58 [ DEBUG ] CachedRegexMatcher.cpp @ 68 - No match for: "lircd-uinput" 2018-11-05 09:56:58 [ INFO ] InputLIRC.cpp @ 103 - LIRC Got Key : "KEY_NUMERIC_7" , repeat count: 1 , from remote "SamsungAA5900559A\n" 2018-11-05 09:56:58 [ DEBUG ] InputComponent.cpp @ 160 - Input received: source: "lircd-uinput" keycode: "KEY_BUTTON_231" : InputBase::InputkeyState(KeyUp)

2018-11-05 09:58:07 [ INFO ] InputLIRC.cpp @ 103 - LIRC Got Key : "KEY_RIGHT" , repeat count: 0 , from remote "SamsungAA5900559A\n" 2018-11-05 09:58:07 [ DEBUG ] InputComponent.cpp @ 160 - Input received: source: "LIRC" keycode: "KEY_RIGHT" : InputBase::InputkeyState(KeyDown) 2018-11-05 09:58:07 [ DEBUG ] CachedRegexMatcher.cpp @ 68 - No match for: "KEY_RIGHT" 2018-11-05 09:58:07 [ DEBUG ] InputComponent.cpp @ 229 - Emit input action: ("right") 2018-11-05 09:58:07 [ DEBUG ] InputComponent.cpp @ 160 - Input received: source: "lircd-uinput" keycode: "KEY_BUTTON_72" : InputBase::InputkeyState(KeyDown) 2018-11-05 09:58:07 [ DEBUG ] CachedRegexMatcher.cpp @ 68 - No match for: "lircd-uinput" 2018-11-05 09:58:07 [ DEBUG ] InputComponent.cpp @ 160 - Input received: source: "Keyboard" keycode: "Right" : InputBase::InputkeyState(KeyDown) 2018-11-05 09:58:07 [ DEBUG ] CachedRegexMatcher.cpp @ 68 - No match for: "Keyboard" 2018-11-05 09:58:07 [ INFO ] InputLIRC.cpp @ 103 - LIRC Got Key : "KEY_RIGHT" , repeat count: 1 , from remote "SamsungAA5900559A\n" 2018-11-05 09:58:07 [ DEBUG ] InputComponent.cpp @ 160 - Input received: source: "Keyboard" keycode: "Right" : InputBase::InputkeyState(KeyUp) 2018-11-05 09:58:07 [ DEBUG ] InputComponent.cpp @ 160 - Input received: source: "lircd-uinput" keycode: "KEY_BUTTON_72" : InputBase::InputkeyState(KeyUp)