On Fri, May 29, 2015 at 12:32:44PM +0200, Andreas Pokorny wrote: > Hi, > > On Fri, May 29, 2015 at 2:07 AM, Peter Hutterer <peter.hutte...@who-t.net> > wrote: > > > On Thu, May 28, 2015 at 02:47:35PM +0200, Andreas Pokorny wrote: > > > There are touch screens that do not provide 'BTN_TOUCH' and hence no > > > 'EV_KEY'. Previously those would not get any properties assigned and > > > > note: I consider this a kernel bug. From the kernel's documentation: > > * BTN_TOUCH: > > BTN_TOUCH is used for touch contact. While an input tool is determined > > to be within meaningful physical contact, the value of this property must > > be set to 1. > > > > Yes and somewhat further down in the text: > > [snip] > Note: Historically a touch device with BTN_TOOL_FINGER and BTN_TOUCH was > interpreted as a touchpad by userspace, while a similar device without > BTN_TOOL_FINGER was interpreted as a touchscreen. For backwards > compatibility > with current userspace it is recommended to follow this distinction. In the > future, this distinction will be deprecated and the device properties ioctl > EVIOCGPROP, defined in linux/input.h, will be used to convey the device > type. > [snip]
hmm, I read this as a comment regarding BTN_TOOL_FINGER, not BTN_TOUCH. so the property tells us direct/indirect but both still have BTN_TOUCH and a touchscreen may have BTN_TOOL_FINGER (though not recommended). > I try to ensure that we capture most devices with and without broken > drivers as possible. > With the various soc vendors it is hard to fix all kernel source providers. > As far as I can > see from the source code I counted three kernel drivers that do not emit > BTN_TOUCH. > But I believe I only have access to one of those devices. Then there are > various other > that have not been sent upstream. But all of those seemed to set the DIRECT > property. > > > [...] > > > > + has_3d_coordinates = has_coordinates && test_bit(ABS_Z, > > bitmask_abs); > > > + has_mt_coordinates = test_bit(ABS_MT_POSITION_X, bitmask_abs) && > > > + test_bit(ABS_MT_POSITION_Y, bitmask_abs); > > > > pretty sure the second line must be aligned with the =, applies for the > > others too > > > > as a follow-up: this test should be updated to check if ABS_MT_SLOT - 1 > > *and* ABS_MT_SLOT are set. If both are set, the device is *not* a > > touchscreen. > > > > ACK to all of the above, but ABS_MT_SLOT - 1? I never saw that axis > anywhere. Oh or is that a joystick device with so so many axis that it > hits the touch pad/screen range of axis? yep, there's a few devices that simply set all ABS bits (or just anything north of ABS_MISC) and interpreting them as touchscreens has interesting effects. > > [...] > > > + else if (has_3d_coordinates && !has_keys) > > > + is_accelerometer = true; > > > > this is a behaviour change: before anything with INPUT_PROP_ACCELEROMETER > > was tagged as such, now some of these may be tagged as other devices. > > specifically, the Wacom cintiq 27QHD remote would be tagged as tablet now. > > BTN_STYLUS + ABS_X/Y/Z + KEY_PROG1/2/3 + INPUT_PROP_ACCELEROMETER > > > > I think you should reinstate the "if it says it's an accelerometer, > > anything > > else doesn't matter". > > > > The initial code was similar to > if (!keys) { > if(has_3d_coordinates) is_accelerometer = true > return false; > } > I had to change that part for nexus4 and nexus10 devices, because there > was no EV_KEY bit set. > Hence test_pointers returned false. But yes I can reinstate a "if 3d axis + > no buttons it is an accelerometer and anything else does not matter"... > But then what about devices like the PS4 controller? As far as I can tell > it has three rotational axis respective joystick axis and a touchpad. So I > believe the device should have all three udev properties set? does that come through as one kernel device? tbh I don't consider a controller like this as touchpad, setting ID_INPUT_JOYSTICK is enough. and anything that wants to deal with it can then see the touchpad on the joystick but trying to use it as a normal touchpad is IMO outside of the use-cases we should aim for. Cheers, Peter > > [...] > > all the other bits go into the first patch. also, please CC me on the next > > version, this way I'll see it quicker. > > > > Thank you for the review. I will make a patch series out of that, and send > it again soon. > > regards > Andreas Pokorny _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel