[RFC v2 libinput 2/2] buttonset: implement buttonset handling for Wacom tablet pads

[Peter Hutterer]

On Thu, Mar 19, 2015 at 02:01:29PM -0700, Jason Gerecke wrote:
> David's already asked the API questions that popped in my head, so I'll just
> wait for feedback on his thread. As far as this patch goes, things look
> pretty good though there are two comments:
> 
> On 3/17/2015 11:58 PM, Peter Hutterer wrote:
[..]
> >+
> >+static double
> >+buttonset_to_phys(struct evdev_device *device, unsigned int axis, double value)
> >+{
> >+	struct buttonset_dispatch *buttonset =
> >+				(struct buttonset_dispatch*)device->dispatch;
> >+
> >+	switch(buttonset->types[axis]) {
> >+	case LIBINPUT_BUTTONSET_AXIS_NONE:
> >+	case LIBINPUT_BUTTONSET_AXIS_X:
> >+	case LIBINPUT_BUTTONSET_AXIS_Y:
> >+	case LIBINPUT_BUTTONSET_AXIS_Z:
> >+	case LIBINPUT_BUTTONSET_AXIS_REL_X:
> >+	case LIBINPUT_BUTTONSET_AXIS_REL_Y:
> >+	case LIBINPUT_BUTTONSET_AXIS_REL_Z:
> >+	case LIBINPUT_BUTTONSET_AXIS_ROTATION_X:
> >+	case LIBINPUT_BUTTONSET_AXIS_ROTATION_Y:
> >+	case LIBINPUT_BUTTONSET_AXIS_ROTATION_Z:
> >+		abort();
> 
> Yikes! I understand the point (no reasonable value to return, no way a
> caller would be given one of these axes in the first place, etc.) but it'd
> be good to document that doing this is bad and that the caller should feel
> bad.

yep, added a fixme locally, will be in the next version.

> >+	case LIBINPUT_BUTTONSET_AXIS_RING:
> >+		value *= 360;
> >+		break;
> >+	case LIBINPUT_BUTTONSET_AXIS_STRIP:
> >+		value *= 52; /* FIXME: correct for Intuos3 and 21UX */
> >+		break;
> >+	}
> >+
> >+	return value;
> >+}
> >+
> >+static unsigned int
> >+buttonset_get_num_axes(struct evdev_device *device)
> >+{
> >+	struct buttonset_dispatch *buttonset =
> >+				(struct buttonset_dispatch*)device->dispatch;
> >+
> >+	return buttonset->naxes;
> >+}
> >+
> >+static enum libinput_buttonset_axis_type
> >+buttonset_get_axis_type(struct evdev_device *device, unsigned int axis)
> >+{
> >+	struct buttonset_dispatch *buttonset =
> >+				(struct buttonset_dispatch*)device->dispatch;
> >+	if (axis < buttonset->naxes)
> >+		return buttonset->types[axis];
> >+	else
> >+		return LIBINPUT_BUTTONSET_AXIS_NONE;
> >+}
> >+
> >+static struct evdev_dispatch_interface buttonset_interface = {
> >+	buttonset_process,
> >+	NULL, /* remove */
> >+	buttonset_destroy,
> >+	NULL, /* device_added */
> >+	NULL, /* device_removed */
> >+	NULL, /* device_suspended */
> >+	NULL, /* device_resumed */
> >+	NULL, /* tag_device */
> >+	NULL, /* post_added */
> >+	buttonset_to_phys,
> >+	buttonset_get_num_axes,
> >+	buttonset_get_axis_type,
> >+};
> >+
> >+static enum libinput_buttonset_axis_type
> >+buttonset_guess_axis_type(struct evdev_device *device,
> >+			  unsigned int evcode)
> >+{
> >+	switch (evcode) {
> >+	case ABS_WHEEL:
> >+	case ABS_THROTTLE:
> >+		return LIBINPUT_BUTTONSET_AXIS_RING;
> >+	case ABS_RX:
> >+	case ABS_RY:
> >+		return LIBINPUT_BUTTONSET_AXIS_STRIP;
> >+	default:
> >+		return LIBINPUT_BUTTONSET_AXIS_NONE;
> >+	}
> >+}
> >+
> >+static int
> >+buttonset_init(struct buttonset_dispatch *buttonset,
> >+	       struct evdev_device *device)
> >+{
> >+	unsigned int naxes = 0;
> >+	int code;
> >+	enum libinput_buttonset_axis_type type;
> >+
> >+	buttonset->base.interface = &buttonset_interface;
> >+	buttonset->device = device;
> >+	buttonset->status = BUTTONSET_NONE;
> >+
> >+	/* We intentionally skip X/Y, they're dead on wacom pads */
> >+	for (code = ABS_Z; code <= ABS_MAX; code++) {
> 
> Note: the 27QHD pad uses ABS_{X,Y,Z} to report accelerometer data. Probably
> should skip Z too unless you consider it appropriate data for this
> interface.

urgh. thanks, fixed locally.

Cheers,
   Peter