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

Nvidia Shield Controller support #9

Open
scaronni opened this issue Feb 1, 2016 · 9 comments
Open

Nvidia Shield Controller support #9

scaronni opened this issue Feb 1, 2016 · 9 comments

Comments

@scaronni
Copy link

scaronni commented Feb 1, 2016

Hello,

I've just asked for a pull request to the UDEV rules for the Nvidia Shield Controller here:

cyndis/shield-controller-config#2

Actually, this is exactly the opposite issue of the UDEV rules for udev-joystick-blacklist, the controller is seen as a mouse (it has a touchpad) but it should be seen as a joystick.

Do you think it makes sense to be added here or it should be kept separate?

Thanks & regards,
--Simone

@denilsonsa
Copy link
Owner

I don't have a Shield Controller here, so I don't know how it works, and I have to trust people that this fixes the issue. That's fine, anyway, because I only own 2 of the several devices listed in this repository.

I have two main questions:

  • The Nvidia Shield Controller seems to include a touchpad area, and as such I'm inclined to believe it should be recognized as both mouse, keyboard and joystick. Am I wrong? Why being a mouse is an issue? Can you explain me, or maybe point to a video or to some description of such issue?
  • The rules in my repository are in a file numbered 51. The number was chosen on purpose, because it must be run after some initializations, but before some later rules. Your Nvidia Shield Controller rule runs at the very end. Is there any reason? Should/can it run sooner?

If these are cleared up, I don't see much problem adding another udev rule for nvidia shield controller. It can be added as another file or as a rule in the current files. The SDL controller db file seems out-of-place here. If we chose to keep stuff separate, I'm going to put a link to your repository, as I believe it will help other peopl.

@scaronni
Copy link
Author

scaronni commented Feb 1, 2016

I am the Steam maintainer in Fedora, and was playing with a few things that were not working in my setup. There is an issue in Steam, ONLY when running in Big Picture Mode, where the devices that expose multiple input "interfaces" of which one is a joystick, to trigger a lot of ghost touches, movements and whatever, making Steam unusable:

ValveSoftware/steam-for-linux#3384

The Valve bug report is then closed referencing the kernel bug:

https://bugzilla.kernel.org/show_bug.cgi?id=28912

And some people point to this Github project to fix their woes; and this is how I discovered your rules and that I have one of the keyboard on the list. By chance, I also have an Nvidia Shield controller, so while testing, I got the same symptoms, ghost clicks everywhere (usually up left) and two distinct input devices.

By looking around, I've seen that it follows the same logic, so it creates a joystick device and a mouse device; so exactly like the devices in your list except the opposite way: the mouse device should be removed.

So I sent the UDEV rule update to the other repository, and asked myself if everything should be in one place instead of two. Number 99 is simply because it was used in the other project, but it can run anywhere. It's triggered only on that specific USB id insertion.

The conrtroller DB file is only used when adding support for SDL, so it's not related to the UDEV rules and can be omitted.

Do you think we should add the controller rules to your 51*.rules files? Or keep them separate?
I've added both to the Steam package in the meantime as a workaround.

@denilsonsa
Copy link
Owner

Sounds good. I have a couple more questions, now regarding the udev rule itself:

SUBSYSTEM=="input", ENV{ID_MODEL}=="NVIDIA_Controller_v01.03", MODE="0666", ENV{ID_INPUT_JOYSTICK}="1", ENV{ID_INPUT_MOUSE}=""
  • Supposing it is a USB device, do you have the vendor id and the product id? They can be found at lsusb, and I believe that is less fragile than comparing the model name and version.
  • Should I include the MODE="0666" action? Shouldn't it have the appropriate permissions automatically?

I believe a line like this should be good:

SUBSYSTEM=="input", ATTRS{idVendor}=="▓▓▓▓", ATTRS{idProduct}=="▓▓▓▓", ENV{ID_INPUT_JOYSTICK}="1", ENV{ID_INPUT_MOUSE}=""

Can you test it? Any issues? Any feedback?

And thank you for your effort!

@scaronni
Copy link
Author

scaronni commented Feb 1, 2016

Works perfectly, using the vendor/product id is a better idea. Here is the line that I added to your 51-these-are-not-joysticks-rm.rules file:

SUBSYSTEM=="input", ATTRS{idVendor}=="0955", ATTRS{idProduct}=="7210", MODE="0666", ENV{ID_INPUT_JOYSTICK}="1", ENV{ID_INPUT_MOUSE}=""

Mode 666 is required for doing any write to the device, like activating the rumble from the program/game or (if it will ever be the case) update the firmware. The Steam Controller as well has mode 666 as the permissions written in an UDEV rule for the same reasons.

Two questions... permissions or rm rules file? Still the name "these-are-not-joysticks"?

@denilsonsa
Copy link
Owner

I've started a branch to push work-in-progress code regarding this issue.

At first, since it is only one device, I was inclined to keep in the same file, even though the name makes no sense. Should I just rename the file to something else? Maybe fixing-joystick-input-devices? I'm slightly worried about the trouble that renaming this might cause to the users; but I also don't like keeping an incorrect name.

Regarding changing permissions or removing the device, yet again I need your help. What happens with NVIDIA Shield Controller? Does it create multiple devices, even if ID_INPUT_MOUSE is empty? If yes, it may make sense to add different versions, one for each udev rule file.

On the other hand, if things get too specific, we may end up choosing to put the rule in a new file. To tell the truth, I believe you have more information on taking a decision than me. You have the device and you can use udevadm monitor -p to monitor the udev behavior and to better understand what happens.

@scaronni
Copy link
Author

scaronni commented Feb 2, 2016

The plug creates multiple devices also when ID_INPUT_MOUSE is empty, so the best thing to do would probably be to remove the device like you did for the other devices:

SUBSYSTEM=="input", ATTRS{idVendor}=="0955", ATTRS{idProduct}=="7210", KERNEL=="mouse[0-9]*", RUN+="/bin/rm %E{DEVNAME}", ENV{ID_INPUT_MOUSE}="", MODE="0666"

This one works as well and I just have a js* device. So it's very similar except for the permissions settings. Btw, why do you have 2 lines for each device? They do the same thing from my understanding:

SUBSYSTEM=="input", ATTRS{idVendor}=="2516", ATTRS{idProduct}=="001f", ENV{ID_INPUT_JOYSTICK}=="?*", RUN+="/bin/rm %E{DEVNAME}", ENV{ID_INPUT_JOYSTICK}="" SUBSYSTEM=="input", ATTRS{idVendor}=="2516", ATTRS{idProduct}=="001f", KERNEL=="js[0-9]*", RUN+="/bin/rm %E{DEVNAME}", ENV{ID_INPUT_JOYSTICK}=""

For the naming of the files I'm fine with anything that makes sense. "fix-input-devices" may be a good candidate as it's generic enough.

@scaronni
Copy link
Author

scaronni commented Feb 2, 2016

Nevermind, got it. The first one removes the event* device. Ok, so the full lines for the controller should be:

SUBSYSTEM=="input", ATTRS{idVendor}=="0955", ATTRS{idProduct}=="7210", ENV{ID_INPUT_MOUSE}=="?*", RUN+="/bin/rm %E{DEVNAME}", ENV{ID_INPUT_MOUSE}="", ENV{ID_INPUT_JOYSTICK}="1"

SUBSYSTEM=="input", ATTRS{idVendor}=="0955", ATTRS{idProduct}=="7210", KERNEL=="mouse[0-9]*", RUN+="/bin/rm %E{DEVNAME}", ENV{ID_INPUT_MOUSE}="", MODE="0666"

So very close to your, just inverted with the mouse and the mode in the second.

@scaronni
Copy link
Author

scaronni commented Feb 2, 2016

Please ignore my last comments, I've made some tests, if the event device for the mouse ceases to exist, the controller does not work. What you pushed to the other branch is correct, that is the best option.

@denilsonsa
Copy link
Owner

Btw, why do you have 2 lines for each device?

Reason: issue #2.

Ok, so the full lines for the controller should be:

So those lines are at the same time setting MODE="0666" and removing the device. It seems redundant, because there is no need to set the MODE if the device is going to be removed. But it looks like you reached the same conclusion already.

I have one more question: do we really need to set MODE to give permission to all users? Because 70-uaccess.rules and 70-udev-acl.rules already tags the device in a way that should give it appropriate permissions:

# joysticks
SUBSYSTEM=="input", ENV{ID_INPUT_JOYSTICK}=="?*", TAG+="uaccess"
# joysticks
SUBSYSTEM=="input", ENV{ID_INPUT_JOYSTICK}=="?*", TAG+="udev-acl"

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

No branches or pull requests

2 participants