[PATCH] evdev: add phys property (EVIOCGPHYS) as stable identifier

[Peter Hutterer]

On Mon, Apr 26, 2010 at 09:33:16PM +0200, Peter Korsgaard wrote:
> xinput ids are not guaranteed to be stable between reboots (or hotplugs),
> so add a "Evdev physical" property containing the output of the
> EVIOCGPHYS ioctl as a stable identifier. This is needed to be able to
> apply device-specific parameters at runtime (E.G. touchscreen
> transformation in multi-head setups).
> 
> EVIOCGPHYS is used rather than E.G. the device node or sysfs path,
> as it is (supposed to be) unique, simple to access and the remaining
> information can be retrieved through /proc/bus/input/devices, which
> doesn't require any special privileges.
> 
> Signed-off-by: Peter Korsgaard <peter.korsgaard at barco.com>
> ---
>  include/evdev-properties.h |    3 +++
>  src/evdev.c                |   16 ++++++++++++++--
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/include/evdev-properties.h b/include/evdev-properties.h
> index 7df2876..6ab42b3 100644
> --- a/include/evdev-properties.h
> +++ b/include/evdev-properties.h
> @@ -66,4 +66,7 @@
>  /* BOOL */
>  #define EVDEV_PROP_SWAP_AXES "Evdev Axes Swap"
>  
> +/* Physical location of input device as defined by the kernel */
> +#define EVDEV_PROP_PHYSICAL "Evdev Physical"
> +

Please name this "Evdev Physical Path".

I'm also wondering if we should make this a two-string property to export
the "Device" option and the Phys path at the same time. especially for
xorg.conf settings, there is no mapping between what had a given device path
and what had a phys.

Which also brings us to the question of whether this should be implemented
in the server instead then? I'm sure the other drivers could benefit from
this as well.

>  #endif
> diff --git a/src/evdev.c b/src/evdev.c
> index ccea90d..accc1d4 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -118,6 +118,7 @@ static Atom prop_calibration = 0;
>  static Atom prop_swap = 0;
>  static Atom prop_axis_label = 0;
>  static Atom prop_btn_label = 0;
> +static Atom prop_phys = 0;
>  #endif
>  
>  /* All devices the evdev driver has allocated and knows about.
> @@ -2453,6 +2454,17 @@ EvdevInitProperty(DeviceIntPtr dev)
>      EvdevPtr     pEvdev = pInfo->private;
>      int          rc;
>      BOOL         invert[2];
> +    char         phys[256];

valgrind complains here, can you make this char phys[256] = {0}; instead
please. (conditional jump uninitialized variables blah blah)

> +
> +    if (ioctl(pInfo->fd, EVIOCGPHYS(sizeof(phys) - 1), phys) >= 0)
> +    {
> +        prop_phys = MakeAtom(EVDEV_PROP_PHYSICAL,
> +                             strlen(EVDEV_PROP_PHYSICAL), TRUE);
> +        rc = XIChangeDeviceProperty(dev, prop_phys, XA_STRING, 8,
> +                                    PropModeReplace, strlen(phys), phys, FALSE);
> +        if (rc != Success)
> +            return;
> +    }
>  
>      if (pEvdev->flags & (EVDEV_RELATIVE_EVENTS | EVDEV_ABSOLUTE_EVENTS))
>      {
> @@ -2561,8 +2573,8 @@ EvdevSetProperty(DeviceIntPtr dev, Atom atom, XIPropertyValuePtr val,
>  
>          if (!checkonly)
>              pEvdev->swap_axes = *((BOOL*)val->data);
> -    } else if (atom == prop_axis_label || atom == prop_btn_label)
> -        return BadAccess; /* Axis/Button labels can't be changed */
> +    } else if (atom == prop_axis_label || atom == prop_btn_label || atom == prop_phys)
> +        return BadAccess; /* Axis/Button labels + phys can't be changed */
>  
>      return Success;
>  }
> -- 
> 1.7.0

as you said on IRC, the man page hunks are missing, please include these in
the next version.
 
Cheers,
  Peter