* [PATCH 0/3] Dualshock4 controller enhancements @ 2016-12-22 5:20 Juha Kuikka 2016-12-22 5:20 ` [PATCH 1/3] Add new DS4 controller PID into special case handler Juha Kuikka ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Juha Kuikka @ 2016-12-22 5:20 UTC (permalink / raw) To: linux-bluetooth; +Cc: Juha Kuikka 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] Add new DS4 controller PID into special case handler 2016-12-22 5:20 [PATCH 0/3] Dualshock4 controller enhancements Juha Kuikka @ 2016-12-22 5:20 ` Juha Kuikka 2016-12-28 16:06 ` Szymon Janc 2016-12-22 5:20 ` [PATCH 2/3] sixaxis: Add support for pairing DS4 over USB Juha Kuikka 2016-12-22 5:20 ` [PATCH 3/3] sixaxis: Set created devices trusted by default Juha Kuikka 2 siblings, 1 reply; 10+ messages in thread From: Juha Kuikka @ 2016-12-22 5:20 UTC (permalink / raw) To: linux-bluetooth; +Cc: Juha Kuikka 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] Add new DS4 controller PID into special case handler 2016-12-22 5:20 ` [PATCH 1/3] Add new DS4 controller PID into special case handler Juha Kuikka @ 2016-12-28 16:06 ` Szymon Janc 0 siblings, 0 replies; 10+ messages in thread From: Szymon Janc @ 2016-12-28 16:06 UTC (permalink / raw) To: Juha Kuikka; +Cc: linux-bluetooth 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] sixaxis: Add support for pairing DS4 over USB 2016-12-22 5:20 [PATCH 0/3] Dualshock4 controller enhancements Juha Kuikka 2016-12-22 5:20 ` [PATCH 1/3] Add new DS4 controller PID into special case handler Juha Kuikka @ 2016-12-22 5:20 ` Juha Kuikka 2016-12-22 10:24 ` Antonio Ospite 2016-12-28 16:14 ` Szymon Janc 2016-12-22 5:20 ` [PATCH 3/3] sixaxis: Set created devices trusted by default Juha Kuikka 2 siblings, 2 replies; 10+ messages in thread From: Juha Kuikka @ 2016-12-22 5:20 UTC (permalink / raw) To: linux-bluetooth; +Cc: Juha Kuikka 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] sixaxis: Add support for pairing DS4 over USB 2016-12-22 5:20 ` [PATCH 2/3] sixaxis: Add support for pairing DS4 over USB Juha Kuikka @ 2016-12-22 10:24 ` Antonio Ospite 2016-12-22 17:29 ` Juha Kuikka 2016-12-28 16:14 ` Szymon Janc 1 sibling, 1 reply; 10+ messages in thread From: Antonio Ospite @ 2016-12-22 10:24 UTC (permalink / raw) To: Juha Kuikka; +Cc: linux-bluetooth On Wed, 21 Dec 2016 21:20:53 -0800 Juha Kuikka <juha.kuikka@synapse.com> 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 <frank.praznik@gmail.com> Roderick Colenbrander <roderick.colenbrander@sony.com> 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? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] sixaxis: Add support for pairing DS4 over USB 2016-12-22 10:24 ` Antonio Ospite @ 2016-12-22 17:29 ` Juha Kuikka 0 siblings, 0 replies; 10+ messages in thread From: Juha Kuikka @ 2016-12-22 17:29 UTC (permalink / raw) To: Antonio Ospite; +Cc: Juha Kuikka, linux-bluetooth On Thu, Dec 22, 2016 at 2:24 AM, Antonio Ospite <ao2@ao2.it> wrote: > On Wed, 21 Dec 2016 21:20:53 -0800 > Juha Kuikka <juha.kuikka@synapse.com> 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 <frank.praznik@gmail.com> > Roderick Colenbrander <roderick.colenbrander@sony.com> > > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] sixaxis: Add support for pairing DS4 over USB 2016-12-22 5:20 ` [PATCH 2/3] sixaxis: Add support for pairing DS4 over USB Juha Kuikka 2016-12-22 10:24 ` Antonio Ospite @ 2016-12-28 16:14 ` Szymon Janc 1 sibling, 0 replies; 10+ messages in thread From: Szymon Janc @ 2016-12-28 16:14 UTC (permalink / raw) To: Juha Kuikka; +Cc: linux-bluetooth 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] sixaxis: Set created devices trusted by default 2016-12-22 5:20 [PATCH 0/3] Dualshock4 controller enhancements Juha Kuikka 2016-12-22 5:20 ` [PATCH 1/3] Add new DS4 controller PID into special case handler Juha Kuikka 2016-12-22 5:20 ` [PATCH 2/3] sixaxis: Add support for pairing DS4 over USB Juha Kuikka @ 2016-12-22 5:20 ` Juha Kuikka 2016-12-28 16:07 ` Szymon Janc 2 siblings, 1 reply; 10+ messages in thread From: Juha Kuikka @ 2016-12-22 5:20 UTC (permalink / raw) To: linux-bluetooth; +Cc: Juha Kuikka 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] sixaxis: Set created devices trusted by default 2016-12-22 5:20 ` [PATCH 3/3] sixaxis: Set created devices trusted by default Juha Kuikka @ 2016-12-28 16:07 ` Szymon Janc 2016-12-28 16:25 ` Bastien Nocera 0 siblings, 1 reply; 10+ messages in thread From: Szymon Janc @ 2016-12-28 16:07 UTC (permalink / raw) To: Juha Kuikka; +Cc: linux-bluetooth 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] sixaxis: Set created devices trusted by default 2016-12-28 16:07 ` Szymon Janc @ 2016-12-28 16:25 ` Bastien Nocera 0 siblings, 0 replies; 10+ messages in thread From: Bastien Nocera @ 2016-12-28 16:25 UTC (permalink / raw) To: Szymon Janc, Juha Kuikka; +Cc: linux-bluetooth 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-12-28 16:25 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-22 5:20 [PATCH 0/3] Dualshock4 controller enhancements Juha Kuikka 2016-12-22 5:20 ` [PATCH 1/3] Add new DS4 controller PID into special case handler Juha Kuikka 2016-12-28 16:06 ` Szymon Janc 2016-12-22 5:20 ` [PATCH 2/3] sixaxis: Add support for pairing DS4 over USB Juha Kuikka 2016-12-22 10:24 ` Antonio Ospite 2016-12-22 17:29 ` Juha Kuikka 2016-12-28 16:14 ` Szymon Janc 2016-12-22 5:20 ` [PATCH 3/3] sixaxis: Set created devices trusted by default Juha Kuikka 2016-12-28 16:07 ` Szymon Janc 2016-12-28 16:25 ` Bastien Nocera
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.