From: Peter Hutterer <peter.hutterer@who-t.net>
To: Benjamin Tissoires <bentiss@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>,
Benjamin Tissoires <benjamin.tissoires@redhat.com>,
Shuah Khan <shuah@kernel.org>,
linux-input@vger.kernel.org, linux-kselftest@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 09/12] selftests/hid: tablets: add variants of states with buttons
Date: Fri, 1 Dec 2023 15:41:27 +1000 [thread overview]
Message-ID: <20231201054127.GA626305@quokka> (raw)
In-Reply-To: <20231129-wip-selftests-v1-9-ba15a1fe1b0d@kernel.org>
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@kernel.org>
> ---
> 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
>
next prev parent reply other threads:[~2023-12-01 5:41 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-29 15:24 [PATCH 00/12] selftests/hid: tablets fixes Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 01/12] selftests/hid: vmtest.sh: update vm2c and container Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 02/12] selftests/hid: vmtest.sh: allow finer control on the build steps Benjamin Tissoires
2023-12-01 5:34 ` Peter Hutterer
2023-11-29 15:24 ` [PATCH 03/12] selftests/hid: base: allow for multiple skip_if_uhdev Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 04/12] selftests/hid: tablets: remove unused class Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 05/12] selftests/hid: tablets: move the transitions to PenState Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 06/12] selftests/hid: tablets: move move_to function to PenDigitizer Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 07/12] selftests/hid: tablets: do not set invert when the eraser is used Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 08/12] selftests/hid: tablets: set initial data for tilt/twist Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 09/12] selftests/hid: tablets: add variants of states with buttons Benjamin Tissoires
2023-12-01 5:41 ` Peter Hutterer [this message]
2023-11-29 15:24 ` [PATCH 10/12] selftests/hid: tablets: convert the primary button tests Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 11/12] selftests/hid: tablets: add a secondary barrel switch test Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 12/12] selftests/hid: tablets: be stricter for some transitions Benjamin Tissoires
2023-12-01 5:50 ` Peter Hutterer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231201054127.GA626305@quokka \
--to=peter.hutterer@who-t.net \
--cc=benjamin.tissoires@redhat.com \
--cc=bentiss@kernel.org \
--cc=jikos@kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=shuah@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.