Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wayland support with keyboard emulation and capture using uinput #1679

Merged
merged 16 commits into from
Feb 6, 2025

Conversation

LilleAila
Copy link
Contributor

@LilleAila LilleAila commented Jul 11, 2024

Summary of changes

This adds support for keyboard emulation and capture using uinput. As opposed to #1461, this implements a uinput-based output backend. Uinput is present in most linux distributions, and works at a lower level. This means that it does not rely on a specific protocol, and will work in Wayland, X11 and even the TTY console, as it essentially emulates a keyboard. The drawback to this is that it varies by the software layout, and there is not a way to directly input characters not in the keyboard layout, so it relies on an IME such as iBus or fcitx5. The latter should work fine in window managers like Hyprland or sway from my testing. It assumes the key binding to type unicode is set to the default, which is ctrl+shift+u.

It adds evdev as a new dependency.

This should close #1655 and close #1050

Remaining stuff to do

  • Add tests
    • The already existing test/test_keyboard.py should work with this, and the tests pass when running tox r -e test
  • Add news
  • Implement suppressing in KeyboardCapture, probably by passing the keys to KeyboardEmulation as a workaround
  • Figure out a way to get the system's keyboard layout used by the emulated keyboard
    • There is a config option. The default value is us, and it will still work fine for most people using a non-us keyboard, as long as the alphanumeric keys are in the same positions.
  • Figure out how to install the udev rule automaticaly (KERNEL=="uinput", GROUP="input", MODE="0660", OPTIONS+="static_node=uinput")
    • The rule now gets installed, but the code for it is not the best
  • Resolve error Qt: Wayland does not support QWindow::requestActivate()

Pull Request Checklist

  • Changes have tests
  • News fragment added in news.d. See documentation for details

and added comments about remaining things that have to be done
added suppressing keys and pass through non-suppressed keys
that function is not supported on wayland, and gave this warning:

Qt: Wayland does not support QWindow::requestActivate()
Copy link

@fearofcode fearofcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mkrnr
Copy link
Contributor

mkrnr commented Feb 1, 2025

@LilleAila after merging the latest main into your branch, the CI/CD is failing with:

build/temp.linux-x86_64-cpython-310/xkbcommon._ffi.c:575:10: fatal error: xkbcommon/xkbcommon.h: No such file or directory

Do you have time to have a look?

@LilleAila
Copy link
Contributor Author

@LilleAila after merging the latest main into your branch, the CI/CD is failing with:


build/temp.linux-x86_64-cpython-310/xkbcommon._ffi.c:575:10: fatal error: xkbcommon/xkbcommon.h: No such file or directory

Do you have time to have a look?

This looks like is because the added python dependency xkbcommon, which in turn depends on the system libxkbcommon, which is seemingly not present in the testing environment. I added a note about it under linux/.

I'm not sure how system dependencies like that should be managed.

@LilleAila
Copy link
Contributor Author

I could remove the dependency on xkbcommon completely, rather manually adding in some simpler general, such as "qwerty", "colemak" and "qwertz", etc, removing the need for traversing a more specific one through xkbcommon. That would make the settings item a dropdown instead of a free input.

@mkrnr Does that sound like a good option?

@mkrnr
Copy link
Contributor

mkrnr commented Feb 1, 2025

That's a tricky one. It's of course nice that your current solution supports all xkb layouts. On the other hand, we need to be careful which dependencies we include and how.

I'm afraid I can't make a call on this based on my limited understanding. Let's maybe raise that in the plover-dev discord to get some more opinions on this?

@LilleAila
Copy link
Contributor Author

That's a tricky one. It's of course nice that your current solution supports all xkb layouts. On the other hand, we need to be careful which dependencies we include and how.

I'm afraid I can't make a call on this based on my limited understanding. Let's maybe raise that in the plover-dev discord to get some more opinions on this?

I implemented my idea to see how the code would look. From what i did now, i prefer doing it this way, as i have more control over what happens layout-wise, which i think could avoid bugs related to xkbcommon in the future.

The changes could always be reverted, but this new solution is a better compromise in my opinion.

@LilleAila
Copy link
Contributor Author

The CI fails with this error

ERROR: Could not find a version that satisfies the requirement evdev>=1.4.0 (from versions: none)
ERROR: No matching distribution found for evdev>=1.4.0

Does someone have any idea what could cause this? I believe i have added the new dependency in the correct locations such that pip should be able to resolve it.

@mkrnr
Copy link
Contributor

mkrnr commented Feb 3, 2025

@LilleAila you also need to add it to reqs/constraints.txt

@LilleAila
Copy link
Contributor Author

LilleAila commented Feb 3, 2025

@LilleAila you also need to add it to reqs/constraints.txt

Should that file contain a specific version ==1.8.0, or the same >=1.4.0?

@mkrnr
Copy link
Contributor

mkrnr commented Feb 3, 2025

@LilleAila you also need to add it to reqs/constraints.txt

Should that file contain a specific version ==1.8.0, or the same >=1.4.0?

Needs to be specific. That's the version that will be installed.

@mkrnr mkrnr requested a review from a team February 5, 2025 16:36
Copy link
Contributor

@mkrnr mkrnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for the great work @LilleAila! Many people have been asking for this 🙂

I tested it on ubuntu 24.04 (Wayland) and it works great for me! I tested both qwerty and qwertz. In addition, I did a quick test on windows and macOS, just in case.

We'll certainly get a lot of requests for adding keyboard layouts but let's start with this list.

Just a super trivial request regarding a doc string. Other than that, it's good to go for me 🚀

"""
Calling .activateWindow() on a window results in this error:
Qt: Wayland does not support QWindow::requestActivate()
This function returns a boolean used to determine whether to run activateWindow()
Copy link
Contributor

@mkrnr mkrnr Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trivial: I can imagine this method being used in other situations in the future. So better move the info regarding .activeWindow() inside the two if statements where the .activeWindow() method is called.
The _is_wayland method is so easy to understand that I'd probably not put any docstring.

@LilleAila LilleAila requested a review from mkrnr February 6, 2025 07:36
@mkrnr mkrnr merged commit b12c617 into openstenoproject:main Feb 6, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[wayland]: KDE on Wayland Not working with Gnome on Wayland in Linux
3 participants