[PATCH kdrive/ephyr v7 4/9] kdrive: update evdev keyboard LEDs (#22302)

[Peter Hutterer]

On Fri, Dec 11, 2015 at 11:43:09AM -0200, Laércio de Sousa wrote:
> From: Mikhail Krivtsov <mikhail.krivtsov at gmail.com>
> 
> When one hits {Num,Caps,Scroll}Lock key on a Xephyr's keyboard,
> keyboard itself works as expected but LEDs are not updated
> and always stay in off.
> 
> Currently logical LEDs are propagated to physical keyboard LEDs
> for "CoreKeyboard" only. All "kdrive" keyboards are not
> "CoreKeyboard" and LEDs of "kdrive" keyboards are always "dead".
> 
> One possible solution is cloning "CoreKeyboard" LEDs to all
> "kdrive" keyboards.
> 
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=22302
> 
> Signed-off-by: Laércio de Sousa <laerciosousa at sme-mogidascruzes.sp.gov.br>
> ---
>  hw/kdrive/linux/evdev.c | 11 ++++++++---
>  hw/kdrive/src/kinput.c  | 23 +++++++++++++++++++++++
>  2 files changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/kdrive/linux/evdev.c b/hw/kdrive/linux/evdev.c
> index 63e8409..fecbae4 100644
> --- a/hw/kdrive/linux/evdev.c
> +++ b/hw/kdrive/linux/evdev.c
> @@ -440,10 +440,16 @@ EvdevKbdEnable(KdKeyboardInfo * ki)
>  static void
>  EvdevKbdLeds(KdKeyboardInfo * ki, int leds)
>  {
> -/*    struct input_event event;
> +    struct input_event event;
>      Kevdev             *ke;
> +    if (!ki) return; // This shouldn't happen but just for safety.

no // comments please, and our indenting style is 4 spaces.
>  
> -    ki->driverPrivate = ke;
> +    ke = ki->driverPrivate;
> +    if (!ke) {
> +	ErrorF("[%s:%u:%s] can't update LEDs of _disabled_ evdev keyboard\n",
> +		__FILE__, __LINE__, __FUNCTION__);
> +	return;
> +    }
>  
>      memset(&event, 0, sizeof(event));
>  
> @@ -466,7 +472,6 @@ EvdevKbdLeds(KdKeyboardInfo * ki, int leds)
>      event.code = LED_COMPOSE;
>      event.value = leds & (1 << 3) ? 1 : 0;
>      write(ke->fd, (char *) &event, sizeof(event));
> -*/
>  }
>  
>  static void
> diff --git a/hw/kdrive/src/kinput.c b/hw/kdrive/src/kinput.c
> index 682b63a..e9a5f24 100644
> --- a/hw/kdrive/src/kinput.c
> +++ b/hw/kdrive/src/kinput.c
> @@ -618,6 +618,28 @@ KdSetLed(KdKeyboardInfo * ki, int led, Bool on)
>      KdSetLeds(ki, ki->dixdev->kbdfeed->ctrl.leds);
>  }
>  
> +/*
> + * For unknown reason, logical keyboard LEDs are propagated to physical
> + * keyboard LEDs for "CoreKeyboard" only. All "kdrive" keyboards are
> + * not "CoreKeyboard" and LEDs of "kdrive" keyboards are always "dead".
> + * As workaround, the following function will clone "CoreKeyboard" LEDs
> + * to all "kdrive" keyboards.
> + */

the idea behind only propagating LED changes to core keyboard is that you
may have multiple keyboards, not all attached to the same master keyboard.
If you toggle caps lock on one of them, you don't want it to toggle on an
otherwise unrelated keyboard. Likewise, there's the niche case of floating
slave devices, where you don't want logical keyboard LEDs to change other
keyboards.

> +static void
> +KdCloneCoreLEDs(void)
> +{
> +    DeviceIntPtr pCoreKeyboard = inputInfo.keyboard;
> +    if (pCoreKeyboard && pCoreKeyboard->kbdfeed) {
> +       Leds leds = pCoreKeyboard->kbdfeed->ctrl.leds;
> +       KdKeyboardInfo *tmp;
> +       for (tmp = kdKeyboards; tmp; tmp = tmp->next) {
> +           if (tmp->leds != leds) {
> +                KdSetLeds(tmp, tmp->leds = leds);
                                            ^^ this almost certainly doesn't
                                            do what you want.


so something else isn't right here. By default, xephyr keyboards should be
attached to a master keyboard and see the LEDs update. Where exactly does
the LED update stop?

Cheers,
   Peter

> +           }
> +        }
> +    }
> +}
> +
>  void
>  KdSetPointerMatrix(KdPointerMatrix * matrix)
>  {
> @@ -2163,6 +2185,7 @@ ProcessInputEvents(void)
>      if (kdSwitchPending)
>          KdProcessSwitch();
>      KdCheckLock();
> +    KdCloneCoreLEDs();
>  }
>  
>  /* At the moment, absolute/relative is up to the client. */
> -- 
> 2.1.4