2016-12-22 05:20:51

by Juha Kuikka

[permalink] [raw]
Subject: [PATCH 0/3] Dualshock4 controller enhancements

This patch set contains enhancements for the Dualshock4 controller.

There are now two Dualshock4 PIDs out there, add second one to
special case handler.

Modify sixaxis plugin to add support for pairing DS4 controllers over USB.
The HID report also allows for the link key to be set but since there is
currently no API to set it from the plugins, this has not been implemented.

Because of this the DS4 will connect to the bluez host automatically but
the bonding still has to happen over the bluetooth connection as normal.

Also set connected controllers "paired" by default. This allows for hands-free
connection to bluez host.

Juha Kuikka (3):
Add new DS4 controller PID into special case handler
sixaxis: Add support for pairing DS4 over USB
sixaxis: Set created devices trusted by default

plugins/sixaxis.c | 87 +++++++++++++++++++++++++++++++++++++------------
profiles/input/server.c | 2 +-
2 files changed, 68 insertions(+), 21 deletions(-)

--
1.9.1



2016-12-28 16:25:17

by Bastien Nocera

[permalink] [raw]
Subject: Re: [PATCH 3/3] sixaxis: Set created devices trusted by default

On Wed, 2016-12-28 at 17:07 +0100, Szymon Janc wrote:
> Hi Juha,
>
> On Wednesday, 21 December 2016 21:20:54 CET Juha Kuikka wrote:
> > If user has enabled the sixaxis plugin and plugs a controller in,
> > it can be assumed they also want the controllers to be able to
> > connect
> > automatically.
> > ---
> >  plugins/sixaxis.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
> > index 1fb2091..ba96b54 100644
> > --- a/plugins/sixaxis.c
> > +++ b/plugins/sixaxis.c
> > @@ -349,6 +349,7 @@ static bool setup_device(int fd, int index,
> > struct
> > btd_adapter *adapter) btd_device_set_pnpid(device,
> > devices[index].source,
> > devices[index].vid, devices[index].pid, devices[index].version);
> >   btd_device_set_temporary(device, false);
> > + btd_device_set_trusted(device, true);
> >
> >   return true;
> >  }
>
> This was already NACKed in the past for security reasons (mostly due
> to 
> possibility of fake device being trusted).

Juha, you could take a look at a way of getting this cleaned up and
merged:
https://www.spinics.net/lists/linux-bluetooth/msg62970.html
https://www.spinics.net/lists/linux-bluetooth/msg62967.html

I couldn't get a straight answer about the patch, as the developers
themselves seemed to disagree on how to solve the problem.

Cheers

2016-12-28 16:14:46

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH 2/3] sixaxis: Add support for pairing DS4 over USB

Hi Juha,

On Wednesday, 21 December 2016 21:20:53 CET Juha Kuikka wrote:
> This patch adds support for "pairing" a Dualshock4 controller over USB
> into the sixaxis plugin.
>
> Pairing is in quotes because we cannot do real bonding due to lack of
> API of setting link keys from the plugin so instead we just tell the
> controller that we are the master and to connect to us.
>
> Actual bonding happens on first connection per usual.
>
> This patch is based on information from sixpair tool.
> ---
> plugins/sixaxis.c | 86
> ++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 66
> insertions(+), 20 deletions(-)
>
> diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
> index fcc93bc..1fb2091 100644
> --- a/plugins/sixaxis.c
> +++ b/plugins/sixaxis.c
> @@ -69,6 +69,20 @@ static const struct {
> .pid = 0x042f,
> .version = 0x0000,
> },
> + {
> + .name = "Wireless Controller",
> + .source = 0x0002,
> + .vid = 0x054c,
> + .pid = 0x05c4,
> + .version = 0x0001,
> + },
> + {
> + .name = "Wireless Controller",
> + .source = 0x0002,
> + .vid = 0x054c,
> + .pid = 0x09cc,
> + .version = 0x0001,
> + },
> };
>
> struct leds_data {
> @@ -86,57 +100,89 @@ static struct udev *ctx = NULL;
> static struct udev_monitor *monitor = NULL;
> static guint watch_id = 0;
>
> -static int get_device_bdaddr(int fd, bdaddr_t *bdaddr)
> +static bool is_dualshock4(int index)
> +{
> + return devices[index].pid == 0x05c4 || devices[index].pid == 0x09cc;
> +}
> +
> +static int get_device_bdaddr(int fd, int index, bdaddr_t *bdaddr)
> {

While passing index isn't that bad for differentiating pads it would be great
to have dedicated get/set callbacks in devices[] structures. AFAIR there are
some chinese DS3 counterfeits that don't work currently with sixaxis plugin
that could possibly use custom calbacks.

What do you think about such approach?

> uint8_t buf[18];
> - int ret;
> + int ret, report_length;
>
> memset(buf, 0, sizeof(buf));
>
> - buf[0] = 0xf2;
> + if (is_dualshock4(index)) {
> + report_length = 7;
> + buf[0] = 0x81;
> + } else {
> + report_length = 18;
> + buf[0] = 0xf2;
> + }
>
> - ret = ioctl(fd, HIDIOCGFEATURE(sizeof(buf)), buf);
> + ret = ioctl(fd, HIDIOCGFEATURE(report_length), buf);
> if (ret < 0) {
> error("sixaxis: failed to read device address (%s)",
> strerror(errno));
> return ret;
> }
>
> - baswap(bdaddr, (bdaddr_t *) (buf + 4));
> + if (is_dualshock4(index))
> + memcpy(bdaddr->b, buf + 1, 6); // little-endian on DS4
> + else
> + baswap(bdaddr, (bdaddr_t *) (buf + 4));
>
> return 0;
> }
>
> -static int get_master_bdaddr(int fd, bdaddr_t *bdaddr)
> +static int get_master_bdaddr(int fd, int index, bdaddr_t *bdaddr)
> {
> - uint8_t buf[8];
> - int ret;
> + uint8_t buf[16];
> + int ret, report_length;
>
> memset(buf, 0, sizeof(buf));
>
> - buf[0] = 0xf5;
> + if (is_dualshock4(index)) {
> + report_length = 16;
> + buf[0] = 0x12;
> + } else {
> + report_length = 8;
> + buf[0] = 0xf5;
> + }
>
> - ret = ioctl(fd, HIDIOCGFEATURE(sizeof(buf)), buf);
> + ret = ioctl(fd, HIDIOCGFEATURE(report_length), buf);
> if (ret < 0) {
> error("sixaxis: failed to read master address (%s)",
> strerror(errno));
> return ret;
> }
>
> - baswap(bdaddr, (bdaddr_t *) (buf + 2));
> + if (is_dualshock4(index))
> + memcpy(bdaddr->b, buf + 10, 6); // little-endian on DS4

I'd prefer use of bacpy(). Also please use /* */ style comments.

> + else
> + baswap(bdaddr, (bdaddr_t *) (buf + 2));
>
> return 0;
> }
>
> -static int set_master_bdaddr(int fd, const bdaddr_t *bdaddr)
> +static int set_master_bdaddr(int fd, int index, const bdaddr_t *bdaddr)
> {
> - uint8_t buf[8];
> - int ret;
> + uint8_t buf[23];
> + int ret, report_length;
>
> - buf[0] = 0xf5;
> - buf[1] = 0x01;
> + if (is_dualshock4(index)) {
> + report_length = 23;
>
> - baswap((bdaddr_t *) (buf + 2), bdaddr);
> + buf[0] = 0x13;
> + memcpy(buf + 1, bdaddr->b, 6);
> + memset(buf + 7, 0, 16); /* TODO: we could put the key here but there
is
> no way to force a re-loading of link keys to the kernel */ + } else {

80 characters limit.

> + report_length = 8;
> +
> + buf[0] = 0xf5;
> + buf[1] = 0x01;
> + baswap((bdaddr_t *) (buf + 2), bdaddr);
> + }
>
> ret = ioctl(fd, HIDIOCSFEATURE(sizeof(buf)), buf);
> if (ret < 0)
> @@ -262,10 +308,10 @@ static bool setup_device(int fd, int index, struct
> btd_adapter *adapter) const bdaddr_t *adapter_bdaddr;
> struct btd_device *device;
>
> - if (get_device_bdaddr(fd, &device_bdaddr) < 0)
> + if (get_device_bdaddr(fd, index, &device_bdaddr) < 0)
> return false;
>
> - if (get_master_bdaddr(fd, &master_bdaddr) < 0)
> + if (get_master_bdaddr(fd, index, &master_bdaddr) < 0)
> return false;
>
> /* This can happen if controller was plugged while already connected
> @@ -279,7 +325,7 @@ static bool setup_device(int fd, int index, struct
> btd_adapter *adapter) adapter_bdaddr = btd_adapter_get_address(adapter);
>
> if (bacmp(adapter_bdaddr, &master_bdaddr)) {
> - if (set_master_bdaddr(fd, adapter_bdaddr) < 0)
> + if (set_master_bdaddr(fd, index, adapter_bdaddr) < 0)
> return false;
> }


--
pozdrawiam
Szymon Janc

2016-12-28 16:07:12

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH 3/3] sixaxis: Set created devices trusted by default

Hi Juha,

On Wednesday, 21 December 2016 21:20:54 CET Juha Kuikka wrote:
> If user has enabled the sixaxis plugin and plugs a controller in,
> it can be assumed they also want the controllers to be able to connect
> automatically.
> ---
> plugins/sixaxis.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
> index 1fb2091..ba96b54 100644
> --- a/plugins/sixaxis.c
> +++ b/plugins/sixaxis.c
> @@ -349,6 +349,7 @@ static bool setup_device(int fd, int index, struct
> btd_adapter *adapter) btd_device_set_pnpid(device, devices[index].source,
> devices[index].vid, devices[index].pid, devices[index].version);
> btd_device_set_temporary(device, false);
> + btd_device_set_trusted(device, true);
>
> return true;
> }

This was already NACKed in the past for security reasons (mostly due to
possibility of fake device being trusted).


--
pozdrawiam
Szymon Janc

2016-12-28 16:06:21

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH 1/3] Add new DS4 controller PID into special case handler

Hi Juha,

On Wednesday, 21 December 2016 21:20:52 CET Juha Kuikka wrote:
> There is a special path for various game controllers where they connect
> to the hid service before bluetoothd knows what they are.
>
> This patch adds another PID for the Dualshock4 controller.
> ---
> profiles/input/server.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/profiles/input/server.c b/profiles/input/server.c
> index eb3fcf8..61f084a 100644
> --- a/profiles/input/server.c
> +++ b/profiles/input/server.c
> @@ -136,7 +136,7 @@ static bool dev_is_sixaxis(const bdaddr_t *src, const
> bdaddr_t *dst) return true;
>
> /* DualShock 4 */
> - if (vid == 0x054c && pid == 0x05c4)
> + if (vid == 0x054c && (pid == 0x05c4 || pid == 0x09cc))
> return true;

Is this ID for new DS4 sold with PS4 pro? if so please add a comment.

>
> /* Navigation Controller */


--
pozdrawiam
Szymon Janc

2016-12-22 17:29:18

by Juha Kuikka

[permalink] [raw]
Subject: Re: [PATCH 2/3] sixaxis: Add support for pairing DS4 over USB

On Thu, Dec 22, 2016 at 2:24 AM, Antonio Ospite <[email protected]> wrote:
> On Wed, 21 Dec 2016 21:20:53 -0800
> Juha Kuikka <[email protected]> wrote:
>
>> This patch adds support for "pairing" a Dualshock4 controller over USB
>> into the sixaxis plugin.
>>
>> Pairing is in quotes because we cannot do real bonding due to lack of
>> API of setting link keys from the plugin so instead we just tell the
>> controller that we are the master and to connect to us.
>>
>
> Hi Juha,

Hi Antonio,

>
> I do not have a DS4, so it is not immediately clear to me what the
> difference is between having or not having this USB "pairing" with DS4
> controllers.
>
> Could you please elaborate a little more? Thanks.
> I thought people were using DS4s via BT just fine without it.

Certainly!

This is a convenience feature. Users often use the same controllers
with both their PS4 and a Linux gaming system. This feature would make
switching a controller back-and-forth between the systems more
convenient as there would be no need for keyboard and mouse to go
through the BT discovery and pairing process every time they change
systems, instead the user could just plug it in.

This is exactly how the PS4 pairs/bonds the controllers
(http://manuals.playstation.net/document/en/ps4/basic/usecontroller.html)
so it would mimic that functionality as well.

>
> And what about setup_leds() for the DS4? Will it still be called when
> the DS4 shows up via BT?

Yes. The DS4 kernel driver exposes the LEDs. I will review this path as well.

> Also consider CCing these developers when you send a possible next
> iteration, they worked on the DS4 kernel driver and could be interested:
>
> Frank Praznik <[email protected]>
> Roderick Colenbrander <[email protected]>
>
> Maybe mention BlueZ in the title of the cover letter when you do that,
> to make clearer to them that the changes are not about the kernel
> driver.

Thank you for the suggestion, I will.

> Some comments inlined below.
>
>> Actual bonding happens on first connection per usual.
>>
>> This patch is based on information from sixpair tool.
>
> Is there an official link to that?

There are several links but I am not certain which one is the
original. I will dig around for this.
>
>> ---
>> plugins/sixaxis.c | 86 ++++++++++++++++++++++++++++++++++++++++++-------------
>> 1 file changed, 66 insertions(+), 20 deletions(-)
>>
>> diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
>> index fcc93bc..1fb2091 100644
>> --- a/plugins/sixaxis.c
>> +++ b/plugins/sixaxis.c
>> @@ -69,6 +69,20 @@ static const struct {
>> .pid = 0x042f,
>> .version = 0x0000,
>> },
>> + {
>> + .name = "Wireless Controller",
>> + .source = 0x0002,
>> + .vid = 0x054c,
>> + .pid = 0x05c4,
>> + .version = 0x0001,
>
> If these devices have a different pairing mechanism maybe we can set
> some function pointers here, and use a "callback" approach to handle it.
>
> This would avoid having a lot of if...else in the functions below.
>
> Something like:
> get_device_bdaddr = ds4_get_device_bdaddr,
> get_master_bdaddr = ds4_get_master_bdaddr,
> set_device_bdaddr = ds4_set_device_bdaddr,
>> + },
>> + {
>> + .name = "Wireless Controller",
>> + .source = 0x0002,
>> + .vid = 0x054c,
>> + .pid = 0x09cc,
>> + .version = 0x0001,
>> + },
>> };
>>
>> struct leds_data {
>> @@ -86,57 +100,89 @@ static struct udev *ctx = NULL;
>> static struct udev_monitor *monitor = NULL;
>> static guint watch_id = 0;
>>
>> -static int get_device_bdaddr(int fd, bdaddr_t *bdaddr)
>
> This function will just become sixaxis_get_device_bdaddr, and a new
> function called ds4_get_device_bdaddr will implement the DS4 paring
> which uses a different feature report, with a different size and
> structure.
>
> This looks cleaner to me.

I agree. I will refactor this.

>
>> +static bool is_dualshock4(int index)
>> +{
>> + return devices[index].pid == 0x05c4 || devices[index].pid == 0x09cc;
>> +}
>
> this will not be needed,

Right.
>
>> +
>> +static int get_device_bdaddr(int fd, int index, bdaddr_t *bdaddr)
>
> and neither changing the function parameters.
>
>> {
>> uint8_t buf[18];
>> - int ret;
>> + int ret, report_length;
>>
>> memset(buf, 0, sizeof(buf));
>>
>> - buf[0] = 0xf2;
>> + if (is_dualshock4(index)) {
>> + report_length = 7;
>> + buf[0] = 0x81;
>> + } else {
>> + report_length = 18;
>> + buf[0] = 0xf2;
>> + }
>>
>> - ret = ioctl(fd, HIDIOCGFEATURE(sizeof(buf)), buf);
>> + ret = ioctl(fd, HIDIOCGFEATURE(report_length), buf);
>> if (ret < 0) {
>> error("sixaxis: failed to read device address (%s)",
>> strerror(errno));
>> return ret;
>> }
>>
>> - baswap(bdaddr, (bdaddr_t *) (buf + 4));
>> + if (is_dualshock4(index))
>> + memcpy(bdaddr->b, buf + 1, 6); // little-endian on DS4
>> + else
>> + baswap(bdaddr, (bdaddr_t *) (buf + 4));
>>
>> return 0;
>> }
>>
>> -static int get_master_bdaddr(int fd, bdaddr_t *bdaddr)
>> +static int get_master_bdaddr(int fd, int index, bdaddr_t *bdaddr)
>
> ditto.
>
>> {
>> - uint8_t buf[8];
>> - int ret;
>> + uint8_t buf[16];
>> + int ret, report_length;
>>
>> memset(buf, 0, sizeof(buf));
>>
>> - buf[0] = 0xf5;
>> + if (is_dualshock4(index)) {
>> + report_length = 16;
>> + buf[0] = 0x12;
>> + } else {
>> + report_length = 8;
>> + buf[0] = 0xf5;
>> + }
>>
>> - ret = ioctl(fd, HIDIOCGFEATURE(sizeof(buf)), buf);
>> + ret = ioctl(fd, HIDIOCGFEATURE(report_length), buf);
>> if (ret < 0) {
>> error("sixaxis: failed to read master address (%s)",
>> strerror(errno));
>> return ret;
>> }
>>
>> - baswap(bdaddr, (bdaddr_t *) (buf + 2));
>> + if (is_dualshock4(index))
>> + memcpy(bdaddr->b, buf + 10, 6); // little-endian on DS4
>> + else
>> + baswap(bdaddr, (bdaddr_t *) (buf + 2));
>>
>> return 0;
>> }
>>
>> -static int set_master_bdaddr(int fd, const bdaddr_t *bdaddr)
>> +static int set_master_bdaddr(int fd, int index, const bdaddr_t *bdaddr)
>
> ditto.
>
>> {
>> - uint8_t buf[8];
>> - int ret;
>> + uint8_t buf[23];
>> + int ret, report_length;
>>
>> - buf[0] = 0xf5;
>> - buf[1] = 0x01;
>> + if (is_dualshock4(index)) {
>> + report_length = 23;
>>
>> - baswap((bdaddr_t *) (buf + 2), bdaddr);
>> + buf[0] = 0x13;
>> + memcpy(buf + 1, bdaddr->b, 6);
>> + memset(buf + 7, 0, 16); /* TODO: we could put the key here but there is no way to force a re-loading of link keys to the kernel */

Here is an interesting possibility, we could pass the link key here so
the controller would be fully bonded after the USB connection but
currently there is no API to pass that key back to the core.

Do you think this would this be something worth looking into?

>> + } else {
>> + report_length = 8;
>> +
>> + buf[0] = 0xf5;
>> + buf[1] = 0x01;
>> + baswap((bdaddr_t *) (buf + 2), bdaddr);
>> + }
>>
>> ret = ioctl(fd, HIDIOCSFEATURE(sizeof(buf)), buf);
>> if (ret < 0)
>> @@ -262,10 +308,10 @@ static bool setup_device(int fd, int index, struct btd_adapter *adapter)
>> const bdaddr_t *adapter_bdaddr;
>> struct btd_device *device;
>>
>> - if (get_device_bdaddr(fd, &device_bdaddr) < 0)
>> + if (get_device_bdaddr(fd, index, &device_bdaddr) < 0)
>> return false;
>>
>> - if (get_master_bdaddr(fd, &master_bdaddr) < 0)
>> + if (get_master_bdaddr(fd, index, &master_bdaddr) < 0)
>> return false;
>>
>
> Here the function pointers will be used, like:
> if (devices[index].get_device_bdaddr(fd, &device_bdaddr) < 0)
> ...
> if (devices[index].get_master_bdaddr(fd, &master_bdaddr) < 0)
>
>> /* This can happen if controller was plugged while already connected
>> @@ -279,7 +325,7 @@ static bool setup_device(int fd, int index, struct btd_adapter *adapter)
>> adapter_bdaddr = btd_adapter_get_address(adapter);
>>
>> if (bacmp(adapter_bdaddr, &master_bdaddr)) {
>> - if (set_master_bdaddr(fd, adapter_bdaddr) < 0)
>> + if (set_master_bdaddr(fd, index, adapter_bdaddr) < 0)
>
> and here too.
>
>> return false;
>> }
>>
>
> If this approach sounds OK, send a preparatory patch which adds the
> needed fields to the global struct and renames the sixaxis functions
> maybe moving them before the definition of the devices[] array, and then
> do the DS4 changes to add the new devices and callbacks.

Will do! Thank you for the review and suggestions.

Cheers,
- Juha

2016-12-22 10:24:18

by Antonio Ospite

[permalink] [raw]
Subject: Re: [PATCH 2/3] sixaxis: Add support for pairing DS4 over USB

On Wed, 21 Dec 2016 21:20:53 -0800
Juha Kuikka <[email protected]> wrote:

> This patch adds support for "pairing" a Dualshock4 controller over USB
> into the sixaxis plugin.
>
> Pairing is in quotes because we cannot do real bonding due to lack of
> API of setting link keys from the plugin so instead we just tell the
> controller that we are the master and to connect to us.
>

Hi Juha,

I do not have a DS4, so it is not immediately clear to me what the
difference is between having or not having this USB "pairing" with DS4
controllers.

Could you please elaborate a little more? Thanks.
I thought people were using DS4s via BT just fine without it.

And what about setup_leds() for the DS4? Will it still be called when
the DS4 shows up via BT?

Also consider CCing these developers when you send a possible next
iteration, they worked on the DS4 kernel driver and could be interested:

Frank Praznik <[email protected]>
Roderick Colenbrander <[email protected]>

Maybe mention BlueZ in the title of the cover letter when you do that,
to make clearer to them that the changes are not about the kernel
driver.

Some comments inlined below.

> Actual bonding happens on first connection per usual.
>
> This patch is based on information from sixpair tool.

Is there an official link to that?

> ---
> plugins/sixaxis.c | 86 ++++++++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 66 insertions(+), 20 deletions(-)
>
> diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
> index fcc93bc..1fb2091 100644
> --- a/plugins/sixaxis.c
> +++ b/plugins/sixaxis.c
> @@ -69,6 +69,20 @@ static const struct {
> .pid = 0x042f,
> .version = 0x0000,
> },
> + {
> + .name = "Wireless Controller",
> + .source = 0x0002,
> + .vid = 0x054c,
> + .pid = 0x05c4,
> + .version = 0x0001,

If these devices have a different pairing mechanism maybe we can set
some function pointers here, and use a "callback" approach to handle it.

This would avoid having a lot of if...else in the functions below.

Something like:
get_device_bdaddr = ds4_get_device_bdaddr,
get_master_bdaddr = ds4_get_master_bdaddr,
set_device_bdaddr = ds4_set_device_bdaddr,
> + },
> + {
> + .name = "Wireless Controller",
> + .source = 0x0002,
> + .vid = 0x054c,
> + .pid = 0x09cc,
> + .version = 0x0001,
> + },
> };
>
> struct leds_data {
> @@ -86,57 +100,89 @@ static struct udev *ctx = NULL;
> static struct udev_monitor *monitor = NULL;
> static guint watch_id = 0;
>
> -static int get_device_bdaddr(int fd, bdaddr_t *bdaddr)

This function will just become sixaxis_get_device_bdaddr, and a new
function called ds4_get_device_bdaddr will implement the DS4 paring
which uses a different feature report, with a different size and
structure.

This looks cleaner to me.

> +static bool is_dualshock4(int index)
> +{
> + return devices[index].pid == 0x05c4 || devices[index].pid == 0x09cc;
> +}

this will not be needed,

> +
> +static int get_device_bdaddr(int fd, int index, bdaddr_t *bdaddr)

and neither changing the function parameters.

> {
> uint8_t buf[18];
> - int ret;
> + int ret, report_length;
>
> memset(buf, 0, sizeof(buf));
>
> - buf[0] = 0xf2;
> + if (is_dualshock4(index)) {
> + report_length = 7;
> + buf[0] = 0x81;
> + } else {
> + report_length = 18;
> + buf[0] = 0xf2;
> + }
>
> - ret = ioctl(fd, HIDIOCGFEATURE(sizeof(buf)), buf);
> + ret = ioctl(fd, HIDIOCGFEATURE(report_length), buf);
> if (ret < 0) {
> error("sixaxis: failed to read device address (%s)",
> strerror(errno));
> return ret;
> }
>
> - baswap(bdaddr, (bdaddr_t *) (buf + 4));
> + if (is_dualshock4(index))
> + memcpy(bdaddr->b, buf + 1, 6); // little-endian on DS4
> + else
> + baswap(bdaddr, (bdaddr_t *) (buf + 4));
>
> return 0;
> }
>
> -static int get_master_bdaddr(int fd, bdaddr_t *bdaddr)
> +static int get_master_bdaddr(int fd, int index, bdaddr_t *bdaddr)

ditto.

> {
> - uint8_t buf[8];
> - int ret;
> + uint8_t buf[16];
> + int ret, report_length;
>
> memset(buf, 0, sizeof(buf));
>
> - buf[0] = 0xf5;
> + if (is_dualshock4(index)) {
> + report_length = 16;
> + buf[0] = 0x12;
> + } else {
> + report_length = 8;
> + buf[0] = 0xf5;
> + }
>
> - ret = ioctl(fd, HIDIOCGFEATURE(sizeof(buf)), buf);
> + ret = ioctl(fd, HIDIOCGFEATURE(report_length), buf);
> if (ret < 0) {
> error("sixaxis: failed to read master address (%s)",
> strerror(errno));
> return ret;
> }
>
> - baswap(bdaddr, (bdaddr_t *) (buf + 2));
> + if (is_dualshock4(index))
> + memcpy(bdaddr->b, buf + 10, 6); // little-endian on DS4
> + else
> + baswap(bdaddr, (bdaddr_t *) (buf + 2));
>
> return 0;
> }
>
> -static int set_master_bdaddr(int fd, const bdaddr_t *bdaddr)
> +static int set_master_bdaddr(int fd, int index, const bdaddr_t *bdaddr)

ditto.

> {
> - uint8_t buf[8];
> - int ret;
> + uint8_t buf[23];
> + int ret, report_length;
>
> - buf[0] = 0xf5;
> - buf[1] = 0x01;
> + if (is_dualshock4(index)) {
> + report_length = 23;
>
> - baswap((bdaddr_t *) (buf + 2), bdaddr);
> + buf[0] = 0x13;
> + memcpy(buf + 1, bdaddr->b, 6);
> + memset(buf + 7, 0, 16); /* TODO: we could put the key here but there is no way to force a re-loading of link keys to the kernel */
> + } else {
> + report_length = 8;
> +
> + buf[0] = 0xf5;
> + buf[1] = 0x01;
> + baswap((bdaddr_t *) (buf + 2), bdaddr);
> + }
>
> ret = ioctl(fd, HIDIOCSFEATURE(sizeof(buf)), buf);
> if (ret < 0)
> @@ -262,10 +308,10 @@ static bool setup_device(int fd, int index, struct btd_adapter *adapter)
> const bdaddr_t *adapter_bdaddr;
> struct btd_device *device;
>
> - if (get_device_bdaddr(fd, &device_bdaddr) < 0)
> + if (get_device_bdaddr(fd, index, &device_bdaddr) < 0)
> return false;
>
> - if (get_master_bdaddr(fd, &master_bdaddr) < 0)
> + if (get_master_bdaddr(fd, index, &master_bdaddr) < 0)
> return false;
>

Here the function pointers will be used, like:
if (devices[index].get_device_bdaddr(fd, &device_bdaddr) < 0)
...
if (devices[index].get_master_bdaddr(fd, &master_bdaddr) < 0)

> /* This can happen if controller was plugged while already connected
> @@ -279,7 +325,7 @@ static bool setup_device(int fd, int index, struct btd_adapter *adapter)
> adapter_bdaddr = btd_adapter_get_address(adapter);
>
> if (bacmp(adapter_bdaddr, &master_bdaddr)) {
> - if (set_master_bdaddr(fd, adapter_bdaddr) < 0)
> + if (set_master_bdaddr(fd, index, adapter_bdaddr) < 0)

and here too.

> return false;
> }
>

If this approach sounds OK, send a preparatory patch which adds the
needed fields to the global struct and renames the sixaxis functions
maybe moving them before the definition of the devices[] array, and then
do the DS4 changes to add the new devices and callbacks.

Thanks,
Antonio

--
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

2016-12-22 05:20:54

by Juha Kuikka

[permalink] [raw]
Subject: [PATCH 3/3] sixaxis: Set created devices trusted by default

If user has enabled the sixaxis plugin and plugs a controller in,
it can be assumed they also want the controllers to be able to connect
automatically.
---
plugins/sixaxis.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
index 1fb2091..ba96b54 100644
--- a/plugins/sixaxis.c
+++ b/plugins/sixaxis.c
@@ -349,6 +349,7 @@ static bool setup_device(int fd, int index, struct btd_adapter *adapter)
btd_device_set_pnpid(device, devices[index].source, devices[index].vid,
devices[index].pid, devices[index].version);
btd_device_set_temporary(device, false);
+ btd_device_set_trusted(device, true);

return true;
}
--
1.9.1


2016-12-22 05:20:53

by Juha Kuikka

[permalink] [raw]
Subject: [PATCH 2/3] sixaxis: Add support for pairing DS4 over USB

This patch adds support for "pairing" a Dualshock4 controller over USB
into the sixaxis plugin.

Pairing is in quotes because we cannot do real bonding due to lack of
API of setting link keys from the plugin so instead we just tell the
controller that we are the master and to connect to us.

Actual bonding happens on first connection per usual.

This patch is based on information from sixpair tool.
---
plugins/sixaxis.c | 86 ++++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 66 insertions(+), 20 deletions(-)

diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
index fcc93bc..1fb2091 100644
--- a/plugins/sixaxis.c
+++ b/plugins/sixaxis.c
@@ -69,6 +69,20 @@ static const struct {
.pid = 0x042f,
.version = 0x0000,
},
+ {
+ .name = "Wireless Controller",
+ .source = 0x0002,
+ .vid = 0x054c,
+ .pid = 0x05c4,
+ .version = 0x0001,
+ },
+ {
+ .name = "Wireless Controller",
+ .source = 0x0002,
+ .vid = 0x054c,
+ .pid = 0x09cc,
+ .version = 0x0001,
+ },
};

struct leds_data {
@@ -86,57 +100,89 @@ static struct udev *ctx = NULL;
static struct udev_monitor *monitor = NULL;
static guint watch_id = 0;

-static int get_device_bdaddr(int fd, bdaddr_t *bdaddr)
+static bool is_dualshock4(int index)
+{
+ return devices[index].pid == 0x05c4 || devices[index].pid == 0x09cc;
+}
+
+static int get_device_bdaddr(int fd, int index, bdaddr_t *bdaddr)
{
uint8_t buf[18];
- int ret;
+ int ret, report_length;

memset(buf, 0, sizeof(buf));

- buf[0] = 0xf2;
+ if (is_dualshock4(index)) {
+ report_length = 7;
+ buf[0] = 0x81;
+ } else {
+ report_length = 18;
+ buf[0] = 0xf2;
+ }

- ret = ioctl(fd, HIDIOCGFEATURE(sizeof(buf)), buf);
+ ret = ioctl(fd, HIDIOCGFEATURE(report_length), buf);
if (ret < 0) {
error("sixaxis: failed to read device address (%s)",
strerror(errno));
return ret;
}

- baswap(bdaddr, (bdaddr_t *) (buf + 4));
+ if (is_dualshock4(index))
+ memcpy(bdaddr->b, buf + 1, 6); // little-endian on DS4
+ else
+ baswap(bdaddr, (bdaddr_t *) (buf + 4));

return 0;
}

-static int get_master_bdaddr(int fd, bdaddr_t *bdaddr)
+static int get_master_bdaddr(int fd, int index, bdaddr_t *bdaddr)
{
- uint8_t buf[8];
- int ret;
+ uint8_t buf[16];
+ int ret, report_length;

memset(buf, 0, sizeof(buf));

- buf[0] = 0xf5;
+ if (is_dualshock4(index)) {
+ report_length = 16;
+ buf[0] = 0x12;
+ } else {
+ report_length = 8;
+ buf[0] = 0xf5;
+ }

- ret = ioctl(fd, HIDIOCGFEATURE(sizeof(buf)), buf);
+ ret = ioctl(fd, HIDIOCGFEATURE(report_length), buf);
if (ret < 0) {
error("sixaxis: failed to read master address (%s)",
strerror(errno));
return ret;
}

- baswap(bdaddr, (bdaddr_t *) (buf + 2));
+ if (is_dualshock4(index))
+ memcpy(bdaddr->b, buf + 10, 6); // little-endian on DS4
+ else
+ baswap(bdaddr, (bdaddr_t *) (buf + 2));

return 0;
}

-static int set_master_bdaddr(int fd, const bdaddr_t *bdaddr)
+static int set_master_bdaddr(int fd, int index, const bdaddr_t *bdaddr)
{
- uint8_t buf[8];
- int ret;
+ uint8_t buf[23];
+ int ret, report_length;

- buf[0] = 0xf5;
- buf[1] = 0x01;
+ if (is_dualshock4(index)) {
+ report_length = 23;

- baswap((bdaddr_t *) (buf + 2), bdaddr);
+ buf[0] = 0x13;
+ memcpy(buf + 1, bdaddr->b, 6);
+ memset(buf + 7, 0, 16); /* TODO: we could put the key here but there is no way to force a re-loading of link keys to the kernel */
+ } else {
+ report_length = 8;
+
+ buf[0] = 0xf5;
+ buf[1] = 0x01;
+ baswap((bdaddr_t *) (buf + 2), bdaddr);
+ }

ret = ioctl(fd, HIDIOCSFEATURE(sizeof(buf)), buf);
if (ret < 0)
@@ -262,10 +308,10 @@ static bool setup_device(int fd, int index, struct btd_adapter *adapter)
const bdaddr_t *adapter_bdaddr;
struct btd_device *device;

- if (get_device_bdaddr(fd, &device_bdaddr) < 0)
+ if (get_device_bdaddr(fd, index, &device_bdaddr) < 0)
return false;

- if (get_master_bdaddr(fd, &master_bdaddr) < 0)
+ if (get_master_bdaddr(fd, index, &master_bdaddr) < 0)
return false;

/* This can happen if controller was plugged while already connected
@@ -279,7 +325,7 @@ static bool setup_device(int fd, int index, struct btd_adapter *adapter)
adapter_bdaddr = btd_adapter_get_address(adapter);

if (bacmp(adapter_bdaddr, &master_bdaddr)) {
- if (set_master_bdaddr(fd, adapter_bdaddr) < 0)
+ if (set_master_bdaddr(fd, index, adapter_bdaddr) < 0)
return false;
}

--
1.9.1


2016-12-22 05:20:52

by Juha Kuikka

[permalink] [raw]
Subject: [PATCH 1/3] Add new DS4 controller PID into special case handler

There is a special path for various game controllers where they connect
to the hid service before bluetoothd knows what they are.

This patch adds another PID for the Dualshock4 controller.
---
profiles/input/server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/input/server.c b/profiles/input/server.c
index eb3fcf8..61f084a 100644
--- a/profiles/input/server.c
+++ b/profiles/input/server.c
@@ -136,7 +136,7 @@ static bool dev_is_sixaxis(const bdaddr_t *src, const bdaddr_t *dst)
return true;

/* DualShock 4 */
- if (vid == 0x054c && pid == 0x05c4)
+ if (vid == 0x054c && (pid == 0x05c4 || pid == 0x09cc))
return true;

/* Navigation Controller */
--
1.9.1