On Tue, Jan 19, 2016 at 06:35:24PM -0800, Ping Cheng wrote: > On Tue, Jan 19, 2016 at 6:17 PM, Peter Hutterer <peter.hutte...@who-t.net> > wrote: > > > On Tue, Jan 19, 2016 at 05:47:50PM -0800, Ping Cheng wrote: > > > On Tue, Jan 19, 2016 at 5:03 PM, Peter Hutterer < > > peter.hutte...@who-t.net> > > > wrote: > > > > > > > On Tue, Jan 19, 2016 at 04:29:09PM -0800, Ping Cheng wrote: > > > > > Group inital values by device types. > > > > > > > > > > Signed-off-by: Ping Cheng <pi...@wacom.com> > > > > > --- > > > > > v2: rely on device type instead of kernel version, as suggested by > > Peter. > > > > > --- > > > > > wacom.c | 67 > > > > +++++++++++++---------------------------------------------------- > > > > > 1 file changed, 13 insertions(+), 54 deletions(-) > > > > > > > > > > diff --git a/wacom.c b/wacom.c > > > > > index 32a98c4..3f5a725 100644 > > > > > --- a/wacom.c > > > > > +++ b/wacom.c > > > > > @@ -211,26 +211,21 @@ static int wacom_intuos_events(struct > > uinput_info > > > > *info) > > > > > set_event(info, UI_SET_KEYBIT, BTN_TOOL_RUBBER); > > > > > set_event(info, UI_SET_KEYBIT, BTN_TOOL_MOUSE); > > > > > set_event(info, UI_SET_KEYBIT, BTN_TOOL_LENS); > > > > > - if (features->type != INTUOS5S && > > > > > - features->type != INTUOS5M && > > > > > - features->type != INTUOS5L) > > > > > + if (features->type != INTUOS) > > > > > set_event(info, UI_SET_KEYBIT, BTN_TOOL_FINGER); > > > > > > > > ok, now Im confused. in the previous version you had > INTUOS which > > > > would've > > > > excluded a couple of devices. Now you're only excluding the Intuos1. Is > > > > this > > > > intended? > > > > > > > > > > Yes, it is intentional. > > > > > > "> INTUOS" meant INTUOS is not included. All types after INTUOS were > > > included, which are Intuos 3 and later, as well as all Cintitqs. Now, I > > > don't have those types in order. I can not use > INTUOS any more. In > > fact, > > > "> INTUOS" was unnecessary then. The type enum rearrangement was not for > > > this statement. It was for the if/switch statement you commented below. > > > > I was referring to the devices < INTUOS that would've been dropped with v1 > > > > There was no < INTUOS in v1. This was a <= INTUOS3L. But, that was for > another loop...
In the current code, the intuos 5 ones don't get the BTN_TOOL_FINGER bit set (at least not here). in the first version [1], you removed the intous5 condition and replaced it with an kernel version check and the condition > + if (features->type > INTUOS) this would've added the tag to everything except PTU, PL, GRAPHIRE, P4, PENPARTNER and INTUOS. In the second version [2], you removed the intuos5 condition and replaced it with > + if (features->type != INTUOS) so PTU, PL, GRAPHIRE, P4 and PENPARTNER now get the bit set. that's equivalent to the current code, so with v2 you're unsetting BTN_TOOL_FINGER from the Intuos, but leaving it in place for everything else (including Intuos 5 models which were previously excluded) [1] http://sourceforge.net/p/linuxwacom/mailman/message/34776688/ [2] http://sourceforge.net/p/linuxwacom/mailman/message/34779774/ It looks correct enough, but I'm just making sure this is indeed what you intended (the commit message should've mentioned this change, "Cleanup" is always a bit generic) Cheers, Peter > > but included in v2. Since this is the same as the current code, I guess this > > was intentational though and maybe an oversight in v1? > > > > I am a bit confused ;-). This is v2 we are looking at. We only had one > version before this one. Do you refer to another version? > > Cheers, > > Ping > > > > > - set_event(info, UI_SET_KEYBIT, BTN_TOUCH); > > > > > set_event(info, UI_SET_KEYBIT, BTN_STYLUS2); > > > > > set_event(info, UI_SET_KEYBIT, BTN_RIGHT); > > > > > set_event(info, UI_SET_KEYBIT, BTN_LEFT); > > > > > set_event(info, UI_SET_KEYBIT, BTN_MIDDLE); > > > > > set_event(info, UI_SET_KEYBIT, BTN_SIDE); > > > > > set_event(info, UI_SET_KEYBIT, BTN_EXTRA); > > > > > - if (features->type != INTUOS5S && > > > > > - features->type != INTUOS5M && > > > > > - features->type != INTUOS5L) > > > > > - set_event(info, UI_SET_KEYBIT, BTN_7); > > > > > - set_event(info, UI_SET_KEYBIT, BTN_MIDDLE); > > > > > > > > > > - if (features->type != INTUOS5S && > > > > > - features->type != INTUOS5M && > > > > > - features->type != INTUOS5L) { > > > > > + if (features->type == INTUOS3S || > > > > > + features->type == INTUOS3 || > > > > > + features->type == INTUOS3L || > > > > > + features->type == CINTIQ || > > > > > + features->type == BEE || > > > > > + features->type == WACOM_21UX2) { > > > > > > > > I'd prefer a switch for simpler fallthrough, but I'm not going to > > fight too > > > > hard for this :) > > > > > > > > > > Yeah, I paid attention to your suggestion ;-). But, there are only two > > > states: with RX/Y, or without. We can definitely go with > > > > > > switch (features->type) { > > > case INTUOS3S: > > > case INTUOS3: > > > case INTUOS3L: > > > case CINTIQ: > > > case BEE: > > > case WACOM_21UX2: > > > set_event(info, UI_SET_ABSBIT, ABS_RX); > > > set_event(info, UI_SET_ABSBIT, ABS_RY); > > > break; > > > } > > > > > > Feel free to merge it in when you push the patch. You have my > > > signed-off-by, either way. > > > > > > Thank you for your review. > > > > > > Ping > > > > > > > > > > > set_event(info, UI_SET_ABSBIT, ABS_RX); > > > > > set_event(info, UI_SET_ABSBIT, ABS_RY); > > > > > } > > > > > @@ -352,26 +347,14 @@ static int wacom_set_events(struct uinput_info > > > > *info, struct uinput_user_dev *de > > > > > break; > > > > > case INTUOS4: > > > > > case INTUOS4L: > > > > > - set_event(info, UI_SET_ABSBIT, ABS_Z); > > > > > - set_event(info, UI_SET_KEYBIT, BTN_7); > > > > > - set_event(info, UI_SET_KEYBIT, BTN_8); > > > > > - /* fall trhu */ > > > > > - case INTUOS4S: > > > > > - set_event(info, UI_SET_KEYBIT, BTN_0); > > > > > - set_event(info, UI_SET_KEYBIT, BTN_1); > > > > > - set_event(info, UI_SET_KEYBIT, BTN_2); > > > > > - set_event(info, UI_SET_KEYBIT, BTN_3); > > > > > - set_event(info, UI_SET_KEYBIT, BTN_4); > > > > > - set_event(info, UI_SET_KEYBIT, BTN_5); > > > > > - set_event(info, UI_SET_KEYBIT, BTN_6); > > > > > - wacom_intuos_events(info); > > > > > - break; > > > > > case INTUOS5M: > > > > > case INTUOS5L: > > > > > set_event(info, UI_SET_KEYBIT, BTN_7); > > > > > set_event(info, UI_SET_KEYBIT, BTN_8); > > > > > case INTUOS5S: > > > > > set_event(info, UI_SET_ABSBIT, ABS_Z); > > > > > + /* fall through */ > > > > > + case INTUOS4S: > > > > > set_event(info, UI_SET_KEYBIT, BTN_0); > > > > > set_event(info, UI_SET_KEYBIT, BTN_1); > > > > > set_event(info, UI_SET_KEYBIT, BTN_2); > > > > > @@ -416,7 +399,6 @@ static int wacom_set_initial_values(struct > > > > uinput_info *info, > > > > > dev->absmax[ABS_WHEEL] = 71; > > > > > /* fall through */ > > > > > case G4: > > > > > - /* fall through */ > > > > > case GRAPHIRE: > > > > > break; > > > > > > > > > > @@ -429,45 +411,22 @@ static int wacom_set_initial_values(struct > > > > uinput_info *info, > > > > > /* fall through */ > > > > > case INTUOS3S: > > > > > dev->absmax[ABS_RX] = 4096; > > > > > - dev->absmax[ABS_Z] = 899; > > > > > - dev->absmin[ABS_Z] = -900; > > > > > /* fall through */ > > > > > - case INTUOS: > > > > > - dev->absmax[ABS_WHEEL] = 1023; > > > > > - dev->absmax[ABS_TILT_X] = 127; > > > > > - dev->absmax[ABS_TILT_Y] = 127; > > > > > - dev->absmin[ABS_RZ] = -900; > > > > > - dev->absmax[ABS_RZ] = 899; > > > > > - dev->absmin[ABS_THROTTLE] = -1023; > > > > > - dev->absmax[ABS_THROTTLE] = 1023; > > > > > - break; > > > > > - case PL: > > > > > - case PTU: > > > > > - break; > > > > > - case PENPARTNER: > > > > > - break; > > > > > - case TABLETPC: > > > > > - dev->absmax[ABS_RX] = 1023; > > > > > - dev->absmax[ABS_RY] = 1023; > > > > > case INTUOS4S: > > > > > case INTUOS4: > > > > > case INTUOS4L: > > > > > - dev->absmin[ABS_Z] = -900; > > > > > - dev->absmax[ABS_Z] = 899; > > > > > - break; > > > > > case INTUOS5S: > > > > > case INTUOS5M: > > > > > case INTUOS5L: > > > > > - dev->absfuzz[ABS_X] = 4; > > > > > - dev->absfuzz[ABS_Y] = 4; > > > > > - dev->absfuzz[ABS_Y] = 4; > > > > > dev->absmin[ABS_Z] = -900; > > > > > dev->absmax[ABS_Z] = 899; > > > > > - dev->absmin[ABS_RZ] = -900; > > > > > - dev->absmax[ABS_RZ] = 899; > > > > > + /* fall through */ > > > > > + case INTUOS: > > > > > dev->absmax[ABS_WHEEL] = 1023; > > > > > dev->absmax[ABS_TILT_X] = 127; > > > > > dev->absmax[ABS_TILT_Y] = 127; > > > > > + dev->absmin[ABS_RZ] = -900; > > > > > + dev->absmax[ABS_RZ] = 899; > > > > > dev->absmin[ABS_THROTTLE] = -1023; > > > > > dev->absmax[ABS_THROTTLE] = 1023; > > > > > break; > > > > > -- > > > > > 1.9.1 > > > > > > > > > > > ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140 _______________________________________________ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
