joshgoebel / keyszer

a smart, flexible keymapper for X11 (a fork/reboot of xkeysnail )
Other
69 stars 15 forks source link

Breaks with latest systemd #137

Open fecet opened 1 year ago

fecet commented 1 year ago

Don't know why, but It makes keyboard unusable with latest systemd version, can confirm systemd 252.4 works fine.

This also happens in xkeysnail.

joshgoebel commented 1 year ago

Any log strangeness or anything else you can provide?

fecet commented 1 year ago

I would like to but don't know to create a debug file, the behavior is like:

For xkeysnail, I can see it do catch my key strike but the system doesn't react to them.

The Maintainer of xkeysnail in AUR also report this https://aur.archlinux.org/packages/xkeysnail.

vladimir-g commented 1 year ago

Hello. I have same issue on archlinux with systemd 253 (I'm that AUR xkeysnail maintainer).

After some experimenting with code I've found that removing 6 (REL_WHEEL) and 8 (REL_HWHEEL) from EV_REL dict item fixes this issue:

https://github.com/joshgoebel/keyszer/blob/f8e50146d35b598bdd8dc0b95fcf906cbfea6849/src/keyszer/output.py#L40-L46

I.e. replacing line 44 with: ecodes.EV_REL: set([0, 1, 9]),

That line also can be removed completely and everything still works. I don't know why these events are listed here (wheels and axes for keyboard?), but I don't know much about Linux input subsystem and this solution may be wrong. I also can't understand how this is related to systemd and what is the cause of the bug.

joshgoebel commented 1 year ago

(wheels and axes for keyboard?)

Some keyboards build in a mouse/trackpad/navigation device - single device. If you aren't using one of those such keyboards, this patch shouldn't hurt anything.

I also can't understand how this is related to systemd and what is the cause of the bug.

Yeah, that's still a mystery.

anaxonda commented 1 year ago

Hello. I have same issue on archlinux with systemd 253 (I'm that AUR xkeysnail maintainer).

After some experimenting with code I've found that removing 6 (REL_WHEEL) and 8 (REL_HWHEEL) from EV_REL dict item fixes this issue:

I tried your solution, unfortunately it did not work for me, on a Thinkpad X1 Carbon. Downgraded systemd as per OP.

vladimir-g commented 1 year ago

I tried your solution, unfortunately it did not work for me, on a Thinkpad X1 Carbon. Downgraded systemd as per OP.

Maybe removing EV_REL line completely may help.

I can confirm that original solution works on Thinkpad Edge E330, although it may be too different from Carbon series (Edge isn't "classic" Thinkpad, but it has trackpoint and has similar-looking keyboard).

fecet commented 1 year ago

This solution works for me, but results in some random unexpected behavior. The keyboard will suddenly freeze then keep pressing one key then I have to unplug this keyboard and kill keyszer.

How can I debug further from there?

vladimir-g commented 1 year ago

I've tried to do some debugging and maybe found source of the issue. For some reason device with REL_HWHEEL and REL_WHEEL have libinput capabilites tablet-pad, but without them they are keyboard pointer (it is from libinput list-devices command output).

When device is detected as tablet-pad, udevadm info displays:

ID_INPUT=1
ID_INPUT_TABLET=1
ID_INPUT_TABLET_PAD=1
ID_INPUT_KEY=1
ID_INPUT_KEYBOARD=1

But with keyboard pointer it is:

ID_INPUT=1
ID_INPUT_MOUSE=1
ID_INPUT_KEY=1
ID_INPUT_KEYBOARD=1

So, there are different set of device properties, and device with tablet-pad capability emits keyboard events (can be seen in evtest) but they are ignored by terminal or other clients. I don't know much about linux input subsystem, but maybe it is intended behavior.

That means something is wrong in device type detection. I think there is only one commit related to detection that was made between systemd 252 and 253: https://github.com/systemd/systemd/commit/0855ce67726f87a5a67b4fb536d58e0e4428a248 Looks like now devices with wheels are also may be classified as tablets.

I don't really know the best way to fix this issue though. In one test I've replaced _KEYBOARD_KEYS = ecodes.keys.keys() - ecodes.BTN with _KEYBOARD_KEYS = set(ecodes.keys.keys()) in output.py and it helped. But this may lead to other bugs with different versions of udev.

joshgoebel commented 1 year ago

But this may lead to other bugs with different versions of udev.

Yeah, the whole reason that line exists is so we get detected as a "keyboard", not a "joystick"... which I presume is bad - this line is before my time. But I'd say if the change works for you with no obvious issues then it's probably safe.

fecet commented 1 year ago

This solution works for me, but results in some random unexpected behavior. The keyboard will suddenly freeze then keep pressing one key the I have to unplug this keyboard and kill keyszer.

How can I debug further from there?

The log looks like

Exception in callback receive_input(InputDevice('...nput/event19')) at /home/rok/.local/lib/python3.10/site-packages/keyszer/input.py:102
handle: <Handle receive_input(InputDevice('...nput/event19')) at /home/rok/.local/lib/python3.10/site-packages/keyszer/input.py:102>
Traceback (most recent call last):
  File "/usr/lib/python3.10/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "/home/rok/.local/lib/python3.10/site-packages/keyszer/input.py", line 117, in receive_input
    on_event(event, device)
  File "/home/rok/.local/lib/python3.10/site-packages/keyszer/transform.py", line 321, in on_event
    if context.x_error:
  File "/home/rok/.local/lib/python3.10/site-packages/keyszer/lib/key_context.py", line 27, in x_error
    self._query_window_context()
  File "/home/rok/.local/lib/python3.10/site-packages/keyszer/lib/key_context.py", line 13, in _query_window_context
    self._X_ctx = get_xorg_context()
  File "/home/rok/.local/lib/python3.10/site-packages/keyszer/xorg.py", line 35, in get_xorg_context
    wm_name = window.get_full_text_property(_display.get_atom("_NET_WM_NAME"))
  File "/usr/lib/python3.10/site-packages/Xlib/xobject/drawable.py", line 486, in get_full_text_property
    prop = self.get_full_property(property, property_type,
  File "/usr/lib/python3.10/site-packages/Xlib/xobject/drawable.py", line 476, in get_full_property
    prop = self.get_property(property, property_type, sizehint,
  File "/usr/lib/python3.10/site-packages/Xlib/xobject/drawable.py", line 455, in get_property
    r = request.GetProperty(display = self.display,
  File "/usr/lib/python3.10/site-packages/Xlib/protocol/rq.py", line 1368, in __init__
    self.reply()
  File "/usr/lib/python3.10/site-packages/Xlib/protocol/rq.py", line 1380, in reply
    self._display.send_and_recv(request = self._serial)
  File "/usr/lib/python3.10/site-packages/Xlib/protocol/display.py", line 612, in send_and_recv
    gotreq = self.parse_response(request)
  File "/usr/lib/python3.10/site-packages/Xlib/protocol/display.py", line 719, in parse_response
    gotreq = self.parse_error_response(request) or gotreq
  File "/usr/lib/python3.10/site-packages/Xlib/protocol/display.py", line 745, in parse_error_response
    req = self.get_waiting_request(e.sequence_number)
AttributeError: 'BadRRModeError' object has no attribute 'sequence_number'

is this relevant to our fix patch @joshgoebel ?

joshgoebel commented 1 year ago

Looks like a pretty weird X11 error... we should probably rescue that as well with all the others in xorg.py.

joshgoebel commented 1 year ago

I'm trying to decide if I wanted to file a bug or issue against systemd exactly how I might do it - obviously this was an intentional change and probably works in most cases for normal devices...

I'd suggest we build the event table for our virtual keyboard dynamically (from looking at the events supported by the real keyboards), but that becomes problematic when you can hot plug keyboards constantly - I don't think you can reconfigure an existing keyboard on the fly to add new types of events...

vladimir-g commented 1 year ago

I'd suggest we build the event table for our virtual keyboard dynamically (from looking at the events supported by the real keyboards), but that becomes problematic when you can hot plug keyboards constantly - I don't think you can reconfigure an existing keyboard on the fly to add new types of events...

Removing and adding virtual device is basically the same hotplug action. So that solution may be ok, although it may require pretty large rewrite.

But I see possible problem here, related to current issue. Users may specify list of devices (like mouse and keyboard) to grab them both, and it looks like valid use case for someone. Combining different capabilities from these devices into single one may result in wrong classification by udev again. Maybe creating a virtual device for every grabbed one? Although this is pretty big rewrite too.

joshgoebel commented 1 year ago

Removing and adding virtual device is basically the same hotplug action.

This also messes up certain keyboard attributes (key repeat speed to name one I'm aware of) that a user has set on the virtual keyboard... so constantly breaking the key repeat speed isn't a great option.

Maybe creating a virtual device for every grabbed one?

Interesting. I was considering a single separate mouse and keyboard, but a proxy output device per input device is certainly an intriguing idea.