linuxwacom / xf86-input-wacom

X.Org driver for Wacom devices
361 stars 45 forks source link

test: add a test for the artpen/airbrush wheel updates #275

Closed whot closed 2 years ago

whot commented 2 years ago

This was slightly too complicated to add to the existing test_axis_updates test, so that test is duplicated here, only testing what we see as "wheel" axis in the driver.

The test is run for all three pens

Because the X driver unconditionally sets ds->abswheel from the kernel events, we must take care only to send the kernel axis that matters for our current tool - otherwise we overwrite ds->abswheel with whichever one of ABS_Z or ABS_WHEEL is sent last in the evdev frame. In other words, we cannot test that artpen ignores ABS_WHEEL and airbrush ignores ABS_Z because ... they don't.

For the generic pen that doesn't matter since we're supposed to ignore both axes anyway.

cc @Pinglinux

The test_axis_updates should probably drop the wheel handling now and instead run for the airbrush as-is (i.e. without checking wheel).

Pinglinux commented 2 years ago

I installed Fedora 36 on one of my testing systems. After getting all the required packages in place, I successfully executed the python-based test suite! I don't understand why we can't test if airbrush ignores ABS_Z and artpen ignores ABS_WHEEL. But, I am happy the patch passes all 20 tests. So, the patch is:

Reviewed-by: Ping Cheng ping.cheng@wacom.com Tested-by: Ping Cheng ping.cheng@wacom.com

whot commented 2 years ago

I don't understand why we can't test if airbrush ignores ABS_Z and artpen ignores ABS_WHEEL

Simple answer: because the driver doesn't currently do this and the test would fail :) Once the driver is fixed we could test for it. But right now, if you look at wcmUSB.c, we have:

                  case ABS_Z:
                          ds->abswheel = event->value;
                          break;

and

                  case ABS_WHEEL:
                          ds->abswheel = event->value;
                          break;

So whichever comes later overwrites the ds->abswheel state. This just hasn't been a problem yet because your pens behave properly.

Thanks for the review + test, merging!