Re: [Linuxwacom-devel] [PATCH wdaemon 1/2] Cleanup initial values

2016-01-19 Thread Ping Cheng
On Tue, Jan 19, 2016 at 5:03 PM, Peter Hutterer 
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 
> > ---
> > 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.


> > - 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 

Re: [Linuxwacom-devel] [PATCH wdaemon 1/2] Cleanup initial values

2016-01-19 Thread Peter Hutterer
On Tue, Jan 19, 2016 at 05:47:50PM -0800, Ping Cheng wrote:
> On Tue, Jan 19, 2016 at 5:03 PM, Peter Hutterer 
> 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 
> > > ---
> > > 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
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?

Cheers,
   Peter


> > > - 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 

Re: [Linuxwacom-devel] [PATCH wdaemon 1/2] Cleanup initial values

2016-01-19 Thread Peter Hutterer
On Tue, Jan 19, 2016 at 04:29:09PM -0800, Ping Cheng wrote:
> Group inital values by device types.
> 
> Signed-off-by: Ping Cheng 
> ---
> 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?

> - 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 :)

Cheers,
   Peter

>   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;
>   

Re: [Linuxwacom-devel] [PATCH wdaemon 1/2] Cleanup initial values

2016-01-19 Thread Ping Cheng
On Tue, Jan 19, 2016 at 6:17 PM, Peter Hutterer 
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 
> > > > ---
> > > > 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...

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);
> > > > - 

Re: [Linuxwacom-devel] [PATCH wdaemon 1/2] Cleanup initial values

2016-01-19 Thread Peter Hutterer
On Tue, Jan 19, 2016 at 06:35:24PM -0800, Ping Cheng wrote:
> On Tue, Jan 19, 2016 at 6:17 PM, Peter Hutterer 
> 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 
> > > > > ---
> > > > > 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 

[Linuxwacom-devel] [PATCH wdaemon 1/2] Cleanup initial values

2016-01-19 Thread Ping Cheng
Group inital values by device types.

Signed-off-by: Ping Cheng 
---
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);
-   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) {
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 

[Linuxwacom-devel] [PATCH wdaemon 2/2] Add Intuos Pro support

2016-01-19 Thread Ping Cheng
Signed-off-by: Ping Cheng 
---
 udev/60-wacom.rules   |  3 +++
 udev/wdaemon_is_uinput.sh |  3 +++
 wacom.c   | 12 
 3 files changed, 18 insertions(+)

diff --git a/udev/60-wacom.rules b/udev/60-wacom.rules
index 7f536e3..0e62b35 100644
--- a/udev/60-wacom.rules
+++ b/udev/60-wacom.rules
@@ -76,6 +76,9 @@ ENV{ID_MODEL_ID}=="0027", 
SYMLINK+="input/wacom-tablets/intuos5-t-m input/wacom-
 ENV{ID_MODEL_ID}=="0028", SYMLINK+="input/wacom-tablets/intuos5-t-l 
input/wacom-tablets/intuos5-t-l-%b"
 ENV{ID_MODEL_ID}=="0029", SYMLINK+="input/wacom-tablets/intuos5-s 
input/wacom-tablets/intuos5-s-%b"
 ENV{ID_MODEL_ID}=="002a", SYMLINK+="input/wacom-tablets/intuos5-m 
input/wacom-tablets/intuos5-m-%b"
+ENV{ID_MODEL_ID}=="0314", SYMLINK+="input/wacom-tablets/intuospro-s 
input/wacom-tablets/intuospro-s-%b"
+ENV{ID_MODEL_ID}=="0315", SYMLINK+="input/wacom-tablets/intuospro-m 
input/wacom-tablets/intuospro-m-%b"
+ENV{ID_MODEL_ID}=="0317", SYMLINK+="input/wacom-tablets/intuospro-l 
input/wacom-tablets/intuospro-l-%b"
 ENV{ID_MODEL_ID}=="00cc", SYMLINK+="input/wacom-tablets/cintiq-21ux2 
input/wacom-tablets/cintiq-21ux2-%b"
 
 LABEL="wacom_end"
diff --git a/udev/wdaemon_is_uinput.sh b/udev/wdaemon_is_uinput.sh
index d93f661..3c56c85 100755
--- a/udev/wdaemon_is_uinput.sh
+++ b/udev/wdaemon_is_uinput.sh
@@ -88,6 +88,9 @@ case "$vendor-$product" in
"056a-0028") echo "intuos5-t-l"; ;;
"056a-0029") echo "intuos5-s"; ;;
"056a-002a") echo "intuos5-m"; ;;
+   "056a-0314") echo "intuospro-s"; ;;
+   "056a-0315") echo "intuospro-m"; ;;
+   "056a-0317") echo "intuospro-l"; ;;
"056a-00CC") echo "cintiq-21ux2"; ;;
"056a-00F0") echo "dtu1631"; ;;
"056a-00CE") echo "dtu2231"; ;;
diff --git a/wacom.c b/wacom.c
index 3f5a725..99f0b3d 100644
--- a/wacom.c
+++ b/wacom.c
@@ -99,6 +99,9 @@ enum {
INTUOS5M,
INTUOS5L,
INTUOS5_FG,
+   INTUOSPS,
+   INTUOSPM,
+   INTUOSPL,
MAX_TYPE
 };
 
@@ -181,6 +184,9 @@ static struct wacom_features {
{ "Wacom Intuos5 touch L Finger",  4096,  4096,0,  0, INTUOS5_FG, 
0x28},
{ "Wacom Intuos5 S Pen",  31496, 19685, 2047, 63, INTUOS5S,   
0x29},
{ "Wacom Intuos5 M Pen",  44704, 27940, 2047, 63, INTUOS5M,   
0x2A},
+   { "Wacom Intuos Pro S Pen",   31496, 19685, 2047, 63, INTUOSPS,   
0x314},
+   { "Wacom Intuos Pro M Pen",   44704, 27940, 2047, 63, INTUOSPM,   
0x315},
+   { "Wacom Intuos Pro L Pen",   65024, 40640, 2047, 63, INTUOSPL,   
0x317},
 };
 #define WACOM_N_TABLETS (sizeof(wacom_features)/sizeof(wacom_features[0]))
 int wacom_check_type(int x)
@@ -349,9 +355,12 @@ static int wacom_set_events(struct uinput_info *info, 
struct uinput_user_dev *de
case INTUOS4L:
case INTUOS5M:
case INTUOS5L:
+   case INTUOSPM:
+   case INTUOSPL:
set_event(info, UI_SET_KEYBIT, BTN_7);
set_event(info, UI_SET_KEYBIT, BTN_8);
case INTUOS5S:
+   case INTUOSPS:
set_event(info, UI_SET_ABSBIT, ABS_Z);
/* fall through */
case INTUOS4S:
@@ -418,6 +427,9 @@ static int wacom_set_initial_values(struct uinput_info 
*info,
case INTUOS5S:
case INTUOS5M:
case INTUOS5L:
+   case INTUOSPS:
+   case INTUOSPM:
+   case INTUOSPL:
dev->absmin[ABS_Z] = -900;
dev->absmax[ABS_Z] = 899;
/* fall through */
-- 
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=/4140
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel


Re: [Linuxwacom-devel] [PATCH] xsetwacom: Remove unnecessary static state from 'get_actions'

2016-01-19 Thread Jason Gerecke
On Mon, Jan 18, 2016 at 9:01 PM, Peter Hutterer
 wrote:
> On Mon, Jan 18, 2016 at 10:48:57AM -0800, Jason Gerecke wrote:
>> The 'last_type' variable within 'get_actions' stores the type of the last
>> action encountered. When dealing with "key" or "button" actions, we use
>> that information to determine if we need to print out the action prefix
>> or not (if the type hasn't changed, its safe to leave the prefix out).
>>
>> For some reason, this variable was marked as 'static', which causes it
>> to retain its value across invocations. The function is only called once
>> for any given button, meaning that we improperly retain the "last_type"
>> across buttons. If the last action on a button is of e.g. type "key" and
>> the first action of the next button is as well, then the "key" prefix
>> will be missing from the printed output of that second button's actions.
>> Making this variable non-static fixes this issue and ensures each run
>> of the function is independent.
>>
>> Signed-off-by: Jason Gerecke 
>> ---
>>  tools/xsetwacom.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/xsetwacom.c b/tools/xsetwacom.c
>> index c1fec71..3374d58 100644
>> --- a/tools/xsetwacom.c
>> +++ b/tools/xsetwacom.c
>> @@ -1981,7 +1981,7 @@ static int get_actions(Display *dpy, XDevice *dev,
>>
>>   for (i = 0; i < nitems; i++)
>>   {
>> - static int last_type;
>> + int last_type = 0;
>
> yes to the removal of static, but don't you need to move it out of the loop
> then? the point of last_type is that it retains the value of data[i-1] for
> comparison. If you remove the static, you can't be guaranteed of that.
>
> Cheers,
>Peter
>

Derp. Update (along with some other related fixes) coming shortly.

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two, /
But you can’t take seven from three,/
So you look at the sixty-fours

--
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=/4140
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel


[Linuxwacom-devel] [PATCH v2 1/3] xsetwacom: Remove unnecessary static state from 'get_actions'

2016-01-19 Thread Jason Gerecke
The 'last_type' variable within 'get_actions' stores the type of the last
action encountered. When dealing with "key" or "button" actions, we use
that information to determine if we need to print out the action prefix
or not (if the type hasn't changed, its safe to leave the prefix out).

For some reason, this variable was marked as 'static', which causes it
to retain its value across invocations. The function is only called once
for any given button, meaning that we improperly retain the "last_type"
across buttons. If the last action on a button is of e.g. type "key" and
the first action of the next button is as well, then the "key" prefix
will be missing from the printed output of that second button's actions.
Making this variable non-static fixes this issue and ensures each run
of the function is independent.

Fixes: http://sourceforge.net/p/linuxwacom/bugs/303/
Fixes: https://github.com/linuxwacom/xf86-input-wacom/issues/3

Signed-off-by: Jason Gerecke 
---
 tools/xsetwacom.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/xsetwacom.c b/tools/xsetwacom.c
index c1fec71..95f7a6c 100644
--- a/tools/xsetwacom.c
+++ b/tools/xsetwacom.c
@@ -1951,6 +1951,7 @@ static int get_actions(Display *dpy, XDevice *dev,
unsigned long nitems, bytes_after, *data;
int i;
char buff[1024] = {0};
+   int last_type;
 
prop = XInternAtom(dpy, param->prop_name, True);
 
@@ -1979,9 +1980,9 @@ static int get_actions(Display *dpy, XDevice *dev,
   AnyPropertyType, , , ,
   _after, (unsigned char**));
 
+   last_type = 0;
for (i = 0; i < nitems; i++)
{
-   static int last_type;
unsigned long action = data[i];
int current_type;
int detail;
-- 
2.7.0


--
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=/4140
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel


[Linuxwacom-devel] [PATCH 3/3] xsetwacom: Use stderr for 'core' and 'displaytogle' deprecation notices

2016-01-19 Thread Jason Gerecke
Signed-off-by: Jason Gerecke 
---
 tools/xsetwacom.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/xsetwacom.c b/tools/xsetwacom.c
index cf43fe7..2f75d78 100644
--- a/tools/xsetwacom.c
+++ b/tools/xsetwacom.c
@@ -1026,7 +1026,7 @@ static int special_map_core(Display *dpy, int argc, char 
**argv, unsigned long *
static int once_only = 1;
if (once_only)
{
-   printf ("Note: The \"core\" keyword is not supported anymore 
and "
+   fprintf(stderr, "Note: The \"core\" keyword is not supported 
anymore and "
"will be ignored.\n");
once_only = 0;
}
@@ -1053,7 +1053,7 @@ static int special_map_displaytoggle(Display *dpy, int 
argc, char **argv, unsign
static int once_only = 1;
if (once_only)
{
-   printf ("Note: The \"displaytoggle\" keyword is not supported "
+   fprintf(stderr, "Note: The \"displaytoggle\" keyword is not 
supported "
"anymore and will be ignored.\n");
once_only = 0;
}
-- 
2.7.0


--
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=/4140
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel


[Linuxwacom-devel] [PATCH 2/3] xsetwacom: Only print action name for "modetoggle" and "displaytoggle"

2016-01-19 Thread Jason Gerecke
Running "xsetwacom get" on a button which contain a "modetoggle" or
"displaytoggle" action will result in output which cannot be parsed
by "xsetwacom set". Both of these actions are treated like buttons,
printing extra data that is not expected by the "set" command.

Signed-off-by: Jason Gerecke 
---
 tools/xsetwacom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/xsetwacom.c b/tools/xsetwacom.c
index 95f7a6c..cf43fe7 100644
--- a/tools/xsetwacom.c
+++ b/tools/xsetwacom.c
@@ -2022,7 +2022,7 @@ static int get_actions(Display *dpy, XDevice *dev,
if (current_type == AC_KEY)
sprintf(str, "%c%s ", press_str,
XKeysymToString(detail));
-   else
+   else if (current_type == AC_BUTTON)
sprintf(str, "%c%d ", press_str, detail);
strcat(buff, str);
last_type = current_type;
-- 
2.7.0


--
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=/4140
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel