Re: XBox One gamecontroller support
On Sun, Mar 20, 2022 at 05:00:13PM -0600, Thomas Frohwein wrote: > I updated the diff for the controller with your diff. Below is the > complete diff for all the files involved. I tested it again with my > controller and sdl-jstest (in ports); it continues to work as intended. > > ok? Thanks! Ok by me.
Re: XBox One gamecontroller support
On Sat, Mar 19, 2022 at 09:15:23PM +0100, Stefan Sperling wrote:
[...]
> > + /* XBox One controller initialization */
>
> Shouldn't this initialization code be under #ifndef SMALL_KERNEL,
> like the rest of the code your patch is adding to this file?
>
> > + if (sc->sc_flags & UHIDEV_F_XB1) {
> > + uint8_t init_data[] = { 0x05, 0x20, 0x00, 0x01, 0x00 };
> > + int init_data_len = sizeof(init_data);
>
> I would use 'size_t init_data_len' instead of 'int init_data_len'.
> Largely a matter of taste, but this way everything stays unsigned.
> usbd_setup_xfer() expects an unsigned int.
[...]
> Both of the above suggestions as a diff:
[...]
I updated the diff for the controller with your diff. Below is the
complete diff for all the files involved. I tested it again with my
controller and sdl-jstest (in ports); it continues to work as intended.
ok?
Index: uhid_rdesc.h
===
RCS file: /cvs/src/sys/dev/usb/uhid_rdesc.h,v
retrieving revision 1.1
diff -u -p -r1.1 uhid_rdesc.h
--- uhid_rdesc.h25 Oct 2013 03:09:59 - 1.1
+++ uhid_rdesc.h20 Mar 2022 22:41:04 -
@@ -273,3 +273,93 @@ static const uByte uhid_xb360gp_report_d
0x81, 0x01,/* INPUT (Constant)*/
0xc0, /* END COLLECTION */
};
+
+static const uByte uhid_xbonegp_report_descr[] = {
+0x05, 0x01,/* USAGE PAGE (Generic Desktop)
*/
+0x09, 0x05,/* USAGE (Gamepad)
*/
+0xa1, 0x01,/* COLLECTION (Application)
*/
+/* Button packet */
+0xa1, 0x00,/* COLLECTION (Physical)
*/
+0x85, 0x20,/* REPORT ID (0x20)
*/
+/* Skip unknown field and counter */
+0x09, 0x00,/* USAGE (Undefined)
*/
+0x75, 0x08,/* REPORT SIZE (8)
*/
+0x95, 0x02,/* REPORT COUNT (2)
*/
+0x81, 0x03,/* INPUT (Constant, Variable,
Absolute) */
+/* Payload size */
+0x09, 0x3b,/* USAGE (Byte Count)
*/
+0x95, 0x01,/* REPORT COUNT (1)
*/
+0x81, 0x02,/* INPUT (Data, Variable, Absolute)
*/
+/* 16 Buttons: 1-2=Unknown, 3=Start, 4=Back, 5-8=ABXY,
+ *9-12=D-Pad(Up,Dn,Lt,Rt), 13=LB, 14=RB, 15=LS, 16=RS
+ */
+/* Skip the first 2 as they are not used */
+0x75, 0x01,/* REPORT SIZE (1)
*/
+0x95, 0x02,/* REPORT COUNT (2)
*/
+0x81, 0x01,/* INPUT (Constant)
*/
+/* Assign buttons Start(7), Back(8), ABXY(1-4) */
+0x15, 0x00,/* LOGICAL MINIMUM (0)
*/
+0x25, 0x01,/* LOGICAL MAXIMUM (1)
*/
+0x95, 0x06,/* REPORT COUNT (6)
*/
+0x05, 0x09,/* USAGE PAGE (Button)
*/
+0x09, 0x08,/* USAGE (Button 8)
*/
+0x09, 0x07,/* USAGE (Button 7)
*/
+0x09, 0x01,/* USAGE (Button 1)
*/
+0x09, 0x02,/* USAGE (Button 2)
*/
+0x09, 0x03,/* USAGE (Button 3)
*/
+0x09, 0x04,/* USAGE (Button 4)
*/
+0x81, 0x02,/* INPUT (Data, Variable, Absolute)
*/
+/* D-Pad */
+0x05, 0x01,/* USAGE PAGE (Generic Desktop)
*/
+0x09, 0x01,/* USAGE (Pointer)
*/
+0xa1, 0x00,/* COLLECTION (Physical)
*/
+0x75, 0x01,/*REPORT SIZE (1)
*/
+0x15, 0x00,/*LOGICAL MINIMUM (0)
*/
+0x25, 0x01,/*LOGICAL MAXIMUM (1)
*/
+0x95, 0x04,/*REPORT COUNT (4)
*/
+0x05, 0x01,/*USAGE PAGE (Generic Desktop)
*/
+0x09, 0x90,
Re: XBox One gamecontroller support
On Fri, Jan 15, 2021 at 06:32:04AM -0700, Thomas Frohwein wrote:
> @@ -557,6 +571,23 @@ uhidev_open(struct uhidev *scd)
> DPRINTF(("uhidev_open: couldn't allocate owxfer\n"));
> error = ENOMEM;
> goto out3;
> + }
> +
> + /* XBox One controller initialization */
Shouldn't this initialization code be under #ifndef SMALL_KERNEL,
like the rest of the code your patch is adding to this file?
> + if (sc->sc_flags & UHIDEV_F_XB1) {
> + uint8_t init_data[] = { 0x05, 0x20, 0x00, 0x01, 0x00 };
> + int init_data_len = sizeof(init_data);
I would use 'size_t init_data_len' instead of 'int init_data_len'.
Largely a matter of taste, but this way everything stays unsigned.
usbd_setup_xfer() expects an unsigned int.
> + usbd_setup_xfer(sc->sc_oxfer, sc->sc_opipe, 0,
> + init_data, init_data_len,
> + USBD_SYNCHRONOUS | USBD_CATCH, USBD_NO_TIMEOUT,
> + NULL);
> + err = usbd_transfer(sc->sc_oxfer);
> + if (err != USBD_NORMAL_COMPLETION) {
> + DPRINTF(("uhidev_open: xb1 init failed, "
> + "error=%d\n", err));
> + error = EIO;
> + goto out3;
> + }
> }
> }
Both of the above suggestions as a diff:
diff 978a5fa5205c9adcf7df6f7cb4e82b0fc5f3612a /usr/src
blob - 2aa48d139f7b1a0ddedd79e53da2f0c846ca2745
file + sys/dev/usb/uhidev.c
--- sys/dev/usb/uhidev.c
+++ sys/dev/usb/uhidev.c
@@ -599,11 +599,11 @@ uhidev_open(struct uhidev *scd)
error = ENOMEM;
goto out3;
}
-
+#ifndef SMALL_KERNEL
/* XBox One controller initialization */
if (sc->sc_flags & UHIDEV_F_XB1) {
uint8_t init_data[] = { 0x05, 0x20 };
- int init_data_len = sizeof(init_data);
+ size_t init_data_len = sizeof(init_data);
usbd_setup_xfer(sc->sc_oxfer, sc->sc_opipe, 0,
init_data, init_data_len,
USBD_SYNCHRONOUS | USBD_CATCH, USBD_NO_TIMEOUT,
@@ -616,6 +616,7 @@ uhidev_open(struct uhidev *scd)
goto out3;
}
}
+#endif
}
return (0);
Re: XBox One gamecontroller support
On Thu, Mar 17, 2022 at 10:58:21PM +0100, Solene Rapenne wrote: [...] > > > > ping > > this diff is still applying to the kernel and allows to use a popular > and affordable game controller The diff was written fairly conservatively based on pre-existing code and NetBSD's solution. It would be nice if someone with more experience with the USB fundamentals would comment, but maybe it could just go in in its current form.
Re: XBox One gamecontroller support
On Fri, Jan 15, 2021 at 06:32:04AM -0700, Thomas Frohwein wrote:
> On Sat, Jan 09, 2021 at 10:50:35AM -0700, Thomas Frohwein wrote:
> > Hi,
> >
> > This diff adds support for the XBox One gamecontroller in a similar way
> > to what we have for the (older) XBox 360 controller [1][2]. This diff
> > is based on the pertinent code in NetBSD's uhidev.c.
> >
> > Similarities include that the device doesn't provide a report
> > descriptor, so this diff adds one to uhid_rdesc.h.
> >
> > The biggest difference (that held this up for a while) is that the
> > controller needs an init byte sequence to "wake up."
> >
> > The original report descriptor from NetBSD just maps the buttons and
> > axes in the order that they appear in the report. I modified the report
> > descriptor to assign the buttons/axes in the most logical order for
> > compatibility, which results in the same layout that we already have
> > for the XBox 360 controller.
> >
> > The addition of the member sc_flags to struct uhidev_softc follows the
> > NetBSD example.
> >
> > I have tested this with usbhidctl(1), sdl2-jstest (from sdl-jstest
> > package), and a couple of SDL2 games (Owlboy, Cryptark).
> >
> > If you have an XBox One controller, the easiest way to test is with:
> >
> > $ usbhidctl -f /dev/uhidX -l
> >
> > Make sure to pick the right uhid device and that you have read
> > permissions for it.
> >
> > Of course, this works only with the controller connected via USB, and
> > doesn't magically add wireless support.
> >
> > If you test actual SDL2 applications, the button layout will likely
> > default to an odd configuration. I am simultaneously preparing a diff
> > for sdl2-2.0.14p0 that improves recognition and automatic mapping and
> > solves any issues with XBox One default button layout.
>
> I have an update to this diff. 2 testers used a more recent XBox One
> controller model (model number 1708 in both cases) and ran into
> problems with the original diff. Below is a new diff that uses a longer
> 5-byte init sequence taken from Linux' xpad.c [1], compared to the
> 2-byte sequence from NetBSD's uhidev.c that I offered in the initial
> diff. This fixed the issue for the 2 testers.
>
> My own XBox One controller is model number 1537 and still works with
> this diff.
>
> [1] https://github.com/paroj/xpad/blob/master/xpad.c#L479
solene@ has tested this and is ok with it, but I was hoping to get an
opinion from someone with more mileage with the USB internals.
The items of note to direct anyone wanting to give a quick opinion
inlined below.
>
> Index: uhid_rdesc.h
> ===
> RCS file: /cvs/src/sys/dev/usb/uhid_rdesc.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 uhid_rdesc.h
> --- uhid_rdesc.h 25 Oct 2013 03:09:59 - 1.1
> +++ uhid_rdesc.h 9 Jan 2021 15:11:30 -
> @@ -272,4 +272,94 @@ static const uByte uhid_xb360gp_report_d
> 0x95, 0x01, /* REPORT COUNT (1)*/
> 0x81, 0x01, /* INPUT (Constant)*/
> 0xc0,/* END COLLECTION */
> +};
> +
> +static const uByte uhid_xbonegp_report_descr[] = {
> +0x05, 0x01, /* USAGE PAGE (Generic Desktop)
> */
> +0x09, 0x05, /* USAGE (Gamepad)
> */
> +0xa1, 0x01, /* COLLECTION (Application)
> */
> +/* Button packet */
> +0xa1, 0x00, /* COLLECTION (Physical)
> */
> +0x85, 0x20, /* REPORT ID (0x20)
> */
> +/* Skip unknown field and counter */
> +0x09, 0x00, /* USAGE (Undefined)
> */
> +0x75, 0x08, /* REPORT SIZE (8)
> */
> +0x95, 0x02, /* REPORT COUNT (2)
> */
> +0x81, 0x03, /* INPUT (Constant, Variable,
> Absolute) */
> +/* Payload size */
> +0x09, 0x3b, /* USAGE (Byte Count)
> */
> +0x95, 0x01, /* REPORT COUNT (1)
> */
> +0x81, 0x02, /* INPUT (Data, Variable, Absolute)
> */
> +/* 16 Buttons: 1-2=Unknown, 3=Start, 4=Back, 5-8=ABXY,
> + * 9-12=D-Pad(Up,Dn,Lt,Rt), 13=LB, 14=RB, 15=LS, 16=RS
> + */
> +/* Skip the first 2 as they are not used */
> +0x75, 0x01, /* REPORT SIZE (1)
> */
> +0x95, 0x02, /* REPORT COUNT (2)
> */
> +0x81, 0x01, /* INPUT (Constant)
> */
> +/* Assign buttons Start(7), Back(8), ABXY(1-4) */
> +
Re: XBox One gamecontroller support
On Sat, Jan 09, 2021 at 10:50:35AM -0700, Thomas Frohwein wrote:
> Hi,
>
> This diff adds support for the XBox One gamecontroller in a similar way
> to what we have for the (older) XBox 360 controller [1][2]. This diff
> is based on the pertinent code in NetBSD's uhidev.c.
>
> Similarities include that the device doesn't provide a report
> descriptor, so this diff adds one to uhid_rdesc.h.
>
> The biggest difference (that held this up for a while) is that the
> controller needs an init byte sequence to "wake up."
>
> The original report descriptor from NetBSD just maps the buttons and
> axes in the order that they appear in the report. I modified the report
> descriptor to assign the buttons/axes in the most logical order for
> compatibility, which results in the same layout that we already have
> for the XBox 360 controller.
>
> The addition of the member sc_flags to struct uhidev_softc follows the
> NetBSD example.
>
> I have tested this with usbhidctl(1), sdl2-jstest (from sdl-jstest
> package), and a couple of SDL2 games (Owlboy, Cryptark).
>
> If you have an XBox One controller, the easiest way to test is with:
>
> $ usbhidctl -f /dev/uhidX -l
>
> Make sure to pick the right uhid device and that you have read
> permissions for it.
>
> Of course, this works only with the controller connected via USB, and
> doesn't magically add wireless support.
>
> If you test actual SDL2 applications, the button layout will likely
> default to an odd configuration. I am simultaneously preparing a diff
> for sdl2-2.0.14p0 that improves recognition and automatic mapping and
> solves any issues with XBox One default button layout.
I have an update to this diff. 2 testers used a more recent XBox One
controller model (model number 1708 in both cases) and ran into
problems with the original diff. Below is a new diff that uses a longer
5-byte init sequence taken from Linux' xpad.c [1], compared to the
2-byte sequence from NetBSD's uhidev.c that I offered in the initial
diff. This fixed the issue for the 2 testers.
My own XBox One controller is model number 1537 and still works with
this diff.
[1] https://github.com/paroj/xpad/blob/master/xpad.c#L479
Index: uhid_rdesc.h
===
RCS file: /cvs/src/sys/dev/usb/uhid_rdesc.h,v
retrieving revision 1.1
diff -u -p -r1.1 uhid_rdesc.h
--- uhid_rdesc.h25 Oct 2013 03:09:59 - 1.1
+++ uhid_rdesc.h9 Jan 2021 15:11:30 -
@@ -272,4 +272,94 @@ static const uByte uhid_xb360gp_report_d
0x95, 0x01,/* REPORT COUNT (1)*/
0x81, 0x01,/* INPUT (Constant)*/
0xc0, /* END COLLECTION */
+};
+
+static const uByte uhid_xbonegp_report_descr[] = {
+0x05, 0x01,/* USAGE PAGE (Generic Desktop)
*/
+0x09, 0x05,/* USAGE (Gamepad)
*/
+0xa1, 0x01,/* COLLECTION (Application)
*/
+/* Button packet */
+0xa1, 0x00,/* COLLECTION (Physical)
*/
+0x85, 0x20,/* REPORT ID (0x20)
*/
+/* Skip unknown field and counter */
+0x09, 0x00,/* USAGE (Undefined)
*/
+0x75, 0x08,/* REPORT SIZE (8)
*/
+0x95, 0x02,/* REPORT COUNT (2)
*/
+0x81, 0x03,/* INPUT (Constant, Variable,
Absolute) */
+/* Payload size */
+0x09, 0x3b,/* USAGE (Byte Count)
*/
+0x95, 0x01,/* REPORT COUNT (1)
*/
+0x81, 0x02,/* INPUT (Data, Variable, Absolute)
*/
+/* 16 Buttons: 1-2=Unknown, 3=Start, 4=Back, 5-8=ABXY,
+ *9-12=D-Pad(Up,Dn,Lt,Rt), 13=LB, 14=RB, 15=LS, 16=RS
+ */
+/* Skip the first 2 as they are not used */
+0x75, 0x01,/* REPORT SIZE (1)
*/
+0x95, 0x02,/* REPORT COUNT (2)
*/
+0x81, 0x01,/* INPUT (Constant)
*/
+/* Assign buttons Start(7), Back(8), ABXY(1-4) */
+0x15, 0x00,/* LOGICAL MINIMUM (0)
*/
+0x25, 0x01,/* LOGICAL MAXIMUM (1)
*/
+0x95, 0x06,/* REPORT COUNT (6)
*/
+0x05, 0x09,/* USAGE PAGE (Button)
*/
+0x09, 0x08,/* USAGE (Button 8)
XBox One gamecontroller support
Hi,
This diff adds support for the XBox One gamecontroller in a similar way
to what we have for the (older) XBox 360 controller [1][2]. This diff
is based on the pertinent code in NetBSD's uhidev.c.
Similarities include that the device doesn't provide a report
descriptor, so this diff adds one to uhid_rdesc.h.
The biggest difference (that held this up for a while) is that the
controller needs an init byte sequence to "wake up."
The original report descriptor from NetBSD just maps the buttons and
axes in the order that they appear in the report. I modified the report
descriptor to assign the buttons/axes in the most logical order for
compatibility, which results in the same layout that we already have
for the XBox 360 controller.
The addition of the member sc_flags to struct uhidev_softc follows the
NetBSD example.
I have tested this with usbhidctl(1), sdl2-jstest (from sdl-jstest
package), and a couple of SDL2 games (Owlboy, Cryptark).
If you have an XBox One controller, the easiest way to test is with:
$ usbhidctl -f /dev/uhidX -l
Make sure to pick the right uhid device and that you have read
permissions for it.
Of course, this works only with the controller connected via USB, and
doesn't magically add wireless support.
If you test actual SDL2 applications, the button layout will likely
default to an odd configuration. I am simultaneously preparing a diff
for sdl2-2.0.14p0 that improves recognition and automatic mapping and
solves any issues with XBox One default button layout.
comments? ok?
[1] https://marc.info/?l=openbsd-cvs=138267062532046=mbox
[2] https://marc.info/?l=openbsd-cvs=144956819605939=mbox
Index: uhid_rdesc.h
===
RCS file: /cvs/src/sys/dev/usb/uhid_rdesc.h,v
retrieving revision 1.1
diff -u -p -r1.1 uhid_rdesc.h
--- uhid_rdesc.h25 Oct 2013 03:09:59 - 1.1
+++ uhid_rdesc.h9 Jan 2021 15:11:30 -
@@ -272,4 +272,94 @@ static const uByte uhid_xb360gp_report_d
0x95, 0x01,/* REPORT COUNT (1)*/
0x81, 0x01,/* INPUT (Constant)*/
0xc0, /* END COLLECTION */
+};
+
+static const uByte uhid_xbonegp_report_descr[] = {
+0x05, 0x01,/* USAGE PAGE (Generic Desktop)
*/
+0x09, 0x05,/* USAGE (Gamepad)
*/
+0xa1, 0x01,/* COLLECTION (Application)
*/
+/* Button packet */
+0xa1, 0x00,/* COLLECTION (Physical)
*/
+0x85, 0x20,/* REPORT ID (0x20)
*/
+/* Skip unknown field and counter */
+0x09, 0x00,/* USAGE (Undefined)
*/
+0x75, 0x08,/* REPORT SIZE (8)
*/
+0x95, 0x02,/* REPORT COUNT (2)
*/
+0x81, 0x03,/* INPUT (Constant, Variable,
Absolute) */
+/* Payload size */
+0x09, 0x3b,/* USAGE (Byte Count)
*/
+0x95, 0x01,/* REPORT COUNT (1)
*/
+0x81, 0x02,/* INPUT (Data, Variable, Absolute)
*/
+/* 16 Buttons: 1-2=Unknown, 3=Start, 4=Back, 5-8=ABXY,
+ *9-12=D-Pad(Up,Dn,Lt,Rt), 13=LB, 14=RB, 15=LS, 16=RS
+ */
+/* Skip the first 2 as they are not used */
+0x75, 0x01,/* REPORT SIZE (1)
*/
+0x95, 0x02,/* REPORT COUNT (2)
*/
+0x81, 0x01,/* INPUT (Constant)
*/
+/* Assign buttons Start(7), Back(8), ABXY(1-4) */
+0x15, 0x00,/* LOGICAL MINIMUM (0)
*/
+0x25, 0x01,/* LOGICAL MAXIMUM (1)
*/
+0x95, 0x06,/* REPORT COUNT (6)
*/
+0x05, 0x09,/* USAGE PAGE (Button)
*/
+0x09, 0x08,/* USAGE (Button 8)
*/
+0x09, 0x07,/* USAGE (Button 7)
*/
+0x09, 0x01,/* USAGE (Button 1)
*/
+0x09, 0x02,/* USAGE (Button 2)
*/
+0x09, 0x03,/* USAGE (Button 3)
*/
+0x09, 0x04,/* USAGE (Button 4)
*/
+0x81, 0x02,/* INPUT (Data, Variable,
