Hi Chris,
Thank you for the patch. Please see my comments inline.
Ping
On Tue, Jun 1, 2010 at 7:18 PM, <ch...@cnpbagwell.com> wrote:
> From: Chris Bagwell <ch...@cnpbagwell.com>
>
> usbChooseChannel() supported in most cases 1 input and 1 pad to
> be concurrently in proximity. DUALINPUT V5 devices (Intuos 1 & 2)
> could handle a combination 2 inputs and 1 pad, up to max of 2 in
> proximity at 1 time though.
>
> This change ports over an update from linuxwacom to allow
> 2 input channels on V4 devices as well as 1 pad to be
> concurrently in proximity. It gives DUALINPUT V5 a total
> of 3 as a side affect.
>
> This change is required to expose multitouch input to any
> pending Gesture logic. This is namely for Bamboo P&T and
> Tablet PC's.
>
> Patch also replaces a few magic numbers with #defines to improve
> understanding.
>
> Signed-off-by: Chris Bagwell <ch...@cnpbagwell.com>
> ---
> src/wcmUSB.c | 34 ++++++++++++++++++++--------------
> src/xf86WacomDefs.h | 8 ++++++--
> 2 files changed, 26 insertions(+), 16 deletions(-)
>
> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> index f0321ee..b804aa9 100644
> --- a/src/wcmUSB.c
> +++ b/src/wcmUSB.c
> @@ -697,13 +697,17 @@ static int usbChooseChannel(WacomCommonPtr common, int
> serial)
>
> if (common->wcmProtocolLevel == 4)
> {
> - /* Protocol 4 doesn't support tool serial numbers */
> - if (serial == 0xf0)
> - channel = 1;
> - else
> - channel = 0;
This is pretty much for backward compatibility support since there was
no serial number reported from the kernel for V4 devices. How about we
change the comment to:
/* default to channel one if serial number is not
received */
> + /* V4 devices imply serial #1 if not sent */
> + if (!serial)
> + serial = 1;
> +
> + if (serial == SINGLEINPUT_PAD_SERIAL)
> + channel = PAD_CHANNEL;
> + else if (serial < MAX_CHANNELS)
> + channel = serial-1;
> }
> - else if (serial) /* serial number should never be 0 for V5 devices */
> + /* serial number should never be 0 for V5 devices */
> + else if (serial)
> {
> /* dual input is supported */
> if (TabletHasFeature(common, WCM_DUALINPUT))
> @@ -712,7 +716,7 @@ static int usbChooseChannel(WacomCommonPtr common, int
> serial)
> for (i=0; i<MAX_CHANNELS; ++i)
> {
> if (common->wcmChannel[i].work.proximity &&
> - common->wcmChannel[i].work.serial_num
> == serial)
> + common->wcmChannel[i].work.serial_num ==
> serial)
> {
> channel = i;
> break;
> @@ -732,14 +736,16 @@ static int usbChooseChannel(WacomCommonPtr common, int
> serial)
> }
> }
> }
> - else /* one transducer plus expresskey (pad) is supported */
> + else
> {
> - if (serial == -1) /* pad */
> - channel = 1;
> - else if ( (common->wcmChannel[0].work.proximity &&
> /* existing transducer */
> - (common->wcmChannel[0].work.serial_num ==
> serial)) ||
> - !common->wcmChannel[0].work.proximity
> ) /* new transducer */
> - channel = 0;
> + /* One transducer plus expresskey (pad) is supported.
> + * Second dual input is not used. */
> + if (serial == DUALINPUT_PAD_SERIAL)
> + channel = PAD_CHANNEL;
This line makes me feel we want to split the pen-only devices from the
pen and touch ones. The middle channel, in this case, is wasted all
the time for this type of devices. Plus, there are only two fingers
for now. What happens if we have more than 2 fingers? We would leave
a bigger gap between channel=0 and PAD_CHANNEL (the last one) for this
type of devices.
The linuxwacom solution was kind of a workaround to meet the deadline.
For xf86-input-wacom, I think we want to do it right. Basically, I'd
like to keep the pen-only flow unchanged. Only update the ones that
support touch. Let me know if you get confused.
> + else if ((common->wcmChannel[0].work.proximity && /*
> existing transducer */
> + (common->wcmChannel[0].work.serial_num ==
> serial)) ||
> + !common->wcmChannel[0].work.proximity ) /*
> new transducer */
> + channel = FIRST_INPUT_CHANNEL;
> }
> }
>
> diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
> index 3d3c176..cbd890c 100644
> --- a/src/xf86WacomDefs.h
> +++ b/src/xf86WacomDefs.h
> @@ -359,8 +359,12 @@ extern WacomDeviceClass gWacomISDV4Device;
>
> #define DEVICE_ISDV4 0x000C
>
> -#define MAX_CHANNELS 2
> -#define MAX_FINGERS 2
> +#define MAX_FINGERS 2
> +#define MAX_CHANNELS MAX_FINGERS+1
> +#define FIRST_INPUT_CHANNEL 0
> +#define PAD_CHANNEL 2
The idea was to make pad channel always the last one. It would be more
future proofing if we:
#define PAD_CHANNEL MAX_CHANNELS-1
> +#define DUALINPUT_PAD_SERIAL -1
> +#define SINGLEINPUT_PAD_SERIAL 0xf0
>
> struct _WacomCommonRec
> {
> --
> 1.7.0.1
>
>
> ------------------------------------------------------------------------------
>
> _______________________________________________
> Linuxwacom-devel mailing list
> Linuxwacom-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
>
------------------------------------------------------------------------------
ThinkGeek and WIRED's GeekDad team up for the Ultimate
GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
lucky parental unit. See the prize list and enter to win:
http://p.sf.net/sfu/thinkgeek-promo
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
