<div dir="ltr"><div>Hi, <br></div><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 29, 2015 at 2:07 AM, Peter Hutterer <span dir="ltr"><<a href="mailto:peter.hutterer@who-t.net" target="_blank">peter.hutterer@who-t.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span>On Thu, May 28, 2015 at 02:47:35PM +0200, Andreas Pokorny wrote:<br>
> There are touch screens that do not provide 'BTN_TOUCH' and hence no<br>
> 'EV_KEY'. Previously those would not get any properties assigned and<br>
<br>
</span>note: I consider this a kernel bug. From the kernel's documentation:<br>
* BTN_TOUCH:<br>
BTN_TOUCH is used for touch contact. While an input tool is determined<br>
to be within meaningful physical contact, the value of this property must<br>
be set to 1.<br></blockquote><div> </div>Yes and somewhat further down in the text:<br><br></div><div class="gmail_quote">[snip]<br></div><div class="gmail_quote">Note: Historically a touch device with BTN_TOOL_FINGER and BTN_TOUCH was<br>interpreted as a touchpad by userspace, while a similar device without<br>BTN_TOOL_FINGER was interpreted as a touchscreen. For backwards compatibility<br>with current userspace it is recommended to follow this distinction. In the<br>future, this distinction will be deprecated and the device properties ioctl<br>EVIOCGPROP, defined in linux/input.h, will be used to convey the device type.<br></div><div class="gmail_quote">[snip]<br><br></div><div class="gmail_quote">I try to ensure that we capture most devices with and without broken drivers as possible. <br></div><div class="gmail_quote">With the various soc vendors it is hard to fix all kernel source providers. As far as I can <br></div><div class="gmail_quote">see from the source code I counted three kernel drivers that do not emit BTN_TOUCH.<br></div><div class="gmail_quote">But I believe I only have access to one of those devices. Then there are various other<br></div><div class="gmail_quote">that have not been sent upstream. But all of those seemed to set the DIRECT property. <br></div><div class="gmail_quote"><br><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div>[...] <br></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div></div></div><span>
> + has_3d_coordinates = has_coordinates && test_bit(ABS_Z, bitmask_abs);<br>
> + has_mt_coordinates = test_bit(ABS_MT_POSITION_X, bitmask_abs) &&<br>
> + test_bit(ABS_MT_POSITION_Y, bitmask_abs);<br>
<br>
</span>pretty sure the second line must be aligned with the =, applies for the<br>
others too<br>
<br>
as a follow-up: this test should be updated to check if ABS_MT_SLOT - 1<br>
*and* ABS_MT_SLOT are set. If both are set, the device is *not* a<br>
touchscreen.<br></blockquote><div><br></div><div>ACK to all of the above, but ABS_MT_SLOT - 1? I never saw that axis anywhere. Oh<br></div><div>or is that a joystick device with so so many axis that it hits the touch pad/screen range of axis?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span></span>[...] <br><div><div>
> + else if (has_3d_coordinates && !has_keys)<br>
> + is_accelerometer = true;<br>
<br>
</div></div>this is a behaviour change: before anything with INPUT_PROP_ACCELEROMETER<br>
was tagged as such, now some of these may be tagged as other devices.<br>
specifically, the Wacom cintiq 27QHD remote would be tagged as tablet now.<br>
BTN_STYLUS + ABS_X/Y/Z + KEY_PROG1/2/3 + INPUT_PROP_ACCELEROMETER<br>
<br>
I think you should reinstate the "if it says it's an accelerometer, anything<br>
else doesn't matter".<br></blockquote><div><br></div><div>The initial code was similar to<br> if (!keys) { <br> if(has_3d_coordinates) is_accelerometer = true<br></div><div> return false;<br> }<br></div><div>I had to change that part for nexus4 and nexus10 devices, because there was no EV_KEY bit set.<br>Hence test_pointers returned false. But yes I can reinstate a "if 3d axis + no buttons it is an <br>accelerometer and anything else does not matter"...<br></div><div>But then what about devices like the PS4 controller? As far as I can tell it has three rotational axis<br></div><div>respective joystick axis and a touchpad. So I believe the device should have all three udev properties<br></div><div>set?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span>[...]</span><br>
all the other bits go into the first patch. also, please CC me on the next<br>
version, this way I'll see it quicker.<br></blockquote></div><br></div><div class="gmail_extra">Thank you for the review. I will make a patch series out of that, and send it again soon. <br><br></div><div class="gmail_extra">regards<br></div><div class="gmail_extra">Andreas Pokorny<br></div></div>