Re: [PATCH v3 xf86-input-synaptics] syndaemon: enable touchpad when pressing a modifier combo
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
> ---
> 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
> > > ---
> >
> > 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;
> > >
Re: [PATCH v3 xf86-input-synaptics] syndaemon: enable touchpad when pressing a modifier combo
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
---
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
> > ---
>
> 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
> pol
