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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linux: add support for more virtual input devices #2315

Closed
wants to merge 10 commits into from

Conversation

ABeltramo
Copy link
Contributor

Description

I had to rename the branch for CI to work, this is a continuation of #2261

Added support for the followings new virtual devices:

  • Nintendo Switch pro joypad
  • DualSense (PS5) joypad, including:
    • Gyroscope
    • Accelerometer
    • Touchpad
    • LED
    • Battery status
    • Force feedback? (is this supported in Moonlight already? I have to check..)
    • Microphone and Headset jack? (we should be able to plug into that as well, to be checked..)

And generally completely refactored all virtual input code using inputtino as a library

Requirements

The DualSense implementation requires access to /dev/uhid, (more on this later..) this is easily done by setting the right permissions, similar to how we deal with udev already:

First we鈥檒l add our user to the input group:

sudo usermod -a -G input $USER

Then we add an udev rule to allow access to /dev/uhid:

echo 'KERNEL=="uhid", GROUP="input", MODE="0660"' | sudo tee /etc/udev/rules.d/99-uhid.rules

Finally, we have to make sure that the uhid kernel module is loaded at boot:

echo "uhid" | sudo tee /etc/modules-load.d/uhid.conf

Code details

The main file is src/platform/linux/inputtino.cpp I've kept the original input.cpp so that we can easily compare the two implementations and possibly add what's missing but the idea would be to completely replace that implementation with this. To switch from one implementation to the other you can just set the Cmake variable -DSUNSHINE_USE_INPUTTINO=OFF.

Unfortunately we have to deal with uhid in order to properly emulate gyro, acceleration and touchpad; there's a bit of a rationale into why this is not possible with udev here. This might be a deal breaker for you guys but I hope it's not; if that's the case, we can still emulate a PS5 joypad using udev without supporting gyro and acceleration.

I think bringing in uhid has also other benefits, since it sits at a lower level than uinput, we can really emulate any kind of usb device and possibly even completely integrate usb over IP if this will ever be considered as an addition to the protocol.

As for the code itself, Inputtino has got unit tests using libinput for mouse, keyboard, pen and touchscreen and SDL2 for testing joypads. These aren't complete (yet) but should cover most of the codebase.

Future plans

  • I'd like to give it a go at implementing gyro and acceleration support for a Nintendo joypad too.
  • Ideally, since we are emulating the real joystick USB messages, this should also be a good foundation for a Windows virtual driver implementation. I know nothing about this but I'd like to do more research on it and possibly add a multiplatform implementation to inputtino.
    • I know of the EV license and all the requirements, for some reason it's hard to convince companies to invest in this without a functioning prototype first 馃槄
  • Add support for trackpads in Moonlight? Generally it seems to not be the same as a touchscreen: it moves relatively instead of absolutely and has got different gestures.

What's missing?

Most notably the XTest fallback that is present in input.cpp if this is something that is used we can easily add it here as well.
Everything else should be implemented, please let me know if something is missing or misbeaving.

I'm probably forgetting something else..

Issues Fixed or Closed

Might help with #1720

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Dependency update (updates to dependencies)
  • Documentation update (changes to documentation)
  • Repository update (changes to repository files, e.g. .github/...)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated the in code docstring/documentation-blocks for new or existing methods/components

Branch Updates

LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch
must be updated before it can be merged. You must also
Allow edits from maintainers.

  • I want maintainers to keep my branch updated

Copy link

codecov bot commented Mar 28, 2024

Codecov Report

Attention: Patch coverage is 0.86580% with 229 lines in your changes are missing coverage. Please review.

Project coverage is 6.76%. Comparing base (cd46565) to head (202b09f).

Additional details and impacted files
@@            Coverage Diff             @@
##           nightly   #2315      +/-   ##
==========================================
+ Coverage     6.51%   6.76%   +0.25%     
==========================================
  Files           85      85              
  Lines        18382   17686     -696     
  Branches      8348    8009     -339     
==========================================
- Hits          1197    1196       -1     
+ Misses       15357   14662     -695     
  Partials      1828    1828              
Flag Coverage 螖
Linux 4.75% <0.86%> (+0.33%) 猬嗭笍
Windows 2.00% <酶> (酶)
macOS-12 8.73% <酶> (-0.13%) 猬囷笍
macOS-13 8.07% <酶> (-0.02%) 猬囷笍
macOS-14 8.39% <酶> (酶)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage 螖
src/platform/linux/inputtino.cpp 0.86% <0.86%> (酶)

... and 2 files with indirect coverage changes

@ReenigneArcher
Copy link
Member

I think for anyone to test this it will need the config ui updated to handle the options.

Should be basically this #2261 (comment) ... except slightly different now that localization was added (https://docs.lizardbyte.dev/projects/sunshine/en/nightly/contributing/localization.html#extraction)

Or, I could update it if you want?

@ABeltramo
Copy link
Contributor Author

Thanks for the offer!
I have to look at how the UI works, but it shouldn't be a hard requirement since Moonlight will already tell which joypad type should be emulated. My understanding is that the UI option is only for forcing a specific type to be picked instead, but admittedly, I haven't looked at this too much.

I've been focusing on testing my real PS5 pad against the virtual one, and that's already being picked up without changing anything in the UI.

@ReenigneArcher
Copy link
Member

My understanding is that the UI option is only for forcing a specific type to be picked instead

Yea, there's an automatic mode. I personally wouldn't be able to test anything other than Xbox using auto mode. Does Moonlight send anything other than xbox360 or ds4 though?

@ABeltramo
Copy link
Contributor Author

Yep, the 3 supported types are currently Xbox, PS and Nintendo. I've personally tested Xbox and PS whilst the Nintendo implementation has been tested by a user using Wolf (plus unit tests for all of them using SDL).
With the force option, I should be able to test them all, I should probably work on it sooner rather than later..

Comment on lines +161 to +165
"gamepad_ds5": "DS5 (PS5)",
"gamepad_switch": "Nintendo Pro (Switch)",
"gamepad_manual": "Manual DS4 options",
"gamepad_x360": "X360 (Xbox 360)",
"gamepad_xone": "XOne (Xbox One)",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will the localization library automatically fallback to EN if missing or should I copy this keys also in the other json files?

Copy link
Member

Choose a reason for hiding this comment

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

Yea, it will fallback to the en.json.

@@ -147,7 +147,7 @@ <h1 class="my-4">{{ $t('config.configuration') }}</h1>
<div class="form-text">{{ $t('config.controller_desc') }}</div>
</div>

<!-- Emulated Gamepad Type -->
<!-- Emulated Gamepad Type (Windows only) -->
Copy link
Member

Choose a reason for hiding this comment

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

We can have one emulated gamepad block that covers Windows and Linux.

        <!-- Emulated Gamepad Type -->
        <div class="mb-3" v-if="config.controller === 'enabled' && platform !== 'macos'">
          <label for="gamepad" class="form-label">{{ $t('config.gamepad') }}</label>
          <select id="gamepad" class="form-select" v-model="config.gamepad">
            <option value="auto">{{ $t('_common.auto') }}</option>
            <option value="ds4" v-if="platform === 'windows'">{{ $t('config.gamepad_ds4') }}</option>
            <option value="ds5" v-if="platform === 'linux'">{{ $t('config.gamepad_ds5') }}</option>
            <option value="switch" v-if="platform === 'linux'">{{ $t('config.gamepad_switch') }}</option>
            <option value="x360" v-if="platform === 'windows'">{{ $t('config.gamepad_x360') }}</option>
            <option value="xone" v-if="platform === 'linux'">{{ $t('config.gamepad_xone') }}</option>
          </select>
          <div class="form-text">{{ $t('config.gamepad_desc') }}</div>
        </div>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I've started with that and then refactored like it is now.
I think it's cleaner with two separate blocks since they don't share any option, but I'd be happy to switch it back if you prefer it that way..

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer the single block since it's less code to maintain, lol

Copy link
Member

Choose a reason for hiding this comment

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

The config layout has been refactored. @Hazer can provide guidance if needed.

@Hazer
Copy link
Member

Hazer commented May 29, 2024

@ReenigneArcher @ABeltramo I'm taking over this PR to #2606 so I can update the PR to allow for merging.

@ABeltramo Can you review my changes to your PR, just to be sure I didn't hurt your work?

@ABeltramo
Copy link
Contributor Author

ABeltramo commented May 29, 2024

@Hazer thanks for picking this up, I thought nobody was interested in this..

I think we should automatically fallback in case some requirements are missing, for example:

  • uhid not available -> uinput
  • uinput not available -> xinput
  • xinput not available -> no input I guess..

This would let us cover more use cases and allow people that don't want to expose uhid and/or uinput (to tighten security/on unprivileged containers or VMs) to still be able to run a session with just mouse and keyboard over Xorg. I believe there's no equivalent solution for Wayland, but I haven't looked too much into it..

Apart from that, there's still some quirk over gyro and acceleration on my PS5 virtual joypad but everything else seems to work, have you had a chance to run it on your end too?

@ReenigneArcher
Copy link
Member

Closing, replaced by #2606

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants