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

Add paddles support for the Elite Series 2 firmware versions < 5.x #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

charlesmunger
Copy link

Tested with firmware 4.7 and evtest. Works in Steam Input when no profile is selected on the controller. Input event codes are the same as what xpad reports for paddles, so most controller profiles should work in both wired and wireless modes.

Tested with firmware 4.7 and evtest. Works in Steam Input when no
profile is selected on the controller. Input event codes are the
same as what xpad reports for paddles, so most controller profiles
should work in both wired and wireless modes.
@Entropy512
Copy link

As architected, this is not extendable without significant rework to support other Elite controllers such as the original Elite

I've added a rework of your commit that is tested to work with the original Elite at https://github.com/Entropy512/xone/commits/paddles/

You can either cherry pick it and add to your PR, or I'll PR a combo of your patch and my rework.

@Riven-Spell
Copy link

Riven-Spell commented May 22, 2024

Adjusted the existing xone-dkms-git PKGBUILD for Arch-based users who'd like to beta test use this work early; credit to Michał Kopeć [email protected]

_pkgname=xone
pkgname=xone-dkms-git
pkgver=makepkg.53964a4
pkgrel=1
pkgdesc='Modern Linux driver for Xbox One and Xbox Series X|S controllers'
arch=('x86_64')
url='https://github.com/Entropy512/xone'
license=('GPL2')
depends=('dkms'
		 'xone-dongle-firmware')
makedepends=('git')
conflicts=('xone-dkms'
		   'xow')
provides=('xone-dkms')
source=("git+https://github.com/Entropy512/xone.git#branch=paddles")
sha256sums=('SKIP')

pkgver() {
  cd "$srcdir/$_pkgname"
  # git describe --long --tags | sed 's/^v//;s/\([^-]*-g\)/r\1/;s/-/./g'
  echo "$(git branch --show-current).$(git rev-parse --short HEAD)"
}

package() {
  cd "${srcdir}/${_pkgname}"

  find . -type f \( -name 'dkms.conf' -o -name '*.c' \) -exec sed -i "s/#VERSION#/$pkgver/" {} +
  echo 'ccflags-y += -DDEBUG' >> "Kbuild"

  echo "* Copying module into /usr/src..."
  install -dm755 "${pkgdir}/usr/src/${_pkgname}-${pkgver}"
  cp -r "${srcdir}/$_pkgname"/* "${pkgdir}/usr/src/${_pkgname}-${pkgver}"

  echo "* Blacklisting xpad module..."
  install -D -m 644 install/modprobe.conf "${pkgdir}/etc/modprobe.d/xone-blacklist.conf"
}

@Entropy512
Copy link

Note that it looks like it's possible to support newer Elite 2 firmwares, but I have no way to test those configurations.

If there's someone with a newer-firmware Elite 2 that wants to try it, I can take a crack at adding it based on xpad's setup.

@Riven-Spell
Copy link

Riven-Spell commented May 22, 2024

I actually do have an Elite 2. I can pop over to Windows and pull the latest firmware and act as a guinea pig. No skin off my back, paddles were never bindable via steam input for me.

@Riven-Spell
Copy link

I actually do have an Elite 2. I can pop over to Windows and pull the latest firmware and act as a guinea pig. No skin off my back, paddles were never bindable via steam input for me.

https://github.com/Riven-Spell/xone/tree/paddles

tried my hand at porting it, but, not real sure where to head from here.

@Entropy512
Copy link

Entropy512 commented May 24, 2024

Yeah the latest Elite firmware seems to be a little more challenging:

An extra init packet needs to be sent: https://github.com/paroj/xpad/blob/master/xpad.c#L612

The equivalent of the already-implemented init packet at https://github.com/paroj/xpad/blob/master/xpad.c#L602 is in bus/protocol.c for xone - https://github.com/medusalix/xone/blob/master/bus/protocol.c#L420 - so something like gip_set_extra_input() would need to be implemented

Then the extra input is in a different packet for the latest Elite2 firmware - https://github.com/paroj/xpad/blob/master/xpad.c#L1090 (GIP_CMD_FIRMWARE is a bit of an odd name for this - definition reuse???) - implementing the equivalent to this appears to be necessary in gip_dispatch_pkt + similar structure to the functions called from there (gip_handle_pkt_input and so on)

I'll take a look at your stuff and try and take a poke at it this afternoon.

Edit: Heh, looks like you already figured that out. Will take a look later. Personally I think they recycled the GIP_CMD_FIRMWARE line so it should probably be called gip_handle_pkt_extinput or something like that.

I STRONGLY disagree with xpad's approach of not allowing a profile to be applied:

  1. If two reports come from a single button, most software will just ignore the report that it's not interested in. It might mess up button-assignment approaches though in some games, so maybe this needs to somehow be an option
  2. There are lots of scenarios where a profile is applied but the paddles are not mapped in the controller, with the intent being to map them host-side. This is EXACTLY my use case, since one of the primary reasons I like to use an Elite controller is to disable the thumbsticks completely and use the lower L/R paddles instead, but also be able to map the upper L/R paddles separately from the RB/LB buttons in some games/applications. People also uses profiles for other things like stick sensitivity mapping, brightness, etc.

I'll see if I still have it around somewhere, but I added a bit of code to print the entire packet in hex, which let me figure out some pesky off-by-one errors. :)

At the current point - now that you've implemented Riven-Spell@cd596fc are you seeing extrainput packets come in? Or are they still not coming in? Maybe the init packet needs to be sent earlier? xpad sends that init packet to Elite2 controllers no matter what firmware revision they are. I'll have to look later, but is the VID/PID set inside the hardware member of the gip_client struct passed to gip_gamepad_probe? That's where GIP_CMD_POWER is sent.

@Riven-Spell
Copy link

I am not seeing the extra input packets come in, unfortunately. I think there might be something wrong with the init packet I sent, or something else along those lines. I have yet to fire both xone and native up under wireshark and go through with a fine-tooth comb.

@medusalix
Copy link
Owner

Yeah the latest Elite firmware seems to be a little more challenging:

An extra init packet needs to be sent: paroj/xpad@master/xpad.c#L612

@Entropy512 I strongly suspect that Microsoft doesn't want to support the paddles as additional buttons on Windows. That's why they don't hesitate to change the offset in the input packet on every new firmware version.
That also explains why the Series 2 Elite Controller requires an extra (client-specific) packet to be sent to activate them on newer firmwares: The actual paddle input is really only used by the Xbox Accessories App. The paddles are meant to be hardware remapped via profiles to arbitrary keys, which is supported by the Elite 2.

I believe hardware-mapped paddles should already work for xone, so I'd suggest that we build something similar to the Xbox Accessories App for Linux instead. That way we can avoid breaking the paddle functionality for every new firmware.

@Riven-Spell
Copy link

@Entropy512 I strongly suspect that Microsoft doesn't want to support the paddles as additional buttons on Windows.

Actually wouldn't be surprised if it was just to screw with us.

I believe hardware-mapped paddles should already work for xone, so I'd suggest that we build something similar to the Xbox Accessories App for Linux instead. That way we can avoid breaking the paddle functionality for every new firmware.

I'd much prefer the paddles were supported as a button. IMO, it's quite clunky/bad UX to have mapping to other controller buttons be the only option. It could be a fallback option, with paddle support being a build flag or something.

Maybe the init packet needs to be sent earlier? xpad sends that init packet to Elite2 controllers no matter what firmware revision they are. I'll have to look later, but is the VID/PID set inside the hardware member of the gip_client struct passed to gip_gamepad_probe? That's where GIP_CMD_POWER is sent.

Not sure. I'll take a look maybe late tonight and see what I can do.

@Riven-Spell
Copy link

I'd much prefer the paddles were supported as a button.

I should clarify. I bought the controller coming from the steam deck, and was mostly hoping I could use the paddles under steam input.

@Entropy512
Copy link

Entropy512 commented Jul 12, 2024

I wound up having to buy an Elite 2 as the USB port on my original Elite was failing, leading to frequent dead batteries due to not charging reliably. It's updated to 5.21, so I had a chance to work on this/needed to fix this last night. Took me about 30 minutes to get it working with your changes as a starting point.

It turns out the issue with your patch was not when the init packet was sent (e.g. Entropy512@d16a4c3 may not be necessary), but that you had the handler in the case block for when HDR_OPT_INTERNAL is set - but input events are handled in a separate case statement outside this block. Also you had an off-by-one error - since indexing starts at 0, an offset of 14 comes after 14 bytes of buffer, not 13.

Entropy512@71b2a95 has things working, but needs to be cleaned up. WARNING: I'm probably going to rebasing/force-pushing this branch until it's cleaned up. I'll probably squash your commits into one, but won't squash them with mine in order to preserve authorship.

Like you I disagree that trying to reimplement the configuration tool on Linux as suggested by @medusalix is a valid replacement for these efforts. After all, my system is dual-boot so I can run the config tool without much hassle, yet I am still putting forth this effort to support the paddles directly because some of my use cases involve mapping paddles to macros separately from any existing buttons. If the config tool alone met my needs I wouldn't be bothering to put in this effort here. It's also why I disagree with xpad's approach of disabling paddle mapping if a profile is enabled, since profiles have MANY more functions than merely paddle mapping.

However in an ideal scenario in the future, paddle handling should be disabled if a paddle has been mapped to another button in hardware. But that will require some additional reverse engineering. I think the consequences of having the raw paddle available when hardware mapping are minimal - basically only a negative when trying to initially bind a hardware-mapped paddle in software with an automatic "press this button to bind" method.

Also, paddle functionality has not been broken for every new firmware. The current setup seems to be stable across multiple firmware revisions from 5.11 up to and including 5.21 (which is the latest as of now)

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

Successfully merging this pull request may close these issues.

4 participants