[RFC libinput 1/2] Add a "buttonset" interface for button-only devices

[Hans de Goede]

Hi Peter,

On 13-02-15 08:42, Peter Hutterer wrote:
> On 13/02/2015 14:32 , Peter Hutterer wrote:
>> On Thu, Feb 12, 2015 at 12:37:12PM +0100, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 12-02-15 08:33, Peter Hutterer wrote:
>>>> A generic interface for devices that provide buttons and axes, but don't
>>>> control the pointer. This caters for the Pad part of Wacom graphics tablets
>>>> but could eventually also deal with remote controls, 3D mice and other
>>>> devices.
>>>>
>>>> This patch adds a new interface "buttonset" with a new capability and two
>>>> events AXIS and BUTTON. The interface is largely modelled after the
>>>> tablet interface, but with different axes and a few different behaviors.
>>>> The button events do the obvious, axis events provide 5 hooks:
>>>>
>>>> libinput_event_buttonset_get_axis_value() -> axis value in physical dimensions
>>>> libinput_event_buttonset_get_axis_value_transformed()
>>>> libinput_event_buttonset_get_axis_delta() -> axis delta in physical dimensions
>>>> libinput_event_buttonset_get_axis_delta_transformed()
>>>> libinput_event_buttonset_get_axis_delta_discrete() -> axis delta in clicks
>>>>
>>>> The latter is similar after the scroll click counts.
>>>>
>>>> Currently implemented are two ring and two strip axes. The axis use depends a
>>>> lot on the device and is hard to predict. e.g. strips have apparently been
>>>> used for physical positioning (like a sliding actuator) whereas the wheel
>>>> usually only cares about deltas. The above set should cater for the common
>>>> use-cases.
>>>>
>>>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>>>
>>> <snip>
>>>
>>>> diff --git a/src/libinput.h b/src/libinput.h
>>>> index a8146c0..d777730 100644
>>>> --- a/src/libinput.h
>>>> +++ b/src/libinput.h
>>>> @@ -55,7 +55,8 @@ enum libinput_device_capability {
>>>>   	LIBINPUT_DEVICE_CAP_KEYBOARD = 0,
>>>>   	LIBINPUT_DEVICE_CAP_POINTER = 1,
>>>>   	LIBINPUT_DEVICE_CAP_TOUCH = 2,
>>>> -	LIBINPUT_DEVICE_CAP_TABLET = 3
>>>> +	LIBINPUT_DEVICE_CAP_TABLET = 3,
>>>> +	LIBINPUT_DEVICE_CAP_BUTTONSET = 4
>>>>   };
>>>>
>>>>   /**
>>>> @@ -148,6 +149,34 @@ enum libinput_tablet_axis {
>>>>   /**
>>>>    * @ingroup device
>>>>    *
>>>> + * Available axis types for a buttonset device. It must have the @ref
>>>> + * LIBINPUT_DEVICE_CAP_BUTTONSET capability.
>>>> + */
>>>> +enum libinput_buttonset_axis {
>>>> +	LIBINPUT_BUTTONSET_AXIS_RING = 1,
>>>> +	LIBINPUT_BUTTONSET_AXIS_RING2 = 2,
>>>> +	LIBINPUT_BUTTONSET_AXIS_STRIP = 3,
>>>> +	LIBINPUT_BUTTONSET_AXIS_STRIP2 = 4,
>>>> +};
>>>
>>> Hmm, I thought that the idea was in the end to be able to use the
>>> button set interface with any random input device, with any random
>>> amount of inputs? Limiting things to just 2 of each type sounds wrong
>>> in that light, I think it would be better to just number axis 0 - #
>>> without looking at their type, and add a separate type field, which
>>> can be: RING, STRIP or UNKNOWN. Axis should probably also have flags
>>> to indicate if they generate abs events, deltas or both.
>>
>> hmm, good point, I honestly didn't think of that. I did a quick edit of the
>> header file to see what the API would look like, see the diff below.
>> The changes are axis from the enum to uint32_t and a few extra calls:
>>      libinput_event_buttonset_has_absolute_value(event, axisnumber)
>>      libinput_device_buttonset_get_num_axes(device)
>>      libinput_device_buttonset_get_axis_type(device, axisnumber)
>>
>> The latter two take a device. I'm not sure I like the API more though, on
>> the caller side it becomes a lot less expressive: before every call into the
>> API would carry enough information to know what you're getting, now it's
>> just an arbitrary number. It's more flexible this way though, that's for
>> sure.
>>
>> As for an UNKNOWN axis: I'd rather avoid this altogether. if we don't know
>> what an axis is we should ignore it since we can't normalize the events or
>> even guarantee that any event sequence is correct.
>>
>> Thinking aloud: and that "no unknown axes" approach is also why I still like
>> the original diff: it guarantees that any axis we export is known to
>> libinput. We don't use the axis type as mask (in the API) so we have a _lot_
>> of axes we can add eventually. I don't think having, eventually,
>> LIBINPUT_BUTTONSET_AXIS_STRIP14 is the worst of
>> all things. We could group them if we really wanted to as well, similar to
>> the events, i.e.
>>   LIBINPUT_BUTTONSET_AXIS_RING = 1
>>   LIBINPUT_BUTTONSET_AXIS_RING2,
>>   ... empty space
>>   LIBINPUT_BUTTONSET_AXIS_STRIP = 50,
>>   ...
>>
>> So that some procedural looping through a specific axis type is possible. If
>> we ever have a device with more than 50 axis of the same type, libinput is
>> probably not the best piece of code to handle it anyway :)
>
> After a bit more thinking about this, I started liking this idea more
> and more. The only change to the original patch would be spacing out the
> enum, with calls to the API becoming both well defined and free-form and
> forward-compatible.
>
> If we know there are devices that use a specific axis we can add them to
> the enum (RING2 for example), otherwise with the proper spacing all
> other axes are still available:
>
> To get the first ring value:
> libinput_event_buttonset_get_axis_value(
>           event,
>           LIBINPUT_BUTTONSET_AXIS_RING);
>
> To get the 14th ring value:
>
> libinput_event_buttonset_get_axis_value(
>           event,
>           LIBINPUT_BUTTONSET_AXIS_RING + 13);
>
> If we make the type spacing even, you can query all *potential* axes of
> one type with:
>    for (i = AXIS_RING; i < AXIS_RING + AXIS_TYPE_COUNT; i++)
>         ....
>
> I think that's a lot better than having just numbered axes, it adds the
> a lot of semantics that are otherwise difficult to get.

Ok, this (spacing the enums out) was actually my first idea too, but I wasn't
sure it would be generic enough, thinking more about it I think it should work
fine. Would be good to add a

int
libinput_device_get_buttonset_axis_count(struct libinput_device *device, libinput_buttonset_axis axis);

Or some such so that API users who want to know how many (if any) of a type
of axis there are in a device with buttonset capability can do so with needing to
do a loop and call has_axis all the time. Our internal API may still use the
loop for all I care, I just think we need something more API user friendly :)

And I do still believe we need an UNKNOWN axis, at XDC we were talking about these
button boxes people could buy to use with midi-apps, which were just a bunch
of dials, I don't want to do a database of those, so for such a device we
should just end up saying this is a buttonbox with UNKNOWN axis, which when
absolute must be used with get_transformed only, and the deltas are in an
unknown unit, but we can cross that bridge when we get there.

Regards,

Hans