[PATCH] xfree86: store the InputAttributes in the input device.

[Peter Hutterer]

On Tue, May 25, 2010 at 05:04:03PM -0700, Dan Nicholson wrote:
> On Tue, May 25, 2010 at 3:48 PM, Peter Hutterer
> <peter.hutterer at who-t.net> wrote:
> > On Tue, May 25, 2010 at 02:52:05PM -0700, Dan Nicholson wrote:
> >> On Tue, May 25, 2010 at 2:23 PM, Peter Hutterer
> >> <peter.hutterer at who-t.net> wrote:
> >> > On Tue, May 25, 2010 at 08:34:29AM -0700, Dan Nicholson wrote:
> >> >> On Tue, May 25, 2010 at 12:15 AM, Peter Hutterer
> >> >> <peter.hutterer at who-t.net> wrote:
> >> >> > InputAttributes largely decide which configuration values get merged from
> >> >> > the xorg.conf.d snippets. While they are available in the config backend,
> >> >> > they are not available for any other callers of NewInputDeviceRequest().
> >> >> >
> >> >> > Drivers implementing driver-side hotplugging do not have access to these
> >> >> > attributes and cannot have xorg.conf.d snippets specific to dependent
> >> >> > devices. For example, the following case cannot work right now:
> >> >> >
> >> >> > Section "InputClass"
> >> >> >        MatchProduct "Wacom"
> >> >> >        Option "PressCurve" "0 0 100 100"
> >> >> >        ...
> >> >> > EndSection
> >> >> >
> >> >> > Section "InputClass"
> >> >> >        MatchProduct "Wacom"
> >> >> >        MatchProduct "eraser"
> >> >> >        Option "PressCurve" "10 10 50 50"
> >> >> >        ...
> >> >> > EndSection
> >> >> >
> >> >> > The second section is not triggered, as the wacom driver cannot supply the
> >> >> > InputAttributes to NewInputDeviceRequest().
> >> >>
> >> >> The second section is also not entirely valid since there can only be
> >> >> one MatchProduct used per section. :) I don't know what the best way
> >> >> to differentiate the wacom multiple devices per device file case is.
> >> >> Maybe you could munge the product name to "Wacom Eraser" or something.
> >> >
> >> > hmm, this is quite bad then. I just assumed it worked and when I tried the
> >> > above it must have worked only because I had just one device with "eraser"
> >> > in the name.
> >> > scrapping the product name isn't really an option, it's complicated enough
> >> > as it is already (at least for wacom devices) and having the model
> >> > identifier in the name is quite useful.
> >> >
> >> > I think there's a distinct need for supporting & on top of the current |,
> >> > it's quite hard to get generic matches for subdevices otherwise.
> >>
> >> Yeah, I'd say subdevices are hard. What are you planning on setting
> >> the product name to? Just tacking " eraser" onto the end of whatever
> >> is currently there? Some options I can think of here:
> >
> > there's a thread on the wacom devel list for this since I already had the
> > wacom patch ready.
> > http://sourceforge.net/mailarchive/forum.php?thread_name=20100525012734.GB8972%40barra.redhat.com&forum_name=linuxwacom-devel
> 
> Gross, sourceforge archives. :)
> 
> >> 1. Set a tag from wacom to differentiate the eraser. I.e. take
> >> attrs->tags, realloc another slot on the array and set it to "eraser".
> >> Then MatchTag "eraser" in the .conf.
> >
> > That's easy enough (and the patch linked above does that) but there's no
> > possibility to set tags by a user configuration, they appear magically from
> > the config backend. So while name matching could (in theory) be combined to
> > whatever match you need, tags are limited by what's added by the backend.
> >
> > Especially when multiple tools must be configured by the user by hand, you
> > can't apply config snippets to them based on tag matches.
> >
> > Tags also are not visible from clients so xinput --list would give you the
> > same name 4 times, without you knowing which of those 4 is the eraser. So we
> > kinda have to merge the type into the name anyway.
> 
> Good point.
> 
> >> 2. Add a subdevice string in InputAttributes + MatchSubdevice in
> >> xorg.conf. Pretty lame, but this probably won't be the last time
> >> multiple devices per device file will be needed.
> >
> > Similar problems here, the subdevice string is not visible from the client
> > unless we merge it into the name.
> 
> True.
> 
> >> 3. Handle & matching by making multiple Match* lines mean they all
> >> have to match. This would unfortunately make the parsing and matching
> >> more difficult, but is doable. On the other hand, you're still munging
> >> the product name in this case, which doesn't seem that nice.
> >
> > This is the best way I think. I originally tried to match on "Wacom *
> > eraser", realising that this doesn't work either. And that's quite
> > counter-intuitive given that the MatchDevice string will happily take
> > placeholders like this.
> 
> Yeah, it was probably an unfortunate decision. I was trying to make it
> be mostly like the "contains" type of matches in the hal fdi since
> that's what people had been using, but now I wished I'd just used
> fnmatch for everything. We could still make that change, but it could
> hurt people that have already started using the .conf files.

Is there some middle-ground? Like automatically appending ** to substrings
for fnmatch if no wildcard patterns were specified at the beginning and end
of the string?

So that MatchProduct "foo|bar" expands into "*foo*|*bar*"
We'd only need to handle the *string* case anyway, because that's
essentially what we're doing now.

> Alternatively, there could be variants like MatchProductPattern, etc.
> Really nice would be if you could specify the matching type.
> 
> MatchProduct Pattern "Foo*"
> MatchVendor Substring "Bar"
> 
> But I agree that multiple matches seems like a good idea. Probably
> this will require turning all the "char **" types in
> XF86ConfInputClassRec into lists and looping over them when doing the
> matching. Doesn't seem too hard, but will be some churn.

At this point, I think we might have to look into waiting for 1.10 with all
this anyway. I don't have a lot of time to work on this and it's not a
crucial problem. While the current wacom hotplugging code is complicated, it
does work. evdev hotplugging can wait, it's a side-project anyway.

So we should probably just push this into a input-hotplug-rework feature
branch and wait for the next merge window to open.

Cheers,
  Peter