[PATCH weston v3] compositor: add a way to change the keyboard locks

[Pekka Paalanen]

On Fri, 22 Aug 2014 13:22:27 +0300
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:

> This adds a function weston_keyboard_set_locks() which can be used to
> change the state of the num lock and the caps locks, changing the leds too.
> Only the evdev and libinput backends supports this, since it doesn't make
> sense for embedded sessions.
> ---
> 
> v3: forgot semicolon
> 
>  src/compositor.h | 10 ++++++++++
>  src/input.c      | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+)
> 
> diff --git a/src/compositor.h b/src/compositor.h
> index c0fc0a6..d80b05e 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -68,6 +68,12 @@ enum weston_keyboard_modifier {
>  	MODIFIER_SHIFT = (1 << 3),
>  };
>  
> +enum weston_keyboard_locks {
> +	NUM_LOCK = (1 << 0),
> +	CAPS_LOCK = (1 << 1),
> +	SCROLL_LOCK = (1 << 2),
> +};

Hmm, this is compositor.h which is exported/installed and used by third
party things, so I think these should have a prefix to avoid name
clashes.

> +
>  enum weston_led {
>  	LED_NUM_LOCK = (1 << 0),
>  	LED_CAPS_LOCK = (1 << 1),
> @@ -393,6 +399,10 @@ weston_keyboard_start_grab(struct weston_keyboard *device,
>  			   struct weston_keyboard_grab *grab);
>  void
>  weston_keyboard_end_grab(struct weston_keyboard *keyboard);
> +int
> +weston_keyboard_set_locks(struct weston_keyboard *keyboard,
> +			  enum weston_keyboard_locks maks,

Typo 'maks'.

> +			  enum weston_keyboard_locks value);

Is this to be used only with a single lock at a time?
If you meant one can 'or' together several locks, you shouldn't use enum
type in the function arguments but uint32_t + documentation.

>  
>  struct weston_touch *
>  weston_touch_create(void);
> diff --git a/src/input.c b/src/input.c
> index 975cd77..195deb3 100644
> --- a/src/input.c
> +++ b/src/input.c
> @@ -1062,6 +1062,61 @@ notify_axis(struct weston_seat *seat, uint32_t time, uint32_t axis,
>  				     value);
>  }
>  
> +WL_EXPORT int
> +weston_keyboard_set_locks(struct weston_keyboard *keyboard,
> +			  enum weston_keyboard_locks mask,
> +			  enum weston_keyboard_locks value)
> +{
> +#ifdef ENABLE_XKBCOMMON
> +	uint32_t mods_depressed, mods_latched, mods_locked, group;
> +	uint32_t serial;
> +	int num, caps;

Shouldn't these be uint32_t too?

> +
> +	/* We don't want the leds to go out of sync with the actual state
> +	 * so if the backend has no way to change the leds don't try to
> +	 * change the state */
> +	if (!keyboard->seat->led_update)
> +		return -1;
> +
> +	mods_depressed = xkb_state_serialize_mods(keyboard->xkb_state.state,
> +						XKB_STATE_DEPRESSED);
> +	mods_latched = xkb_state_serialize_mods(keyboard->xkb_state.state,
> +						XKB_STATE_LATCHED);
> +	mods_locked = xkb_state_serialize_mods(keyboard->xkb_state.state,
> +						XKB_STATE_LOCKED);
> +	group = xkb_state_serialize_group(keyboard->xkb_state.state,
> +                                      XKB_STATE_EFFECTIVE);
> +
> +	num = (1 << keyboard->xkb_info->mod2_mod);
> +	caps = (1 << keyboard->xkb_info->caps_mod);
> +	if (mask & NUM_LOCK) {
> +		if (value & NUM_LOCK)
> +			mods_locked |= num;
> +		else
> +			mods_locked = mods_locked & ~num;

Could also do mods_locked &= ~num; :-)

I would be careful making sure the types match.

> +	}
> +	if (mask & CAPS_LOCK) {
> +		if (value & CAPS_LOCK)
> +			mods_locked |= caps;
> +		else
> +			mods_locked = mods_locked & ~caps;
> +	}
> +	if (mask & SCROLL_LOCK)
> +		weston_log("Changing the SCROLL_LOCK value is not supported.");

Return failure here?

> +
> +	xkb_state_update_mask(keyboard->xkb_state.state, mods_depressed,
> +			      mods_latched, mods_locked, 0, 0, group);
> +
> +	serial = wl_display_next_serial(
> +				keyboard->seat->compositor->wl_display);
> +	notify_modifiers(keyboard->seat, serial);
> +
> +	return 0;
> +#else
> +	return -1;
> +#endif
> +}
> +
>  #ifdef ENABLE_XKBCOMMON
>  WL_EXPORT void
>  notify_modifiers(struct weston_seat *seat, uint32_t serial)

The #ifdeffing looks ok.


Thanks,
pq