Hi On Thu, Dec 18, 2014 at 4:15 PM, David Herrmann <dh.herrm...@gmail.com> wrote: > Hi > > On Thu, Dec 18, 2014 at 3:58 PM, Carlos Garnacho <carl...@gnome.org> wrote: >> This rule is only run on tablet/touchscreen devices, and extracts their size >> in millimeters, as it can be found out through their struct input_absinfo. >> >> Conceivably, that information can be changed through EVIOCSABS anywhere >> else, but we're only interested in values prior to any calibration, this >> rule is thus only run on "add", and no tracking of changes is performed. >> This should only remain a problem if calibration were automatically applied >> by an earlier udev rule (read: don't). >> --- >> Makefile.am | 14 +++ >> rules/60-input_abs_size.rules | 8 ++ >> src/udev/input_abs_size/input_abs_size.c | 177 >> +++++++++++++++++++++++++++++++ >> 3 files changed, 199 insertions(+) >> create mode 100644 rules/60-input_abs_size.rules >> create mode 100644 src/udev/input_abs_size/input_abs_size.c >> >> diff --git a/Makefile.am b/Makefile.am >> index ab07d3b..a23705a 100644 >> --- a/Makefile.am >> +++ b/Makefile.am >> @@ -3741,6 +3741,20 @@ dist_udevrules_DATA += \ >> rules/61-accelerometer.rules >> >> # >> ------------------------------------------------------------------------------ >> +input_abs_size_SOURCES = \ >> + src/udev/input_abs_size/input_abs_size.c >> + >> +input_abs_size_LDADD = \ >> + libudev-internal.la -lm \ >> + libsystemd-shared.la >> + >> +udevlibexec_PROGRAMS += \ >> + input_abs_size_ >> + >> +dist_udevrules_DATA += \ >> + rules/60-input_abs_size.rules >> + >> +# >> ------------------------------------------------------------------------------ >> if ENABLE_GUDEV >> if ENABLE_GTK_DOC >> SUBDIRS += \ >> diff --git a/rules/60-input_abs_size.rules b/rules/60-input_abs_size.rules >> new file mode 100644 >> index 0000000..5f613ac >> --- /dev/null >> +++ b/rules/60-input_abs_size.rules >> @@ -0,0 +1,8 @@ >> +# do not edit this file, it will be overwritten on update >> + >> +ACTION!="add", GOTO="input_abs_size_end" >> + >> +ENV{ID_INPUT_TOUCHSCREEN}=="1", IMPORT{program}="input_abs_size %p" >> +ENV{ID_INPUT_TABLET}=="1", IMPORT{program}="input_abs_size %p" >> + >> +LABEL="input_abs_size_end" >> diff --git a/src/udev/input_abs_size/input_abs_size.c >> b/src/udev/input_abs_size/input_abs_size.c >> new file mode 100644 >> index 0000000..34a6bf3 >> --- /dev/null >> +++ b/src/udev/input_abs_size/input_abs_size.c >> @@ -0,0 +1,177 @@ >> +/* >> + * input_abs_size - extracts abs X/Y size in millimeters from input devices >> + * >> + * Copyright (C) 2014 Red Hat >> + * Author: >> + * Carlos Garnacho <carl...@gnome.org> >> + * >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, but >> + * WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + * General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with keymap; if not, write to the Free Software Foundation, >> + * Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA. >> + */ >> + >> +#include <stdio.h> >> +#include <string.h> >> +#include <math.h> >> +#include <sys/types.h> >> +#include <sys/stat.h> >> +#include <fcntl.h> >> +#include <stdlib.h> >> +#include <unistd.h> >> +#include <getopt.h> >> +#include <limits.h> >> +#include <linux/limits.h> >> +#include <linux/input.h> >> + >> +#include "libudev.h" >> +#include "libudev-private.h" >> + >> +/* Resolution is defined to be in units/mm for ABS_X/Y */ >> +#define ABS_SIZE_MM(absinfo) ((absinfo.maximum - absinfo.minimum) / >> absinfo.resolution) >> + >> +static void extract_info(struct udev *udev, >> + struct udev_device *dev, >> + const char *devpath) >> +{ >> + struct input_absinfo xabsinfo = { 0 }, yabsinfo = { 0 }; >> + _cleanup_close_ int fd = -1; >> + char text[1024]; >> + >> + if ((fd = open(devpath, O_RDONLY)) < 0) >> + return; >> + >> + if (ioctl(fd, EVIOCGABS(ABS_X), &xabsinfo) < 0 || >> + ioctl(fd, EVIOCGABS(ABS_Y), &yabsinfo) < 0) >> + return; >> + >> + if (xabsinfo.resolution <= 0 || yabsinfo.resolution <= 0) >> + return; >> + >> + snprintf (text, sizeof(text), >> + "ID_INPUT_WIDTH_MM=%d\n" >> + "ID_INPUT_HEIGHT_MM=%d", >> + ABS_SIZE_MM (xabsinfo), >> + ABS_SIZE_MM (yabsinfo)); >> + puts (text); >> +} >> + >> +static void help(void) >> +{ >> + printf("Usage: input_abs_size [options] <device path>\n" >> + " --debug debug to stderr\n" >> + " --help print this help text\n\n"); >> +} >> + >> +int main(int argc, char** argv) >> +{ >> + struct udev *udev; >> + struct udev_device *dev; >> + >> + static const struct option options[] = { >> + { "debug", no_argument, NULL, 'd' }, >> + { "help", no_argument, NULL, 'h' }, >> + {} >> + }; >> + >> + char devpath[PATH_MAX]; >> + char *devnode; >> + struct udev_enumerate *enumerate; >> + struct udev_list_entry *list_entry; >> + >> + udev = udev_new(); >> + if (udev == NULL) >> + return 1; >> + >> + log_parse_environment(); >> + log_open(); >> + >> + /* CLI argument parsing */ >> + while (1) { >> + int option; >> + >> + option = getopt_long(argc, argv, "dxh", options, NULL); >> + if (option == -1) >> + break; >> + >> + switch (option) { >> + case 'd': >> + log_set_target(LOG_TARGET_CONSOLE); >> + log_set_max_level(LOG_DEBUG); >> + log_open(); >> + break; >> + case 'h': >> + help(); >> + exit(0); >> + default: >> + exit(1); >> + } >> + } >> + >> + if (argv[optind] == NULL) { >> + help(); >> + exit(1); >> + } >> + >> + /* get the device */ >> + snprintf(devpath, sizeof(devpath), "/sys/%s", argv[optind]); >> + dev = udev_device_new_from_syspath(udev, devpath); >> + if (dev == NULL) { >> + fprintf(stderr, "unable to access '%s'\n", devpath); >> + return 1; >> + } >> + >> + /* Get the children devices and find the devnode */ >> + devnode = NULL; >> + enumerate = udev_enumerate_new(udev); >> + udev_enumerate_add_match_parent(enumerate, dev); >> + udev_enumerate_scan_devices(enumerate); >> + udev_list_entry_foreach(list_entry, >> udev_enumerate_get_list_entry(enumerate)) { > > Who guarantees that there are any children at the time you run this > program? In fact, you run this program immediately once the input > device is found, but the evdev device might get registered later. On > modern machines, the kernel is fast enough to register both before any > uevent can get scheduled. However, there is no such guarantee! I > dislike relying on such races to not hit us.. > > Regardless of that race: What is the point of exposing that > information? Why would any process but a reader of the evdev device be > interested in this? Your commit message lacks any explanation why this > interface is useful. Please elaborate!
Ok, your head-letter includes that information. Could you merge that into the commit-message on v2? I'd recommend doing this whole thing as a built-in as Tom suggested and then write those properties on the "eventXYZ" devices (the actual evdev nodes). Everyone should be using those instead of the input-device, so we should be fine. Thanks David _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel