#149 Repeated buttons on lirc-0.9.3

[Jon]

Since upgrading to lirc-0.9.3 I'm sometimes seeing buttons repeated twice, similar to ticket #148. However, according to git bisect the offending commit is 7dca803fc6b6b7090d057194f2828c21e186dfd6, which is different from that ticket.

I'm using a Logitech Harmony 200 (programmable remote) emulating an Xbox MCE remote with an USB-UIRT [1] receiver, on Arch Linux with kernel 4.2.1. I've tried emulating a different remote but still had issues.

(The next paragraphs are some observations I've done that may or may not be helpful)

Sometimes when I press a button, the previous button is repeated once at the same time that the new button is pressed. This happens both when pressing the same button and a different button. This seems to happen every time if the previous button was held down (the second column in irw's output is increasing). The delay between the two button presses doesn't seem to matter (I can wait a few minutes and still get the previous button repeated).

I've also noticed that before this commit, when holding down buttons, there seemed to be a small delay after I release the button before the last repeat is reported. This last repeat seems to now be missing, so I'm taking a wild guess that maybe this repeat is what I'm now seeing when pressing a new button?

Also, unless I'm mistaken, the timeout argument to poll() seems to be wrong. It should be milliseconds but is given in nanoseconds (maxusec * 1000). I tried changing it to maxusec / 1000 but that made no difference.

1. http://www.usbuirt.com/

[Alec Leamas]

Hm... here are many things lurking around. Your observation on the timeout is certainly correct, it should be maxusec /1000, nothing else. This is an example on what's called a "brain drop" in the primitive culture I live in (Sweden).

Could you please:

• Make a try while reverting same commit as in [#148] just to rule out side-effects from that (given that you kernel is updated).
• If the situation is the same, make a new try with maxusec / 1000.
• If the situation still is the same, add a "suppress_repeat 3" to the config file header

and report back?

[Jon]

Done, done and done, but no difference on all three. I already had suppress_repeat 2 in the config, so I just changed that. I tried setting it as high as 5, but still no difference.

[Alec Leamas]

Hm... this is, as they say, "odd". Just to double-check, could you make a try while reverting both the [#148] patch [b59b88] and your own [7dca80]. If this indeed solves the problem, I need to do some thinking...

[Jon]

Reverting both [7dca80] and [b59b88] (against release-0_9_3) works fine without any issues.

[Bengt Martensson]

Just an idea: Note that poll and select have a different interpretations for timeout == 0: select interprets it as infinite timeout, poll interprets it as "return immediately", and requires a negative timeout parameter to wait forever, Might be worth to check...(too lazy to do it myself...)

[Jon]

That seems almost right, select's timeout parameter was set to NULL when maxusec == 0 (which blocks indefinitely), so something like this I guess:

ret = poll(&pfd, 1, (maxusec > 0) ? (maxusec / 1000) : -1);

That doesn't help. However, I just made a small discovery when looking over the code. Previously, when given a timeout and select() returned 0, waitfordata() would also return 0. This part of the code was removed when it was replaced with poll(). Adding the following after the call to poll() seems to solve the issue:

if (maxusec > 0 && ret == 0)
    return 0;

Does this seem right? If so I can make a proper patch with the fix.

[Alec Leamas]

Yes! It seems very right... If you prepare and test a patch I would be really grateful. I'll make a more thorough review if this, possibly enhancing the patch in current master.

(Sorry for delay. Occasionally, I have a life ;) )

[Jon]

Added a separate patch for each of the changes, as they are separate fixes. The patches are applied against the current master.

[Alec Leamas]

Fixed in [951fa8] and [c824c0] (master) and [c824c0] and [02c248] (release). Closing bug. Many thanks for testing and providing a patch!