[PATCH libinput] Add libinput_device_keyboard_has_key()

[Peter Hutterer]

On Thu, Apr 23, 2015 at 05:15:01PM -0500, Derek Foreman wrote:
> On 22/04/15 07:05 PM, Peter Hutterer wrote:
> > On Wed, Apr 22, 2015 at 08:45:21AM -0500, Derek Foreman wrote:
> >> On 21/04/15 11:44 PM, Peter Hutterer wrote:
> >>> Similar to libinput_device_pointer_has_button(), this function returns whether
> >>> a given device has a specific keycode.
> >>>
> >>> This enables a caller to determine if the device is really a keyboard (check
> >>> for KEY_A-KEY_Z) or just a media key device (check for KEY_PLAY or somesuch),
> >>> depending on the context required.
> >>
> >> Cool, thanks.  :)
> >>
> >> Trivial comments below.
> >>
> >>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >>> ---
> >>> Had a deja-vu implementing this, I thought this already existed.
> >>>
> >>>  src/evdev.c      |  9 +++++++++
> >>>  src/evdev.h      |  3 +++
> >>>  src/libinput.c   |  6 ++++++
> >>>  src/libinput.h   | 16 ++++++++++++++++
> >>>  src/libinput.sym |  5 +++++
> >>>  test/keyboard.c  | 20 ++++++++++++++++++++
> >>>  6 files changed, 59 insertions(+)
> >>>
> > 
> > [...]
> > 
> >>> diff --git a/test/keyboard.c b/test/keyboard.c
> >>> index cb7ad52..a477cfb 100644
> >>> --- a/test/keyboard.c
> >>> +++ b/test/keyboard.c
> >>> @@ -290,12 +290,32 @@ START_TEST(keyboard_key_auto_release)
> >>>  }
> >>>  END_TEST
> >>>  
> >>> +START_TEST(keyboard_has_key)
> >>> +{
> >>> +	struct litest_device *dev = litest_current_device();
> >>> +	struct libinput_device *device = dev->libinput_device;
> >>> +	unsigned int code;
> >>> +	int evdev_has, libinput_has;
> >>> +
> >>> +	ck_assert(libinput_device_has_capability(
> >>> +					 device,
> >>> +					 LIBINPUT_DEVICE_CAP_KEYBOARD));
> >>> +
> >>> +	for (code = 0; code < KEY_CNT; code++) {
> >>> +		evdev_has = libevdev_has_event_code(dev->evdev, EV_KEY, code);
> >>> +		libinput_has = libinput_device_keyboard_has_key(device, code);
> >>> +		ck_assert_int_eq(evdev_has, libinput_has);
> >>> +	}
> >>> +}
> >>> +END_TEST
> >>> +
> >>>  int
> >>>  main(int argc, char **argv)
> >>>  {
> >>>  	litest_add_no_device("keyboard:seat key count", keyboard_seat_key_count);
> >>>  	litest_add_no_device("keyboard:key counting", keyboard_ignore_no_pressed_release);
> >>>  	litest_add_no_device("keyboard:key counting", keyboard_key_auto_release);
> >>> +	litest_add("keyboard:keys", keyboard_has_key, LITEST_KEYS, LITEST_ANY);
> >>
> >> We sort of wrote this in parallel, the only difference in our
> >> implementations is that I also had a negative test with LITEST_ANY,
> >> LITEST_KEYS that would always fail.
> >>
> >> Not sure it's that important to test the -1 case anyway...
> > 
> > it is, please send me a patch for that test. It doesn't matter that much
> > right now, but for the tablet interface we'll have a similar one like
> > libinput_device_tablet_has_button(). And since tablets/buttonsets can have
> > keys that are sent as keys and others that are sent as buttons there is some
> > chance of overlap. So having the test won't hurt now, but will be helpful in
> > the future.
> 
> Sent, but I foolishly forgot to --in-reply-to this thread.
> 
> I can't actually use LITEST_ANY, LITEST_KEYS for the devices to test
> though, because the roccat mouse and the wheel only device don't have
> LITEST_KEYS in their feature lists.
> 
> Is it a bug that the roccat mouse doesn't feature LITEST_KEYS?  If it
> is, it'll break the existing libinput_device_keyboard_has_key() test to
> switch it.

hmm, the test still passes here when I enable it (that's the existing one,
not your new one). What fails for you?
I think we should set it, at least until we have separate tests for the
various multimedia keys.
 
> Not sure what to think about wheel only.  It doesn't actually generate
> key events, so despite being labeled a keyboard by udev rule, it's not
> really a source of keys...  So I guess that makes sense.

that's a recent addition to input_id. thing is that if we only have a
REL_WHEEL or REL_HWEEL none of the other tags are appropriate, so
ID_INPUT_KEYBOARD is somewhat of a fallback here.
I think we can ignore that.

Cheers,
   Peter


> > thanks for the reviews
> > 
> > Cheers,
> >    Peter
> > 
> >>
> >> Reviewed-by: Derek Foreman <derekf at osg.samsung.com>
> >>>  
> >>>  	return litest_run(argc, argv);
> >>>  }
> >>>
> >>
>