denilsonsa / udev-joystick-blacklist

Fix for keyboard/mouse/tablet being detected as joystick in Linux
118 stars 27 forks source link

Nvidia Shield Controller support #9

Open scaronni opened 8 years ago

scaronni commented 8 years ago

Hello,

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

https://github.com/cyndis/shield-controller-config/pull/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 commented 8 years ago

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:

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

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:

https://github.com/ValveSoftware/steam-for-linux/issues/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 commented 8 years ago

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}=""

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

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

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

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

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

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

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"