On Wed, Nov 17, 2010 at 7:54 PM, Peter Hutterer
<[email protected]> wrote:
> On Wed, Nov 17, 2010 at 12:33:49PM -0800, Ping Cheng wrote:
>> From: Ping Cheng <[email protected]>
>>
>> They said using BTN_TOOL_FINGER is too confusing. But we still
>
> Who is "they"? the people in the street? some guy in a pub? :)

Maybe some guys from the pub. Who knows ;).

>> need to configure those tablet buttons and strips/ring. Let's
>> detect the actual buttons we are going to receive to determine
>> the existence of a pad tool.
>>
>> BTN_0 and BTN_FORWARD are the two unique BTN_s that we use for
>> tablet buttons in the kernel driver.
>>
>> Signed-off-by: Ping Cheng <[email protected]>
>> Reviewed-by: Chris Bagwell <[email protected]>
>> ---
>>  src/wcmUSB.c            |    3 ++-
>>  src/wcmValidateDevice.c |   40 ++++++++++++++++++++++------------------
>>  2 files changed, 24 insertions(+), 19 deletions(-)
>>
>> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
>> index 4d8a0fb..98d3c05 100644
>> --- a/src/wcmUSB.c
>> +++ b/src/wcmUSB.c
>> @@ -846,7 +846,8 @@ static struct
>>       { CURSOR_ID, BTN_TOOL_LENS      },
>>       { TOUCH_ID,  BTN_TOOL_DOUBLETAP },
>>       { TOUCH_ID,  BTN_TOOL_TRIPLETAP },
>> -     { PAD_ID,    BTN_TOOL_FINGER    }
>> +     { PAD_ID,    BTN_FORWARD        },
>> +     { PAD_ID,    BTN_0              }
>>  };
>>
>>  #define MOD_BUTTONS(bit, value) do { \
>> diff --git a/src/wcmValidateDevice.c b/src/wcmValidateDevice.c
>> index 0968991..93857ec 100644
>> --- a/src/wcmValidateDevice.c
>> +++ b/src/wcmValidateDevice.c
>> @@ -120,20 +120,20 @@ ret:
>>  static struct
>>  {
>>       const char* type;
>> -     __u16 tool;
>> +     __u16 tool [3];
>
> no space before [] please.
>
>>  } wcmType [] =
>>  {
>> -     { "stylus", BTN_TOOL_PEN       },
>> -     { "eraser", BTN_TOOL_RUBBER    },
>> -     { "cursor", BTN_TOOL_MOUSE     },
>> -     { "touch",  BTN_TOOL_DOUBLETAP },
>> -     { "pad",    BTN_TOOL_FINGER    }
>> +     { "stylus", { BTN_TOOL_PEN,       0        } },
>> +     { "eraser", { BTN_TOOL_RUBBER,    0        } },
>> +     { "cursor", { BTN_TOOL_MOUSE,     0        } },
>> +     { "touch",  { BTN_TOOL_DOUBLETAP, 0        } },
>> +     { "pad",    { BTN_FORWARD,        BTN_0, 0 } }
>>  };
>
> this needs a comment that this is a zero-terminated array.

I feared you would say it is implied in the code if I had a comment
there ;). Anyway, comment added.

>>  /* validate tool type for device/product */
>>  Bool wcmIsAValidType(InputInfoPtr pInfo, const char* type)
>>  {
>> -     int j, ret = FALSE;
>> +     int j, k, ret = FALSE;
>>       WacomDevicePtr priv = (WacomDevicePtr)pInfo->private;
>>       WacomCommonPtr common = priv->common;
>>       char* dsource = xf86CheckStrOption(pInfo->options, "_source", "");
>> @@ -146,18 +146,22 @@ Bool wcmIsAValidType(InputInfoPtr pInfo, const char* 
>> type)
>>       {
>>               if (!strcmp(wcmType[j].type, type))
>>               {
>> -                     if (ISBITSET (common->wcmKeys, wcmType[j].tool))
>> +                     k = 0;
>> +                     do
>>                       {
>> -                             ret = TRUE;
>> -                             break;
>> -                     }
>> -                     else if (!strlen(dsource)) /* an user defined type */
>> -                     {
>> -                             /* assume it is a valid type */
>> -                             SETBIT(common->wcmKeys, wcmType[j].tool);
>> -                             ret = TRUE;
>> -                             break;
>> -                     }
>> +                             if (ISBITSET (common->wcmKeys, 
>> wcmType[j].tool[k]))
>> +                             {
>> +                                     ret = TRUE;
>> +                                     break;
>> +                             }
>> +                             else if (!strlen(dsource)) /* an user defined 
>> type */
>> +                             {
>> +                                     /* assume it is a valid type */
>> +                                     SETBIT(common->wcmKeys, 
>> wcmType[j].tool[k]);
>> +                                     ret = TRUE;
>> +                                     break;
>> +                             }
>> +                     } while (wcmType[j].tool[++k]);
>
> wouldn't a for loop be a little bit easier to read. do {} while loops are
> somewhat uncommon and not needed here.

Excellent point. New version will be submitted.

Thank you.

Ping

> for (k = 0; wcmType[j].tool[k] && !ret; k++)
>
> does the same job (and allows to remove the "break")
>
> ack in principle though
>
> Cheers,
>  Peter
>>               }
>>       }
>>       return ret;
>> --
>> 1.7.2.3
>>
>

------------------------------------------------------------------------------
Beautiful is writing same markup. Internet Explorer 9 supports
standards for HTML5, CSS3, SVG 1.1,  ECMAScript5, and DOM L2 & L3.
Spend less time writing and  rewriting code and more time creating great
experiences on the web. Be a part of the beta today
http://p.sf.net/sfu/msIE9-sfdev2dev
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to