[PATCH v3 xf86-input-synaptics] syndaemon: enable touchpad when pressing a modifier combo

Anton Lindqvist anton.lindqvist at gmail.com
Fri Aug 5 08:21:59 UTC 2016


When ignoring modifiers, ensure the touchpad is enabled once a modifier
key is pressed disregarding any previous key press that caused the
touchpad to be disabled.

Signed-off-by: Anton Lindqvist <anton.lindqvist at gmail.com>
---
On Fri, Aug 05, 2016 at 08:17:17AM +1000, Peter Hutterer wrote:
> On Thu, Aug 04, 2016 at 01:06:37PM +0200, Anton Lindqvist wrote:
> > Thanks for the feedback. Here's a revised patch trying to resolve the
> > issues. Should the new patch be re-submitted in a separate thread?
> 
> sticking it in the same thread makes them easier to associate, but for next
> time please add a v2, v3, etc. right after PATCH
> 
> > 
> > ------------------------ >8 ------------------------
> > 
> > When ignoring modifiers, ensure the touchpad is enabled once a modifier
> > key is pressed disregarding any previous key press that caused the
> > touchpad to be disabled.
> > 
> > Signed-off-by: Anton Lindqvist <anton.lindqvist at gmail.com>
> > ---
> 
> fwiw, if you write the general comments here, after the --- then git am will
> automatically remove it and there's no need to get the scissors out.

Thanks, didn't know. I am able to apply this message as a patch using
git-am(1).

> 
> >  tools/syndaemon.c | 48 +++++++++++++++++++++++++++++++++++-------------
> >  1 file changed, 35 insertions(+), 13 deletions(-)
> > 
> > diff --git a/tools/syndaemon.c b/tools/syndaemon.c
> > index 29e75f5..1d5ce1d 100644
> > --- a/tools/syndaemon.c
> > +++ b/tools/syndaemon.c
> > @@ -47,6 +47,12 @@
> >  
> >  #include "synaptics-properties.h"
> >  
> > +enum KeyboardActivity {
> > +    ActivityNew,
> > +    ActivityNone,
> > +    ActivityReset
> > +};
> > +
> >  enum TouchpadState {
> >      TouchpadOn = 0,
> >      TouchpadOff = 1,
> > @@ -181,29 +187,29 @@ install_signal_handler(void)
> >      }
> >  }
> >  
> > -/**
> > - * Return non-zero if the keyboard state has changed since the last call.
> > - */
> > -static int
> > +static enum KeyboardActivity
> >  keyboard_activity(Display * display)
> >  {
> >      static unsigned char old_key_state[KEYMAP_SIZE];
> >      unsigned char key_state[KEYMAP_SIZE];
> >      int i;
> > -    int ret = 0;
> > +    int ret = ActivityNone;
> >  
> >      XQueryKeymap(display, (char *) key_state);
> >  
> >      for (i = 0; i < KEYMAP_SIZE; i++) {
> >          if ((key_state[i] & ~old_key_state[i]) & keyboard_mask[i]) {
> > -            ret = 1;
> > +            ret = ActivityNew;
> >              break;
> >          }
> >      }
> >      if (ignore_modifier_combos) {
> >          for (i = 0; i < KEYMAP_SIZE; i++) {
> >              if (key_state[i] & ~keyboard_mask[i]) {
> > -                ret = 0;
> > +                if (old_key_state[i] & ~keyboard_mask[i])
> > +                    ret = ActivityNone;
> > +                else
> > +                    ret = ActivityReset;
> >                  break;
> >              }
> >          }
> > @@ -232,8 +238,17 @@ main_loop(Display * display, double idle_time, int poll_delay)
> >  
> >      for (;;) {
> >          current_time = get_time();
> > -        if (keyboard_activity(display))
> > -            last_activity = current_time;
> > +        switch (keyboard_activity(display)) {
> > +            case ActivityNew:
> > +                last_activity = current_time;
> > +                break;
> > +            case ActivityNone:
> > +                /* NOP */;
> > +                break;
> > +            case ActivityReset:
> > +                last_activity = 0.0;
> > +                break;
> > +        }
> >  
> >          /* If system times goes backwards, touchpad can get locked. Make
> >           * sure our last activity wasn't in the future and reset if it was. */
> > @@ -423,6 +438,7 @@ record_main_loop(Display * display, double idle_time)
> >          fd_set read_fds;
> >          int ret;
> >          int disable_event = 0;
> > +	int modifier_event = 0;
> 
> all the record bits have tabs instead of spaces. I fixed that up locally,
> but...

Sorry, my bad. The expandtab option is disabled in Vim from the modeline
present in the file. This seems to be the only file with a modeline in
this repository, maybe it should be removed? Anyway, I fixed the
whitespace in the attached patch.

> 
> >          struct timeval timeout;
> >  
> >          FD_ZERO(&read_fds);
> > @@ -454,9 +470,12 @@ record_main_loop(Display * display, double idle_time)
> >                  disable_event = 1;
> >              }
> >  
> > -            if (cbres.non_modifier_event &&
> > -                !(ignore_modifier_combos && is_modifier_pressed(&cbres))) {
> > -                disable_event = 1;
> > +            if (cbres.non_modifier_event) {
> > +		if (ignore_modifier_combos && is_modifier_pressed(&cbres)) {
> > +		    modifier_event = 1;
> 
> this doesn't work the same way. in poll mode, any modifier will re-enable
> the touchpad immediately. but this path is only hit for the non-modifier
> keys while a modifer is down. 
> 
> - press a
> - touchpad is disabled
> - press shift
>     poll mode: touchpad re-enables
>     record mode: no change
> - press s
>     poll mode: no change
>     record mode: touchpad re-enables
> - release keys, touchpad remains enabled
> 
> I can't quite decide which one is the more sensible behaviour but I think
> more compatible with normal -k mode would be the your poll mode one so let's
> stick with that. Please make sure both behave the same.

Both poll and record mode should behave the same by now.

> 
> Cheers,
>    Peter
> 
> 
> 
> 
> > +		} else {
> > +		    disable_event = 1;
> > +		}
> >              }
> >          }
> >  
> > @@ -468,10 +487,13 @@ record_main_loop(Display * display, double idle_time)
> >              toggle_touchpad(False);
> >          }
> >  
> > +	if (modifier_event && pad_disabled) {
> > +	    toggle_touchpad(True);
> > +	}
> > +
> >          if (ret == 0 && pad_disabled) { /* timeout => enable event */
> >              toggle_touchpad(True);
> >          }
> > -
> >      }                           /* end while(1) */
> >  
> >      XFreeModifiermap(cbres.modifiers);
> > -- 
> > 2.7.0

 tools/syndaemon.c | 50 +++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/tools/syndaemon.c b/tools/syndaemon.c
index 29e75f5..f716827 100644
--- a/tools/syndaemon.c
+++ b/tools/syndaemon.c
@@ -47,6 +47,12 @@
 
 #include "synaptics-properties.h"
 
+enum KeyboardActivity {
+    ActivityNew,
+    ActivityNone,
+    ActivityReset
+};
+
 enum TouchpadState {
     TouchpadOn = 0,
     TouchpadOff = 1,
@@ -181,29 +187,29 @@ install_signal_handler(void)
     }
 }
 
-/**
- * Return non-zero if the keyboard state has changed since the last call.
- */
-static int
+static enum KeyboardActivity
 keyboard_activity(Display * display)
 {
     static unsigned char old_key_state[KEYMAP_SIZE];
     unsigned char key_state[KEYMAP_SIZE];
     int i;
-    int ret = 0;
+    int ret = ActivityNone;
 
     XQueryKeymap(display, (char *) key_state);
 
     for (i = 0; i < KEYMAP_SIZE; i++) {
         if ((key_state[i] & ~old_key_state[i]) & keyboard_mask[i]) {
-            ret = 1;
+            ret = ActivityNew;
             break;
         }
     }
     if (ignore_modifier_combos) {
         for (i = 0; i < KEYMAP_SIZE; i++) {
             if (key_state[i] & ~keyboard_mask[i]) {
-                ret = 0;
+                if (old_key_state[i] & ~keyboard_mask[i])
+                    ret = ActivityNone;
+                else
+                    ret = ActivityReset;
                 break;
             }
         }
@@ -232,8 +238,17 @@ main_loop(Display * display, double idle_time, int poll_delay)
 
     for (;;) {
         current_time = get_time();
-        if (keyboard_activity(display))
-            last_activity = current_time;
+        switch (keyboard_activity(display)) {
+            case ActivityNew:
+                last_activity = current_time;
+                break;
+            case ActivityNone:
+                /* NOP */;
+                break;
+            case ActivityReset:
+                last_activity = 0.0;
+                break;
+        }
 
         /* If system times goes backwards, touchpad can get locked. Make
          * sure our last activity wasn't in the future and reset if it was. */
@@ -423,6 +438,7 @@ record_main_loop(Display * display, double idle_time)
         fd_set read_fds;
         int ret;
         int disable_event = 0;
+        int modifier_event = 0;
         struct timeval timeout;
 
         FD_ZERO(&read_fds);
@@ -454,9 +470,14 @@ record_main_loop(Display * display, double idle_time)
                 disable_event = 1;
             }
 
-            if (cbres.non_modifier_event &&
-                !(ignore_modifier_combos && is_modifier_pressed(&cbres))) {
-                disable_event = 1;
+            if (cbres.non_modifier_event) {
+                if (ignore_modifier_combos && is_modifier_pressed(&cbres)) {
+                    modifier_event = 1;
+                } else {
+                    disable_event = 1;
+                }
+            } else if (ignore_modifier_keys) {
+                modifier_event = 1;
             }
         }
 
@@ -468,10 +489,13 @@ record_main_loop(Display * display, double idle_time)
             toggle_touchpad(False);
         }
 
-        if (ret == 0 && pad_disabled) { /* timeout => enable event */
+        if (modifier_event && pad_disabled) {
             toggle_touchpad(True);
         }
 
+        if (ret == 0 && pad_disabled) { /* timeout => enable event */
+            toggle_touchpad(True);
+        }
     }                           /* end while(1) */
 
     XFreeModifiermap(cbres.modifiers);
-- 
2.7.0


More information about the xorg-devel mailing list