On Mon, Mar 08, 2010 at 12:16:00PM -0600, Chris Bagwell wrote:
> Have we verified yet if these ISDV4 devices return 2 sizes of packets?
> The logic in isdv4Parse() reads to me like it must or else your only
> going to be able to parse pen data or touch data but not both.
>
> This development branch changes things so that packet size is only
> computed in isdv4GetRanges(). Because of order of checks, its going
> to prefer length of touch packets. This means your only going to
> execute touch (finger) logic in isdv4Parse(); even in the case of pen
> packets.
>
> Assuming pen packets are length 9 and touch length 5 for Thomas's
> device, I think that has side affect of throwing away roughly every
> other pen packet... and thus the resolution issue. But if this is the
> case, I'm surprised that touch packets are close enough to pen packets
> to get any movement.
I think I may have it now, thanks much for the pointers. The patch is
attached but it also needs a revert of the "Remove superfluous packet-length
assignment" patch on the isdv4 branch.
I'm a bit confused though, in the original code, all data was read into a
buffer, then the first byte of this buffer was checked and the
packet length decided on. Then the code would read as many packets as viable
from this buffer, until the remainder was < pktlen. I'm a bit confused on
how this code handled packets on different lengths though since it'd always
check the first byte in the buffer, never of the actual packet.
With the devel tree, I think the issue is with one line - that we only
checked the first byte of the buffer as well but not of the data passed in.
Given that each time we get data this is supposed to be a whole packet we
can assume that the first byte contains the info we need. does this make
sense?
I've pushed the tree to my freedesktop repo for testing, branch devel and
isdv4 is rebased on top of it.
> My assumptions align with the bisect below because thats the one where
> you stopped updating length between 9/5 values at each packet read.
>
> Chris
>
> p.s. I like the direction your heading. It would be nice to somehow
> pull isdv4ParseQuery() and isdv4ParseTouchQuery() over to isdv4Parse()
> because they are much easier to read. I think thats somehow your
> intent because you removed isdv4ValidateSerial() from isdv4Parse() but
> you didn't replace it with any new parsing or validation logic.
That'll come, I had the whole parsing code but ran into issues with the
different packet length for touch packets (my original design didn't handle
this too well). So I figured I get some early testing for the stuff that's
supposed to work.
Cheers,
Peter
> On Mon, Mar 8, 2010 at 2:19 AM, Peter Hutterer <peter.hutte...@who-t.net>
> wrote:
> > On Mon, Mar 08, 2010 at 02:40:17AM -0500, Thomas Jaeger wrote:
> >> I see a regression introduced in dc513e8e1e6813e6d8d0f101a56fbf1b2f388dbc:
> >> Strokes are noticeably less smooth, they look more like sequences of
> >> line segments. My guess is that this is either due to increased jitter
> >> or to a reduced sampling frequency -- no idea why the patch would
> >> trigger this, though. Let me know if I should investigate further.
> >
> > hmm. weird. I can see how 7b89318ed235cbbad0312e56efa9443bf96b47c9 might
> > have this effect if there's a bug in the code but the other one? not sure
> > about this. can you put some xf86Msg in to see which code path gets
> > triggered - both before and after the move - and if those paths are the
> > same?
> >
> > Cheers,
> > Peter
> >
> >> On 03/07/2010 11:30 PM, Peter Hutterer wrote:
> >> > Lads and lasses,
> >> >
> >> > I've just pushed the isdv4 branch onto
> >> > git://people.freedesktop.org/~whot/xf86-input-wacom.git. This is the
> >> > beginning of a rework/cleanup of the ISDV4-related code with the possible
> >> > end solution of moving it into the kernel (there's more work needed for
> >> > this
> >> > though and I need to understand the protocol much better first).
> >> >
> >> > The branch is based on my devel branch from last week and most of the
> >> > changes should be harmless (it's only 10 patches so far). if you have an
> >> > ISDV4 device, please give this a test run and let me know if it breaks
> >> > (or
> >> > doesn't) and if it does maybe also which commit breaks it. I don't have a
> >> > device so anything that gcc doesn't catch will go unnoticed by me.
> >> >
> >> > The plan with this branch is to move the whole ISDV4 parsing code into
> >> > little helper functions to make the other isdv4-related code a bit more
> >> > streamlined and easier to understand for someone new to the code.
> >> >
> >> > Thanks in advance for testing and feedback!
> >> >
> >> > Cheers,
> >> > Peter
>From b945afc82e83aab24d9d3d3320717e58c23b5c8f Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutte...@who-t.net>
Date: Tue, 9 Mar 2010 15:13:56 +1000
Subject: [PATCH] Fix invalid buffer handling in isdv4Parse().
Check the first byte of each packet for the bits that define the packet
length, not just the first byte in the buffer.
Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
---
src/wcmISDV4.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/src/wcmISDV4.c b/src/wcmISDV4.c
index 1b365dc..0c8b0a8 100644
--- a/src/wcmISDV4.c
+++ b/src/wcmISDV4.c
@@ -398,7 +398,6 @@ 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;
--
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
