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

[linux] Add DualShock 3 button map for 3.14+ kernels #143

Merged
merged 1 commit into from Jul 29, 2018

Conversation

@psyke83
Copy link

commented Apr 3, 2018

Kernel 3.14 and later's hid-sony driver uses a new button mapping for DS3 pads.
Thankfully, the axis/button count has been changed, so the button map should not
interfere with older kernels that would otherwise enumerate the same device name.

@garbear

This comment has been minimized.

Copy link
Member

commented Apr 3, 2018

I'm glad I used button/axis count to differentiate controllers! You've mapped the triggers to digital buttons. This works, but I suspect they can be mapped to axes for analog events. Can you try the instructions here: https://kodi.wiki/view/HOW-TO:PlayStation_4_controller ?

@psyke83

This comment has been minimized.

Copy link
Author

commented Apr 3, 2018

Ah yes, I overlooked the issue with the analog triggers; in this PR I simply replaced all values using the old DS3 configuration as a template.

I gave a quick test, and the "ignore input" works as expected. After a quick remap via the interface, here's the diff:

--- Sony_PLAYSTATION_R_3_Controller_17b_6a.xml  2018-04-03 04:21:03.845236844 +0000
+++ /home/pi/.kodi/userdata/addon_data/peripheral.joystick/resources/buttonmaps/xml/linux/Sony_PLAYSTATION_R_3_Controller_17b_6a.xml    2018-04-03 04:40:31.159456293 +0000
@@ -1,6 +1,12 @@
 <?xml version="1.0" ?>
 <buttonmap>
     <device name="Sony PLAYSTATION(R)3 Controller" provider="linux" buttoncount="17" axiscount="6">
+        <configuration>
+            <axis index="2" center="-1" range="2" />
+            <axis index="5" center="-1" range="2" />
+            <button index="6" ignore="true" />
+            <button index="7" ignore="true" />
+        </configuration>
         <controller id="game.controller.default">
             <feature name="a" button="0" />
             <feature name="b" button="1" />
@@ -16,7 +22,7 @@
                 <left axis="-0" />
             </feature>
             <feature name="leftthumb" button="11" />
-            <feature name="lefttrigger" button="6" />
+            <feature name="lefttrigger" axis="+2" />
             <feature name="right" button="16" />
             <feature name="rightbumper" button="5" />
             <feature name="rightstick">
@@ -26,7 +32,7 @@
                 <left axis="-3" />
             </feature>
             <feature name="rightthumb" button="12" />
-            <feature name="righttrigger" button="7" />
+            <feature name="righttrigger" axis="+5" />
             <feature name="start" button="9" />
             <feature name="up" button="13" />
             <feature name="x" button="3" />

Do you really prefer to use the analog events, though? I'm currently working on DS3 integration for RetroPie, and I've also seen conflicts in our EmulationStation frontend when mapping the shoulder buttons, but the solution I opted for was to manually set a high fuzz on the analog axes to effectively disable the analog inputs: https://github.com/psyke83/sixaxis/blob/master/sixaxis-helper.sh#L54

The script above is still a work in progress, aimed to do basic axis calibration and set up a very simple 10 minute idle timeout handler, as the BlueZ sixaxis plugin doesn't respect the Bluetooth IdleTimeout setting correctly. That's not important to you, but if you use analog events for this Kodi mapping, it will mean that my script causes the shoulder buttons not to work in Kodi, as only the digital events will be registered.

@garbear

This comment has been minimized.

Copy link
Member

commented Apr 3, 2018

Kodi opportunistically uses analog axes in all emulators that support them. If the emulator doesn't, Kodi will convert analog to digital (threshold). If the emulator supports analog and Kodi sees digital buttons, it will convert the button to 0.0/1.0 for the emulator.

So in most cases it doesn't matter. Only if the emulator has analog trigger support AND physical triggers are mapped to axes. In this case, the only difference is the user can control the axis amount instead of only having 0.0/1.0 values. You should decide if this edge case is worth it.

@psyke83 psyke83 force-pushed the psyke83:master branch from e4e1e00 to 45d03af Apr 3, 2018

@psyke83

This comment has been minimized.

Copy link
Author

commented Apr 3, 2018

OK, I've updated the mapping to use the analog axes and will try to figure out a better solution to the mapping issues in non-Kodi programs on the RetroPie end. Thanks.

@garbear

This comment has been minimized.

Copy link
Member

commented Apr 3, 2018

The issue is only for mapping triggers in ES, right? Could they solve this the same way I did?

@psyke83 psyke83 closed this Apr 3, 2018

[linux] Add DualShock 3 button map for 3.14+ kernels
Kernel 3.14 and later's hid-sony driver uses a new button mapping for DS3 pads.
Thankfully, the axis/button count has been changed, so the button map should not
interfere with older kernels that would otherwise enumerate the same device name.

@psyke83 psyke83 reopened this Apr 3, 2018

@psyke83 psyke83 force-pushed the psyke83:master branch from 45d03af to 513a308 Apr 3, 2018

@psyke83

This comment has been minimized.

Copy link
Author

commented Apr 3, 2018

(Closed briefly as I realized that I missed the N64 profile's "z" left trigger mapping, but it's now changed to the axis map too. )

Yes, I think we can come up with a solution in EmulationStation. It's probably best to use the analog axes. Since the DS4 has the same issue, it's probably for the best that a solution is devised rather than ignoring it, but the new hid-sony driver is still quite troublesome due to enumerating a different device node for the sensors; it causes emulators such as RetroArch to incorrectly detect a single controller as player 2. I also planned to work around that by manually building the driver with the sensor node disabled, since we don't really need gyro sensors etc. in most cases.

@garbear

This comment has been minimized.

Copy link
Member

commented Apr 3, 2018

it causes emulators to incorrectly detect a single controller as player 2

In Kodi we simply ignore a device if it's not mapped. Wouldn't this be simpler than modifying the driver to ignore the sensors?

@psyke83

This comment has been minimized.

Copy link
Author

commented Apr 3, 2018

Actually, I may have been mistaken. It may be the case that the first pad would be assigned as player 1, and the second pad as player 3 (and can be verified by the pads' lit LEDs). So it's an issue with the driver too, since no userspace software actually sets the LED assignment.

Here's the kernel log:

[   28.055969] input: Sony PLAYSTATION(R)3 Controller Motion Sensors as /devices/platform/soc/3f201000.serial/tty/ttyAMA0/hci0/hci0:11/0005:054C:0268.0005/input/input2
[   28.057197] input: Sony PLAYSTATION(R)3 Controller as /devices/platform/soc/3f201000.serial/tty/ttyAMA0/hci0/hci0:11/0005:054C:0268.0005/input/input1

The motion sensors is being seen as player 2 in emulators, and connecting a second pad will light LED #3 (presumably also assigning player 4 as the motion sensors on that pad too).

@garbear

This comment has been minimized.

Copy link
Member

commented Apr 3, 2018

The question is, should player management be done by the driver or by the frontend? On windows with XInput, it's done by the driver. However, XInput doesn't f*k up often.

IMO player management is something that must be done by the frontend. This must be abstracted from driver code. Adding player management logic to a driver is breaking this abstraction.

@psyke83

This comment has been minimized.

Copy link
Author

commented Apr 3, 2018

To be fair, Windows doesn't officially support DS3 pads at all via inbox drivers. Sony does provide a USB driver, but they only package it with their paid PS Now service, and of course it doesn't support Bluetooth. On Windows, users usually have to resort to userspace wrappers such as SCPToolkit to get their pads usable.

I'll have to look into a better workaround to the player numbering - I'd be OK with implementing a workaround in userspace if it's indeed possible, but I still think that the hid-sony driver might be the right place to fix the LED numbering on the actual pad, as the new driver has changed the behaviour due to the splitting of input nodes.

The only reason why I was planning to patch the kernel driver was because it's already necessary to backport a patch from the 4.15 kernel for the hid-sony driver (to prevent eternal USB rumbling with third-party Shanwan pads). The overall plan is to replace the "ps3controller" (sixad userspace driver) entirely since recent BlueZ versions support third-party controller pairing, but my changes haven't been committed yet, so it's no big deal.

@garbear

This comment has been minimized.

Copy link
Member

commented Apr 3, 2018

Ha, forgot the sorry state of non-XInput controllers on windows.

Yea, driver work sounds necessary. Can LEDs be controlled from userspace?

Speaking of player management, I've put a lot of thought into this: garbear/xbmc#87. You can see the depth-first strategy for auto-configure in the second image.

Do you have any ideas for player number management? I put that task on hold, but I'd like to resume before release.

@psyke83

This comment has been minimized.

Copy link
Author

commented Apr 3, 2018

The leds are accessible via userspace, yes:

/sys/devices/platform/soc/3f201000.serial/tty/ttyAMA0/hci0/hci0:11/0005:054C:0268.0006/leds/0005:054C:0268.0006::sony4/brightness
/sys/devices/platform/soc/3f201000.serial/tty/ttyAMA0/hci0/hci0:11/0005:054C:0268.0006/leds/0005:054C:0268.0006::sony2/brightness
/sys/devices/platform/soc/3f201000.serial/tty/ttyAMA0/hci0/hci0:11/0005:054C:0268.0006/leds/0005:054C:0268.0006::sony3/brightness
/sys/devices/platform/soc/3f201000.serial/tty/ttyAMA0/hci0/hci0:11/0005:054C:0268.0006/leds/0005:054C:0268.0006::sony1/brightness

I'll have to delve into the code to see how the LEDs are assigned; perhaps there's a simple way for the motion sensor node to be ignored by the driver's default LED selection.

@psyke83 psyke83 referenced this pull request Apr 11, 2018
@garbear

This comment has been minimized.

Copy link
Member

commented Jul 23, 2018

I just found this open PR, should it be merged?

@psyke83

This comment has been minimized.

Copy link
Author

commented Jul 29, 2018

Apologies for the delay. The PR should be OK to merge - it's using the analog trigger mappings instead of the digital buttons, as you requested.

@garbear

This comment has been minimized.

Copy link
Member

commented Jul 29, 2018

No worries, I'm the one who forgot about this for 4 months :) thanks!

@garbear garbear merged commit 08e4a1a into xbmc:master Jul 29, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mirh

This comment has been minimized.

Copy link

commented Jul 29, 2018

Ehrm.. Friendly reminder that 4.12 reinvented the wheel altogether in sony-hid

@psyke83

This comment has been minimized.

Copy link
Author

commented Jul 29, 2018

@mirh - whoops. I was testing directly on the Pi, where the firmware jumped from a 4.9 kernel directly to 4.14, so I assumed the mappings changed in that revision. Not sure how I managed to state the wrong major revision as well.

Thanks @garbear. The file itself should be fine, but considering the versioning, if you want to revert the change, I'd be happy to resubmit a new PR with the correct kernel version stated in the commit topic/message.

@garbear

This comment has been minimized.

Copy link
Member

commented Jul 29, 2018

don't really care about the version in the commit message, so it's all good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.