delay and repeat were swapped

[dani-hs]

delay and repeat were swapped

[sezanzeb]

Hi. Could you please elaborate a bit why this change is necessary?

[dani-hs]

Hi sezanzeb,

if I run the following commands in python as root

import evdev
kbd = evdev.InputDevice('/dev/input/event4')
print(kbd.repeat)

the output is

repeat 250, delay 33

With evtest I get the correct data:

Key repeat handling:
Repeat type 20 (EV_REP)
Repeat code 0 (REP_DELAY)
Value 250
Repeat code 1 (REP_PERIOD)
Value 33

I need to know the real values to fix a keyboard problem with my Thinkpad E15 Gen 3 (other Thinkpads are effected as well). On that keyboard there is a second backspace key which sends scancodes for key down and key hold but none for key up. Thus this key behaves sticky in gnome.
To fix this I wrote a script which waits for backspace key down or key hold events. If then no further backspace key related event occurs within REP_DELAY milliseconds or 1/REP_PERIOD seconds the script fires the missing key up event and the keyboard behaves as it should.

[sezanzeb]

Thanks. I can confirm the issue.

Event: time 1737047777.026238, type 1 (EV_KEY), code 30 (KEY_A), value 1
Event: time 1737047777.026238, -------------- SYN_REPORT ------------
Event: time 1737047777.276443, type 1 (EV_KEY), code 30 (KEY_A), value 2
Event: time 1737047777.276443, -------------- SYN_REPORT ------------
Event: time 1737047777.310443, type 1 (EV_KEY), code 30 (KEY_A), value 2
Event: time 1737047777.310443, -------------- SYN_REPORT ------------
Event: time 1737047777.345443, type 1 (EV_KEY), code 30 (KEY_A), value 2

Starts repeating after ~250ms, every ~33ms, while it stores them in reverse:

>>> kbd = evdev.InputDevice('/dev/input/event3')
>>> kbd.repeat
KbdInfo(repeat=250, delay=33)

One the one hand, if someone is using the old order of the tuple in old code, they would assume that KbdInfo[0] is the repeat, and KbdInfo[1] the delay. They would get broken values, but they did follow the python-evdev api correctly. This merge request would fix it for them.

On the other hand, the order of values proposed in this merge request would be consistent with the one reported by EVIOCGREP.

Any thoughts?

[sezanzeb]

No, wait.

So if someone had k = KbdInfo(250, 33), they'd assume (based on docs or python-evdev code or whatever) that k[0] is the repeat, and k[1] the delay. Merging this would not change those values.

If they did k.delay, they used to get 33, and would get 250 now. In this case, this merge request fixes it for them.

If someone knew that it was reverse, they would have done repeat = k.delay. This merge request would break their code, but to my knowledge they didn't report the issue (at least not since I have been maintainer), and it is obviously a bug no matter how you look at it, so I don't think I would really take that into account. Or they would have already done repeat = k[1]. In this case, nothing would change for them.

So I think this is fine. I'll look at this again tomorrow with a clear head.

[sezanzeb]

Or am I missing something? I just need to take people with legacy code into consideration before merging things.

[sezanzeb]

(I'll remove python 3.7 from the ci soon, has reached EOL https://devguide.python.org/versions/)

[dani-hs]

I was confused about the different naming period (evtest) vs repeat (python-evdev).

A repeat rate of 33 per second would mean a period 1s / 33 = 0.03030... s = 30,30 ... ms which is quit close to 33 ms.
So it does not make a big difference, but we should do it correct.

In the source of truth (the kernel headers, https://github.com/torvalds/linux/blob/master/include/uapi/linux/kd.h) I found this:

struct kbd_repeat {
int delay; /* in msec; <= 0: don't change /
int period; / in msec; <= 0: don't change /
/ earlier this field was misnamed "rate" */
};

So the naming and the documentation should be updated as well.

[sezanzeb]

Thanks, that would make sense, but eab6649 is a breaking change. I'll merge the first commit f8ee702 instead. Are you ok with removing the second one from the branch?

[dani-hs]

Yes, I'm fine with just merging the first commit.
I'm totally new to github and I did the commits directly from within the browser but I can't figure out how to remove them. Sorry for that and many thanks for your efforts.

[sezanzeb]

No problem. You could try:

git clone https://github.com/dani-hs/python-evdev.git
cd python-evdev
git checkout patch-1
git reset --hard f8ee702a265e2141ed6827c6a02b7a130c2e3d9c
git push --force

I made a local copy of your branch already, so there isn't really anything you could do wrong.

[dani-hs]

Thanks a lot. It worked.

[sezanzeb]

Thank you, too!