feat(vacs-client): support joystick input#768
Conversation
Add gamepad/joystick button support for push-to-talk and other input codes using the gilrs crate. Introduces an InputCode enum (Key|Joystick) with parsing and display helpers and switches TransmitConfig to use InputCode. Implements a tauri command to capture a joystick button from the frontend and updates the frontend KeyCapture to call it. Keybind engine now merges keyboard and joystick events via a forwarded channel, spawns a blocking joystick poll loop that maps gilrs buttons to sentinel F13..F30 codes, and handles task shutdown. Adds gilrs to Cargo.toml and registers the new tauri command.
MorpheusXAUT
left a comment
MorpheusXAUT
left a comment
There was a problem hiding this comment.
Hi! Sorry for the delay, didn't get around to taking a closer look until now.
Firstly, thanks for the work, but I'm afraid that in the current state, this is not something we can merge.
I'll summarize more general feedback and will comment on more specific parts in code where relevant.
In general, I would prefer to do a "proper" refactor of our capture system instead of "hacking" this on top just to make it work. In the long run, this will just be a lot of extra headache to maintain.
Secondly, this does not work on Linux at all, which is somewhat to be expected, but definitely a downside we need to think about. Whilst gilrs is linux capable, it access /dev/input/event*, which requires elevated permissions or specific udev rules or other changes that are not something I just want to add without careful consideration. Running vacs as a superuser constantly is not an option for me right now. I've been considering options for a while, but haven't come to a good solution yet.
I'm sure there's something else I'm missing, but that's all off the top of my head right now.
In general, once again, it'd be great to discuss large changes like this beforehand so we can agree on an architecture/implementation and avoid lengthy reworks.
Thanks again for the work in advance, though, appreciated!
| TransmitMode::PushToTalk => Some(Code::F33), | ||
| TransmitMode::PushToMute => Some(Code::F34), | ||
| TransmitMode::RadioIntegration => Some(Code::F35), |
There was a problem hiding this comment.
This does not compile under Linux. Aside from the general build failure in CI due to missing build dependencies (which would need an update of ci-rust.yml and similar), the change of return type is missing here.
| pub struct KeybindsConfig { | ||
| /// Key code to accept an incoming call. | ||
| pub accept_call: Option<Code>, | ||
| /// Key code to end an active call. | ||
| pub end_call: Option<Code>, | ||
| /// Key code to toggle radio prio during an active call. | ||
| pub toggle_radio_prio: Option<Code>, | ||
| } |
There was a problem hiding this comment.
This is missing support for additional input devices. If we support binding them for transmit config, we should do so for the other keybinds of vacs as well.
| toml::Value::String(s) => val_str = Some(s), | ||
| toml::Value::Integer(n) => val_int = Some(n as u8), |
There was a problem hiding this comment.
We're relying on toml::Value here, which would make the general deserialization of serde potentially fail for JSON (or any other supported) input.
Our config is TOML, but serialization for tauri's IPC is JSON for instance. It works, since these formats don't necessarily differ with their string/int serialization, but we should try to stay format agnostic here for future compatibility.
| /// We use F13–F30 (18 buttons) as they exist in keyboard_types | ||
| /// and are never produced by real keyboards. F31–F35 are reserved | ||
| /// for Wayland portal shortcuts so we stay below F31. | ||
| fn gilrs_button_to_code(button: gilrs::Button) -> Option<Code> { |
There was a problem hiding this comment.
With the current Wayland implementation using a handful of "fake" keys, the risk of conflict of some user binding their own (unused) keys to something like a mouse side button is low, but if we bind the whole range from F13 to F36, we might run into some actual binding conflicts.
Additionally, this blocks us from ever extending keybind support for Wayland, effectively leaving us with just what we have right now, or the need to reduce these bindings in the future, leading to potential conflicts or invalid configuration.
| gilrs::Button::DPadDown => 15, | ||
| gilrs::Button::DPadLeft => 16, | ||
| gilrs::Button::DPadRight => 17, | ||
| _ => return None, |
There was a problem hiding this comment.
From a quick test on the hardware I have at home (throttle, joystick), the range of buttons actually mappable is severely limited and quite unintuitive from a user perspective. I'm aware that's mostly due to the way gilrs maps these device buttons internally, but I'm not sure it'll make for good UX.
The actual gamepad I have at home (8Bitdo Ultimate 2C controller) strangely enough failed to bind even a single button, btw.
| return Ok::<Option<String>, Error>(None); | ||
| } | ||
|
|
||
| while let Some(gilrs::Event { event, .. }) = gilrs.next_event() { |
There was a problem hiding this comment.
In order to avoid the classic nuisance of some joystick, rudder, throttle or latched switch input triggering immediately, we need some way to either exclude devices or other option to make sure we can properly press the desired button...
| self.spawn_rx_loop(rx); | ||
| // Create a merged channel that both keyboard and joystick feed into. | ||
| // The main rx loop only needs to know about KeyEvent — source doesn't matter. | ||
| let (merged_tx, merged_rx) = tokio::sync::mpsc::unbounded_channel::<KeyEvent>(); |
There was a problem hiding this comment.
I'd prefer to use a bounded channel and handle errors gracefully to avoid memory runoff if the consumer dies. We can put a decently high limit, but shouldn't run into issues anyway since our receiving end doesn't block for long anyways.
|
|
||
| /// Forward keyboard events from the platform listener into the merged channel. | ||
| /// This lets the joystick task share the same channel without touching the rx loop. | ||
| fn spawn_forward_task( |
There was a problem hiding this comment.
Since this basically just receives from one channel to another, we can just write to the original tx directly as it's all the same type and save unnecessary latency/complexity.
| // --------------------------------------------------------------------------- | ||
|
|
||
| fn joystick_sentinel(n: u8) -> Code { | ||
| format!("F{}", 13 + n) |
There was a problem hiding this comment.
This should be a const lookup table instead of a new string allocation on every capture etc.
| dashmap = "6.1.0" | ||
| flate2 = "1.1.9" | ||
| futures-util = "0.3.32" | ||
| gilrs = "0.11.1" |
There was a problem hiding this comment.
From the documentation:
Windows Gaming Input requires an in focus window to be associated with the process to receive events. You can still switch back to using xInput by turning off default features and enabling the xinput feature.
Might have to enable thexinputfeature here to get global captures. A quick test on Window showed that it works sometimes, but I wasn't able to get consistent global PTT behavior while vacs was not in focus.
2 participants
Solves #69.
Implemented gamepad support via gilrs by mapping joystick buttons to F13–F30 sentinel codes, allowing for controller-based PTT without a full event-engine rewrite.
Open to feedback regarding how to display the selection in a nicer way