Key-repeat or release magic in RCU drivers

  I demand that Marko Mäkelä may or may not have written...

> On Tue, Feb 07, 2006 at 06:24:02PM +0000, Darren Salt wrote:
>>>> I've altered ir-functions.c:ir_input_keydown() here to make this easier:
>>>> there should no longer any need for other modules to alter
>>>> ir->keypressed.
[snip]
>>> The function should never convert a repeat code to a keypress or vice
>>> versa.  Two examples:

>>> (1) If I press the '1' key two times very quickly, I want VDR to see
>>> '11'. In your scheme, I'm afraid it will produce '1' and '1'|kRepeat, and
>>> VDR will ignore the second keypress.

>> That won't be a problem unless interrupts are being missed. AFAICT
>> (printk), that isn't happening.

> The RC5 repeat rate is 113.8 ms.

That matches what I'm seeing. It seems safe to assume that my remote control
does indeed use RC5.

> If I press the same button twice within that time frame, it does happen.  I
> must admit that this seems to happen with my own patch as well.  I'll have
> to add some debug code to display the RC5 frames to see whether the toggle
> bit really is toggling.

I just stuck in a printk: either it wasn't being toggled or I wasn't properly
releasing the button. (I could probably tell, but I don't plan on opening up
the remote control until I have to for other reasons.)

>>> (2) When I press the '1' button close to the maximum range of the remote
>>> (which is not much with the Hauppauge receiver that is driven out of
>>> spec), the driver should deliver the events 12022020 instead of 12012010,

>> Meaning what?

> 0=key-release
> 1=key-press
> 2=key-repeat

Oh, right: input_event last parameter.

Both are possible, depending on the number of consecutive unreceived frames.

[snip]
>>> so that VDR will switch to channel 1 and not 111.
>> Well... it'll generate a repeat for each received signal (assuming that
>> the key is held down) and it may time out, causing the next received
>> signal to be treated as a key-down event.

> With 240 ms key-release timeout, it will time out if two RC5 frames are
> lost.

Which seems reasonable to me, although this should perhaps be a module
parameter with, at least, a fixed minimum value. The right place for it is in
ir-common, but one step at a time...

> If you hold the RCU in the wrong angle or far away from the receiver and
> press the button longer to ensure that the event will be noticed, you will
> end up getting multiple keypress events instead of one keypress and
> multiple repeat events.

You might get a mixture of both.

>> With the /other/ scheme (using the input layer's own repeat handler), you
>> will get repeats - and maybe a few more 1s (although I'm enforcing lower
>> limits on the repeat/delay times, and the timeout is shorter than the
>> repeat time).

> I tested patch 3326 of the hg/v4l-dvb tree with your patches
> dvb-ir-20060206.patch.tar.gz applied.  I changed the key-release timeout in
> cx88-input.c from 120 to 240 ms.  Few keys were recognized.  Among the
> working keys were the number keys, although they were shifted by 1 (e.g.,
> '6' would be reported as '7').

Oops. The last patch in that series is broken - part of it got lost. It's
attached.

> The input layer's own repeat handler was not disabled, that is, I think
> there were too many repeat events. I couldn't figure out how budget-ci.c
> disables the repeat timer.

If ir->rep[0] and ir->rep[1] are both zero then input_register_device() will
set default delay and repeat values and install its repeat handler.

> I was unable to use this driver, because one of the cx88 modules failed to
> load and the computer crashed after some time.

All that I can say is that budget-ci is fine...
  

On Tue, Feb 07, 2006 at 11:00:23PM +0000, Darren Salt wrote:
> > If I press the same button twice within that time frame, it does happen.  I
> > must admit that this seems to happen with my own patch as well.  I'll have
> > to add some debug code to display the RC5 frames to see whether the toggle
> > bit really is toggling.
> 
> I just stuck in a printk: either it wasn't being toggled or I wasn't properly
> releasing the button. (I could probably tell, but I don't plan on opening up
> the remote control until I have to for other reasons.)

I loaded cx88xx with ir_debug=1 or something like that, and got debug output
in dmesg.  Indeed, the toggle bit did not toggle.  If this is the case with
two RCUs, then this is probably not an issue (that is, the bug is already
in the RCU).

[snip]

> > With 240 ms key-release timeout, it will time out if two RC5 frames are
> > lost.
> 
> Which seems reasonable to me, although this should perhaps be a module
> parameter with, at least, a fixed minimum value. The right place for it is in
> ir-common, but one step at a time...

Agreed.

[snip]

> > I tested patch 3326 of the hg/v4l-dvb tree with your patches
> > dvb-ir-20060206.patch.tar.gz applied.  I changed the key-release timeout in
> > cx88-input.c from 120 to 240 ms.  Few keys were recognized.  Among the
> > working keys were the number keys, although they were shifted by 1 (e.g.,
> > '6' would be reported as '7').
> 
> Oops. The last patch in that series is broken - part of it got lost. It's
> attached.

Thanks, I'll try it out after some time (hopefully, with a snapshot that
has better working cx88 drivers).

> > The input layer's own repeat handler was not disabled, that is, I think
> > there were too many repeat events. I couldn't figure out how budget-ci.c
> > disables the repeat timer.
> 
> If ir->rep[0] and ir->rep[1] are both zero then input_register_device() will
> set default delay and repeat values and install its repeat handler.

Thanks, I'll try setting those.

	Marko
  

On Tue, Feb 07, 2006 at 11:00:23PM +0000, Darren Salt wrote:
> > I tested patch 3326 of the hg/v4l-dvb tree with your patches
> > dvb-ir-20060206.patch.tar.gz applied.  I changed the key-release timeout in
> > cx88-input.c from 120 to 240 ms.  Few keys were recognized.  Among the
> > working keys were the number keys, although they were shifted by 1 (e.g.,
> > '6' would be reported as '7').
> 
> Oops. The last patch in that series is broken - part of it got lost. It's
> attached.

Thanks.  All keys seem to be mapped correctly now.  The only difference
I was able to spot was that previously, the number keys were mapped to
KP0..KP9 instead of 0..9.

> > I was unable to use this driver, because one of the cx88 modules failed to
> > load and the computer crashed after some time.
> 
> All that I can say is that budget-ci is fine...

cx88 sure isn't.  The tuner module could not be loaded, and vdr drops frames
when playing recordings via softdevice.  I'll try fixing the cx88-input.c
auto-repeat later based on hg/v4l-dvb and your patches.

	Marko
  

On Thu, Feb 09, 2006 at 09:30:50AM +0200, Marko Mäkelä wrote:
> > All that I can say is that budget-ci is fine...
> 
> cx88 sure isn't.  The tuner module could not be loaded, and vdr drops frames
> when playing recordings via softdevice.  I'll try fixing the cx88-input.c
> auto-repeat later based on hg/v4l-dvb and your patches.

Done; see the video4linux list for details.

I also improved my patch to the Hauppauge Nova-T PCI 90002 driver
in Linux 2.6.15.2.  Now it will discard the first repeated RC5 frame,
effectively doubling the repeat delay from 113.8 milliseconds to
227.6 milliseconds.  The 113.8-millisecond repeat delay of the previous
patch was too short for my wife, and occasionally also too short for me.
The repeat rate remains at 113.8 milliseconds, that is, the built-in
repeat rate (64*T, T=16/9 ms) of the remote control unit.

You can get the updated 2.6.15.2 patch at the following location:
http://www.iki.fi/~msmakela/software/vdr/linux-2.6.15.2-cx88_input2.patch

	Marko