Re: [Linuxwacom-devel] [PATCH 1/4] Remove channel duplication code for generic devices

2012-12-14 Thread Ping Cheng
Hi Chris,

Long time no "say". What's your opinion about the patchset? Do you care to
give an acked-by or nacked-by? Without your help, the bamboo PAD is broken
every so often these days

Ping

On Fri, Dec 14, 2012 at 2:58 PM, Chris Bagwell  wrote:

>
> On Thu, Dec 13, 2012 at 9:37 PM, Peter Hutterer 
> wrote:
>
>> On Wed, Dec 12, 2012 at 11:08:17AM -0800, Ping Cheng wrote:
>> > On Wed, Dec 12, 2012 at 10:54 AM, Jason Gerecke > >wrote:
>> >
>> > > It certainly doesn't hurt :) I think my biggest problem is that I
>> never
>> > > really understood exactly when this code was required.
>> >
>> >
>> > I bet you don't want to know ;-). Chris knows this chunk of code way
>> better
>> > than me. It is a long story and it is history already. I'll share it
>> > offline, with those who are curious...
>>
>> no, please share it online so it's archived for future generations.
>> this code isn't that old, so if we can't even remember what it was written
>> for we really need to document better.
>>
>> commit messages are free, we might as well use them properly.
>>
>> afaict, this code was there because before the MT protocol we used the
>> BTN_TOOL_DOUBLETAP and friends to switch between tools. but because this
>> was
>> abusing the system a bit, switching to a different tool wouldn't update
>> the
>> other coordinates for that tool (the kernel would skip those events if
>> they
>> were on the last-sent coordinates for the previous tool), so we had to
>> copy
>> that around in the driver.
>>
>> except for the pad which is always in proximity and didn't rely on
>> multiplexing, so we could skip that.
>>
>> does this sound correct?
>>
>> Cheers,
>>Peter
>>
>>
> Right, except that "tool" means fingers in this case.  The "old kernel
> way" is explained in some detail on this page under the "Touchpad Overview"
> section.
>
>
> https://sourceforge.net/apps/mediawiki/linuxwacom/index.php?title=Kernel_Input_Event_Overview
>
> The concept of that deleted code is that any user land apps need to track
> last values sent by kernel that are shared between tools and when switching
> tools you need to continue using the last sent values because of event
> filtering.  Since we are storing X/Y/Pressure in different "channels", when
> switching channels the X/Y/Pressure fields need to be copied over at switch
> time.
>
> For MT devices, there is not same concept of shared events so the code can
> be deleted.
>
> Protocol 5 devices and any Protocol 4 with mice do in fact need similar
> logic but we don't have it; as the Protocol 5 deleted comment alludes to.
> We are most likely losing data during some tool/finger switch overs for
> those devices but since they mostly work in Absolute mode almost no one
> notices it because of fast recovery.
>
> FYI: I see now that I documented "Synaptics" wrong on wiki.The
> X/Y/Pressure for syntaptics are really always bound to the FINGER tool and
> DOUBLETAP is more an extra tool with no X/Y/Pressure associated directly
> with it.  Something closer to this:
>
>  * BTN_TOOL_FINGER
>* BTN_TOUCH
>* ABS_X
>* ABS_Y
>* ABS_PRESSURE
>  * BTN_TOOL_DOUBLETAP
>  * BTN_LEFT
>  * BTN_RIGHT
>
> Chris
>
>
>> >
>> > Ping
>> >
>> > It makes sense that only MT using *TAP events would need it (or
>> dual-track,
>> > > but that's protocol 5), so I don't really see any problem given the
>> testing
>> > > you've done.
>> > >
>> > >
>> > > Jason
>> > >
>> > > ---
>> > > When you're rife with devastation / There's a simple explanation:
>> > > You're a toymaker's creation / Trapped inside a crystal ball.
>> > > And whichever way he tilts it / Know that we must be resilient
>> > > We won't let them break our spirits / As we sing our silly song.
>> > >
>> > >
>> > >
>> > > On Wed, Dec 12, 2012 at 10:39 AM, Ping Cheng 
>> wrote:
>> > >
>> > >> On Wed, Dec 12, 2012 at 9:45 AM, Jason Gerecke > >wrote:
>> > >>
>> > >>> Though I can't quite convince myself that this is safe, I don't see
>> any
>> > >>> problems with it.
>> > >>>
>> > >>> Acked-By: Jason Gerecke 
>> > >>>
>> > >>
>> > >> Will I say it is tested on older and new systems make it easier to
>> > >> convince you?
>> > >>
>> > >> Ping
>> > >>
>> > >>
>> > >>>  Jason
>> > >>>
>> > >>> ---
>> > >>> When you're rife with devastation / There's a simple explanation:
>> > >>> You're a toymaker's creation / Trapped inside a crystal ball.
>> > >>> And whichever way he tilts it / Know that we must be resilient
>> > >>> We won't let them break our spirits / As we sing our silly song.
>> > >>>
>> > >>>
>> > >>>
>> > >>> On Thu, Dec 6, 2012 at 4:20 PM, Ping Cheng 
>> wrote:
>> > >>>
>> >  We use true MT protocol for MT devices in kernel now. This code
>> >  was introduced to deal with ABS_TOOL_*TAP events loss issue. It
>> >  is uncessary any more. And its existence makes it hard to support
>> >  generic PAD cleanly.
>> > 
>> >  Signed-off-by: Ping Cheng 
>> >  ---
>> >   src/wcmUSB.c |   34 ++

Re: [Linuxwacom-devel] [PATCH 1/4] Remove channel duplication code for generic devices

2012-12-14 Thread Chris Bagwell
On Thu, Dec 13, 2012 at 9:37 PM, Peter Hutterer wrote:

> On Wed, Dec 12, 2012 at 11:08:17AM -0800, Ping Cheng wrote:
> > On Wed, Dec 12, 2012 at 10:54 AM, Jason Gerecke  >wrote:
> >
> > > It certainly doesn't hurt :) I think my biggest problem is that I never
> > > really understood exactly when this code was required.
> >
> >
> > I bet you don't want to know ;-). Chris knows this chunk of code way
> better
> > than me. It is a long story and it is history already. I'll share it
> > offline, with those who are curious...
>
> no, please share it online so it's archived for future generations.
> this code isn't that old, so if we can't even remember what it was written
> for we really need to document better.
>
> commit messages are free, we might as well use them properly.
>
> afaict, this code was there because before the MT protocol we used the
> BTN_TOOL_DOUBLETAP and friends to switch between tools. but because this
> was
> abusing the system a bit, switching to a different tool wouldn't update the
> other coordinates for that tool (the kernel would skip those events if they
> were on the last-sent coordinates for the previous tool), so we had to copy
> that around in the driver.
>
> except for the pad which is always in proximity and didn't rely on
> multiplexing, so we could skip that.
>
> does this sound correct?
>
> Cheers,
>Peter
>
>
Right, except that "tool" means fingers in this case.  The "old kernel way"
is explained in some detail on this page under the "Touchpad Overview"
section.

https://sourceforge.net/apps/mediawiki/linuxwacom/index.php?title=Kernel_Input_Event_Overview

The concept of that deleted code is that any user land apps need to track
last values sent by kernel that are shared between tools and when switching
tools you need to continue using the last sent values because of event
filtering.  Since we are storing X/Y/Pressure in different "channels", when
switching channels the X/Y/Pressure fields need to be copied over at switch
time.

For MT devices, there is not same concept of shared events so the code can
be deleted.

Protocol 5 devices and any Protocol 4 with mice do in fact need similar
logic but we don't have it; as the Protocol 5 deleted comment alludes to.
We are most likely losing data during some tool/finger switch overs for
those devices but since they mostly work in Absolute mode almost no one
notices it because of fast recovery.

FYI: I see now that I documented "Synaptics" wrong on wiki.The
X/Y/Pressure for syntaptics are really always bound to the FINGER tool and
DOUBLETAP is more an extra tool with no X/Y/Pressure associated directly
with it.  Something closer to this:

 * BTN_TOOL_FINGER
   * BTN_TOUCH
   * ABS_X
   * ABS_Y
   * ABS_PRESSURE
 * BTN_TOOL_DOUBLETAP
 * BTN_LEFT
 * BTN_RIGHT

Chris


> >
> > Ping
> >
> > It makes sense that only MT using *TAP events would need it (or
> dual-track,
> > > but that's protocol 5), so I don't really see any problem given the
> testing
> > > you've done.
> > >
> > >
> > > Jason
> > >
> > > ---
> > > When you're rife with devastation / There's a simple explanation:
> > > You're a toymaker's creation / Trapped inside a crystal ball.
> > > And whichever way he tilts it / Know that we must be resilient
> > > We won't let them break our spirits / As we sing our silly song.
> > >
> > >
> > >
> > > On Wed, Dec 12, 2012 at 10:39 AM, Ping Cheng 
> wrote:
> > >
> > >> On Wed, Dec 12, 2012 at 9:45 AM, Jason Gerecke  >wrote:
> > >>
> > >>> Though I can't quite convince myself that this is safe, I don't see
> any
> > >>> problems with it.
> > >>>
> > >>> Acked-By: Jason Gerecke 
> > >>>
> > >>
> > >> Will I say it is tested on older and new systems make it easier to
> > >> convince you?
> > >>
> > >> Ping
> > >>
> > >>
> > >>>  Jason
> > >>>
> > >>> ---
> > >>> When you're rife with devastation / There's a simple explanation:
> > >>> You're a toymaker's creation / Trapped inside a crystal ball.
> > >>> And whichever way he tilts it / Know that we must be resilient
> > >>> We won't let them break our spirits / As we sing our silly song.
> > >>>
> > >>>
> > >>>
> > >>> On Thu, Dec 6, 2012 at 4:20 PM, Ping Cheng 
> wrote:
> > >>>
> >  We use true MT protocol for MT devices in kernel now. This code
> >  was introduced to deal with ABS_TOOL_*TAP events loss issue. It
> >  is uncessary any more. And its existence makes it hard to support
> >  generic PAD cleanly.
> > 
> >  Signed-off-by: Ping Cheng 
> >  ---
> >   src/wcmUSB.c |   34 ++
> >   1 file changed, 2 insertions(+), 32 deletions(-)
> > 
> >  diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> >  index e192489..4b5f53b 100644
> >  --- a/src/wcmUSB.c
> >  +++ b/src/wcmUSB.c
> >  @@ -37,7 +37,6 @@ typedef struct {
> >  Bool wcmPenTouch;
> >  Bool wcmUseMT;
> >  int wcmMTChannel;
> >  -   int wcmPrevChannel;
> >  int wcmEventCnt;
> >    

[Linuxwacom-devel] libwacom: [PATCH] test: check number of switches against number of modes

2012-12-14 Thread Olivier Fourdan


>From 5fe5a58ad6eada45471e670625b60cd10b52b0be Mon Sep 17 00:00:00 2001
From: Olivier Fourdan 
Date: Fri, 14 Dec 2012 12:08:09 +0100
Subject: [PATCH] test: check number of switches against number of modes

Most tablets have one mode-switch button and multiple
modes, but some like the Cintiq 24HD have dedicated mode-
switch buttons.

In this case, the number of modes must match the number of
switches otherwise client applications have no way to tell
if a button is used as a mode-switch button to cycle through
all the modes or a mode-switch button to select a given mode.

Add this in the tablet-validity check, ie make sure that
if we have more than one mode-switch button, then the
number of modes matches the number of mode-switch buttons.

Also checks that if more than one mode is declared, at least
one mode-switch button is found.

Signed-off-by: Olivier Fourdan 
---
 test/tablet-validity.c |   46 ++
 1 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/test/tablet-validity.c b/test/tablet-validity.c
index 75fe265..19a4b98 100644
--- a/test/tablet-validity.c
+++ b/test/tablet-validity.c
@@ -40,6 +40,8 @@
 #include 
 #include 
 
+typedef int (*NumModesFn) (const WacomDevice *device);
+
 static int buttons_have_direction (WacomDevice *device)
 {
 	char   button;
@@ -59,6 +61,42 @@ static int buttons_have_direction (WacomDevice *device)
 	return 0;
 }
 
+static int match_mode_switch (WacomDevice *device, NumModesFn get_num_modes, WacomButtonFlags flag)
+{
+	char   button;
+	intnum_buttons;
+	intnum_switches;
+	intnum_modes;
+
+	num_buttons  = libwacom_get_num_buttons (device);
+	num_modes= get_num_modes (device);
+	num_switches = 0;
+
+	for (button = 'A'; button < 'A' + num_buttons; button++) {
+		WacomButtonFlags  flags;
+		flags = libwacom_get_button_flag(device, button);
+
+		if (flags & flag)
+			num_switches++;
+	}
+
+	/*
+	 * If we have more than one mode-switch button, then the
+	 * number of modes must match the number of mode-switch buttons.
+	 */
+	if (num_switches > 1 && num_modes != num_switches)
+		return 0;
+
+	/*
+	 * If we have more than one mode, then we should find at least
+	 * one mode-switch button.
+	 */
+	if (num_modes > 1 && num_switches == 0)
+		return 0;
+
+	return 1;
+}
+
 static int eraser_is_present(WacomDeviceDatabase *db, const int *styli, int nstyli, WacomStylusType type)
 {
 	int i;
@@ -146,6 +184,14 @@ static void verify_tablet(WacomDeviceDatabase *db, WacomDevice *device)
 	assert(libwacom_get_strips_num_modes(device) >= 0);
 	assert(libwacom_get_bustype(device) != WBUSTYPE_UNKNOWN);
 	assert(buttons_have_direction(device) > 0);
+	if (libwacom_has_ring(device))
+		assert(match_mode_switch (device, libwacom_get_ring_num_modes, WACOM_BUTTON_RING_MODESWITCH));
+	if (libwacom_has_ring2(device))
+		assert(match_mode_switch (device, libwacom_get_ring2_num_modes, WACOM_BUTTON_RING2_MODESWITCH));
+	if (libwacom_get_num_strips(device) > 1)
+		assert(match_mode_switch (device, libwacom_get_strips_num_modes, WACOM_BUTTON_TOUCHSTRIP2_MODESWITCH));
+	if (libwacom_get_num_strips(device) > 0)
+		assert(match_mode_switch (device, libwacom_get_strips_num_modes, WACOM_BUTTON_TOUCHSTRIP_MODESWITCH));
 }
 
 int main(int argc, char **argv)
-- 
1.7.1

--
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel