Re: [systemd-devel] [RFC 06/12] gfx: add keyboard layer

2013-12-10 Thread Lennart Poettering
On Thu, 28.11.13 09:36, David Herrmann (dh.herrm...@gmail.com) wrote:

 
 Hi
 
 On Wed, Nov 27, 2013 at 11:17 PM, Lennart Poettering
 lenn...@poettering.net wrote:
  On Wed, 27.11.13 19:48, David Herrmann (dh.herrm...@gmail.com) wrote:
 
  +
  +enum {
  +SD_GFX__LED_NUML,
  +SD_GFX__LED_CAPSL,
  +SD_GFX__LED_SCROLLL,
  +SD_GFX__LED_COUNT,
  +};
 
  Double underscores?
 
 Yes.

Why?

  If these are bools, then make them bools! C99 bools (i.e. the type
  bool from stdbool.h) are awesome for bit fields! [ Please use C99 bools
  everywhere in internal code, and int as bool type for public APIs,
  since that's what pre-C99 code usually did, despite the stupidity this
  results in when people use bit fields ]
 
 You can use _Bool/bool for bitfields? 

Yes, that's what makes them so useful! Otherwise C99 bools are just
meh. But in bitfields, yuppideefuckindoo!

 Didn't know that. I always use bool for boolean values, I only
 thought they don't work for bitfields. Will fix that up.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [RFC 06/12] gfx: add keyboard layer

2013-11-28 Thread David Herrmann
Hi

On Wed, Nov 27, 2013 at 11:17 PM, Lennart Poettering
lenn...@poettering.net wrote:
 On Wed, 27.11.13 19:48, David Herrmann (dh.herrm...@gmail.com) wrote:

 +
 +enum {
 +SD_GFX__LED_NUML,
 +SD_GFX__LED_CAPSL,
 +SD_GFX__LED_SCROLLL,
 +SD_GFX__LED_COUNT,
 +};

 Double underscores?

Yes.

 +
 +struct sd_gfx_kbd {
 +char *name;
 +struct libevdev *evdev;
 +int fd;
 +struct xkb_state *state;
 +sd_event *ev;
 +sd_event_source *fd_source;
 +sd_event_source *timer;
 +sd_gfx_kbd_event_fn event_fn;
 +void *data;
 +void *fn_data;
 +
 +unsigned int delay;
 +unsigned int rate;
 +
 +unsigned int sym_count;
 +sd_gfx_kbd_event event;
 +sd_gfx_kbd_event repeat;
 +
 +xkb_mod_index_t xkb_mods[SD_GFX__MOD_COUNT];
 +xkb_led_index_t xkb_leds[SD_GFX__LED_COUNT];
 +
 +unsigned int awake : 1;
 +unsigned int need_sync : 1;
 +unsigned int repeating : 1;

 If these are bools, then make them bools! C99 bools (i.e. the type
 bool from stdbool.h) are awesome for bit fields! [ Please use C99 bools
 everywhere in internal code, and int as bool type for public APIs,
 since that's what pre-C99 code usually did, despite the stupidity this
 results in when people use bit fields ]

You can use _Bool/bool for bitfields? Didn't know that. I always use
bool for boolean values, I only thought they don't work for
bitfields. Will fix that up.

Thanks
David

 +if (r  0) {
 +if (r == -EACCES)
 +log_debug(kbd %s: EACCES on led-update, 
 kbd-name);
 +else
 +log_error(kbd %s: cannot update leds: %d,
 +  kbd-name, r);

 Please do not log from libraries. (Well, except when that's the primary
 purpose of your function, or when you do so on LOG_DEBUG level, since
 that is not visible normally anyway.)

 Lennart

 --
 Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [RFC 06/12] gfx: add keyboard layer

2013-11-27 Thread David Herrmann
To replace the kernel kbd layer, we need keyboard handling in user-space.
The current de-facto standard is XKB and luckily, in the last 3 years
there was an effort to make XKB indepedent of X11, namely libxkbcommon.

libxkbcommon provides keyboard handling compatible (and even superior) to
classic XKB. The default wayland kbd-protocol was designed around the
xkbcommon API and libxkbcommon is used by all known wayland-compositors.
Its only dependency is glibc. However, you also need the keymap-files,
which are part of xkeyboard-config, which itself has no dependencies and
only provides the uncompiled keymap source files.

The kbd sd-gfx layer takes an evdev fd as input and provides keyboard
presses as events. Everything in between is handled transparently to the
application.

To access the kernel evdev layer, we use libevdev. It is a small wrapper
around the evdev ioctls with no dependencies but libc. The main reason it
was created is to handle SYN_DROPPED (input even-queue overflow)
transparently. It is a mandatory dependency of xf86-input-evdev as of 2013
and will probably be used in most other wayland compositors, too.

The sd_gfx_kbd API should be straightforward. The only thing to mention is
async-sleep support. That is, whenever your session is moved into
background, the evdev fd is revoked. sd_gfx_kbd detects that and puts the
device asleep. Explicit sleep is also supported via sd_gfx_kbd_sleep().
You need to provide a new fd to wake the device up.
---
 Makefile.am  |   8 +-
 configure.ac |   7 +-
 src/libsystemd-gfx/gfx-kbd.c | 629 +++
 src/systemd/sd-gfx.h |  84 ++
 4 files changed, 725 insertions(+), 3 deletions(-)
 create mode 100644 src/libsystemd-gfx/gfx-kbd.c

diff --git a/Makefile.am b/Makefile.am
index 2edb091..6ecbd50 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3859,12 +3859,18 @@ noinst_LTLIBRARIES += \
 
 libsystemd_gfx_la_SOURCES = \
src/libsystemd-gfx/sd-gfx.h \
+   src/libsystemd-gfx/gfx-kbd.c \
src/libsystemd-gfx/gfx-unifont.c
 
 libsystemd_gfx_la_CFLAGS = \
-   $(AM_CFLAGS)
+   $(AM_CFLAGS) \
+   $(GFX_CFLAGS)
 
 libsystemd_gfx_la_LIBADD = \
+   $(GFX_LIBS) \
+   libsystemd-bus-internal.la \
+   libsystemd-daemon-internal.la \
+   libsystemd-id128-internal.la \
libsystemd-shared.la \
src/libsystemd-gfx/unifont.bin.lo
 
diff --git a/configure.ac b/configure.ac
index 354673a..74c37ae 100644
--- a/configure.ac
+++ b/configure.ac
@@ -294,8 +294,11 @@ AM_CONDITIONAL(HAVE_KMOD, [test $have_kmod = yes])
 have_gfx=no
 AC_ARG_ENABLE(gfx, AS_HELP_STRING([--disable-gfx], [disable sd-gfx graphics 
and input support]))
 if test x$enable_gfx != xno; then
-AC_DEFINE(HAVE_GFX, 1, [Define if sd-gfx is built])
-have_gfx=yes
+PKG_CHECK_MODULES(GFX, [ libevdev = 0.4 xkbcommon = 0.3 ],
+[AC_DEFINE(HAVE_GFX, 1, [Define if sd-gfx is built]) 
have_gfx=yes], have_gfx=no)
+if test x$have_gfx = xno -a x$enable_gfx = xyes; then
+AC_MSG_ERROR([*** sd-gfx support requested, but libraries not 
found])
+fi
 fi
 AM_CONDITIONAL(HAVE_GFX, [test $have_gfx = yes])
 
diff --git a/src/libsystemd-gfx/gfx-kbd.c b/src/libsystemd-gfx/gfx-kbd.c
new file mode 100644
index 000..4de21dd
--- /dev/null
+++ b/src/libsystemd-gfx/gfx-kbd.c
@@ -0,0 +1,629 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2013 David Herrmann dh.herrm...@gmail.com
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+#include errno.h
+#include fcntl.h
+#include inttypes.h
+#include libevdev/libevdev.h
+#include stdbool.h
+#include stdlib.h
+#include string.h
+#include strings.h
+#include unistd.h
+#include xkbcommon/xkbcommon.h
+
+#include def.h
+#include log.h
+#include macro.h
+#include missing.h
+#include sd-event.h
+#include sd-gfx.h
+#include time-util.h
+#include util.h
+
+enum {
+SD_GFX__LED_NUML,
+SD_GFX__LED_CAPSL,
+SD_GFX__LED_SCROLLL,
+SD_GFX__LED_COUNT,
+};
+
+struct sd_gfx_kbd {
+char *name;
+struct libevdev *evdev;
+int fd;
+struct xkb_state *state;
+sd_event *ev;
+sd_event_source *fd_source;
+sd_event_source *timer;
+sd_gfx_kbd_event_fn event_fn;
+

Re: [systemd-devel] [RFC 06/12] gfx: add keyboard layer

2013-11-27 Thread Lennart Poettering
On Wed, 27.11.13 19:48, David Herrmann (dh.herrm...@gmail.com) wrote:

 +
 +enum {
 +SD_GFX__LED_NUML,
 +SD_GFX__LED_CAPSL,
 +SD_GFX__LED_SCROLLL,
 +SD_GFX__LED_COUNT,
 +};

Double underscores?

 +
 +struct sd_gfx_kbd {
 +char *name;
 +struct libevdev *evdev;
 +int fd;
 +struct xkb_state *state;
 +sd_event *ev;
 +sd_event_source *fd_source;
 +sd_event_source *timer;
 +sd_gfx_kbd_event_fn event_fn;
 +void *data;
 +void *fn_data;
 +
 +unsigned int delay;
 +unsigned int rate;
 +
 +unsigned int sym_count;
 +sd_gfx_kbd_event event;
 +sd_gfx_kbd_event repeat;
 +
 +xkb_mod_index_t xkb_mods[SD_GFX__MOD_COUNT];
 +xkb_led_index_t xkb_leds[SD_GFX__LED_COUNT];
 +
 +unsigned int awake : 1;
 +unsigned int need_sync : 1;
 +unsigned int repeating : 1;

If these are bools, then make them bools! C99 bools (i.e. the type
bool from stdbool.h) are awesome for bit fields! [ Please use C99 bools
everywhere in internal code, and int as bool type for public APIs,
since that's what pre-C99 code usually did, despite the stupidity this
results in when people use bit fields ]

 +if (r  0) {
 +if (r == -EACCES)
 +log_debug(kbd %s: EACCES on led-update, kbd-name);
 +else
 +log_error(kbd %s: cannot update leds: %d,
 +  kbd-name, r);

Please do not log from libraries. (Well, except when that's the primary
purpose of your function, or when you do so on LOG_DEBUG level, since
that is not visible normally anyway.)

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel