[PATCH libinput] Add libinput_device_keyboard_has_key()

[Derek Foreman]

On 23/04/15 08:22 PM, Peter Hutterer wrote:
> 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.

Sorry, nothing actually fails.

I didn't notice that the fancy "mouse" would have
LIBINPUT_DEVICE_CAP_KEYBOARD set, and I thought the has_key test would fail.

>> 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);
>>>>>  }
>>>>>
>>>>
>>