On Sat, Oct 23, 2010 at 3:53 PM, Chris Bagwell <ch...@cnpbagwell.com> wrote:
> On Thu, Oct 21, 2010 at 2:43 PM, Chris Bagwell <ch...@cnpbagwell.com> wrote:
>> On Thu, Oct 21, 2010 at 12:38 PM, Ping Cheng <pingli...@gmail.com> wrote:
>>> On Wed, Oct 20, 2010 at 6:16 PM, <ch...@cnpbagwell.com> wrote:
>>>> From: Chris Bagwell <ch...@cnpbagwell.com>
>>>>
>>>> The core of xf86-input-wacom strictly enforces buttons
>>>> on tools that are out-of-proximity must be cleared except
>>>> for the special case of the PAD device that is always
>>>> considered in proximity.
>>>>
>>>> Simple/Generic tablets (non-wacom) and touchpads will send
>>>> button presses associated with tablet itself even though no
>>>> tools are reported as in proximity.
>>>>
>>>> Work around this by forcing all non-styus button presses
>>>> to be routed to hard coded PAD channel and post multiple
>>>> channel events per sync window.
>>>>
>>>> MT packets could be implemented using same concept and
>>>> routing to hard coded channel 0 or 1 based on finger #.
>>>>
>>>> Signed-off-by: Chris Bagwell <ch...@cnpbagwell.com>
>>>> Reviewed-by: Peter Hutterer <peter.hutte...@who-t.net>
>>>> ---
>>>> src/wcmUSB.c | 110
>>>> +++++++++++++++++++++++++++++++++++++++++++++++-----------
>>>> 1 files changed, 89 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
>>>> index eed755e..973e358 100644
>>>> --- a/src/wcmUSB.c
>>>> +++ b/src/wcmUSB.c
>>>> @@ -35,6 +35,7 @@
>>>>
>>>> typedef struct {
>>>> int wcmLastToolSerial;
>>>> + int wcmPadChannel;
>>>> int wcmEventCnt;
>>>> struct input_event wcmEvents[MAX_USB_EVENTS];
>>>> } wcmUSBData;
>>>> @@ -752,6 +753,13 @@ static int usbChooseChannel(WacomCommonPtr common)
>>>> */
>>>> channel = 0;
>>>> serial = 1;
>>>> +
>>>> + /* Generic devices are only ones that will send
>>>> + * pad buttons without a PAD serial # so hardcode
>>>> + * the reserved channel here. The rest can protocols
>>>> + * can rely on normal channel logic.
>>>> + */
>>>> + private->wcmPadChannel = MAX_CHANNELS-1;
>>>> }
>>>> else if (common->wcmProtocolLevel == WCM_PROTOCOL_4)
>>>> {
>>>> @@ -784,6 +792,7 @@ static int usbChooseChannel(WacomCommonPtr common)
>>>> channel = serial-1;
>>>> else
>>>> channel = 0;
>>>> + private->wcmPadChannel = channel;
>>>> }
>>>> else if (serial) /* serial number should never be 0 for V5 devices
>>>> */
>>>> {
>>>> @@ -834,6 +843,7 @@ static int usbChooseChannel(WacomCommonPtr common)
>>>>
>>>> !common->wcmChannel[0].work.proximity ) /* new transducer */
>>>> channel = 0;
>>>> }
>>>> + private->wcmPadChannel = channel;
>>>> }
>>>>
>>>> /* fresh out of channels */
>>>> @@ -950,9 +960,11 @@ skipEvent:
>>>> private->wcmEventCnt = 0;
>>>> }
>>>>
>>>> -static void usbParseAbsEvent(WacomCommonPtr common,
>>>> - struct input_event *event, WacomDeviceState
>>>> *ds)
>>>> +static int usbParseAbsEvent(WacomCommonPtr common,
>>>> + struct input_event *event, WacomDeviceState
>>>> *ds)
>>>> {
>>>> + int change = 1;
>>>> +
>>>> switch(event->code)
>>>> {
>>>> case ABS_X:
>>>> @@ -1003,7 +1015,10 @@ static void usbParseAbsEvent(WacomCommonPtr common,
>>>> if (event->value)
>>>> ds->device_id = event->value;
>>>> break;
>>>> + default:
>>>> + change = 0;
>>>> }
>>>> + return change;
>>>> }
>>>>
>>>> static struct
>>>> @@ -1024,16 +1039,18 @@ static struct
>>>> { PAD_ID, BTN_TOOL_FINGER }
>>>> };
>>>>
>>>> -static void usbParseKeyEvent(WacomCommonPtr common,
>>>> - struct input_event *event, WacomDeviceState
>>>> *ds,
>>>> - WacomDeviceState *dslast)
>>>> +#define MOD_BUTTONS(bit, value) do { \
>>>> + shift = 1<<bit; \
>>>> + ds->buttons = (((value) != 0) ? \
>>>> + (ds->buttons | (shift)) : (ds->buttons & ~(shift))); \
>>>> + } while (0)
>>>> +
>>>> +static int usbParseKeyEvent(WacomCommonPtr common,
>>>> + struct input_event *event, WacomDeviceState
>>>> *ds,
>>>> + WacomDeviceState *dslast)
>>>> {
>>>> - int shift, nkeys;
>>>> - #define MOD_BUTTONS(bit, value) do { \
>>>> - shift = 1<<bit; \
>>>> - ds->buttons = (((value) != 0) ? \
>>>> - (ds->buttons | (shift)) : (ds->buttons & ~(shift))); \
>>>> - } while (0)
>>>> + int shift;
>>>> + int change = 1;
>>>>
>>>> /* BTN_TOOL_* are sent to indicate when a specific tool is going
>>>> * in our out of proximity. When going out of proximity, ds
>>>> @@ -1046,8 +1063,9 @@ static void usbParseKeyEvent(WacomCommonPtr common,
>>>> * that map to different channels can be in proximity at same
>>>> * time with no confusion.
>>>> *
>>>> - * Remaining part of case state (after BTN_TOOL_*) handle normal
>>>> - * button presses.
>>>> + * Remaining part of case state (after BTN_TOOL_*) handle buttons
>>>> + * on stylus that validaty depends on its tool being in
>>>> + * proximity.
>>>> */
>>>> switch (event->code)
>>>> {
>>>> @@ -1165,16 +1183,38 @@ static void usbParseKeyEvent(WacomCommonPtr common,
>>>> * combination with the first finger data */
>>>> break;
>>>>
>>>> + case BTN_STYLUS:
>>>> + MOD_BUTTONS(1, event->value);
>>>> + break;
>>>> +
>>>> + case BTN_STYLUS2:
>>>> + MOD_BUTTONS(2, event->value);
>>>> + break;
>>>> + default:
>>>> + change = 0;
>>>> + }
>>>> + return change;
>>>> +}
>>>> +
>>>> +/* Track buttons associated with always in proximity PAD device. These
>>>> + * are buttons located on tablet itself.
>>>> + */
>>>
>>> BTN_LEFT, BTN_MIDDLE, BTN_RIGHT, BTN_SIDE, and BTN_EXTRA are the
>>> buttons for CURSOR (mouse/puck, whichever name you use ;-) device.
>>> They are reported through BTN_TOOL_MOUSE. So, PAD should not use them.
>>> That is why BTN_0...9, BTX_A..Z, BTN_BASE... are used for PAD and
>>> those five BTN_* have never been reused for PAD.
>>>
>>> Bamboo doesn't come with a mouse. So, this isn't an issue for you.
>>> But, I have to care about those users who have pucks. Otherwise they
>>> are going to cry ;).
>>>
>>> There are two options for us, I think.
>>>
>>> 1. We don't support BTN_LEFT, etc as default values for PAD, as
>>> we are doing it now, except the freshly baked Bamboo MT patchset. That
>>> is, we'll need to update the Bamboo MT patch to BTN_0 or something.
>>>
>>> The drawback for this option is: we lose the chance to set meaningful
>>> defaults for the buttons in the kernel. The benefit for us is it is
>>> easy to implement.
>>>
>>> 2. We support BTN_LEFT, etc. for PAD as well so we can have good
>>> default settings in the kernel. Then we need something to distinguish
>>> PAD from CURSOR so we could reroute the buttons here.
>>>
>>> Putting an Acked-by tag is easy. Making all the users happy is not so
>>> easy...
>>>
>>> Ping
>>
>> Great comments.
>>
>> I think I can somehow wrap this stuff with WCM_PROTOCOL_GENERIC and
>> resolve the issue. I mean put these "CURSOR" and "PAD" overlapping
>> buttons in both usbParsePadKeyEvent() and usbParseKeyEvent() but only
>> support them in later when wcmProtocolLevel != WCM_PROTOCOL_GENERIC...
>> or maybe only when device_type == CURSOR_ID.
>>
>> Anyways, I'll make a list of the overlapping values and see what
>> patterns emerge. Somehow I had convinced myself there were no
>> overlapping buttons.
>>
>> Stay tuned for v3 of patch #5 and patch #1.
>>
>> Chris
>>
>
> It turns out this version of patch does correctly handle MOUSE devices
> and their buttons because wcmPadChannel == usbChooseChannel() always
> for Protocol 4 and 5 devices.
I really should read the whole code and try your driver before making
any more comments. TBH, I am not sure if what I am talking about
matches exactly with what you are doing. But, I am running out of time
to try your driver and I do have serious concerns, which is not about
your implementation, it is about the fact that you do not have a
protocol 5 device to test with.
Once we have your patch in the driver, we will need to update the PAD
code for Intuos and Cintiqs so they don't rely on BTN_TOOL_FINGER for
pad events. The major difference between your Bamboo and the other
models is which port the PAD events are reported from. For Bamboo, it
is from the touch port. For all the others, including Graphires, which
are protocol 4 devices, it is from the digitizer port. If you have
assumed that the PAD data will only come from the PROTOCOL_GENERIC
devcies, i.e. only one tool is associated with the device/port, we'll
run into trouble to split the PAD events from the other tools (stylus,
eraser, and cursor) for other models.
Do you get what I am talking about?
I might have time to test your patchset next month (well, the earliest
time could be next week, That's next month, right? :). But I'd like to
understand your patch as soon as I can.
> The comments read like it always forces to PAD devices which isn't the
> case so I've updated the patch to read better and will resend.
Let's see if we are on the same page or not.
Thank you so much for your support, Chris.
Ping
------------------------------------------------------------------------------
Nokia and AT&T present the 2010 Calling All Innovators-North America contest
Create new apps & games for the Nokia N8 for consumers in U.S. and Canada
$10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing
Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store
http://p.sf.net/sfu/nokia-dev2dev
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
