[PATCH 1/2] Emit TOUCH with DOUBLETAP for single touch

[PATCH 1/2] Emit TOUCH with DOUBLETAP for single touch

[Rafi Rubin]

Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
---

I squelched TipSwitch in a recent patch which resulted in the loss
of Touch events for single touch firmwares.  This patch just puts Touch back
in for single touch, and bundles it with DoubleTap (like the multitouch code).
The two events are used to convey the same message to different drivers.

I still would like to remove DoubleTap, though I think that should wait until
after users have a more motivation to migrate away from the wacom driver for
touch.

 drivers/hid/hid-ntrig.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index a418f9e..58ba0d3 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -173,6 +173,8 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 			if (!nd->reading_mt) {
 				input_report_key(input, BTN_TOOL_DOUBLETAP,
 						 (nd->confidence != 0));
+				input_report_key(input, BTN_TOUCH,
+						 (nd->confidence != 0));
 				input_event(input, EV_ABS, ABS_X, nd->x);
 				input_event(input, EV_ABS, ABS_Y, nd->y);
 			}
-- 
1.7.0.4

[PATCH 2/2] Use TIPSWITCH for TOUCH

[Rafi Rubin]

TipSwitch is a better match for touch.  Confidence and InRange work
for more current firmwares, but with some older versions only
TipSwitch is appropriate.

Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
---

TipSwitch seems to be the prevailing convention for Touch, I was
mistaken to depend on Confidence, at least for single touch firmwares.
While some firmwares send the same value for confidence and tipswitch,
some older versions only send the correct state with tipswitch.

I've also taken the opportunity to update the types and names of some
of the driver data fields.

 drivers/hid/hid-ntrig.c |   25 +++++++++++++++++--------
 1 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 58ba0d3..8a145e9 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -31,10 +31,12 @@ struct ntrig_data {
 	/* Incoming raw values for a single contact */
 	__u16 x, y, w, h;
 	__u16 id;
-	__u8 confidence;
+
+	bool tipswitch;
+	bool confidence;
+	bool first_contact_touch;
 
 	bool reading_mt;
-	__u8 first_contact_confidence;
 
 	__u8 mt_footer[4];
 	__u8 mt_foot_count;
@@ -141,9 +143,10 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 		case 0xff000001:
 			/* Tag indicating the start of a multitouch group */
 			nd->reading_mt = 1;
-			nd->first_contact_confidence = 0;
+			nd->first_contact_touch = 0;
 			break;
 		case HID_DG_TIPSWITCH:
+			nd->tipswitch = value;
 			/* Prevent emission of touch until validated */
 			return 1;
 		case HID_DG_CONFIDENCE:
@@ -171,10 +174,15 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 			 * to emit a normal (X, Y) position
 			 */
 			if (!nd->reading_mt) {
-				input_report_key(input, BTN_TOOL_DOUBLETAP,
-						 (nd->confidence != 0));
+				/*
+				 * TIPSWITCH indicates the presence of a
+				 * finger.  DOUBLETAP is emitted are both
+				 * emitted to support legacy drivers.
+				 */
 				input_report_key(input, BTN_TOUCH,
-						 (nd->confidence != 0));
+						 nd->tipswitch);
+				input_report_key(input, BTN_TOOL_DOUBLETAP,
+						 nd->tipswitch);
 				input_event(input, EV_ABS, ABS_X, nd->x);
 				input_event(input, EV_ABS, ABS_Y, nd->y);
 			}
@@ -213,7 +221,8 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 
 			/* emit a normal (X, Y) for the first point only */
 			if (nd->id == 0) {
-				nd->first_contact_confidence = nd->confidence;
+				nd->first_contact_touch = nd->confidence &&
+					nd->tipswitch;
 				input_event(input, EV_ABS, ABS_X, nd->x);
 				input_event(input, EV_ABS, ABS_Y, nd->y);
 			}
@@ -243,7 +252,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 
 			nd->reading_mt = 0;
 
-			if (nd->first_contact_confidence) {
+			if (nd->first_contact_touch) {
 				switch (value) {
 				case 0:	/* for single touch devices */
 				case 1:
-- 
1.7.0.4

Re: [PATCH 1/2] Emit TOUCH with DOUBLETAP for single touch

[Jiri Kosina]

On Fri, 9 Apr 2010, Rafi Rubin wrote:

> Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
> ---
> 
> I squelched TipSwitch in a recent patch which resulted in the loss
> of Touch events for single touch firmwares.  This patch just puts Touch back
> in for single touch, and bundles it with DoubleTap (like the multitouch code).
> The two events are used to convey the same message to different drivers.

Thanks Rafi. This should probably still be .34 material, right?

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 2/2] Use TIPSWITCH for TOUCH

[Jiri Kosina]

On Fri, 9 Apr 2010, Rafi Rubin wrote:

> TipSwitch is a better match for touch.  Confidence and InRange work
> for more current firmwares, but with some older versions only
> TipSwitch is appropriate.

Thanks.

> @@ -171,10 +174,15 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  			 * to emit a normal (X, Y) position
>  			 */
>  			if (!nd->reading_mt) {
> -				input_report_key(input, BTN_TOOL_DOUBLETAP,
> -						 (nd->confidence != 0));
> +				/*
> +				 * TIPSWITCH indicates the presence of a
> +				 * finger.  DOUBLETAP is emitted are both
> +				 * emitted to support legacy drivers.
> +				 */

This comment has some slightly strange wording.

>  				input_report_key(input, BTN_TOUCH,
> -						 (nd->confidence != 0));
> +						 nd->tipswitch);
> +				input_report_key(input, BTN_TOOL_DOUBLETAP,
> +						 nd->tipswitch);
>  				input_event(input, EV_ABS, ABS_X, nd->x);
>  				input_event(input, EV_ABS, ABS_Y, nd->y);
>  			}
> @@ -213,7 +221,8 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  
>  			/* emit a normal (X, Y) for the first point only */
>  			if (nd->id == 0) {
> -				nd->first_contact_confidence = nd->confidence;
> +				nd->first_contact_touch = nd->confidence &&
> +					nd->tipswitch;
>  				input_event(input, EV_ABS, ABS_X, nd->x);
>  				input_event(input, EV_ABS, ABS_Y, nd->y);
>  			}
> @@ -243,7 +252,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  
>  			nd->reading_mt = 0;
>  
> -			if (nd->first_contact_confidence) {
> +			if (nd->first_contact_touch) {
>  				switch (value) {
>  				case 0:	/* for single touch devices */
>  				case 1:

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 1/2] Emit TOUCH with DOUBLETAP for single touch

[Rafi Rubin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jiri Kosina wrote:
> On Fri, 9 Apr 2010, Rafi Rubin wrote:
> 
>> Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
>> ---
>>
>> I squelched TipSwitch in a recent patch which resulted in the loss
>> of Touch events for single touch firmwares.  This patch just puts Touch back
>> in for single touch, and bundles it with DoubleTap (like the multitouch code).
>> The two events are used to convey the same message to different drivers.
> 
> Thanks Rafi. This should probably still be .34 material, right?

Yes, this fix became more necessary with the previous Touch patch, which
squelched the tipswitch event on behalf of the multi touch firmwares.  So it
would be nice to keep them relatively close together.


Rafi
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkvEzZYACgkQwuRiAT9o60/PaACgmzf/cLF+vyms9JZcR9zZab04
mNsAoPXBPJjFmXdnhITd6NEMf9qjdyR5
=stpn
-----END PGP SIGNATURE-----

Re: [PATCH 2/2] Use TIPSWITCH for TOUCH

[Rafi Rubin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

>> +				/*
>> +				 * TIPSWITCH indicates the presence of a
>> +				 * finger.  DOUBLETAP is emitted are both
>> +				 * emitted to support legacy drivers.
>> +				 */
> 
> This comment has some slightly strange wording.

Yeah, the wording wasn't great before, but it looks like something happened to
the comment I thought I wrote.  I will clean up that comment and resubmit later
today.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkvEzlUACgkQwuRiAT9o609lyQCfS8LgCIT3XPHyH0113lRicYFt
qWsAn07U5muHivTFk1ovYcdzWvaM4gK3
=Bd+a
-----END PGP SIGNATURE-----

[PATCH] Use TipSwitch for Touch

[Rafi Rubin]

TipSwitch is a better match for touch.  Confidence and InRange work
for more current firmwares, but with some older versions only
TipSwitch is appropriate.

Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
---
 drivers/hid/hid-ntrig.c |   27 +++++++++++++++++++--------
 1 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 58ba0d3..071f86d 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -31,10 +31,12 @@ struct ntrig_data {
 	/* Incoming raw values for a single contact */
 	__u16 x, y, w, h;
 	__u16 id;
-	__u8 confidence;
+
+	bool tipswitch;
+	bool confidence;
+	bool first_contact_touch;
 
 	bool reading_mt;
-	__u8 first_contact_confidence;
 
 	__u8 mt_footer[4];
 	__u8 mt_foot_count;
@@ -141,9 +143,10 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 		case 0xff000001:
 			/* Tag indicating the start of a multitouch group */
 			nd->reading_mt = 1;
-			nd->first_contact_confidence = 0;
+			nd->first_contact_touch = 0;
 			break;
 		case HID_DG_TIPSWITCH:
+			nd->tipswitch = value;
 			/* Prevent emission of touch until validated */
 			return 1;
 		case HID_DG_CONFIDENCE:
@@ -171,10 +174,17 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 			 * to emit a normal (X, Y) position
 			 */
 			if (!nd->reading_mt) {
-				input_report_key(input, BTN_TOOL_DOUBLETAP,
-						 (nd->confidence != 0));
+				/*
+				 * TipSwitch indicates the presence of a
+				 * finger.  Touch is the preferred event for
+				 * downstream support.  DoubleTap is also
+				 * emitted to support proper operation with
+				 * the wacom X driver (for now).
+				 */
 				input_report_key(input, BTN_TOUCH,
-						 (nd->confidence != 0));
+						 nd->tipswitch);
+				input_report_key(input, BTN_TOOL_DOUBLETAP,
+						 nd->tipswitch);
 				input_event(input, EV_ABS, ABS_X, nd->x);
 				input_event(input, EV_ABS, ABS_Y, nd->y);
 			}
@@ -213,7 +223,8 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 
 			/* emit a normal (X, Y) for the first point only */
 			if (nd->id == 0) {
-				nd->first_contact_confidence = nd->confidence;
+				nd->first_contact_touch = nd->confidence &&
+					nd->tipswitch;
 				input_event(input, EV_ABS, ABS_X, nd->x);
 				input_event(input, EV_ABS, ABS_Y, nd->y);
 			}
@@ -243,7 +254,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 
 			nd->reading_mt = 0;
 
-			if (nd->first_contact_confidence) {
+			if (nd->first_contact_touch) {
 				switch (value) {
 				case 0:	/* for single touch devices */
 				case 1:
-- 
1.7.0.4

Re: [PATCH] Use TipSwitch for Touch

[Dmitry Torokhov]

On Wed, Apr 14, 2010 at 01:04:54AM -0400, Rafi Rubin wrote:
> TipSwitch is a better match for touch.  Confidence and InRange work
> for more current firmwares, but with some older versions only
> TipSwitch is appropriate.
> 
> Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
> ---
>  drivers/hid/hid-ntrig.c |   27 +++++++++++++++++++--------
>  1 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
> index 58ba0d3..071f86d 100644
> --- a/drivers/hid/hid-ntrig.c
> +++ b/drivers/hid/hid-ntrig.c
> @@ -31,10 +31,12 @@ struct ntrig_data {
>  	/* Incoming raw values for a single contact */
>  	__u16 x, y, w, h;
>  	__u16 id;
> -	__u8 confidence;
> +
> +	bool tipswitch;
> +	bool confidence;
> +	bool first_contact_touch;
>  
>  	bool reading_mt;
> -	__u8 first_contact_confidence;
>  
>  	__u8 mt_footer[4];
>  	__u8 mt_foot_count;
> @@ -141,9 +143,10 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  		case 0xff000001:
>  			/* Tag indicating the start of a multitouch group */
>  			nd->reading_mt = 1;
> -			nd->first_contact_confidence = 0;
> +			nd->first_contact_touch = 0;
>  			break;
>  		case HID_DG_TIPSWITCH:
> +			nd->tipswitch = value;
>  			/* Prevent emission of touch until validated */
>  			return 1;
>  		case HID_DG_CONFIDENCE:
> @@ -171,10 +174,17 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  			 * to emit a normal (X, Y) position
>  			 */
>  			if (!nd->reading_mt) {
> -				input_report_key(input, BTN_TOOL_DOUBLETAP,
> -						 (nd->confidence != 0));
> +				/*
> +				 * TipSwitch indicates the presence of a
> +				 * finger.  Touch is the preferred event for
> +				 * downstream support.  DoubleTap is also
> +				 * emitted to support proper operation with
> +				 * the wacom X driver (for now).
> +				 */

I am not sure if we want to propagate unfortunate event selection in
wacom drievr into other drivers. Any chance you could lend Ping hand in
adding proper multitouch support to wacom X driver instead?

-- 
Dmitry

Re: [PATCH] Use TipSwitch for Touch

[Rafi Rubin]

Dmitry Torokhov wrote:
> On Wed, Apr 14, 2010 at 01:04:54AM -0400, Rafi Rubin wrote:
>> TipSwitch is a better match for touch.  Confidence and InRange work
>> for more current firmwares, but with some older versions only
>> TipSwitch is appropriate.
>>
>> Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
>> ---
>>  drivers/hid/hid-ntrig.c |   27 +++++++++++++++++++--------
>>  1 files changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
>> index 58ba0d3..071f86d 100644
>> --- a/drivers/hid/hid-ntrig.c
>> +++ b/drivers/hid/hid-ntrig.c
>> @@ -31,10 +31,12 @@ struct ntrig_data {
>>  	/* Incoming raw values for a single contact */
>>  	__u16 x, y, w, h;
>>  	__u16 id;
>> -	__u8 confidence;
>> +
>> +	bool tipswitch;
>> +	bool confidence;
>> +	bool first_contact_touch;
>>  
>>  	bool reading_mt;
>> -	__u8 first_contact_confidence;
>>  
>>  	__u8 mt_footer[4];
>>  	__u8 mt_foot_count;
>> @@ -141,9 +143,10 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>>  		case 0xff000001:
>>  			/* Tag indicating the start of a multitouch group */
>>  			nd->reading_mt = 1;
>> -			nd->first_contact_confidence = 0;
>> +			nd->first_contact_touch = 0;
>>  			break;
>>  		case HID_DG_TIPSWITCH:
>> +			nd->tipswitch = value;
>>  			/* Prevent emission of touch until validated */
>>  			return 1;
>>  		case HID_DG_CONFIDENCE:
>> @@ -171,10 +174,17 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>>  			 * to emit a normal (X, Y) position
>>  			 */
>>  			if (!nd->reading_mt) {
>> -				input_report_key(input, BTN_TOOL_DOUBLETAP,
>> -						 (nd->confidence != 0));
>> +				/*
>> +				 * TipSwitch indicates the presence of a
>> +				 * finger.  Touch is the preferred event for
>> +				 * downstream support.  DoubleTap is also
>> +				 * emitted to support proper operation with
>> +				 * the wacom X driver (for now).
>> +				 */
> 
> I am not sure if we want to propagate unfortunate event selection in
> wacom drievr into other drivers. Any chance you could lend Ping hand in
> adding proper multitouch support to wacom X driver instead?

I agree.  Unfortunately I made the mistake of starting with the wacom 
driver a year ago, and now am trying to avoid breaking things for users, 
at least for a while longer.

I'm confident evdev or Henrik's multitouch driver will dominate in the 
near future.  Evdev already supports the touch modes as well or better 
than the wacom for generic touch devices.  I'm hoping users will migrate 
over the next few months and plan to remove the redundant tap events 
after the transition.

But you raise perhaps a better point.  Migrate the wacom stack to match 
the event conventions of everything else.  I'd certainly be willing to help.

Re: [PATCH] Use TipSwitch for Touch

[Stéphane Chatty]

Le 14 avr. 10 à 08:32, Dmitry Torokhov a écrit :

>
>
> I am not sure if we want to propagate unfortunate event selection in
> wacom drievr into other drivers. Any chance you could lend Ping  
> hand in
> adding proper multitouch support to wacom X driver instead?

I proposed my help a few weeks ago, but got no response.

St.

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Use TipSwitch for Touch

[Mohamed Ikbel Boulabiar]

Hi,

Maybe off-topic, but what about the support of Bamboo multitouch
devices from Wacom ?
http://www.wacom.com/bamboo/

M-I


On Wed, Apr 14, 2010 at 8:32 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Apr 14, 2010 at 01:04:54AM -0400, Rafi Rubin wrote:
>> TipSwitch is a better match for touch.  Confidence and InRange work
>> for more current firmwares, but with some older versions only
>> TipSwitch is appropriate.
>>
>> Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
>> ---
>>  drivers/hid/hid-ntrig.c |   27 +++++++++++++++++++--------
>>  1 files changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
>> index 58ba0d3..071f86d 100644
>> --- a/drivers/hid/hid-ntrig.c
>> +++ b/drivers/hid/hid-ntrig.c
>> @@ -31,10 +31,12 @@ struct ntrig_data {
>>       /* Incoming raw values for a single contact */
>>       __u16 x, y, w, h;
>>       __u16 id;
>> -     __u8 confidence;
>> +
>> +     bool tipswitch;
>> +     bool confidence;
>> +     bool first_contact_touch;
>>
>>       bool reading_mt;
>> -     __u8 first_contact_confidence;
>>
>>       __u8 mt_footer[4];
>>       __u8 mt_foot_count;
>> @@ -141,9 +143,10 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>>               case 0xff000001:
>>                       /* Tag indicating the start of a multitouch group */
>>                       nd->reading_mt = 1;
>> -                     nd->first_contact_confidence = 0;
>> +                     nd->first_contact_touch = 0;
>>                       break;
>>               case HID_DG_TIPSWITCH:
>> +                     nd->tipswitch = value;
>>                       /* Prevent emission of touch until validated */
>>                       return 1;
>>               case HID_DG_CONFIDENCE:
>> @@ -171,10 +174,17 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>>                        * to emit a normal (X, Y) position
>>                        */
>>                       if (!nd->reading_mt) {
>> -                             input_report_key(input, BTN_TOOL_DOUBLETAP,
>> -                                              (nd->confidence != 0));
>> +                             /*
>> +                              * TipSwitch indicates the presence of a
>> +                              * finger.  Touch is the preferred event for
>> +                              * downstream support.  DoubleTap is also
>> +                              * emitted to support proper operation with
>> +                              * the wacom X driver (for now).
>> +                              */
>
> I am not sure if we want to propagate unfortunate event selection in
> wacom drievr into other drivers. Any chance you could lend Ping hand in
> adding proper multitouch support to wacom X driver instead?
>
> --
> Dmitry
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html