[RFC PATCH libevdev] Revamp the API once again

[Peter Hutterer]

On Mon, Sep 09, 2013 at 05:25:41PM +0200, David Herrmann wrote:
> Hi Peter
> 
> On Fri, Aug 30, 2013 at 3:13 AM, Peter Hutterer
> <peter.hutterer at who-t.net> wrote:
> > Another look at the current API showed some inconsistencies, rectified
> > in this commit:
> >
> > libevdev_kernel_*: modify the underlying kernel device
> > libevdev_event_type_*: something with an event type
> > libevdev_event_code_*: something with an event code
> > libevdev_event_*: struct input_event-related functions (i.e. not device-related)
> > libevdev_property_*: something with a property
> > libevdev_*: anything applying to a device
> >
> > Hopefully that's the last API change. Current symbols deprecated and aliased.
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> > Bikeshedding appreciated here. I think I'm happy with this API, or at least
> > one that prefixes everything similarly. A couple of more comments:
> 
> The API looks good to me. It's kernel-style, I like that. But there is
> one thing missing: A reverse-lookup of libevdev_get_event_type_name()
> and friends.
> We often have the situation where we allow text-based configurations
> and then we want code/type/..-name to code lookups. xkbcommon provides
> this for keysyms, I'd like to put the same for kernel-style names into
> libevdev. Otherwise, we have to duplicate it everywhere we parse
> configurations.

ack to the principle, having the inverse lookup could be useful.

> xkbcommon uses a simple binary-search on strings with a python-script
> to create the tables. I wrote that, so we could easily copy it over to
> libevdev without thinking of copyright issues. But before doing the
> work, I'd like to know whether that would actually be accepted?

we already have a python script to generate one mapping, adding the matching
script should be simple enough. so, yes, I'd accept that.

> Proposed API is:
> 
>   int libevdev_parse_{type,syn,key,rel,abs,sw,msc,led,rep,snd,bus,mt_tool,ff}(const
> char *name);
> This takes a string, looks it up and returns the given ID. For each
> type, names with and without prefix are accepted. That means, when
> parsing keys, we allow "KEY_ESC" and "ESC" and both result in KEY_ESC.
> The given types are:
>   type: EV_* constants
>   syn: SYN_* constants
>   key: KEY_* and BTN_* constants. I think libevdev_parse_btn() should
> be an alias for libevdev_parse_key().
>   rel: REL_* constants
>   abs: ABS_* constants
>   sw: SW_* constants
>   msc: MSC_* constants
>   led: LED_* constants
>   rep: REP_* constants
>   snd: SND_* constants
>   bus: BUS_* constants
>   mt_tool: MT_TOOL_* constants
>   ff: FF_* constants
> I would omit any *_MAX and *_CNT constants as these are not constant..

_MAX_ and _CNT are constant from within libevdev as they're compiled in.
so we could add them, with the usual warnings that all other related
functions have.

> This includes basically all constants from linux/input.h

a few comments here:
given the API revamp proposed with this patch, I think the naming should
match the new API, so something like this:
    libevdev_event_type_from_name(const char *name)
    libevdev_event_code_from_name(const char *name)

this should be fairly obvious as-is. where it gets tricky is allowing
non-prefixed types. There is a fair bit of overlap. REL_ and ABS_ overlap
for X/Y/Z/RX/RY/RZ/WHEEL/MISC (i.e. all but 2 of the rel axes overlap).
so an API to work around that would be:

    libevdev_event_code_from_typed_name(unsigned int type, const char *name)

except that breaks for KEY_, because KEY_0 and BTN_0 (and a lot of others)
would overlap within the same event type (which would also prevent a direct
alias for key/btn). tbh, I'm not a big fan of having separate calls in the
form of parse_btn, parse_key.

the question is here: what is the use-case where you need to convert an
event name into an event code but you don't have the REL_ prefix?
addressing this would be best to figure out how complicated the parser needs
to be.

for the others:
    libevdev_bus_from_name()
    libevdev_mt_tool_from_name()
    libevdev_ff_status_from_name()
seems the closest match for consistency with the rest.

>   int libevdev_parse_event(struct input_event *out, const char *description);
> This is a convenience helper which first parses the prefix of
> @description for EV,SYN,KEY,... and then the suffix via the helpers
> above. It fills in @type and @code of @out.

here too: what is the use-case for this?
it's a simple helper if we get the above right of course, but I'm curious
where this is needed.

Cheers,
   Peter