Re: [PATCH 4/4] input: wacom - add 0xE5 (MT device) support
From: Chris Bagwell <hidden>
Date: 2011-12-22 04:14:54
On Wed, Dec 21, 2011 at 12:43 PM, Ping Cheng [off-list ref] wrote:
On Sun, Dec 18, 2011 at 6:07 PM, Chris Bagwell [off-list ref] wrote:quoted
On Fri, Dec 16, 2011 at 6:38 PM, Ping Cheng [off-list ref] wrote:quoted
Signed-off-by: Ping Cheng <redacted> --- drivers/input/tablet/wacom_sys.c | 26 ++++++------ drivers/input/tablet/wacom_wac.c | 82 +++++++++++++++++++++++++++++++++++++- drivers/input/tablet/wacom_wac.h | 8 ++++ 3 files changed, 101 insertions(+), 15 deletions(-)diff --git a/drivers/input/tablet/wacom_sys.cb/drivers/input/tablet/wacom_sys.c index b9736fb..eee06ef 100644--- a/drivers/input/tablet/wacom_sys.c +++ b/drivers/input/tablet/wacom_sys.c@@ -297,6 +297,10 @@ static int wacom_parse_hid(struct usb_interface*intf, features->pktlen = WACOM_PKGLEN_TPC2FG; features->touch_max = 2; } + + if (features->type == MTSCREEN) + features->pktlen = WACOM_PKGLEN_MTOUCH; + if (features->type == BAMBOO_PT) { /* need to reset back */ features->pktlen = WACOM_PKGLEN_BBTOUCH;@@ -331,18 +335,15 @@ static int wacom_parse_hid(struct usb_interface*intf, case HID_USAGE_Y: if (usage == WCM_DESKTOP) { if (finger) {
Let me ask this way. On Bamboo there are 2 interfaces, 1 pen and 1
touch. I need to set feature report 2 to a specific value on the pen
interface to get useful packets. If I set feature report 2 to
something on touch interface, I get an error response back from
device. This aligns with HID descriptor because only the pen
interface declares that feature report #2.
I'm sure its similar for Tablet PC's. Its also writing to feature
report #2 but unique data values. What ever the values are, it
probably also doesn't make sense to write to both pen interface and
touch interface on Tablet PC.
If the feature report is only on one of two interfaces I do suggest
changing the if() to something like below to make it more obvious ("if
(TOUCH)" is more obvious that your looking for specific interface then
"if (!PEN)"). If its on both and only 1 needs to be written to then
that would probably be a good comment as well.
quoted
if (TOUCH) /* do TOUCH set's */ if (PEN) /* do PEN set's */quoted
do { rep_data[0] = 3; rep_data[1] = 4;@@ -479,9 +477,9 @@ static int wacom_retrieve_hid_descriptor(structusb_interface *intf, features->pressure_fuzz = 0; features->distance_fuzz = 0; - /* only Tablet PCs and Bamboo P&T need to retrieve the info */ + /* only devices that support touch need to retrieve the info */ if ((features->type != TABLETPC) && (features->type != TABLETPC2FG) && - (features->type != BAMBOO_PT)) + (features->type != BAMBOO_PT) && (features->type != MTSCREEN)) goto out; if (usb_get_extra_descriptor(interface, HID_DEVICET_HID, &hid_desc)) {diff --git a/drivers/input/tablet/wacom_wac.cb/drivers/input/tablet/wacom_wac.c index 8dc185d..1a887a1 100644--- a/drivers/input/tablet/wacom_wac.c +++ b/drivers/input/tablet/wacom_wac.c@@ -729,6 +729,73 @@ static int wacom_intuos_irq(struct wacom_wac*wacom) return 1; } +static int wacom_mt_touch(struct wacom_wac *wacom) +{
OK. I understand now. It would be nice to add that to above comment (the fact that num_contacts_left is total on screen).
quoted
quoted
+ + for (i = 0; i < contacts_to_send; i++) { + id = le16_to_cpup((__le16 *)&data[k]); + + /* is there an existing slot for this contact? */ + for (j = 0; j < features->touch_max; j++) { + mt = &input->mt[j]; + if (input_mt_get_value(mt, ABS_MT_TRACKING_ID) == id ) + break; + } + + /* no. then find an unused slot to fill */ + if (j >= features->touch_max) { + for (j = 0; j < features->touch_max; j++) { + mt = &input->mt[j]; + if (input_mt_get_value(mt, ABS_MT_TRACKING_ID) == -1 ) + break; + } + }This may need a little more thought since hid-multitouch doesn't query old value. Is there a case were a touch is removed and new touch added in same packet? Probably that would never send the -1 to user land.Once a touch leaves the screen, a -1 will always be sent to indicate the touch is out. When a new touch is added, it may use any available packet. Hence the one used by the just removed touch can also be used. Do I get your question properly?
I'm thinking of this sequence inside 1 packet (probably not easy to produce in the wild): * 1st location in packet shows Finger with ID of 100 as not touching any more. * Loops threw and finds matching ID and sets to -1. * 2nd location in packet shows Finger with ID of 101 as touching. Its new. * Loops threw to find 1st slot with -1 and uses one we just cleared. * Sync window occurs. User only sees Contact ID change from 100 and 101.
quoted
quoted
+ + touch = data[k - 1] & 0x1; + input_mt_slot(input, j); + if (touch) { + x = le16_to_cpup((__le16 *)&data[k + 6]); + y = le16_to_cpup((__le16 *)&data[k + 8]); + + input_report_abs(input, ABS_MT_POSITION_X, x); + input_report_abs(input, ABS_MT_POSITION_Y, y); + } else + id = -1; + + /* keep the id from firmware since we need + * it to process future events + */ + input_report_abs(input, ABS_MT_TRACKING_ID, id); + + k += WACOM_BYTES_PER_MT_PACKET; + } + + input_mt_report_pointer_emulation(input, true); + + features->num_contacts_left -= contacts_to_send; + if (features->num_contacts_left < 0) + features->num_contacts_left = 0;Same comment as above. Could use a comment on why storing this would be needed.If you understood the value of current_num_contacts, it would be easier to see the need for num_contacts_left. I'll add a comment for current_num_contacts.
I guess I still don't see why its needed or how its being used. Now that I understand that current_num_contacts is total touches, I do understand you need to know when a packet contains 5 full touches and when it contains less than 5. For example if 6 touches then 1st packet could have 5 and next packet could have 1. So you need to know to stop looping after 1. But I do not see how this num_contacts_left helps that. It looks like it would set contacts_to_send to 5 in both packets. Chris
Thank you for the review. Ping
-- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html