htrefil / rkvm

Virtual KVM switch for Linux machines
MIT License
386 stars 50 forks source link

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 zodmaner opened 8 months ago

zodmaner commented 8 months ago

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. :)

slartibart70 commented 8 months ago

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

zodmaner commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

zodmaner commented 8 months ago

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 commented 7 months ago

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 commented 7 months ago

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.