Hi Antonio, On Wednesday 14 May 2014 23:40:05 Antonio Ospite wrote: > On recent kernels the hid-sony driver exposes leds class entries in > sysfs for setting the Sixaxis LEDs, use this interface and fall back to > hidraw in case using sysfs fails (e.g. on older hid-sony versions). > > Setting the LEDs via sysfs is the preferred way on newer kernels, the > rationale behind that is: > > 1. the Sixaxis uses the same HID output report for setting both LEDs > and rumble effects; > 2. hid-sony remembers the state of LEDs in order to preserve them when > setting rumble effects; > 3. when the LEDs are set via hidraw hid-sony has no way to know the > new LEDs state and thus can change the LEDs in an inconsistent way > when setting rumble effects later. > > Also require libudev >= 172, this is where > udev_enumerate_add_match_parent() has been first introduced. > > NOTE: using udev_enumerate_add_match_parent() results in a memory leak > when enumerating child devices, this has been fixed in udev 207; the > commit which fixes the issue is this one: > http://cgit.freedesktop.org/systemd/systemd/commit/?id=51cc07576e119dea6e654 > 78eeba9472979fd0936 --- > > Changes since v1: > - Use capital letter after colon in the short commit message. > - Bump the libudev version here. > - Make set_leds_sysfs() return a bool. > - Use implicit NULL checks for pointers. > - Rename get_leds_devices() to get_leds_syspath_prefix() and make it > return a char * representing the common prefix for all LEDs sysfs paths. - > Start the loop from 1 in set_leds_sysfs(), now that we use the syspath > prefix the loop index can match the LEDs numbers. > - Check the return value of set_leds_sysfs() directly in the condition, do > not use a 'ret' variable. Being used to the linux kernel style I don't > particularly like calling functions in conditions but Szymon said it's OK > in BlueZ for function retuning boolean values. > - Fix the style of a multi-line comment. > > configure.ac | 4 +-- > plugins/sixaxis.c | 84 > ++++++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 82 > insertions(+), 6 deletions(-) > > diff --git a/configure.ac b/configure.ac > index 54a387f..4208ad8 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -125,8 +125,8 @@ AM_CONDITIONAL(MONITOR, test "${enable_monitor}" != > "no") AC_ARG_ENABLE(udev, AC_HELP_STRING([--disable-udev], > [disable udev device support]), [enable_udev=${enableval}]) > if (test "${enable_tools}" != "no" && test "${enable_udev}" != "no"); then > - PKG_CHECK_MODULES(UDEV, libudev >= 143, dummy=yes, > - AC_MSG_ERROR(libudev >= 143 is required)) > + PKG_CHECK_MODULES(UDEV, libudev >= 172, dummy=yes, > + AC_MSG_ERROR(libudev >= 172 is required)) > AC_SUBST(UDEV_CFLAGS) > AC_SUBST(UDEV_LIBS) > AC_CHECK_LIB(udev, udev_hwdb_new, > diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c > index 56110a4..c6a3078 100644 > --- a/plugins/sixaxis.c > +++ b/plugins/sixaxis.c > @@ -193,10 +193,40 @@ static void set_leds_hidraw(int fd, uint8_t > leds_bitmap) error("sixaxis: failed to set LEDS (%d bytes written)", ret); > } > > +static bool set_leds_sysfs(struct leds_data *data) > +{ > + int i; > + > + /* start from 1, LED0 is never used */ > + for (i = 1; i <= 4; i++) { > + char path[PATH_MAX] = { 0 }; > + char buf[2] = { 0 }; > + int fd; > + int ret; > + > + if (!data->syspath_prefix) > + return FALSE; Use true/false if using bool type. You can also check this once before loop. > + > + snprintf(path, PATH_MAX, "%s%d/brightness", data->syspath_prefix, i); This is not fitting 80 chars limit. > + fd = open(path, O_WRONLY); > + if (fd < 0) { > + error("sixaxis: cannot open %s (%s)", path, strerror(errno)); > + return FALSE; > + } > + > + buf[0] = '0' + !!(data->bitmap & (1 << i)); > + ret = write(fd, buf, sizeof(buf)); > + close(fd); > + if (ret != sizeof(buf)) > + return FALSE; > + } > + > + return TRUE; > +} > + > static gboolean setup_leds(GIOChannel *channel, GIOCondition cond, > gpointer user_data) > { > - int fd; > struct leds_data *data = user_data; > > if (!data) > @@ -205,9 +235,10 @@ static gboolean setup_leds(GIOChannel *channel, > GIOCondition cond, if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) > goto out; > > - fd = g_io_channel_unix_get_fd(channel); > - > - set_leds_hidraw(fd, data->bitmap); > + if(!set_leds_sysfs(data)) { > + int fd = g_io_channel_unix_get_fd(channel); > + set_leds_hidraw(fd, data->bitmap); > + } > > out: > leds_data_destroy(&data); > @@ -330,6 +361,45 @@ next: > return number; > } > > +static char *get_leds_syspath_prefix(struct udev_device *udevice) > +{ > + struct udev_list_entry *dev_list_entry; > + struct udev_enumerate *enumerate; > + struct udev_device *hid_parent; > + const char *syspath; > + char *syspath_prefix; > + > + hid_parent = udev_device_get_parent_with_subsystem_devtype(udevice, > + "hid", NULL); > + > + enumerate = udev_enumerate_new(udev_device_get_udev(udevice)); > + udev_enumerate_add_match_parent(enumerate, hid_parent); > + udev_enumerate_add_match_subsystem(enumerate, "leds"); > + udev_enumerate_scan_devices(enumerate); > + > + dev_list_entry = udev_enumerate_get_list_entry(enumerate); > + if (!dev_list_entry) { > + syspath_prefix = NULL; > + goto out; > + } > + > + syspath = udev_list_entry_get_name(dev_list_entry); > + > + /* > + * All the sysfs paths of the LEDs have the same structure, just the > + * number changes, so strip it and store only the common prefix. > + * > + * Subtracting 1 here means assuming that the LED number is a single > + * digit, this is safe as the kernel driver only exposes 4 LEDs. > + */ > + syspath_prefix = strndup(syspath, strlen(syspath) - 1); > + > +out: > + udev_enumerate_unref(enumerate); > + > + return syspath_prefix; > +} > + > static struct leds_data *get_leds_data(struct udev_device *udevice) > { > struct leds_data *data; > @@ -346,6 +416,12 @@ static struct leds_data *get_leds_data(struct > udev_device *udevice) if (data->bitmap == 0) > goto err; > > + /* > + * It's OK if this fails, set_leds_hidraw() will be used in > + * case data->syspath_prefix is NULL. > + */ > + data->syspath_prefix = get_leds_syspath_prefix(udevice); > + > return data; > > err: -- Szymon K. Janc szymon.janc@xxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
