On Thu, Mar 04, 2010 at 07:29:05PM -0600, Chris Bagwell wrote:
> For all in series:
>
> Reviewed-by: Chris Bagwell <ch...@cnpbagwell.com>
>
> 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.
Ping, can you confirm this? is this true?
As I understood it so far, the data size is constant once the tablet has
been initialized and it's more model-specific than anything else.
> 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. :-)
I didn't even notice that :)
thanks for the review!
Cheers,
Peter
> On Thu, Mar 4, 2010 at 6:22 PM, Peter Hutterer <peter.hutte...@who-t.net>
> wrote:
> > The packet length only matters on ISDV4, the code should be in the matching
> > part of the source.
> >
> > Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
> > ---
> > 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® 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
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
