> Hi,
>
> I wouldn't mind to see such a feature in wsmouse, but there are two
> things that I don't like about the patch in this form: 1) It breaks
> existing configurations, ...

I know sometimes breaking changes are made in the name of forward
progress, and I wasn't sure if this was one of those times.

> ... and 2) it may bother the users who don't care about such an
> extension - probably the majority of those who enable tapping.
>
> A better approach might be to extend wsconsctl in such a way that
> it accepts both a single value as well as a triple of values as
> input, and prints them in alternating formats, depending on whether
> all values are the same or not.

While I handle single value assignment, I defer on printing. I worked
on it some, but it turned into a real mess. I also don't see much harm
in printing the full triple. Users don't see it except when it is
assigned during boot and when using wsconsctl, so it shouldn't be an
annoyance. And it may also serve as an indicator that something has
changed with regards to touchpad features, prompting investigation.

> (Moreover, if we add special handlers for the 'tapping' field in
> wsconsctrl, then it's possible to use the WSMOUSECFG_TAPPING
> parameter as a bit field, which might be more adequate here.)

This would require a few changes on the kernel side. Right now,
WSTPAD_TAPPING is a single bit within wstpad.features, which is why I
added two more flags. To store the configuration, adding a "tapping"
field to wstpad.params makes the most sense (to me). It wouldn't be too
much work to transition to this layout, and this has the advantage of
keeping all of the tapping configuration parameters together in one
place instead of as three feature flags. I assume that the user would
still see a triple and not the underlying integer representation when
configuring the touchpad via wsconsctl.

Below you will find my updated diff, which adds single value assignment
to wsconsctl for mouse.tp.tapping. I have also updated the man page to
reflect this change. However, it sounds like more changes are in store,
so I will wait for your feedback on what to implement.

Index: sbin/wsconsctl/mousecfg.c
===================================================================
RCS file: /cvs/src/sbin/wsconsctl/mousecfg.c,v
retrieving revision 1.7
diff -u -p -u -p -r1.7 mousecfg.c
--- sbin/wsconsctl/mousecfg.c   2 Apr 2020 17:17:04 -0000       1.7
+++ sbin/wsconsctl/mousecfg.c   16 Feb 2021 08:58:27 -0000
@@ -40,7 +40,7 @@
 #define TP_FILTER_FIRST                WSMOUSECFG_DX_MAX
 #define TP_FILTER_LAST         WSMOUSECFG_SMOOTHING
 #define TP_FEATURES_FIRST      WSMOUSECFG_SOFTBUTTONS
-#define TP_FEATURES_LAST       WSMOUSECFG_TAPPING
+#define TP_FEATURES_LAST       WSMOUSECFG_THREEFINGERTAPPING
 #define TP_SETUP_FIRST         WSMOUSECFG_LEFT_EDGE
 #define TP_SETUP_LAST          WSMOUSECFG_TAP_LOCKTIME
 #define LOG_FIRST              WSMOUSECFG_LOG_INPUT
@@ -71,8 +71,10 @@ static const int touchpad_types[] = {

 struct wsmouse_parameters cfg_tapping = {
        (struct wsmouse_param[]) {
-           { WSMOUSECFG_TAPPING, 0 }, },
-       1
+           { WSMOUSECFG_ONEFINGERTAPPING, 0 },
+           { WSMOUSECFG_TWOFINGERTAPPING, 0 },
+           { WSMOUSECFG_THREEFINGERTAPPING, 0 }, },
+       3
 };

 struct wsmouse_parameters cfg_scaling = {
@@ -262,6 +264,24 @@ set_percent(struct wsmouse_parameters *f
 }

 static int
+set_tapping(struct wsmouse_parameters *field, char *tapping)
+{
+       int i1, i2, i3;
+
+       switch (sscanf(tapping, "%d,%d,%d", &i1, &i2, &i3)) {
+       case 1:
+               i2 = i3 = i1;
+               /* FALLTHROUGH */
+       case 3:
+               set_value(field, WSMOUSECFG_ONEFINGERTAPPING, i1);
+               set_value(field, WSMOUSECFG_TWOFINGERTAPPING, i2);
+               set_value(field, WSMOUSECFG_THREEFINGERTAPPING, i3);
+               return (0);
+       }
+       return (-1);
+}
+
+static int
 set_edges(struct wsmouse_parameters *field, char *edges)
 {
        float f1, f2, f3, f4;
@@ -359,6 +379,12 @@ mousecfg_rd_field(struct wsmouse_paramet
        if (field == &cfg_param) {
                if (read_param(field, val))
                        errx(1, "invalid input (param)");
+               return;
+       }
+
+       if (field == &cfg_tapping) {
+               if (set_tapping(field, val))
+                       errx(1, "invalid input (tapping)");
                return;
        }

Index: share/man/man4/wsmouse.4
===================================================================
RCS file: /cvs/src/share/man/man4/wsmouse.4,v
retrieving revision 1.20
diff -u -p -u -p -r1.20 wsmouse.4
--- share/man/man4/wsmouse.4    4 Feb 2018 20:29:59 -0000       1.20
+++ share/man/man4/wsmouse.4    16 Feb 2021 08:58:27 -0000
@@ -26,7 +26,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: February 4 2018 $
+.Dd $Mdocdate: February 15 2021 $
 .Dt WSMOUSE 4
 .Os
 .Sh NAME
@@ -87,14 +87,23 @@ is omitted, commands apply to
 .Pa /dev/wsmouse0 .
 .Bl -tag -width Ds
 .It Cm mouse.tp.tapping
-Setting this parameter to a non-zero value enables tap gestures.
+This list of three parameters sets the enabled tap gestures, in the order:
+.Bd -literal -offset indent
+.Sm off
+.Ar one-finger , two-finger , three-finger
+.Sm on
+.Ed
+.Pp
+Setting a parameter to a non-zero value enables that tap gesture.
+Providing a single value sets all three parameters to that value.
 Contacts on the touchpad that are immediately released again
-trigger click events.
+trigger click events only if the corresponding tap gesture is enabled.
 One-finger, two-finger, and three-finger taps generate left-button,
 right-button, and middle-button clicks, respectively.
 If, within a short time interval, a second touch follows a one-finger
 tap, the button-up event is not issued until that touch ends
 .Pq Dq tap-and-drag .
+This requires the one-finger tap gesture to be enabled.
 .It Cm mouse.tp.scaling
 The value is a scale coefficient that is applied to the relative
 coordinates.
Index: sys/dev/wscons/wsconsio.h
===================================================================
RCS file: /cvs/src/sys/dev/wscons/wsconsio.h,v
retrieving revision 1.95
diff -u -p -u -p -r1.95 wsconsio.h
--- sys/dev/wscons/wsconsio.h   1 Oct 2020 17:28:14 -0000       1.95
+++ sys/dev/wscons/wsconsio.h   16 Feb 2021 08:58:29 -0000
@@ -319,7 +319,9 @@ enum wsmousecfg {
        WSMOUSECFG_SWAPSIDES,           /* invert soft-button/scroll areas */
        WSMOUSECFG_DISABLE,             /* disable all output except for
                                           clicks in the top-button area */
-       WSMOUSECFG_TAPPING,             /* enable tapping */
+       WSMOUSECFG_ONEFINGERTAPPING,    /* enable one-finger tapping */
+       WSMOUSECFG_TWOFINGERTAPPING,    /* enable two-finger tapping */
+       WSMOUSECFG_THREEFINGERTAPPING,  /* enable three-finger tapping */

        /*
         * Touchpad options
@@ -345,7 +347,7 @@ enum wsmousecfg {
        WSMOUSECFG_LOG_INPUT = 256,
        WSMOUSECFG_LOG_EVENTS,
 };
-#define WSMOUSECFG_MAX 39      /* max size of param array per ioctl */
+#define WSMOUSECFG_MAX 41      /* max size of param array per ioctl */

 struct wsmouse_param {
        enum wsmousecfg key;
Index: sys/dev/wscons/wstpad.c
===================================================================
RCS file: /cvs/src/sys/dev/wscons/wstpad.c,v
retrieving revision 1.26
diff -u -p -u -p -r1.26 wstpad.c
--- sys/dev/wscons/wstpad.c     13 Sep 2020 10:05:46 -0000      1.26
+++ sys/dev/wscons/wstpad.c     16 Feb 2021 08:58:29 -0000
@@ -139,18 +139,19 @@ struct tpad_touch {
 /*
  * wstpad.features
  */
-#define WSTPAD_SOFTBUTTONS     (1 << 0)
-#define WSTPAD_SOFTMBTN                (1 << 1)
-#define WSTPAD_TOPBUTTONS      (1 << 2)
-#define WSTPAD_TWOFINGERSCROLL (1 << 3)
-#define WSTPAD_EDGESCROLL      (1 << 4)
-#define WSTPAD_HORIZSCROLL     (1 << 5)
-#define WSTPAD_SWAPSIDES       (1 << 6)
-#define WSTPAD_DISABLE         (1 << 7)
-#define WSTPAD_TAPPING         (1 << 8)
-
-#define WSTPAD_MT              (1 << 31)
+#define WSTPAD_SOFTBUTTONS             (1 << 0)
+#define WSTPAD_SOFTMBTN                        (1 << 1)
+#define WSTPAD_TOPBUTTONS              (1 << 2)
+#define WSTPAD_TWOFINGERSCROLL         (1 << 3)
+#define WSTPAD_EDGESCROLL              (1 << 4)
+#define WSTPAD_HORIZSCROLL             (1 << 5)
+#define WSTPAD_SWAPSIDES               (1 << 6)
+#define WSTPAD_DISABLE                 (1 << 7)
+#define WSTPAD_ONEFINGERTAPPING                (1 << 8)
+#define WSTPAD_TWOFINGERTAPPING                (1 << 9)
+#define WSTPAD_THREEFINGERTAPPING      (1 << 10)

+#define WSTPAD_MT                      (1 << 31)

 struct wstpad {
        u_int features;
@@ -725,6 +726,16 @@ tap_btn(struct wstpad *tp, int nmasked)
        return (n == 2 ? RIGHTBTN : (n == 3 ? MIDDLEBTN : LEFTBTN));
 }

+static inline int
+tap_btn_ignore(struct wstpad *tp)
+{
+       u_int btn = tp->tap.button;
+
+       return (btn == LEFTBTN ? !(tp->features & WSTPAD_ONEFINGERTAPPING)
+           : btn == RIGHTBTN ? !(tp->features & WSTPAD_TWOFINGERTAPPING)
+           : !(tp->features & WSTPAD_THREEFINGERTAPPING));
+}
+
 /*
  * This handler supports one-, two-, and three-finger-taps, which
  * are mapped to left-button, right-button and middle-button events,
@@ -793,9 +804,13 @@ wstpad_tap(struct wsmouseinput *input, u
                                tp->tap.centered = 0;
                        }
                        if (tp->tap.state == TAP_LIFTED) {
-                               *cmds |= 1 << TAPBUTTON_DOWN;
-                               err = !timeout_add_msec(&tp->tap.to,
-                                   tp->tap.clicktime);
+                               if (!tap_btn_ignore(tp)) {
+                                       *cmds |= 1 << TAPBUTTON_DOWN;
+                                       err = !timeout_add_msec(&tp->tap.to,
+                                           tp->tap.clicktime);
+                               } else {
+                                       tp->tap.state = TAP_DETECT;
+                               }
                        }
                }
                break;
@@ -1626,7 +1641,8 @@ wstpad_configure(struct wsmouseinput *in
        else if (tp->features & WSTPAD_EDGESCROLL)
                tp->handlers |= 1 << EDGESCROLL_HDLR;

-       if (tp->features & WSTPAD_TAPPING) {
+       if (tp->features & (WSTPAD_ONEFINGERTAPPING | WSTPAD_TWOFINGERTAPPING |
+           WSTPAD_THREEFINGERTAPPING)) {
                tp->tap.clicktime = imin(imax(tp->tap.clicktime, 80), 350);
                if (tp->tap.locktime)
                        tp->tap.locktime =
@@ -1669,7 +1685,7 @@ wstpad_set_param(struct wsmouseinput *in
                return (EINVAL);

        switch (key) {
-       case WSMOUSECFG_SOFTBUTTONS ... WSMOUSECFG_TAPPING:
+       case WSMOUSECFG_SOFTBUTTONS ... WSMOUSECFG_THREEFINGERTAPPING:
                switch (key) {
                case WSMOUSECFG_SOFTBUTTONS:
                        flag = WSTPAD_SOFTBUTTONS;
@@ -1695,8 +1711,14 @@ wstpad_set_param(struct wsmouseinput *in
                case WSMOUSECFG_DISABLE:
                        flag = WSTPAD_DISABLE;
                        break;
-               case WSMOUSECFG_TAPPING:
-                       flag = WSTPAD_TAPPING;
+               case WSMOUSECFG_ONEFINGERTAPPING:
+                       flag = WSTPAD_ONEFINGERTAPPING;
+                       break;
+               case WSMOUSECFG_TWOFINGERTAPPING:
+                       flag = WSTPAD_TWOFINGERTAPPING;
+                       break;
+               case WSMOUSECFG_THREEFINGERTAPPING:
+                       flag = WSTPAD_THREEFINGERTAPPING;
                        break;
                }
                if (val)
@@ -1757,7 +1779,7 @@ wstpad_get_param(struct wsmouseinput *in
                return (EINVAL);

        switch (key) {
-       case WSMOUSECFG_SOFTBUTTONS ... WSMOUSECFG_TAPPING:
+       case WSMOUSECFG_SOFTBUTTONS ... WSMOUSECFG_THREEFINGERTAPPING:
                switch (key) {
                case WSMOUSECFG_SOFTBUTTONS:
                        flag = WSTPAD_SOFTBUTTONS;
@@ -1783,8 +1805,14 @@ wstpad_get_param(struct wsmouseinput *in
                case WSMOUSECFG_DISABLE:
                        flag = WSTPAD_DISABLE;
                        break;
-               case WSMOUSECFG_TAPPING:
-                       flag = WSTPAD_TAPPING;
+               case WSMOUSECFG_ONEFINGERTAPPING:
+                       flag = WSTPAD_ONEFINGERTAPPING;
+                       break;
+               case WSMOUSECFG_TWOFINGERTAPPING:
+                       flag = WSTPAD_TWOFINGERTAPPING;
+                       break;
+               case WSMOUSECFG_THREEFINGERTAPPING:
+                       flag = WSTPAD_THREEFINGERTAPPING;
                        break;
                }
                *pval = !!(tp->features & flag);

Reply via email to