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

Add support to rkvm-server for a new configuration option, input-device-paths, which is an array that let's specify which input devices to register with the server #55

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

zodmaner
Copy link

@zodmaner zodmaner commented Nov 8, 2023

Add support to rkvm-server for a new configuration option, input-device-paths, which is an array that let's specify which input devices to register, to the server.toml configuration file.

Leaving the array empty, e.g., input-device-paths = [], means that rkvm will register all detected input devices, which is the same as the current default behavior.

Since it's inconvenient to have user look up which event file (e.g., /dev/input/event1) is associated with which device, users can alternatively pass in symlinks from the folder /dev/input/by-id/ to specify which devices to register with the server.

For example, users can use input-device-paths = [ "/dev/input/by-id/usb-BenQ_ZOWIE_Gaming_Mouse-event-mouse", "/dev/input/by-id/usb-Logitech_Gaming_Keyboard-event-kbd" ] to register a mouse and a keyboard, instead of input-device-paths = [ "/dev/input/event4", "/dev/input/event2" ] (and both are basically equivalent from the perspective of the server).

The motivation behind this pull request came from the fact that, because currently rkvm-server registers all input devices it finds on the server, sometimes it registers input devices that users might not want to share between machines, e.g., a gamepad, or a stylus.

This feature is also useful as a "workaround" for devices which do not play nice with rkvm for one reason or another. (I have one gamepad which, if rkvm-server is running and registered the gamepad, will slow all input from the gamepad to a crawl. Using the feature in this pull request to make rkvm not register the gamepad "fix" the problem completely).

I've been using my own fork of rkvm which has these changes for a couple of days, and things seem to be working fine.
However, I'm quite new to Rust in general, so please excuse any mistakes, and please don't hesitate to tell me to fix any mistakes that you find.

And, lastly, thank you for rkvm. It's a great little program that makes me life better. :)

Modify the server's run function to accept the device_names array
configuration and use it to decide which input devices to register.
@slartibart70
Copy link

i tried your PR, but i get an error message : file open/not found.
Any ideas why? (master branch works fine, though)

@zodmaner
Copy link
Author

Hi there.

i tried your PR, but i get an error message : file open/not found. Any ideas why? (master branch works fine, though)

Hm... that's weird.

Have you add the new entry device-names = [] to the server.toml configuration file before starting rkvm-server?

@zodmaner
Copy link
Author

Ah, yes. I think I can reproduce the issue you're having now, @slartibart70.

The strange thing is, if you restart the server, sometimes the error doesn't happen.

Will look into the cause of this issue Anyway, thanks for notifying me of this issue!

@slartibart70
Copy link

slartibart70 commented Nov 12, 2023

when you are on it, could you also fix the problem with the race condition #46 ?

… be used directly

Resolve user-supplied paths in the `input-device-paths` configuration
array to resolved, absolute paths, so that users can directly use
paths from the `/dev/input/by-id/` in the configuration file.
@zodmaner zodmaner changed the title Add support to rkvm-server for a new configuration option, device-names, which is an array that let's specify which input devices to register Add support to rkvm-server for a new configuration option, input-device-paths, which is an array that let's specify which input devices to register Nov 13, 2023
@zodmaner zodmaner changed the title Add support to rkvm-server for a new configuration option, input-device-paths, which is an array that let's specify which input devices to register Add support to rkvm-server for a new configuration option, input-device-paths, which is an array that let's specify which input devices to register with the server Nov 13, 2023
@zodmaner
Copy link
Author

Okay, after poking around, I've found that my previous approach to specify which input devices to register with the server is flawed (and is the cause of the error discovered by @slartibart70).

Anyway, I've completely revamped how we filter which input devices to register, by moving all the filtering logic into the input monitor, right when rkvm-server open and read input device event files from the /dev/input folder.

This has resulted in, IMHO, a cleaner and less intrusive change, while still preserving the same functionalities. Although there is some different between the earlier version:

The configuration array is now named input-device-paths, which reflects the fact that now users must specify absolute path to input device event files that he or she want to register with the server.

Since using raw absolute path to event files (e.g., /dev/input/event0, /dev/input/event1, etc.) is less than desirable, users can, alternatively, use symlinks in the /dev/input/by-id/ folder instead. Note that these symlinks must be pass as absolute paths in the configuration file.

Anyway, thanks again @slartibart70 for catching the original error, and help make this pull request better. If you have time, I'd really appreciated it if you could try the latest version of this pull request again (and if you find any problems, please don't hesitate to report them).

Also:

when you are on it, could you also fix the problem with the race condition #46 ?

I'll see what I can do, once I have time.

@htrefil
Copy link
Owner

htrefil commented Nov 16, 2023

Thank you for the MR. I was thinking of something similar, but matching device vendor ID, version, name. I think that would be more robust than matching by device paths.

What do you think?

@zodmaner
Copy link
Author

Thank you for the MR. I was thinking of something similar, but matching device vendor ID, version, name. I think that would be more robust than matching by device paths.

What do you think?

Hi! Thank you for the feedback, and I agree with you, that would be more robust.

I see that there is already code in the Interceptor type in rkvm-input for retrieving vendor ID, version, and name of each device using libevdev glue code that you wrote earlier. If you think it's okay, then I'll think I'll adapt these to write a new matching logic when we first open each input devices.

Also, do you have any specific configuration format for specifying an input device in mind? If not, I'll try and come up with something (maybe something like: ${vendor_id}-${version}-${name}).

Anyway, I'll come up with some concrete, initial implementation, so we can discuss the details further.

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.

3 participants