For all in series:

Reviewed-by: Chris Bagwell <[email protected]>

For this patch, I also noticed this mis-placed code while reviewing
isdv4GetRanges() recently.  It seems the basic issue is that serial
ports can return variable sized packets based on what user is doing.
That tells me setting initial values for wcmPktLength in
isdv4GetRanges() is a waste of time and also to lesser extent the
setting in isdv4InitISDV4().  Possible future cleanup item.

I'm glad in patch 4/5, you got rid of a while() that referenced
wcmPktLength because what it was really wanting to do was check a
non-existing wcmMinimumPktLength value.  Also, because wcmPktLength
was initialized outside the while(), it required all packets in single
data buffer to be of same type which seems a wrong assumption to me.
Hopefully, that patch fixes bug tracker #2952501.

So I guess I'm saying I think your patch 4/5 is more important then
your description implies. :-)

Chris

On Thu, Mar 4, 2010 at 6:22 PM, Peter Hutterer <[email protected]> wrote:
> The packet length only matters on ISDV4, the code should be in the matching
> part of the source.
>
> Signed-off-by: Peter Hutterer <[email protected]>
> ---
>  src/wcmISDV4.c  |   15 +++++++++++++++
>  src/xf86Wacom.c |   22 ----------------------
>  2 files changed, 15 insertions(+), 22 deletions(-)
>
> diff --git a/src/wcmISDV4.c b/src/wcmISDV4.c
> index 2b46ff1..69fe7c8 100644
> --- a/src/wcmISDV4.c
> +++ b/src/wcmISDV4.c
> @@ -398,6 +398,21 @@ static int isdv4Parse(LocalDevicePtr local, const 
> unsigned char* data, int len)
>
>        DBG(10, common, "\n");
>
> +       data = common->buffer;
> +       /* choose wcmPktLength if it is not an out-prox event */
> +       if (data[0])
> +               common->wcmPktLength = WACOM_PKGLEN_TPCPEN;
> +
> +       if ( data[0] & 0x10 )
> +       {
> +               /* set touch PktLength */
> +               common->wcmPktLength = WACOM_PKGLEN_TOUCH93;
> +               if ((common->tablet_id == 0x9A) || (common->tablet_id == 
> 0x9F))
> +                       common->wcmPktLength = WACOM_PKGLEN_TOUCH9A;
> +               if ((common->tablet_id == 0xE2) || (common->tablet_id == 
> 0xE3))
> +                       common->wcmPktLength = WACOM_PKGLEN_TOUCH2FG;
> +       }
> +
>        if (len < common->wcmPktLength)
>                return 0;
>
> diff --git a/src/xf86Wacom.c b/src/xf86Wacom.c
> index 3da591b..8685805 100644
> --- a/src/xf86Wacom.c
> +++ b/src/xf86Wacom.c
> @@ -1096,7 +1096,6 @@ void wcmReadPacket(LocalDevicePtr local)
>        WacomDevicePtr priv = (WacomDevicePtr)local->private;
>        WacomCommonPtr common = priv->common;
>        int len, pos, cnt, remaining;
> -       unsigned char * data;
>
>        DBG(10, common, "fd=%d\n", local->fd);
>
> @@ -1128,27 +1127,6 @@ void wcmReadPacket(LocalDevicePtr local)
>        common->bufpos += len;
>        DBG(10, common, "buffer has %d bytes\n", common->bufpos);
>
> -       /* while there are whole packets present, check the packet length
> -        * for serial ISDv4 packet since it's different for pen and touch
> -        */
> -       if (common->wcmForceDevice == DEVICE_ISDV4 && common->wcmDevCls != 
> &gWacomUSBDevice)
> -       {
> -               data = common->buffer;
> -               /* choose wcmPktLength if it is not an out-prox event */
> -               if (data[0])
> -                       common->wcmPktLength = WACOM_PKGLEN_TPCPEN;
> -
> -               if ( data[0] & 0x10 )
> -               {
> -                       /* set touch PktLength */
> -                       common->wcmPktLength = WACOM_PKGLEN_TOUCH93;
> -                       if ((common->tablet_id == 0x9A) || (common->tablet_id 
> == 0x9F))
> -                               common->wcmPktLength = WACOM_PKGLEN_TOUCH9A;
> -                       if ((common->tablet_id == 0xE2) || (common->tablet_id 
> == 0xE3))
> -                               common->wcmPktLength = WACOM_PKGLEN_TOUCH2FG;
> -               }
> -       }
> -
>        len = common->bufpos;
>        pos = 0;
>
> --
> 1.6.6.1
>
>
> ------------------------------------------------------------------------------
> Download Intel&#174; Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> _______________________________________________
> Linuxwacom-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
>

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to