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

[Pekka Paalanen]

On Thu, 28 Aug 2014 12:12:59 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> 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.

I noticed only later, that there is also other stuff unprefixed. I
wouldn't change the existing things now, but adding prefixes to them
too later would be nice IMHO.

So, at least not a blocker this one.

> > +
> >  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?

Ah, Daniel says all these should be xkb_mod_mask_t, actually.


Thanks,
pq