Hi PrasannaKumar On Mon, Feb 15, 2016 at 04:21:23PM +0530, PrasannaKumar Muralidharan wrote: > From: PrasannaKumar Muralidharan <prasannatsmku...@gmail.com> > > I am implementing dial input device support and would like to get feedback > on the code that I have implemented so far. Did not implement test cases > as of now, any pointers on how the test framework works will be helpful. > > Smartwatch can use dial input, Car can use steering wheel rotation as > input for maps may be. But I would like to know if dial input device > support is useful before proceeding further? > > Appreciate positive and negative comments.
First: yes, implementing support for dial interfaces is within the scope of libinput, I think it makes sense to add this. But not quite in this form, the various CAP flags are libinput interfaces that denote a specific functionality. We should not add one per event group. A while ago we started work on the so-called buttonset interface which is a generic interface to support a bunch of devices like yours: https://lists.freedesktop.org/archives/wayland-devel/2016-January/026670.html Since that patchset we've split out the wacom-specific parts so you can ignore any strip/ring handling. There is no dial axis atm, but there could be. Have a look at that patchset, the documentation should explain what the interface is for, etc. I think adding dial to buttonset and then getting that interface ready for merging is the best approach here. One comment on the API (to save you from scrolling down): libinput_event_dial_get_rotation_count() should simply be libinput_event_dial_get_rotation() and return the value in degrees. This is more flexible than the current one which reads like it's multiples of 360 degrees? Cheers, Peter > --- > src/Makefile.am | 2 ++ > src/evdev-dial.c | 80 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > src/evdev-dial.h | 6 ++++ > src/evdev.c | 12 ++++++++ > src/evdev.h | 6 ++++ > src/libinput-private.h | 5 ++++ > src/libinput.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++ > src/libinput.h | 46 +++++++++++++++++++++++++++++ > src/libinput.sym | 5 ++++ > tools/event-debug.c | 23 +++++++++++++++ > 10 files changed, 259 insertions(+) > create mode 100644 src/evdev-dial.c > create mode 100644 src/evdev-dial.h > > diff --git a/src/Makefile.am b/src/Makefile.am > index 343e75c..20a32a5 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -11,6 +11,8 @@ libinput_la_SOURCES = \ > libinput-private.h \ > evdev.c \ > evdev.h \ > + evdev-dial.c \ > + evdev-dial.h \ > evdev-middle-button.c \ > evdev-mt-touchpad.c \ > evdev-mt-touchpad.h \ > diff --git a/src/evdev-dial.c b/src/evdev-dial.c > new file mode 100644 > index 0000000..2427724 > --- /dev/null > +++ b/src/evdev-dial.c > @@ -0,0 +1,80 @@ > +#include "config.h" > + > +#include <assert.h> > +#include <math.h> > +#include <stdbool.h> > +#include <limits.h> > + > +#include "evdev-dial.h" > + > +static void > +dial_process(struct evdev_dispatch *dispatch, > + struct evdev_device *device, > + struct input_event *event, > + uint64_t time) > +{ > + struct libinput_device *base = &device->base; > + dial_notify(base, time, (int16_t)event->value); > +} > + > +/* > +static void > +dial_suspend(struct evdev_dispatch *dispatch, > + struct evdev_device *device) > +{ > + > +} > + > +static void > +dial_remove(struct evdev_dispatch *dispatch) > +{ > + > +} > +*/ > + > +static void > +dial_destroy(struct evdev_dispatch *dispatch) > +{ > + free(dispatch); > +} > + > +/* > +static void > +dial_device_added(struct evdev_device *device, > + struct evdev_device *added_device) > +{ > + > +} > + > +static void > +dial_device_removed(struct evdev_device *device, > + struct evdev_device *removed_device) > +{ > + > +} > +*/ > + > +/* TODO: Check whether other callbacks have to be implemented? */ > +static struct evdev_dispatch_interface dial_interface = { > + dial_process, > + NULL, /* device suspend */ > + NULL, /* device remove */ > + dial_destroy, > + NULL, /* device added */ > + NULL, /* device removed */ > + NULL, /* device suspended */ > + NULL, /* device resumed */ > + NULL, /* device post added */ > +}; > + > +struct evdev_dispatch * > +evdev_dial_create(struct evdev_device *device) > +{ > + struct evdev_dispatch *dispatch = zalloc(sizeof(struct evdev_dispatch)); > + if (!dispatch) > + return NULL; > + > + dispatch->interface = &dial_interface; > + > + return dispatch; > +} > diff --git a/src/evdev-dial.h b/src/evdev-dial.h > new file mode 100644 > index 0000000..0e191e7 > --- /dev/null > +++ b/src/evdev-dial.h > @@ -0,0 +1,6 @@ > +#ifndef EVDEV_DIAL_H > +#define EVDEV_DIAL_H > + > +#include "evdev.h" > + > +#endif /* EVDEV_DIAL_H */ > diff --git a/src/evdev.c b/src/evdev.c > index 66673a8..1e044fb 100644 > --- a/src/evdev.c > +++ b/src/evdev.c > @@ -63,6 +63,7 @@ enum evdev_device_udev_tags { > EVDEV_UDEV_TAG_ACCELEROMETER = (1 << 7), > EVDEV_UDEV_TAG_BUTTONSET = (1 << 8), > EVDEV_UDEV_TAG_POINTINGSTICK = (1 << 9), > + EVDEV_UDEV_TAG_DIAL = (1 << 10), > }; > > struct evdev_udev_tag_match { > @@ -82,6 +83,7 @@ static const struct evdev_udev_tag_match > evdev_udev_tag_matches[] = { > {"ID_INPUT_JOYSTICK", EVDEV_UDEV_TAG_JOYSTICK}, > {"ID_INPUT_ACCELEROMETER", EVDEV_UDEV_TAG_ACCELEROMETER}, > {"ID_INPUT_POINTINGSTICK", EVDEV_UDEV_TAG_POINTINGSTICK}, > + {"ID_INPUT_DIAL", EVDEV_UDEV_TAG_DIAL}, > > /* sentinel value */ > { 0 }, > @@ -779,6 +781,7 @@ evdev_need_touch_frame(struct evdev_device *device) > switch (device->pending_event) { > case EVDEV_NONE: > case EVDEV_RELATIVE_MOTION: > + case EVDEV_RELATIVE_DIAL: > break; > case EVDEV_ABSOLUTE_MT_DOWN: > case EVDEV_ABSOLUTE_MT_MOTION: > @@ -2157,6 +2160,15 @@ evdev_configure_device(struct evdev_device *device) > return -1; > } > > + if (udev_tags & EVDEV_UDEV_TAG_DIAL) { > + device->dispatch = evdev_dial_create(device); > + device->seat_caps |= EVDEV_DEVICE_DIAL; > + log_info(libinput, "input device '%s', '%s' is a rotatory > dial\n", > + device->devname, devnode); > + > + return device->dispatch == NULL ? -1 : 0; > + } > + > return 0; > } > > diff --git a/src/evdev.h b/src/evdev.h > index 8b567a8..b26ae10 100644 > --- a/src/evdev.h > +++ b/src/evdev.h > @@ -54,6 +54,7 @@ enum evdev_event_type { > EVDEV_ABSOLUTE_MT_MOTION, > EVDEV_ABSOLUTE_MT_UP, > EVDEV_RELATIVE_MOTION, > + EVDEV_RELATIVE_DIAL, > }; > > enum evdev_device_seat_capability { > @@ -62,6 +63,7 @@ enum evdev_device_seat_capability { > EVDEV_DEVICE_TOUCH = (1 << 2), > EVDEV_DEVICE_TABLET = (1 << 3), > EVDEV_DEVICE_GESTURE = (1 << 5), > + EVDEV_DEVICE_DIAL = (1 << 9), > }; > > enum evdev_device_tags { > @@ -69,6 +71,7 @@ enum evdev_device_tags { > EVDEV_TAG_INTERNAL_TOUCHPAD = (1 << 1), > EVDEV_TAG_TRACKPOINT = (1 << 2), > EVDEV_TAG_KEYBOARD = (1 << 3), > + EVDEV_TAG_DIAL = (1 << 4), > }; > > enum evdev_middlebutton_state { > @@ -309,6 +312,9 @@ struct evdev_dispatch * > evdev_touchpad_create(struct evdev_device *device); > > struct evdev_dispatch * > +evdev_dial_create(struct evdev_device *device); > + > +struct evdev_dispatch * > evdev_mt_touchpad_create(struct evdev_device *device); > > struct evdev_dispatch * > diff --git a/src/libinput-private.h b/src/libinput-private.h > index bc7000d..eabed47 100644 > --- a/src/libinput-private.h > +++ b/src/libinput-private.h > @@ -503,6 +503,11 @@ gesture_notify_pinch_end(struct libinput_device *device, > int cancelled); > > void > +dial_notify(struct libinput_device *device, > + uint64_t time, > + int16_t value); > + > +void > touch_notify_frame(struct libinput_device *device, > uint64_t time); > > diff --git a/src/libinput.c b/src/libinput.c > index 2bcd416..8fc6ed8 100644 > --- a/src/libinput.c > +++ b/src/libinput.c > @@ -125,6 +125,12 @@ struct libinput_event_gesture { > double angle; > }; > > +struct libinput_event_dial { > + struct libinput_event base; > + uint64_t time; > + int16_t count; > +}; > + > struct libinput_event_tablet_tool { > struct libinput_event base; > uint32_t button; > @@ -318,6 +324,15 @@ libinput_event_get_tablet_tool_event(struct > libinput_event *event) > return (struct libinput_event_tablet_tool *) event; > } > > +LIBINPUT_EXPORT struct libinput_event_dial * > +libinput_event_get_dial_event(struct libinput_event *event) > +{ > + require_event_type(libinput_event_get_context(event), event->type, NULL, > + LIBINPUT_EVENT_DIAL); > + > + return (struct libinput_event_dial *) event; > +} > + > LIBINPUT_EXPORT struct libinput_event_device_notify * > libinput_event_get_device_notify_event(struct libinput_event *event) > { > @@ -1473,6 +1488,39 @@ libinput_tablet_tool_unref(struct libinput_tablet_tool > *tool) > return NULL; > } > > +LIBINPUT_EXPORT uint32_t > +libinput_event_dial_get_time(struct libinput_event_dial *event) > +{ > + require_event_type(libinput_event_get_context(&event->base), > + event->base.type, > + 0, > + LIBINPUT_EVENT_DIAL); > + > + return us2ms(event->time); > +} > + > +LIBINPUT_EXPORT uint64_t > +libinput_event_dial_get_time_usec(struct libinput_event_dial *event) > +{ > + require_event_type(libinput_event_get_context(&event->base), > + event->base.type, > + 0, > + LIBINPUT_EVENT_DIAL); > + > + return event->time; > +} > + > +LIBINPUT_EXPORT int16_t > +libinput_event_dial_get_rotation_count(struct libinput_event_dial *event) > +{ > + require_event_type(libinput_event_get_context(&event->base), > + event->base.type, > + 0, > + LIBINPUT_EVENT_DIAL); > + > + return event->count; > +} > + > struct libinput_source * > libinput_add_fd(struct libinput *libinput, > int fd, > @@ -1953,6 +2001,9 @@ device_has_cap(struct libinput_device *device, > case LIBINPUT_DEVICE_CAP_TABLET_TOOL: > capability = "CAP_TABLET"; > break; > + case LIBINPUT_DEVICE_CAP_DIAL: > + capability = "CAP_DIAL"; > + break; > } > > log_bug_libinput(device->seat->libinput, > @@ -2451,6 +2502,7 @@ event_type_to_str(enum libinput_event_type type) > CASE_RETURN_STRING(LIBINPUT_EVENT_GESTURE_PINCH_BEGIN); > CASE_RETURN_STRING(LIBINPUT_EVENT_GESTURE_PINCH_UPDATE); > CASE_RETURN_STRING(LIBINPUT_EVENT_GESTURE_PINCH_END); > + CASE_RETURN_STRING(LIBINPUT_EVENT_DIAL); > case LIBINPUT_EVENT_NONE: > abort(); > } > @@ -2458,6 +2510,28 @@ event_type_to_str(enum libinput_event_type type) > return NULL; > } > > +void > +dial_notify(struct libinput_device *device, > + uint64_t time, > + int16_t value) > +{ > + struct libinput_event_dial *dial_event; > + > + if (!device_has_cap(device, LIBINPUT_DEVICE_CAP_DIAL)) > + return; > + > + dial_event = zalloc(sizeof *dial_event); > + if (!dial_event) > + return; > + > + *dial_event = (struct libinput_event_dial) { > + .time = time, > + .count = value, > + }; > + > + post_device_event(device, time, LIBINPUT_EVENT_DIAL, &dial_event->base); > +} > + > static void > libinput_post_event(struct libinput *libinput, > struct libinput_event *event) > diff --git a/src/libinput.h b/src/libinput.h > index 8ed5632..fdba4f2 100644 > --- a/src/libinput.h > +++ b/src/libinput.h > @@ -60,6 +60,7 @@ enum libinput_device_capability { > LIBINPUT_DEVICE_CAP_TOUCH = 2, > LIBINPUT_DEVICE_CAP_TABLET_TOOL = 3, > LIBINPUT_DEVICE_CAP_GESTURE = 5, > + LIBINPUT_DEVICE_CAP_DIAL = 8, > }; > > /** > @@ -345,6 +346,8 @@ enum libinput_event_type { > LIBINPUT_EVENT_GESTURE_PINCH_BEGIN, > LIBINPUT_EVENT_GESTURE_PINCH_UPDATE, > LIBINPUT_EVENT_GESTURE_PINCH_END, > + > + LIBINPUT_EVENT_DIAL = 900, > }; > > /** > @@ -552,6 +555,20 @@ libinput_event_get_gesture_event(struct libinput_event > *event); > /** > * @ingroup event > * > + * Return the dial event that is this input event. If the event type does > + * not match the device event types, this function returns NULL. > + * > + * The inverse of this function is > + * libinput_event_device_notify_get_base_event(). > + * > + * @return A device event, or NULL for other events > + */ > +struct libinput_event_dial * > +libinput_event_get_dial_event(struct libinput_event *event); > + > +/** > ++ * @ingroup event > ++ * > * Return the tablet event that is this input event. If the event type does > not > * match the tablet event types, this function returns NULL. > * > @@ -2082,6 +2099,35 @@ libinput_tablet_tool_set_user_data(struct > libinput_tablet_tool *tool, > void *user_data); > > /** > + * @ingroup event_dial > + * > + * @return The event time for this event > + */ > +uint32_t > +libinput_event_dial_get_time(struct libinput_event_dial *event); > + > +/** > + * @ingroup event_dial > + * > + * @return The event time for this event in microseconds > + */ > +uint64_t > +libinput_event_dial_get_time_usec(struct libinput_event_dial *event); > + > +/** > + * @ingroup event_dial > + * > + * Return the number of rotation made in the dial. > + * > + * Positive value indicates clockwise dial rotation, negavtive value > indicates > + * anticlockwise dial rotation. > + * > + * @return the number of dial rotation > + */ > +int16_t > +libinput_event_dial_get_rotation_count(struct libinput_event_dial *event); > + > +/** > * @defgroup base Initialization and manipulation of libinput contexts > */ > > diff --git a/src/libinput.sym b/src/libinput.sym > index a211388..ce0d859 100644 > --- a/src/libinput.sym > +++ b/src/libinput.sym > @@ -233,4 +233,9 @@ LIBINPUT_1.2 { > libinput_tablet_tool_ref; > libinput_tablet_tool_set_user_data; > libinput_tablet_tool_unref; > + > + libinput_event_get_dial_event; > + libinput_event_dial_get_time; > + libinput_event_dial_get_time_usec; > + libinput_event_dial_get_rotation_count; > } LIBINPUT_1.1; > diff --git a/tools/event-debug.c b/tools/event-debug.c > index 648111e..039c64d 100644 > --- a/tools/event-debug.c > +++ b/tools/event-debug.c > @@ -121,6 +121,9 @@ print_event_header(struct libinput_event *ev) > case LIBINPUT_EVENT_TABLET_TOOL_BUTTON: > type = "TABLET_BUTTON"; > break; > + case LIBINPUT_EVENT_DIAL: > + type = "DIAL_ROTATE"; > + break; > } > > printf("%-7s %-16s ", libinput_device_get_sysname(dev), type); > @@ -543,6 +546,23 @@ print_gesture_event_without_coords(struct libinput_event > *ev) > } > > static void > +print_dial_event(struct libinput_event *ev) > +{ > + struct libinput_event_dial *t = libinput_event_get_dial_event(ev); > + enum libinput_event_type type; > + int16_t count = 0; > + > + type = libinput_event_get_type(ev); > + > + print_event_time(libinput_event_dial_get_time(t)); > + count = libinput_event_dial_get_rotation_count(t); > + if (count > 0) > + printf("Dial rotated clockwise %d times\n", count); > + else > + printf("Dial rotated anti-clockwise %d times\n", count); > +} > + > +static void > print_gesture_event_with_coords(struct libinput_event *ev) > { > struct libinput_event_gesture *t = libinput_event_get_gesture_event(ev); > @@ -647,6 +667,9 @@ handle_and_print_events(struct libinput *li) > case LIBINPUT_EVENT_TABLET_TOOL_BUTTON: > print_tablet_button_event(ev); > break; > + case LIBINPUT_EVENT_DIAL: > + print_dial_event(ev); > + break; > } > > libinput_event_destroy(ev); > -- > 2.5.0 _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
