On Wed, Nov 29, 2023 at 04:24:34PM +0100, Benjamin Tissoires wrote: > Turns out that there are transitions that are unlikely to happen: > for example, having both the tip switch and a button being changed > at the same time (in the same report) would require either a very talented > and precise user or a very bad hardware with a very low sampling rate. > > So instead of manually building the button test by hand and forgetting > about some cases, let's reuse the state machine and transitions we have. > > This patch only adds the states and the valid transitions. The actual > tests will be replaced later. > > Signed-off-by: Benjamin Tissoires <bentiss@xxxxxxxxxx> > --- > tools/testing/selftests/hid/tests/test_tablet.py | 170 +++++++++++++++++++++-- > 1 file changed, 157 insertions(+), 13 deletions(-) > > diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py > index 83f6501fe984..80269d1a0f0a 100644 > --- a/tools/testing/selftests/hid/tests/test_tablet.py > +++ b/tools/testing/selftests/hid/tests/test_tablet.py > @@ -21,22 +21,66 @@ logger = logging.getLogger("hidtools.test.tablet") > class PenState(Enum): > """Pen states according to Microsoft reference: > https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states > - """ > > - PEN_IS_OUT_OF_RANGE = (False, None) > - PEN_IS_IN_RANGE = (False, libevdev.EV_KEY.BTN_TOOL_PEN) > - PEN_IS_IN_CONTACT = (True, libevdev.EV_KEY.BTN_TOOL_PEN) > - PEN_IS_IN_RANGE_WITH_ERASING_INTENT = (False, libevdev.EV_KEY.BTN_TOOL_RUBBER) > - PEN_IS_ERASING = (True, libevdev.EV_KEY.BTN_TOOL_RUBBER) > + We extend it with the various buttons when we need to check them. > + """ > > - def __init__(self, touch, tool): It'd be nice to have a comment here what the False refers to. Even nicer would be an enum class BtnTouch.DOWN so the code is instantly readable :) Cheers, Peter > + PEN_IS_OUT_OF_RANGE = (False, None, None) > + PEN_IS_IN_RANGE = (False, libevdev.EV_KEY.BTN_TOOL_PEN, None) > + PEN_IS_IN_RANGE_WITH_BUTTON = ( > + False, > + libevdev.EV_KEY.BTN_TOOL_PEN, > + libevdev.EV_KEY.BTN_STYLUS, > + ) > + PEN_IS_IN_RANGE_WITH_SECOND_BUTTON = ( > + False, > + libevdev.EV_KEY.BTN_TOOL_PEN, > + libevdev.EV_KEY.BTN_STYLUS2, > + ) > + PEN_IS_IN_CONTACT = (True, libevdev.EV_KEY.BTN_TOOL_PEN, None) > + PEN_IS_IN_CONTACT_WITH_BUTTON = ( > + True, > + libevdev.EV_KEY.BTN_TOOL_PEN, > + libevdev.EV_KEY.BTN_STYLUS, > + ) > + PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON = ( > + True, > + libevdev.EV_KEY.BTN_TOOL_PEN, > + libevdev.EV_KEY.BTN_STYLUS2, > + ) > + PEN_IS_IN_RANGE_WITH_ERASING_INTENT = (False, libevdev.EV_KEY.BTN_TOOL_RUBBER, None) > + PEN_IS_IN_RANGE_WITH_ERASING_INTENT_WITH_BUTTON = ( > + False, > + libevdev.EV_KEY.BTN_TOOL_RUBBER, > + libevdev.EV_KEY.BTN_STYLUS, > + ) > + PEN_IS_IN_RANGE_WITH_ERASING_INTENT_WITH_SECOND_BUTTON = ( > + False, > + libevdev.EV_KEY.BTN_TOOL_RUBBER, > + libevdev.EV_KEY.BTN_STYLUS2, > + ) > + PEN_IS_ERASING = (True, libevdev.EV_KEY.BTN_TOOL_RUBBER, None) > + PEN_IS_ERASING_WITH_BUTTON = ( > + True, > + libevdev.EV_KEY.BTN_TOOL_RUBBER, > + libevdev.EV_KEY.BTN_STYLUS, > + ) > + PEN_IS_ERASING_WITH_SECOND_BUTTON = ( > + True, > + libevdev.EV_KEY.BTN_TOOL_RUBBER, > + libevdev.EV_KEY.BTN_STYLUS2, > + ) > + > + def __init__(self, touch, tool, button): > self.touch = touch > self.tool = tool > + self.button = button > > @classmethod > def from_evdev(cls, evdev) -> "PenState": > touch = bool(evdev.value[libevdev.EV_KEY.BTN_TOUCH]) > tool = None > + button = None > if ( > evdev.value[libevdev.EV_KEY.BTN_TOOL_RUBBER] > and not evdev.value[libevdev.EV_KEY.BTN_TOOL_PEN] > @@ -53,7 +97,17 @@ class PenState(Enum): > ): > raise ValueError("2 tools are not allowed") > > - return cls((touch, tool)) > + # we take only the highest button in account > + for b in [libevdev.EV_KEY.BTN_STYLUS, libevdev.EV_KEY.BTN_STYLUS2]: > + if bool(evdev.value[b]): > + button = b > + > + # the kernel tends to insert an EV_SYN once removing the tool, so > + # the button will be released after > + if tool is None: > + button = None > + > + return cls((touch, tool, button)) > > def apply(self, events) -> "PenState": > if libevdev.EV_SYN.SYN_REPORT in events: > @@ -62,6 +116,8 @@ class PenState(Enum): > touch_found = False > tool = self.tool > tool_found = False > + button = self.button > + button_found = False > > for ev in events: > if ev == libevdev.InputEvent(libevdev.EV_KEY.BTN_TOUCH): > @@ -76,12 +132,22 @@ class PenState(Enum): > if tool_found: > raise ValueError(f"duplicated BTN_TOOL_* in {events}") > tool_found = True > - if ev.value: > - tool = ev.code > - else: > - tool = None > + tool = ev.code if ev.value else None > + elif ev in ( > + libevdev.InputEvent(libevdev.EV_KEY.BTN_STYLUS), > + libevdev.InputEvent(libevdev.EV_KEY.BTN_STYLUS2), > + ): > + if button_found: > + raise ValueError(f"duplicated BTN_STYLUS* in {events}") > + button_found = True > + button = ev.code if ev.value else None > > - new_state = PenState((touch, tool)) > + # the kernel tends to insert an EV_SYN once removing the tool, so > + # the button will be released after > + if tool is None: > + button = None > + > + new_state = PenState((touch, tool, button)) > assert ( > new_state in self.valid_transitions() > ), f"moving from {self} to {new_state} is forbidden" > @@ -97,14 +163,20 @@ class PenState(Enum): > return ( > PenState.PEN_IS_OUT_OF_RANGE, > PenState.PEN_IS_IN_RANGE, > + PenState.PEN_IS_IN_RANGE_WITH_BUTTON, > + PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON, > PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT, > PenState.PEN_IS_IN_CONTACT, > + PenState.PEN_IS_IN_CONTACT_WITH_BUTTON, > + PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON, > PenState.PEN_IS_ERASING, > ) > > if self == PenState.PEN_IS_IN_RANGE: > return ( > PenState.PEN_IS_IN_RANGE, > + PenState.PEN_IS_IN_RANGE_WITH_BUTTON, > + PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON, > PenState.PEN_IS_OUT_OF_RANGE, > PenState.PEN_IS_IN_CONTACT, > ) > @@ -112,6 +184,8 @@ class PenState(Enum): > if self == PenState.PEN_IS_IN_CONTACT: > return ( > PenState.PEN_IS_IN_CONTACT, > + PenState.PEN_IS_IN_CONTACT_WITH_BUTTON, > + PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON, > PenState.PEN_IS_IN_RANGE, > PenState.PEN_IS_OUT_OF_RANGE, > ) > @@ -130,6 +204,38 @@ class PenState(Enum): > PenState.PEN_IS_OUT_OF_RANGE, > ) > > + if self == PenState.PEN_IS_IN_RANGE_WITH_BUTTON: > + return ( > + PenState.PEN_IS_IN_RANGE_WITH_BUTTON, > + PenState.PEN_IS_IN_RANGE, > + PenState.PEN_IS_OUT_OF_RANGE, > + PenState.PEN_IS_IN_CONTACT_WITH_BUTTON, > + ) > + > + if self == PenState.PEN_IS_IN_CONTACT_WITH_BUTTON: > + return ( > + PenState.PEN_IS_IN_CONTACT_WITH_BUTTON, > + PenState.PEN_IS_IN_CONTACT, > + PenState.PEN_IS_IN_RANGE_WITH_BUTTON, > + PenState.PEN_IS_OUT_OF_RANGE, > + ) > + > + if self == PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON: > + return ( > + PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON, > + PenState.PEN_IS_IN_RANGE, > + PenState.PEN_IS_OUT_OF_RANGE, > + PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON, > + ) > + > + if self == PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON: > + return ( > + PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON, > + PenState.PEN_IS_IN_CONTACT, > + PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON, > + PenState.PEN_IS_OUT_OF_RANGE, > + ) > + > return tuple() > > @staticmethod > @@ -364,26 +470,64 @@ class PenDigitizer(base.UHIDTestDevice): > pen.xtilt = 0 > pen.ytilt = 0 > pen.twist = 0 > + pen.barrelswitch = False > + pen.secondarybarrelswitch = False > elif state == PenState.PEN_IS_IN_RANGE: > pen.tipswitch = False > pen.inrange = True > pen.invert = False > pen.eraser = False > + pen.barrelswitch = False > + pen.secondarybarrelswitch = False > elif state == PenState.PEN_IS_IN_CONTACT: > pen.tipswitch = True > pen.inrange = True > pen.invert = False > pen.eraser = False > + pen.barrelswitch = False > + pen.secondarybarrelswitch = False > + elif state == PenState.PEN_IS_IN_RANGE_WITH_BUTTON: > + pen.tipswitch = False > + pen.inrange = True > + pen.invert = False > + pen.eraser = False > + pen.barrelswitch = True > + pen.secondarybarrelswitch = False > + elif state == PenState.PEN_IS_IN_CONTACT_WITH_BUTTON: > + pen.tipswitch = True > + pen.inrange = True > + pen.invert = False > + pen.eraser = False > + pen.barrelswitch = True > + pen.secondarybarrelswitch = False > + elif state == PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON: > + pen.tipswitch = False > + pen.inrange = True > + pen.invert = False > + pen.eraser = False > + pen.barrelswitch = False > + pen.secondarybarrelswitch = True > + elif state == PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON: > + pen.tipswitch = True > + pen.inrange = True > + pen.invert = False > + pen.eraser = False > + pen.barrelswitch = False > + pen.secondarybarrelswitch = True > elif state == PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT: > pen.tipswitch = False > pen.inrange = True > pen.invert = True > pen.eraser = False > + pen.barrelswitch = False > + pen.secondarybarrelswitch = False > elif state == PenState.PEN_IS_ERASING: > pen.tipswitch = False > pen.inrange = True > pen.invert = False > pen.eraser = True > + pen.barrelswitch = False > + pen.secondarybarrelswitch = False > > pen.current_state = state > > > -- > 2.41.0 >