On Thu, Aug 10, 2017 at 03:00:21PM +0100, Sean Young wrote: > On Thu, Aug 10, 2017 at 12:45:42PM +0200, Hans Verkuil wrote: > > On 09/08/17 23:17, Sean Young wrote: > > > On Tue, Aug 08, 2017 at 10:11:13AM +0200, Hans Verkuil wrote: > > >> On 07/08/17 22:58, Sean Young wrote: > > >>> On Mon, Aug 07, 2017 at 03:31:24PM +0200, Hans Verkuil wrote: > > >>>> From: Hans Verkuil <hans.verkuil@xxxxxxxxx> > > >>>> > > >>>> The 'Press and Hold' operation was not correctly implemented, in > > >>>> particular the requirement that the repeat doesn't start until > > >>>> the second identical keypress arrives. The REP_DELAY value also > > >>>> had to be adjusted (see the comment in the code) to achieve the > > >>>> desired behavior. > > >>> > > >>> I'm afraid I've caused some confusion; I had not read your last message > > >>> about autorepeat on irc correctly, when I said "exactly". > > >>> > > >>> So if the input layer has not received a key up event after a key down > > >>> event, after REP_DELAY it will generate another key down event every > > >>> REP_PERIOD. So for example, here I'm holding a button on a rc-5 device > > >>> for some time. Comments on lines with parentheses. > > >>> > > >>> # ir-keytable -t > > >>> Testing events. Please, press CTRL-C to abort. > > >>> 1502138577.703695: event type EV_MSC(0x04): scancode = 0x1e11 > > >>> (each time a driver receives something, scancode is reported.) > > >>> 1502138577.703695: event type EV_KEY(0x01) key_down: KEY_VOLUMEDOWN(0x0072) > > >>> 1502138577.703695: event type EV_SYN(0x00). > > >>> 1502138577.817682: event type EV_MSC(0x04): scancode = 0x1e11 > > >>> 1502138577.817682: event type EV_SYN(0x00). > > >>> (rc-5 repeats the command after 115ms). > > >>> 1502138577.930676: event type EV_MSC(0x04): scancode = 0x1e11 > > >>> 1502138577.930676: event type EV_SYN(0x00). > > >>> 1502138578.044682: event type EV_MSC(0x04): scancode = 0x1e11 > > >>> 1502138578.044682: event type EV_SYN(0x00). > > >>> 1502138578.181690: event type EV_MSC(0x04): scancode = 0x1e11 > > >>> 1502138578.181690: event type EV_SYN(0x00). > > >>> 1502138578.205667: event type EV_KEY(0x01) key_down: KEY_VOLUMEDOWN(0x0072) > > >>> (this is 500ms after the initial key down, so this key down is generated > > >>> by the input layer). > > >>> 1502138578.205667: event type EV_SYN(0x00). > > >>> 1502138578.333667: event type EV_KEY(0x01) key_down: KEY_VOLUMEDOWN(0x0072) > > >>> (this is 500 + 125 ms, so another key down event generated by input layer). > > >>> 1502138578.333667: event type EV_SYN(0x00). > > >>> 1502138578.437662: event type EV_KEY(0x01) key_up: KEY_VOLUMEDOWN(0x0072) > > >>> 1502138578.437662: event type EV_SYN(0x00). > > >>> (key up generated by rc-core after 250ms after last scancode received) > > >>> > > >>> So I think the autorepeat can do exactly what you want, without cec > > >>> having any special code for it. > > >> > > >> It comes close, but not quite, to what I need. It has more to do with the > > >> CEC peculiarities than the rc code. > > >> > > >> Specifically the CEC spec strongly recommends that the first reported key > > >> press is always handled as a single non-repeating key press. Only if a second > > >> identical key press is received within 550 ms will the 'Press and Hold' feature > > >> kick in and will the key start repeating. This until a Release message is > > >> received, a different key press is received or nothing is received for 550 ms. > > > > > > Right, that make sense. > > > > > >> Effectively the REP_DELAY is equal to the time between the first and second > > >> key press message, and it immediately switches to repeat mode once the second > > >> key press is received. > > >> > > >> This code models this behavior exactly. > > > > > > Ok, although you're sending a keyup directly after the first keydown. > > > > Yes, that's to prevent it from going into repeat mode. It shouldn't for > > the first CEC key press message. Remember that CEC is slow, so it may > > well take 500ms before you get the next message. And if REP_DELAY < 500ms > > it will already start repeating which is not what you want for the first > > key press. Calling keyup immediately will prevent this from happening. > > > > > Another way of doing this is to set REP_DELAY/REP_PERIOD to 0 and > > > sending the repeats yourself. > > > > No, because you want the user to be able to influence the repeat speed. > > And the repeat speed is supposed to be that of linux, the repeated CEC > > messages are just to tell linux that it has to remain in key repeating > > mode. I.e. if you receive 5 CEC messages while in Press and Hold mode, > > then that can translate to 20 actual repeats (depending on the REP_PERIOD). > > > > Besides, I don't want to have to write the timer code to repeat the keys > > myself, after all, it's already there. > > Yes, agreed. I don't think the key up is ideal, but the alternatives > are worse. Acked-by: Sean Young <sean@xxxxxxxx> Thanks Sean
