On Thu, Jun 30, 2011 at 10:20:33AM -0600, Erich Hoover wrote:
> The HP TC1100 has three buttons on the side of the display that are triggered
> with the stylus. This patch adds support for these buttons, with the buttons
> available under the following xsetwacom button IDs:
> ID 13 = Rotate
> ID 14 = Writing tool
> ID 15 = On-screen keyboard

can we test for these buttons on startup? If so, we could apply the
appropriate labels.

If we can't label them ahead of time, is there any overlap with any tablets
that already provide buttons 13-15? if so, do we need to increase
WCM_MAX_MOUSE_BUTTONS?

> Signed-off-by: Erich E. Hoover <[email protected]>
> ---
>  src/wcmISDV4.c |   31 +++++++++++++++++++++++++++++++
>  1 files changed, 31 insertions(+), 0 deletions(-)
> 
> diff --git a/src/wcmISDV4.c b/src/wcmISDV4.c
> index 82c709d..436e973 100644
> --- a/src/wcmISDV4.c
> +++ b/src/wcmISDV4.c
> @@ -634,6 +634,37 @@ static int isdv4Parse(InputInfoPtr pInfo, const
> unsigned char* data, int len)
>               }
>       }
> 
> +     /* handle TC1100 stylus buttons */
> +     if ((data[0] == 0xC1) && ((data[1] & 0xF0) == 0))

With the exception of the control bit, we try not to have direct protocol
parsing in the code here. Please add these fields to the device state and
parse them accordingly. See the stuff in isdv4.h.
(the isdv4-serial-debugger should also be extended to support this bit as
well if it is set)

> +     {
> +             int oldTPCButton = common->wcmTPCButton;
> +             int button = (int) data[1];
> +
> +             DBG(2, common, "Tablet Button 0x%x\n", data[1]);
> +
> +             common->wcmTPCButton = 0;
> +
> +             /* do not reset relative values here */
> +             ds = &common->wcmChannel[0].work;
> +
> +             /* first send the tablet button press event */
> +             ds->proximity = 1;
> +             ds->buttons = button << (WCM_MAX_MOUSE_BUTTONS - 4);
> +             ds->pressure = common->wcmMaxZ;
> +             ds->device_id = STYLUS_DEVICE_ID;
> +             ds->device_type = STYLUS_ID;

shouldn't this be PAD_ID?

> +             wcmEvent(common, channel, ds);
> +             
> +             /* then immediately after send the release event */

should this be sent on the first packet that doesn't have the matching bit
set? what happens here if you keep holding the button down?

> +             ds->buttons = 0;
> +             ds->pressure = 0;
> +             ds->proximity = 0;
> +             wcmEvent(common, channel, ds);
> +
> +             common->wcmTPCButton = oldTPCButton;
> +             return common->wcmPktLength;

so even though you only seem to parse the second byte, is there any
information in that packet? You seem to still be advancing by
ISDV4_PKGLEN_TPCPEN.

having said that, this should really be a small function that is called if
the header matches. isdv4Parse is unwieldly enough as-is, adding another 20
lines to it doesn't make it easier to read.

also - why is this before the PktLength != ISDV4_PKGLEN_TPCPEN condition?
can this bit be set in both touch and pen packages?

Cheers,
  Peter

> +     }
> +
>       /* Coordinate data bit check */
>       if (data[0] & CONTROL_BIT) /* control data */
>               return common->wcmPktLength;
> -- 
> 1.7.1

------------------------------------------------------------------------------
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security 
threats, fraudulent activity, and more. Splunk takes this data and makes 
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2d-c2
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to