Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Sun, Apr 03, 2011 at 08:38:37PM -0700, Ping Cheng wrote:
On Sun, Apr 3, 2011 at 7:51 PM, Peter Hutterer
peter.hutte...@who-t.netwrote:
On Sun, Apr 03, 2011 at 07:23:34PM -0700, Ping Cheng wrote:
On Sun, Apr 3, 2011 at 6:35 PM, Peter Hutterer peter.hutte...@who-t.net
wrote:
What information can the user use for Uniq to associate the devices,
which
we do not have in the driver? Both devices have the same product ID
and
name.
I only have one Bamboo, but the test code below gives me the right
answer.
argv[1,2] are the two device nodes (/dev/input/eventX and
/dev/input/eventY).
static struct udev_device *udev_from_file(struct udev *udev, const char
*filename)
{
struct stat st;
stat(filename, st);
return udev_device_new_from_devnum(udev, 'c', st.st_rdev);
}
int main(int argc, char** argv)
{
int rc = 1;
struct udev *udev = NULL;
struct udev_device *dev1, *dev2 = NULL;
struct udev_device *parent1, *parent2;
const char *syspath1, *syspath2;
if (argc 3)
goto out;
udev = udev_new();
dev1 = udev_from_file(udev, argv[1]);
dev2 = udev_from_file(udev, argv[2]);
if (!dev1 || !dev2)
goto out;
parent1 = udev_device_get_parent_with_subsystem_devtype(dev1, usb,
usb_device);
parent2 = udev_device_get_parent_with_subsystem_devtype(dev2, usb,
usb_device);
syspath1 = udev_device_get_syspath(parent1);
syspath2 = udev_device_get_syspath(parent2);
This looks very promising. So, we do not need anything from the user.
Everything could be done inside the driver. Are you going to make a
patch?
can't right now, got too many other things on my slate, sorry.
That's fine. I may be able to get to it after next week.
Can you test the code with two of your Intuos4's? If we get Nope, different
devices for two identical devices, we are ready to move forward. Otherwise,
...
I will not be in the office next whole week. So, I do not have two identical
devices to play with.
yeah, as expected, it says that the devices are different.
Cheers,
Peter
--
Xperia(TM) PLAY
It's a major breakthrough. An authentic gaming
smartphone on the nation's most reliable network.
And it wants your games.
http://p.sf.net/sfu/verizon-sfdev
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Thu, Mar 31, 2011 at 11:13:36PM -0700, Ping Cheng wrote:
On Thu, Mar 31, 2011 at 10:57 PM, Peter Hutterer
peter.hutte...@who-t.netwrote:
On Thu, Mar 31, 2011 at 10:37:54PM -0700, Ping Cheng wrote:
On Thu, Mar 31, 2011 at 4:43 PM, Peter Hutterer
peter.hutte...@who-t.netwrote:
/* FIXME: why strstr and not strcmp? */
if (!strstr(device-drv-driverName, wacom))
continue;
I guess, the intention was to allow driverName has extra characters
instead
of exact wacom. As long as it has wacom in it, we take care of it.
Not
sure if it is necessary though.
I'd rather be precise on the match. I don't think anyone has a driver that
could possibly conflict but substring matches have a nasty habit of finding
things humans don't necessarily see.
would it make sense to have them share the common struct once we've
linked
them?
It makes sense. One issue, even with the current code, is how to tell the
difference between two identical tablets in the driver. How do we know if
we
are linking the proper devices together?
So far, I assume all devices attached to the same system have different
product IDs.
maybe we need an Option Uniq then to let the user associate the devices?
What information can the user use for Uniq to associate the devices, which
we do not have in the driver? Both devices have the same product ID and
name.
I only have one Bamboo, but the test code below gives me the right answer.
argv[1,2] are the two device nodes (/dev/input/eventX and /dev/input/eventY).
static struct udev_device *udev_from_file(struct udev *udev, const char
*filename)
{
struct stat st;
stat(filename, st);
return udev_device_new_from_devnum(udev, 'c', st.st_rdev);
}
int main(int argc, char** argv)
{
int rc = 1;
struct udev *udev = NULL;
struct udev_device *dev1, *dev2 = NULL;
struct udev_device *parent1, *parent2;
const char *syspath1, *syspath2;
if (argc 3)
goto out;
udev = udev_new();
dev1 = udev_from_file(udev, argv[1]);
dev2 = udev_from_file(udev, argv[2]);
if (!dev1 || !dev2)
goto out;
parent1 = udev_device_get_parent_with_subsystem_devtype(dev1, usb,
usb_device);
parent2 = udev_device_get_parent_with_subsystem_devtype(dev2, usb,
usb_device);
syspath1 = udev_device_get_syspath(parent1);
syspath2 = udev_device_get_syspath(parent2);
if (strcmp(syspath1, syspath2) == 0)
printf(Yep, they're the same physical device\n);
else
printf(Nope, different devices\n);
out:
udev_device_unref(dev1);
udev_device_unref(dev2);
udev_unref(udev);
return rc;
}
Cheers,
Peter
--
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself;
WebMatrix provides all the features you need to develop and
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Sun, Apr 3, 2011 at 6:35 PM, Peter Hutterer peter.hutte...@who-t.netwrote:
What information can the user use for Uniq to associate the devices,
which
we do not have in the driver? Both devices have the same product ID and
name.
I only have one Bamboo, but the test code below gives me the right answer.
argv[1,2] are the two device nodes (/dev/input/eventX and
/dev/input/eventY).
static struct udev_device *udev_from_file(struct udev *udev, const char
*filename)
{
struct stat st;
stat(filename, st);
return udev_device_new_from_devnum(udev, 'c', st.st_rdev);
}
int main(int argc, char** argv)
{
int rc = 1;
struct udev *udev = NULL;
struct udev_device *dev1, *dev2 = NULL;
struct udev_device *parent1, *parent2;
const char *syspath1, *syspath2;
if (argc 3)
goto out;
udev = udev_new();
dev1 = udev_from_file(udev, argv[1]);
dev2 = udev_from_file(udev, argv[2]);
if (!dev1 || !dev2)
goto out;
parent1 = udev_device_get_parent_with_subsystem_devtype(dev1, usb,
usb_device);
parent2 = udev_device_get_parent_with_subsystem_devtype(dev2, usb,
usb_device);
syspath1 = udev_device_get_syspath(parent1);
syspath2 = udev_device_get_syspath(parent2);
This looks very promising. So, we do not need anything from the user.
Everything could be done inside the driver. Are you going to make a patch?
Thank you for the info.
Ping
if (strcmp(syspath1, syspath2) == 0)
printf(Yep, they're the same physical device\n);
else
printf(Nope, different devices\n);
out:
udev_device_unref(dev1);
udev_device_unref(dev2);
udev_unref(udev);
return rc;
}
Cheers,
Peter
--
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself;
WebMatrix provides all the features you need to develop and
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Sun, Apr 03, 2011 at 07:23:34PM -0700, Ping Cheng wrote:
On Sun, Apr 3, 2011 at 6:35 PM, Peter Hutterer
peter.hutte...@who-t.netwrote:
What information can the user use for Uniq to associate the devices,
which
we do not have in the driver? Both devices have the same product ID and
name.
I only have one Bamboo, but the test code below gives me the right answer.
argv[1,2] are the two device nodes (/dev/input/eventX and
/dev/input/eventY).
static struct udev_device *udev_from_file(struct udev *udev, const char
*filename)
{
struct stat st;
stat(filename, st);
return udev_device_new_from_devnum(udev, 'c', st.st_rdev);
}
int main(int argc, char** argv)
{
int rc = 1;
struct udev *udev = NULL;
struct udev_device *dev1, *dev2 = NULL;
struct udev_device *parent1, *parent2;
const char *syspath1, *syspath2;
if (argc 3)
goto out;
udev = udev_new();
dev1 = udev_from_file(udev, argv[1]);
dev2 = udev_from_file(udev, argv[2]);
if (!dev1 || !dev2)
goto out;
parent1 = udev_device_get_parent_with_subsystem_devtype(dev1, usb,
usb_device);
parent2 = udev_device_get_parent_with_subsystem_devtype(dev2, usb,
usb_device);
syspath1 = udev_device_get_syspath(parent1);
syspath2 = udev_device_get_syspath(parent2);
This looks very promising. So, we do not need anything from the user.
Everything could be done inside the driver. Are you going to make a patch?
can't right now, got too many other things on my slate, sorry.
Cheers,
Peter
--
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself;
WebMatrix provides all the features you need to develop and
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Sun, Apr 3, 2011 at 7:51 PM, Peter Hutterer peter.hutte...@who-t.netwrote:
On Sun, Apr 03, 2011 at 07:23:34PM -0700, Ping Cheng wrote:
On Sun, Apr 3, 2011 at 6:35 PM, Peter Hutterer peter.hutte...@who-t.net
wrote:
What information can the user use for Uniq to associate the devices,
which
we do not have in the driver? Both devices have the same product ID
and
name.
I only have one Bamboo, but the test code below gives me the right
answer.
argv[1,2] are the two device nodes (/dev/input/eventX and
/dev/input/eventY).
static struct udev_device *udev_from_file(struct udev *udev, const char
*filename)
{
struct stat st;
stat(filename, st);
return udev_device_new_from_devnum(udev, 'c', st.st_rdev);
}
int main(int argc, char** argv)
{
int rc = 1;
struct udev *udev = NULL;
struct udev_device *dev1, *dev2 = NULL;
struct udev_device *parent1, *parent2;
const char *syspath1, *syspath2;
if (argc 3)
goto out;
udev = udev_new();
dev1 = udev_from_file(udev, argv[1]);
dev2 = udev_from_file(udev, argv[2]);
if (!dev1 || !dev2)
goto out;
parent1 = udev_device_get_parent_with_subsystem_devtype(dev1, usb,
usb_device);
parent2 = udev_device_get_parent_with_subsystem_devtype(dev2, usb,
usb_device);
syspath1 = udev_device_get_syspath(parent1);
syspath2 = udev_device_get_syspath(parent2);
This looks very promising. So, we do not need anything from the user.
Everything could be done inside the driver. Are you going to make a
patch?
can't right now, got too many other things on my slate, sorry.
That's fine. I may be able to get to it after next week.
Can you test the code with two of your Intuos4's? If we get Nope, different
devices for two identical devices, we are ready to move forward. Otherwise,
...
I will not be in the office next whole week. So, I do not have two identical
devices to play with.
Ping
--
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself;
WebMatrix provides all the features you need to develop and
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Thu, Mar 31, 2011 at 10:57 PM, Peter Hutterer peter.hutte...@who-t.netwrote: On Thu, Mar 31, 2011 at 10:37:54PM -0700, Ping Cheng wrote: On Thu, Mar 31, 2011 at 4:43 PM, Peter Hutterer peter.hutte...@who-t.netwrote: /* FIXME: why strstr and not strcmp? */ if (!strstr(device-drv-driverName, wacom)) continue; I guess, the intention was to allow driverName has extra characters instead of exact wacom. As long as it has wacom in it, we take care of it. Not sure if it is necessary though. I'd rather be precise on the match. I don't think anyone has a driver that could possibly conflict but substring matches have a nasty habit of finding things humans don't necessarily see. would it make sense to have them share the common struct once we've linked them? It makes sense. One issue, even with the current code, is how to tell the difference between two identical tablets in the driver. How do we know if we are linking the proper devices together? So far, I assume all devices attached to the same system have different product IDs. maybe we need an Option Uniq then to let the user associate the devices? What information can the user use for Uniq to associate the devices, which we do not have in the driver? Both devices have the same product ID and name. Ping -- Create and publish websites with WebMatrix Use the most popular FREE web apps or write code yourself; WebMatrix provides all the features you need to develop and publish your website. http://p.sf.net/sfu/ms-webmatrix-sf ___ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Wed, Mar 30, 2011 at 04:25:12PM -0700, Ping Cheng wrote:
With the introduction of multi-touch, the chances of getting touch
events while pen is in prox have been increased. One obvious use
case is that the touch events could be used for gestures while pen
is in prox. However, we do not want two cursors compete on the screen.
Link the pen and touch device once during the initialization stage
instead of every time when we receive a pen event. Then, centralize
pen and touch arbitration process so we can store the touch data in
wcmUSB.c instead of discarding them. The touch events will only be
ignored if it is a single touch event that causes a cursor movement
while pen is in prox.
Some cleanup in wcmUSB.c is needed. It will be considered when we
make MAX_CHANNEL a dynamic value based on MAX_FINGERS. The
MAX_FINGERS is going to be the maximum of ABS_MT_SLOT that we
retrieve from the kernel. That brings us to the state to support
XInput 2.1 and devcies that have dynamic number of fingers.
^ typo, 'devcies'
Signed-off-by: Ping Cheng pingli...@gmail.com
---
src/wcmCommon.c | 31 ---
src/wcmConfig.c | 45 +
src/xf86WacomDefs.h |4
3 files changed, 61 insertions(+), 19 deletions(-)
diff --git a/src/wcmCommon.c b/src/wcmCommon.c
index a370389..615ac6a 100644
--- a/src/wcmCommon.c
+++ b/src/wcmCommon.c
@@ -1138,30 +1138,23 @@ static void commonDispatchDevice(WacomCommonPtr
common, unsigned int channel,
return;
}
- /* send a touch out for USB Tablet PCs */
- if (IsUSBDevice(common) !IsTouch(priv)
- common-wcmTouchDefault !priv-oldProximity)
+ /* send touch out when pen coming in-prox for devices that provide
+ * both pen and touch events so system cursor won't jump between tools
+ */
+ if (IsPen(priv) TabletHasFeature(common, WCM_PENTOUCH))
{
- InputInfoPtr device = xf86FirstLocalDevice();
- WacomCommonPtr tempcommon = NULL;
- WacomDevicePtr temppriv = NULL;
-
- /* Lookup to see if associated touch was enabled */
- for (; device != NULL; device = device-next)
+ if (IsPen(priv))
{
- if (strstr(device-drv-driverName, wacom))
+ if (common-wcmPointerToTouch-oldProximity)
temporary variable please. as a general rule, if you have to use two - to
get to the field you want (and you're doing so twice), you should use a
temporary variable to make the code easier to read.
{
- temppriv = (WacomDevicePtr) device-private;
- tempcommon = temppriv-common;
-
- if ((tempcommon-tablet_id ==
common-tablet_id)
- IsTouch(temppriv)
temppriv-oldProximity)
- {
- /* Send soft prox-out for touch first */
- wcmSoftOutEvent(device);
- }
+ /* Send touch out so pen takes control */
+
wcmSoftOutEvent(common-wcmPointerToTouch-pInfo);
+ return;
}
}
+ else if (IsTouch(priv) common-wcmPenInProx)
+ /* Ignore touch events when pen is in prox */
+ return;
we won't ever get to this condition, IsPen(priv) is always true in this
block
}
if (IsPen(priv))
diff --git a/src/wcmConfig.c b/src/wcmConfig.c
index 6235d3c..21a124e 100644
--- a/src/wcmConfig.c
+++ b/src/wcmConfig.c
@@ -397,6 +397,45 @@ wcmInitModel(InputInfoPtr pInfo)
return TRUE;
}
+/* Link the touch tool to the pen of the same device
asciidoc /** please
+ * so we can arbitrate the events.
+ */
+static void wcmLinkTouchAndPen(InputInfoPtr pInfo)
+{
+ WacomDevicePtr priv = pInfo-private;
+ WacomCommonPtr common = priv-common;
+ InputInfoPtr device = xf86FirstLocalDevice();
+ WacomCommonPtr tmpcommon = NULL;
+ WacomDevicePtr tmppriv = NULL;
+
+ /* Lookup to find the associated pen and touch */
+ for (; device != NULL; device = device-next)
+ {
+ if (strstr(device-drv-driverName, wacom))
+ {
+ tmppriv = (WacomDevicePtr) device-private;
+ tmpcommon = tmppriv-common;
+
I'm a big fan of using continues for simple conditions to skip. in this
case, a
if (tmppriv == priv)
continue;
rather than adding this to the next condition that deals with the actual
comparison of the tools.
+ if ((tmpcommon-tablet_id == common-tablet_id)
+
Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Wed, Mar 30, 2011 at 11:17 PM, Peter Hutterer
peter.hutte...@who-t.netwrote:
{
- temppriv = (WacomDevicePtr)
device-private;
- tempcommon = temppriv-common;
-
- if ((tempcommon-tablet_id ==
common-tablet_id)
- IsTouch(temppriv)
temppriv-oldProximity)
- {
- /* Send soft prox-out for touch
first */
- wcmSoftOutEvent(device);
- }
+ /* Send touch out so pen takes control */
+
wcmSoftOutEvent(common-wcmPointerToTouch-pInfo);
+ return;
}
}
+ else if (IsTouch(priv) common-wcmPenInProx)
+ /* Ignore touch events when pen is in prox */
+ return;
we won't ever get to this condition, IsPen(priv) is always true in this
block
Right, it should be outside of the if-statement.
}
if (IsPen(priv))
diff --git a/src/wcmConfig.c b/src/wcmConfig.c
index 6235d3c..21a124e 100644
--- a/src/wcmConfig.c
+++ b/src/wcmConfig.c
@@ -397,6 +397,45 @@ wcmInitModel(InputInfoPtr pInfo)
return TRUE;
}
+/* Link the touch tool to the pen of the same device
asciidoc /** please
Oh, I only remembered there is no need to specify pInfo and common. I forgot
this detail.
+ * so we can arbitrate the events.
+ */
+static void wcmLinkTouchAndPen(InputInfoPtr pInfo)
+{
+ WacomDevicePtr priv = pInfo-private;
+ WacomCommonPtr common = priv-common;
+ InputInfoPtr device = xf86FirstLocalDevice();
+ WacomCommonPtr tmpcommon = NULL;
+ WacomDevicePtr tmppriv = NULL;
+
+ /* Lookup to find the associated pen and touch */
+ for (; device != NULL; device = device-next)
+ {
+ if (strstr(device-drv-driverName, wacom))
+ {
+ tmppriv = (WacomDevicePtr) device-private;
+ tmpcommon = tmppriv-common;
+
I'm a big fan of using continues for simple conditions to skip. in this
case, a
if (tmppriv == priv)
continue;
rather than adding this to the next condition that deals with the actual
comparison of the tools.
No problem. Will do.
+ if ((tmpcommon-tablet_id == common-tablet_id)
+ tmppriv != priv)
+ {
+ if (IsTouch(tmppriv) IsPen(priv))
+ {
+ common-wcmPointerToTouch =
tmppriv;
+ common-tablet_type |=
WCM_PENTOUCH;
+ tmpcommon-tablet_type |=
WCM_PENTOUCH;
+ }
+ else if (IsTouch(priv) IsPen(tmppriv))
+ {
+ tmpcommon-wcmPointerToTouch =
priv;
+ common-tablet_type |=
WCM_PENTOUCH;
+ tmpcommon-tablet_type |=
WCM_PENTOUCH;
+ }
this can be simplified with the if/else if condition setting a tmp variable
for the new priv assignment and then a shared block for the three actual
assignments.
Not really. Although priv can be set to tmppriv, both common and tmpcommon
are needed which can not be used with one name. Plus, since it is a if/else
if, there would be cases outside of those statements that we should not
assign any values except continue.
+ }
+ }
+ }
+}
+
/* wcmPreInit - called for each input devices with the driver set to
* wacom */
#if GET_ABI_MAJOR(ABI_XINPUT_VERSION) 12
@@ -518,6 +557,12 @@ static int wcmPreInit(InputDriverPtr drv,
InputInfoPtr pInfo, int flags)
pInfo-fd = -1;
}
+ /* only link them once per port. We need to try for both pen and
touch
+ * since we do not know which tool (touch or pen) will be added
first.
+ */
+ if (IsTouch(priv) || (IsPen(priv) !common-wcmPointerToTouch))
+ wcmLinkTouchAndPen(pInfo);
+
return Success;
SetupProc_fail:
diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
index 43348fc..60d6426 100644
--- a/src/xf86WacomDefs.h
+++ b/src/xf86WacomDefs.h
@@ -173,6 +173,7 @@ struct _WacomModel
#define WCM_TPC (0x0200 | WCM_LCD) /* TabletPC
(special
button handling,
always an LCD) */
+#define WCM_PENTOUCH 0x0400 /* Tablet supports pen and
