Return-Path: From: Szymon Janc To: Antonio Ospite Cc: linux-bluetooth@vger.kernel.org, Bastien Nocera , Frank Praznik Subject: Re: [PATCH BlueZ] plugins/sixaxis: Fix get_js_number() for devices connected via BT Date: Sun, 08 Jun 2014 14:58:39 +0200 Message-ID: <1403928.yDT5MoIIEn@athlon> In-Reply-To: <1401270620-27080-1-git-send-email-ao2@ao2.it> References: <1401270620-27080-1-git-send-email-ao2@ao2.it> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi Antonio, On Wednesday 28 May 2014 11:50:20 Antonio Ospite wrote: > Match hid devices and input devices using HID_UNIQ and UNIQ when these > are available, this is the correct way to get matching devices when the > controllers are connected via BT (UNIQ refers to the device bdaddr, PHYS > is the adapter bdaddr, so matching against PHYS will result in all > devices with the same LED number). > > Fall back to HID_PHYS and PHYS when needed, hid devices do not define > HID_UNIQ when connected via USB. > --- > > Hi, > > this is a possible fix for assigning the correct LED numbers via BT. > > Frank, can you take a look? > > I thought about using the information about the bus explicitly but it was > slightly more work; considering that I still intend to look into simplifying > get_js_number() and solving the race we have the first time the joydev > module is loaded the changes proposed in this patch seemed a viable > temporary fix. > > Successfully tested on kernel 3.13 and 3.15-rc4: the js number is retrieved > correctly for the second controller (even though hidraw over BT was broken > in 3.13 so setting the actual LED failed there). > > Thanks, > Antonio > > plugins/sixaxis.c | 33 +++++++++++++++++++++++++-------- > 1 file changed, 25 insertions(+), 8 deletions(-) > > diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c > index de878d1..b802a9c 100644 > --- a/plugins/sixaxis.c > +++ b/plugins/sixaxis.c > @@ -302,15 +302,24 @@ static int get_js_number(struct udev_device *udevice) > struct udev_enumerate *enumerate; > struct udev_device *hid_parent; > const char *hidraw_node; > - const char *hid_phys; > + const char *hid_id; > int number = 0; > > hid_parent = udev_device_get_parent_with_subsystem_devtype(udevice, > "hid", NULL); > > - hid_phys = udev_device_get_property_value(hid_parent, "HID_PHYS"); > + /* > + * Look for HID_UNIQ first for the correct behavior via BT, if > + * HID_UNIQ is not available it means the USB bus is being used and we > + * can rely on HID_PHYS. > + */ > + hid_id = udev_device_get_property_value(hid_parent, "HID_UNIQ"); > + if (!hid_id) > + hid_id = udev_device_get_property_value(hid_parent, > + "HID_PHYS"); > + > hidraw_node = udev_device_get_devnode(udevice); > - if (!hid_phys || !hidraw_node) > + if (!hid_id || !hidraw_node) > return 0; > > enumerate = udev_enumerate_new(udev_device_get_udev(udevice)); > @@ -321,7 +330,7 @@ static int get_js_number(struct udev_device *udevice) > udev_list_entry_foreach(dev_list_entry, devices) { > struct udev_device *input_parent; > struct udev_device *js_dev; > - const char *input_phys; > + const char *input_id; > const char *devname; > > devname = udev_list_entry_get_name(dev_list_entry); > @@ -336,12 +345,20 @@ static int get_js_number(struct udev_device *udevice) > > /* check if this is the joystick relative to the hidraw device > * above */ > - input_phys = udev_device_get_sysattr_value(input_parent, > - "phys"); > - if (!input_phys) > + input_id = udev_device_get_sysattr_value(input_parent, "uniq"); > + > + /* > + * A strlen() check is needed because input device over USB > + * have the UNIQ attribute defined but with an empty value. > + */ > + if (!input_id || strlen(input_id) == 0) > + input_id = udev_device_get_sysattr_value(input_parent, > + "phys"); > + > + if (!input_id) > goto next; > > - if (!strcmp(input_phys, hid_phys)) { > + if (!strcmp(input_id, hid_id)) { > number = atoi(udev_device_get_sysnum(js_dev)); > > /* joystick numbers start from 0, leds from 1 */ Patch applied, thanks. -- Szymon K. Janc szymon.janc@gmail.com