[PATCH v3 xf86-input-synaptics] syndaemon: enable touchpad when pressing a modifier combo
Peter Hutterer
peter.hutterer at who-t.net
Mon Aug 8 01:46:23 UTC 2016
On Fri, Aug 05, 2016 at 10:21:59AM +0200, Anton Lindqvist wrote:
> 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).
hehe, adding the thread in here is creative :)
for next time - normal replies just as is, and then a separate email with
the patch with a short list of changes, like this:
https://patchwork.freedesktop.org/patch/100367/
this example has the v2 changes as part of the commit message but I always
put them under the --- divider, it's personal preference.
anyway, thanks for the patch, pushed
248c593..cd9f979 master -> master
Cheers,
Peter
> >
> > > 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