On Fri, Jul 24, 2020 at 09:49:55PM +0200, Maarten van Gompel wrote:
> From: Miles Alan <m...@milesalan.com>
> 
> Fix SIGTERM handler - flip terminate flag in sigterm handler & cleanup 
> properly
> 
> Modify run function to use select() with a timeout since X events will be
> blocked otherwise and terminate wouldn't apply for a while.
> 
> Run XFlush instead of XSync before starting main loop; fixes bug where rending
> of keys fails when used in conjunction w/ dwm dock patch
> 
> Add pipe key to backslash key
> ---
>  layout.en.h   |  70 -------------------------------
>  layout.sxmo.h | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  svkbd.c       |  73 +++++++++++++++++++++++++++++----
>  3 files changed, 175 insertions(+), 79 deletions(-)
>  delete mode 100644 layout.en.h
>  create mode 100644 layout.sxmo.h
> 
> diff --git a/layout.en.h b/layout.en.h
> deleted file mode 100644
> index b7291b5..0000000
> --- a/layout.en.h
> +++ /dev/null
> @@ -1,70 +0,0 @@
> -static Key keys[] = {
> -     { "1!", XK_1, 1 },
> -     { "2@", XK_2, 1 },
> -     { "3#", XK_3, 1 },
> -     { "4$", XK_4, 1 },
> -     { "5%", XK_5, 1 },
> -     { "6^", XK_6, 1 },
> -     { "7&", XK_7, 1 },
> -     { "8*", XK_8, 1 },
> -     { "9(", XK_9, 1 },
> -     { "0)", XK_0, 1 },
> -     { "-_", XK_minus, 1 },
> -     { "=+", XK_plus, 1 },
> -     { "<-", XK_BackSpace, 2 },
> -     { 0 }, /* New row */
> -     { "->|", XK_Tab, 1 },
> -     { 0, XK_q, 1 },
> -     { 0, XK_w, 1 },
> -     { 0, XK_e, 1 },
> -     { 0, XK_r, 1 },
> -     { 0, XK_t, 1 },
> -     { 0, XK_y, 1 },
> -     { 0, XK_u, 1 },
> -     { 0, XK_i, 1 },
> -     { 0, XK_o, 1 },
> -     { 0, XK_p, 1 },
> -     { "[", XK_bracketleft, 1 },
> -     { "]", XK_bracketright, 1 },
> -     { "Return", XK_Return, 3 },
> -     { 0 }, /* New row */
> -     { 0, XK_Caps_Lock, 2 },
> -     { 0, XK_a, 1 },
> -     { 0, XK_s, 1 },
> -     { 0, XK_d, 1 },
> -     { 0, XK_f, 1 },
> -     { 0, XK_g, 1 },
> -     { 0, XK_h, 1 },
> -     { 0, XK_j, 1 },
> -     { 0, XK_k, 1 },
> -     { 0, XK_l, 1 },
> -     { ":;", XK_semicolon, 1 },
> -     { "'\"", XK_exclam, 1 },
> -     { "\\|", XK_backslash, 1 },
> -     { 0 }, /* New row */
> -     { 0, XK_Shift_L, 3 },
> -     { 0, XK_z, 1 },
> -     { 0, XK_x, 1 },
> -     { 0, XK_c, 1 },
> -     { 0, XK_v, 1 },
> -     { 0, XK_b, 1 },
> -     { 0, XK_n, 1 },
> -     { 0, XK_m, 1 },
> -     { ",", XK_colon, 1 },
> -     { ".", XK_period, 1 },
> -     { "/?", XK_slash, 1 },
> -     { 0, XK_Shift_R, 2 },
> -     { 0 }, /* New row */
> -     { "Ctrl", XK_Control_L, 2 },
> -     { "Alt", XK_Alt_L, 2 },
> -     { "", XK_space, 5 },
> -     { "Alt", XK_Alt_R, 2 },
> -     { "Ctrl", XK_Control_R, 2 },
> -     { "[X]", XK_Cancel, 1},
> -};
> -
> -Buttonmod buttonmods[] = {
> -     { XK_Shift_L, Button2 },
> -     { XK_Alt_L, Button3 },
> -};
> -
> diff --git a/layout.sxmo.h b/layout.sxmo.h
> new file mode 100644
> index 0000000..52ce781
> --- /dev/null
> +++ b/layout.sxmo.h
> @@ -0,0 +1,111 @@
> +static Key keys[40] = { NULL };
> +
> +static Key keys_en[40] = {
> +        { 0, XK_q, 1 },
> +        { 0, XK_w, 1 },
> +        { 0, XK_e, 1 },
> +        { 0, XK_r, 1 },
> +        { 0, XK_t, 1 },
> +        { 0, XK_y, 1 },
> +        { 0, XK_u, 1 },
> +        { 0, XK_i, 1 },
> +        { 0, XK_o, 1 },
> +        { 0, XK_p, 1 },
> +
> +        { 0 }, /* New row */
> +
> +        { 0, XK_a, 1 },
> +        { 0, XK_s, 1 },
> +        { 0, XK_d, 1 },
> +        { 0, XK_f, 1 },
> +        { 0, XK_g, 1 },
> +        { 0, XK_h, 1 },
> +        { 0, XK_j, 1 },
> +        { 0, XK_k, 1 },
> +        { 0, XK_l, 1 },
> +        { ";:", XK_colon, 1 },
> +        /*{ "'", XK_apostrophe, 2 },*/
> +
> +        { 0 }, /* New row */
> +
> +        { 0, XK_z, 1 },
> +        { 0, XK_x, 1 },
> +        { 0, XK_c, 1 },
> +        { 0, XK_v, 1 },
> +        { 0, XK_b, 1 },
> +        { 0, XK_n, 1 },
> +        { 0, XK_m, 1 },
> +        /*{ "/?", XK_slash, 1 },*/
> +        { "Tab", XK_Tab, 1 },
> +        { "<-", XK_BackSpace, 2 },
> +
> +        { 0 }, /* New row */
> +        { "Layer 2", XK_Cancel, 1},
> +        { "Shift", XK_Shift_L, 1 },
> +        /*{ "L", XK_Left, 1 },*/
> +        { "D", XK_Down, 1 },
> +        { "U", XK_Up, 1 },
> +        /*{ "R", XK_Right, 1 },*/
> +        { "", XK_space, 2 },
> +        { "Esc", XK_Escape, 1 },
> +        { "Ctrl", XK_Control_L, 1 },
> +        /*{ "Alt", XK_Alt_L, 1 },*/
> +        { "Enter", XK_Return, 2 },
> +};
> +
> +static Key keys_symbols[40] = {
> +  { "1!", XK_1, 1 },
> +  { "2@", XK_2, 1 },
> +  { "3#", XK_3, 1 },
> +  { "4$", XK_4, 1 },
> +  { "5%", XK_5, 1 },
> +  { "6^", XK_6, 1 },
> +  { "7&", XK_7, 1 },
> +  { "8*", XK_8, 1 },
> +  { "9(", XK_9, 1 },
> +  { "0)", XK_0, 1 },
> +
> +  { 0 }, /* New row */
> +
> +  { "'\"", XK_apostrophe, 1 },
> +  { "`~", XK_grave, 1 },
> +  { "-_", XK_minus, 1 },
> +  { "=+", XK_plus, 1 },
> +  { "[{", XK_bracketleft, 1 },
> +  { "]}", XK_bracketright, 1 },
> +  { ",<", XK_comma, 1 },
> +  { ".>", XK_period, 1 },
> +  { "/?", XK_slash, 1 },
> +  { "\\|", XK_backslash, 1 },
> +
> +  { 0 }, /* New row */
> +
> +  { " ", XK_Shift_L|XK_bar, 1 },
> +  { " ", XK_Shift_L|XK_bar, 1 },
> +  { "L", XK_Left, 1 },
> +  { "R", XK_Right, 1 },
> +  { " ", XK_Shift_L|XK_bar, 1 },
> +  { " ", XK_Shift_L|XK_bar, 1 },
> +  { " ", XK_Shift_L|XK_bar, 1 },
> +  { "Tab", XK_Tab, 1 },
> +  { "<-", XK_BackSpace, 2 },
> +
> +  { 0 }, /* New row */
> +  { "Layer 1", XK_Cancel, 1},
> +  { "Shift", XK_Shift_L, 1 },
> +  /*{ "L", XK_Left, 1 },*/
> +  { "D", XK_Down, 1 },
> +  { "U", XK_Up, 1 },
> +  /*{ "R", XK_Right, 1 },*/
> +  { "", XK_space, 2 },
> +  { "Esc", XK_Escape, 1 },
> +  { "Ctrl", XK_Control_L, 1 },
> +  /*{ "Alt", XK_Alt_L, 1 },*/
> +  { "Enter", XK_Return, 2 },
> +};
> +
> +Buttonmod buttonmods[] = {
> +        { XK_Shift_L, Button2 },
> +        { XK_Alt_L, Button3 },
> +};
> +

Is this layout specific for sxmo or the Pinephone in general?


> diff --git a/svkbd.c b/svkbd.c
> index 337f769..7d93d78 100644
> --- a/svkbd.c
> +++ b/svkbd.c
> @@ -13,6 +13,9 @@
>  #include <X11/Xutil.h>
>  #include <X11/Xproto.h>
>  #include <X11/extensions/XTest.h>
> +#include <signal.h>
> +#include <sys/select.h>
> +
> 
>  /* macros */
>  #define MAX(a, b)       ((a) > (b) ? (a) : (b))
> @@ -98,6 +101,8 @@ static int rows = 0, ww = 0, wh = 0, wx = 0, wy = 0;
>  static char *name = "svkbd";
> 
>  Bool ispressing = False;
> +Bool baselayer = True;
> +Bool sigtermd = False;
> 
>  /* configuration, allows nested code to access above variables */
>  #include "config.h"
> @@ -185,6 +190,21 @@ buttonrelease(XEvent *e) {
> 
>  void
>  cleanup(void) {
> +     int i;
> +
> +     // E.g. Generally in scripts we call SIGTERM on svkbd in which case
> +     //      if the user is holding for example the enter key (to execute
> +     //      the kill or script that does the kill), that causes an issue
> +     //      since then X doesn't know the keyup is never coming.. (since
> +     //      process will be dead before finger lifts - in that case we
> +     //      just trigger out fake up presses for all keys
> +     if (sigtermd) {
> +             for (i = 0; i < LENGTH(keys); i++) {
> +                     XTestFakeKeyEvent(dpy, XKeysymToKeycode(dpy, 
> keys[i].keysym), False, 0);
> +             }
> +     }
> +     XSync(dpy, False);
> +
>       if(dc.font.set)
>               XFreeFontSet(dpy, dc.font.set);
>       else
> @@ -238,7 +258,6 @@ drawkeyboard(void) {
>               if(keys[i].keysym != 0)
>                       drawkey(&keys[i]);
>       }
> -     XSync(dpy, False);
>  }
> 
>  void
> @@ -393,7 +412,10 @@ unpress(Key *k, KeySym mod) {
>       if(k != NULL) {
>               switch(k->keysym) {
>               case XK_Cancel:
> -                     exit(0);
> +                     togglelayer();
> +                     break;
> +             case XK_Break:
> +               running = False;
>               default:
>                       break;
>               }
> @@ -432,13 +454,29 @@ unpress(Key *k, KeySym mod) {
>  void
>  run(void) {
>       XEvent ev;
> -
> -     /* main event loop */
> -     XSync(dpy, False);
> -     while(running) {
> -             XNextEvent(dpy, &ev);
> -             if(handler[ev.type])
> -                     (handler[ev.type])(&ev); /* call handler */
> +     int xfd;
> +     fd_set fds;
> +     struct timeval tv;
> +
> +
> +     xfd = ConnectionNumber(dpy);
> +     tv.tv_usec = 0;
> +     tv.tv_sec = 2;
> +
> +     //XSync(dpy, False);
> +     XFlush(dpy);
> +
> +     while (running) {
> +             FD_ZERO(&fds);
> +             FD_SET(xfd, &fds);
> +             if (select(xfd + 1, &fds, NULL, NULL, &tv)) {
> +                     while (XPending(dpy)) {
> +                             XNextEvent(dpy, &ev);
> +                             if(handler[ev.type]) {
> +                                     (handler[ev.type])(&ev); /* call 
> handler */
> +                             }
> +                     }
> +             }
>       }
>  }
> 

The select code looks wrong to me. Instead of the workaround as described near
the comment and XTestFakeKeyEvent the error condition of select() should be
checked also. On many systems select() should also receive EINTR and return -1
when interrupted by a signal, maybe that solves it?

> @@ -580,11 +618,28 @@ usage(char *argv0) {
>       exit(1);
>  }
> 
> +void
> +togglelayer() {
> +     memcpy(&keys, baselayer ? &keys_symbols : &keys_en, sizeof(keys_en));
> +     updatekeys();
> +     drawkeyboard();
> +     baselayer = !baselayer;
> +}
> +
> +void
> +sigterm(int sig)
> +{
> +     running = False;
> +     sigtermd = True;
> +}
> +
>  int
>  main(int argc, char *argv[]) {
>       int i, xr, yr, bitm;
>       unsigned int wr, hr;
> 
> +     memcpy(&keys, &keys_en, sizeof(keys_en));
> +     signal(SIGTERM, sigterm);
>       for (i = 1; argv[i]; i++) {
>               if(!strcmp(argv[i], "-v")) {
>                       die("svkbd-"VERSION", © 2006-2016 svkbd engineers,"
> --
> 2.27.0
> 
> 

Furthermore can you clean up the code-style a bit and also separate them into
incremental commits? This would make reviewing them easier.

-- 
Kind regards,
Hiltjo

Reply via email to