* [PATCH 01/12] selftests/hid: vmtest.sh: update vm2c and container
2023-11-29 15:24 [PATCH 00/12] selftests/hid: tablets fixes Benjamin Tissoires
@ 2023-11-29 15:24 ` Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 02/12] selftests/hid: vmtest.sh: allow finer control on the build steps Benjamin Tissoires
` (10 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
boot2container is now on an official project, so let's use that.
The container image is now the same I use for the CI, so let's keep
to it.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/vmtest.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/hid/vmtest.sh b/tools/testing/selftests/hid/vmtest.sh
index 4da48bf6b328..301b4e162336 100755
--- a/tools/testing/selftests/hid/vmtest.sh
+++ b/tools/testing/selftests/hid/vmtest.sh
@@ -19,12 +19,12 @@ esac
SCRIPT_DIR="$(dirname $(realpath $0))"
OUTPUT_DIR="$SCRIPT_DIR/results"
KCONFIG_REL_PATHS=("${SCRIPT_DIR}/config" "${SCRIPT_DIR}/config.common" "${SCRIPT_DIR}/config.${ARCH}")
-B2C_URL="https://gitlab.freedesktop.org/mupuf/boot2container/-/raw/master/vm2c.py"
+B2C_URL="https://gitlab.freedesktop.org/gfx-ci/boot2container/-/raw/main/vm2c.py"
NUM_COMPILE_JOBS="$(nproc)"
LOG_FILE_BASE="$(date +"hid_selftests.%Y-%m-%d_%H-%M-%S")"
LOG_FILE="${LOG_FILE_BASE}.log"
EXIT_STATUS_FILE="${LOG_FILE_BASE}.exit_status"
-CONTAINER_IMAGE="registry.freedesktop.org/libevdev/hid-tools/fedora/37:2023-02-17.1"
+CONTAINER_IMAGE="registry.freedesktop.org/bentiss/hid/fedora/39:2023-11-22.1"
TARGETS="${TARGETS:=$(basename ${SCRIPT_DIR})}"
DEFAULT_COMMAND="pip3 install hid-tools; make -C tools/testing/selftests TARGETS=${TARGETS} run_tests"
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 02/12] selftests/hid: vmtest.sh: allow finer control on the build steps
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 ` 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
` (9 subsequent siblings)
11 siblings, 1 reply; 16+ messages in thread
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
vmtest.sh works great for a one shot test, but not so much for CI where
I want to build (with different configs) the bzImage in a separate
job than the one I am running it.
Add a "build_only" option to specify whether we need to boot the currently
built kernel in the vm.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/vmtest.sh | 42 ++++++++++++++++++++---------------
1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/hid/vmtest.sh b/tools/testing/selftests/hid/vmtest.sh
index 301b4e162336..52ada972833b 100755
--- a/tools/testing/selftests/hid/vmtest.sh
+++ b/tools/testing/selftests/hid/vmtest.sh
@@ -32,7 +32,7 @@ DEFAULT_COMMAND="pip3 install hid-tools; make -C tools/testing/selftests TARGETS
usage()
{
cat <<EOF
-Usage: $0 [-i] [-s] [-d <output_dir>] -- [<command>]
+Usage: $0 [-j N] [-s] [-b] [-d <output_dir>] -- [<command>]
<command> is the command you would normally run when you are in
the source kernel direcory. e.g:
@@ -55,6 +55,7 @@ Options:
-u) Update the boot2container script to a newer version.
-d) Update the output directory (default: ${OUTPUT_DIR})
+ -b) Run the only build steps for the kernel and the selftests
-j) Number of jobs for compilation, similar to -j in make
(default: ${NUM_COMPILE_JOBS})
-s) Instead of powering off the VM, start an interactive
@@ -191,8 +192,9 @@ main()
local command="${DEFAULT_COMMAND}"
local update_b2c="no"
local debug_shell="no"
+ local build_only="no"
- while getopts ':hsud:j:' opt; do
+ while getopts ':hsud:j:b' opt; do
case ${opt} in
u)
update_b2c="yes"
@@ -207,6 +209,9 @@ main()
command="/bin/sh"
debug_shell="yes"
;;
+ b)
+ build_only="yes"
+ ;;
h)
usage
exit 0
@@ -226,8 +231,7 @@ main()
shift $((OPTIND -1))
# trap 'catch "$?"' EXIT
-
- if [[ "${debug_shell}" == "no" ]]; then
+ if [[ "${build_only}" == "no" && "${debug_shell}" == "no" ]]; then
if [[ $# -eq 0 ]]; then
echo "No command specified, will run ${DEFAULT_COMMAND} in the vm"
else
@@ -267,24 +271,26 @@ main()
update_kconfig "${kernel_checkout}" "${kconfig_file}"
recompile_kernel "${kernel_checkout}" "${make_command}"
+ update_selftests "${kernel_checkout}" "${make_command}"
- if [[ "${update_b2c}" == "no" && ! -f "${b2c}" ]]; then
- echo "vm2c script not found in ${b2c}"
- update_b2c="yes"
- fi
+ if [[ "${build_only}" == "no" ]]; then
+ if [[ "${update_b2c}" == "no" && ! -f "${b2c}" ]]; then
+ echo "vm2c script not found in ${b2c}"
+ update_b2c="yes"
+ fi
- if [[ "${update_b2c}" == "yes" ]]; then
- download $B2C_URL $b2c
- chmod +x $b2c
- fi
+ if [[ "${update_b2c}" == "yes" ]]; then
+ download $B2C_URL $b2c
+ chmod +x $b2c
+ fi
- update_selftests "${kernel_checkout}" "${make_command}"
- run_vm "${kernel_checkout}" $b2c "${kernel_bzimage}" "${command}"
- if [[ "${debug_shell}" != "yes" ]]; then
- echo "Logs saved in ${OUTPUT_DIR}/${LOG_FILE}"
- fi
+ run_vm "${kernel_checkout}" $b2c "${kernel_bzimage}" "${command}"
+ if [[ "${debug_shell}" != "yes" ]]; then
+ echo "Logs saved in ${OUTPUT_DIR}/${LOG_FILE}"
+ fi
- exit $(cat ${OUTPUT_DIR}/${EXIT_STATUS_FILE})
+ exit $(cat ${OUTPUT_DIR}/${EXIT_STATUS_FILE})
+ fi
}
main "$@"
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 02/12] selftests/hid: vmtest.sh: allow finer control on the build steps
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
0 siblings, 0 replies; 16+ messages in thread
From: Peter Hutterer @ 2023-12-01 5:34 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Benjamin Tissoires, Shuah Khan, linux-input,
linux-kselftest, linux-kernel
On Wed, Nov 29, 2023 at 04:24:27PM +0100, Benjamin Tissoires wrote:
> vmtest.sh works great for a one shot test, but not so much for CI where
> I want to build (with different configs) the bzImage in a separate
> job than the one I am running it.
>
> Add a "build_only" option to specify whether we need to boot the currently
> built kernel in the vm.
>
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
> tools/testing/selftests/hid/vmtest.sh | 42 ++++++++++++++++++++---------------
> 1 file changed, 24 insertions(+), 18 deletions(-)
>
> diff --git a/tools/testing/selftests/hid/vmtest.sh b/tools/testing/selftests/hid/vmtest.sh
> index 301b4e162336..52ada972833b 100755
> --- a/tools/testing/selftests/hid/vmtest.sh
> +++ b/tools/testing/selftests/hid/vmtest.sh
> @@ -32,7 +32,7 @@ DEFAULT_COMMAND="pip3 install hid-tools; make -C tools/testing/selftests TARGETS
> usage()
> {
> cat <<EOF
> -Usage: $0 [-i] [-s] [-d <output_dir>] -- [<command>]
> +Usage: $0 [-j N] [-s] [-b] [-d <output_dir>] -- [<command>]
>
> <command> is the command you would normally run when you are in
> the source kernel direcory. e.g:
> @@ -55,6 +55,7 @@ Options:
>
> -u) Update the boot2container script to a newer version.
> -d) Update the output directory (default: ${OUTPUT_DIR})
> + -b) Run the only build steps for the kernel and the selftests
typo: "run only the"
Cheers,
Peter
> -j) Number of jobs for compilation, similar to -j in make
> (default: ${NUM_COMPILE_JOBS})
> -s) Instead of powering off the VM, start an interactive
> @@ -191,8 +192,9 @@ main()
> local command="${DEFAULT_COMMAND}"
> local update_b2c="no"
> local debug_shell="no"
> + local build_only="no"
>
> - while getopts ':hsud:j:' opt; do
> + while getopts ':hsud:j:b' opt; do
> case ${opt} in
> u)
> update_b2c="yes"
> @@ -207,6 +209,9 @@ main()
> command="/bin/sh"
> debug_shell="yes"
> ;;
> + b)
> + build_only="yes"
> + ;;
> h)
> usage
> exit 0
> @@ -226,8 +231,7 @@ main()
> shift $((OPTIND -1))
>
> # trap 'catch "$?"' EXIT
> -
> - if [[ "${debug_shell}" == "no" ]]; then
> + if [[ "${build_only}" == "no" && "${debug_shell}" == "no" ]]; then
> if [[ $# -eq 0 ]]; then
> echo "No command specified, will run ${DEFAULT_COMMAND} in the vm"
> else
> @@ -267,24 +271,26 @@ main()
> update_kconfig "${kernel_checkout}" "${kconfig_file}"
>
> recompile_kernel "${kernel_checkout}" "${make_command}"
> + update_selftests "${kernel_checkout}" "${make_command}"
>
> - if [[ "${update_b2c}" == "no" && ! -f "${b2c}" ]]; then
> - echo "vm2c script not found in ${b2c}"
> - update_b2c="yes"
> - fi
> + if [[ "${build_only}" == "no" ]]; then
> + if [[ "${update_b2c}" == "no" && ! -f "${b2c}" ]]; then
> + echo "vm2c script not found in ${b2c}"
> + update_b2c="yes"
> + fi
>
> - if [[ "${update_b2c}" == "yes" ]]; then
> - download $B2C_URL $b2c
> - chmod +x $b2c
> - fi
> + if [[ "${update_b2c}" == "yes" ]]; then
> + download $B2C_URL $b2c
> + chmod +x $b2c
> + fi
>
> - update_selftests "${kernel_checkout}" "${make_command}"
> - run_vm "${kernel_checkout}" $b2c "${kernel_bzimage}" "${command}"
> - if [[ "${debug_shell}" != "yes" ]]; then
> - echo "Logs saved in ${OUTPUT_DIR}/${LOG_FILE}"
> - fi
> + run_vm "${kernel_checkout}" $b2c "${kernel_bzimage}" "${command}"
> + if [[ "${debug_shell}" != "yes" ]]; then
> + echo "Logs saved in ${OUTPUT_DIR}/${LOG_FILE}"
> + fi
>
> - exit $(cat ${OUTPUT_DIR}/${EXIT_STATUS_FILE})
> + exit $(cat ${OUTPUT_DIR}/${EXIT_STATUS_FILE})
> + fi
> }
>
> main "$@"
>
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 03/12] selftests/hid: base: allow for multiple skip_if_uhdev
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-11-29 15:24 ` Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 04/12] selftests/hid: tablets: remove unused class Benjamin Tissoires
` (8 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
We can actually have multiple occurences of `skip_if_uhdev` if we follow
the information from the pytest doc[0].
This is not immediately used, but can be if we need multiple conditions
on a given test.
[0] https://docs.pytest.org/en/latest/historical-notes.html#update-marker-code
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/base.py | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/base.py b/tools/testing/selftests/hid/tests/base.py
index 1305cfc9646e..5d9c26dfc460 100644
--- a/tools/testing/selftests/hid/tests/base.py
+++ b/tools/testing/selftests/hid/tests/base.py
@@ -238,8 +238,7 @@ class BaseTestCase:
try:
with HIDTestUdevRule.instance():
with new_uhdev as self.uhdev:
- skip_cond = request.node.get_closest_marker("skip_if_uhdev")
- if skip_cond:
+ for skip_cond in request.node.iter_markers("skip_if_uhdev"):
test, message, *rest = skip_cond.args
if test(self.uhdev):
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 04/12] selftests/hid: tablets: remove unused class
2023-11-29 15:24 [PATCH 00/12] selftests/hid: tablets fixes Benjamin Tissoires
` (2 preceding siblings ...)
2023-11-29 15:24 ` [PATCH 03/12] selftests/hid: base: allow for multiple skip_if_uhdev Benjamin Tissoires
@ 2023-11-29 15:24 ` Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 05/12] selftests/hid: tablets: move the transitions to PenState Benjamin Tissoires
` (7 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
Looks like this is a leftover
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 4 ----
1 file changed, 4 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index 303ffff9ee8d..cd9c1269afa6 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -133,10 +133,6 @@ class PenState(Enum):
return tuple()
-class Data(object):
- pass
-
-
class Pen(object):
def __init__(self, x, y):
self.x = x
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 05/12] selftests/hid: tablets: move the transitions to PenState
2023-11-29 15:24 [PATCH 00/12] selftests/hid: tablets fixes Benjamin Tissoires
` (3 preceding siblings ...)
2023-11-29 15:24 ` [PATCH 04/12] selftests/hid: tablets: remove unused class Benjamin Tissoires
@ 2023-11-29 15:24 ` Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 06/12] selftests/hid: tablets: move move_to function to PenDigitizer Benjamin Tissoires
` (6 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
Those transitions have nothing to do with `Pen`, so migrate them to
`PenState`.
The hidden agenda is to remove `Pen` and integrate it into `PenDigitizer`
so that we can tweak the events in each state to emulate firmware bugs.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 212 +++++++++++------------
1 file changed, 106 insertions(+), 106 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index cd9c1269afa6..18961758e4aa 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -132,104 +132,8 @@ class PenState(Enum):
return tuple()
-
-class Pen(object):
- def __init__(self, x, y):
- self.x = x
- self.y = y
- self.tipswitch = False
- self.tippressure = 15
- self.azimuth = 0
- self.inrange = False
- self.width = 10
- self.height = 10
- self.barrelswitch = False
- self.invert = False
- self.eraser = False
- self.x_tilt = 0
- self.y_tilt = 0
- self.twist = 0
- self._old_values = None
- self.current_state = None
-
- def _restore(self):
- if self._old_values is not None:
- for i in [
- "x",
- "y",
- "tippressure",
- "azimuth",
- "width",
- "height",
- "twist",
- "x_tilt",
- "y_tilt",
- ]:
- setattr(self, i, getattr(self._old_values, i))
-
- def move_to(self, state):
- # fill in the previous values
- if self.current_state == PenState.PEN_IS_OUT_OF_RANGE:
- self._restore()
-
- print(f"\n *** pen is moving to {state} ***")
-
- if state == PenState.PEN_IS_OUT_OF_RANGE:
- self._old_values = copy.copy(self)
- self.x = 0
- self.y = 0
- self.tipswitch = False
- self.tippressure = 0
- self.azimuth = 0
- self.inrange = False
- self.width = 0
- self.height = 0
- self.invert = False
- self.eraser = False
- self.x_tilt = 0
- self.y_tilt = 0
- self.twist = 0
- elif state == PenState.PEN_IS_IN_RANGE:
- self.tipswitch = False
- self.inrange = True
- self.invert = False
- self.eraser = False
- elif state == PenState.PEN_IS_IN_CONTACT:
- self.tipswitch = True
- self.inrange = True
- self.invert = False
- self.eraser = False
- elif state == PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT:
- self.tipswitch = False
- self.inrange = True
- self.invert = True
- self.eraser = False
- elif state == PenState.PEN_IS_ERASING:
- self.tipswitch = False
- self.inrange = True
- self.invert = True
- self.eraser = True
-
- self.current_state = state
-
- def __assert_axis(self, evdev, axis, value):
- if (
- axis == libevdev.EV_KEY.BTN_TOOL_RUBBER
- and evdev.value[libevdev.EV_KEY.BTN_TOOL_RUBBER] is None
- ):
- return
-
- assert (
- evdev.value[axis] == value
- ), f"assert evdev.value[{axis}] ({evdev.value[axis]}) != {value}"
-
- def assert_expected_input_events(self, evdev):
- assert evdev.value[libevdev.EV_ABS.ABS_X] == self.x
- assert evdev.value[libevdev.EV_ABS.ABS_Y] == self.y
- assert self.current_state == PenState.from_evdev(evdev)
-
@staticmethod
- def legal_transitions() -> Dict[str, Tuple[PenState, ...]]:
+ def legal_transitions() -> Dict[str, Tuple["PenState", ...]]:
"""This is the first half of the Windows Pen Implementation state machine:
we don't have Invert nor Erase bits, so just move in/out-of-range or proximity.
https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
@@ -255,7 +159,7 @@ class Pen(object):
}
@staticmethod
- def legal_transitions_with_invert() -> Dict[str, Tuple[PenState, ...]]:
+ def legal_transitions_with_invert() -> Dict[str, Tuple["PenState", ...]]:
"""This is the second half of the Windows Pen Implementation state machine:
we now have Invert and Erase bits, so move in/out or proximity with the intend
to erase.
@@ -293,7 +197,7 @@ class Pen(object):
}
@staticmethod
- def tolerated_transitions() -> Dict[str, Tuple[PenState, ...]]:
+ def tolerated_transitions() -> Dict[str, Tuple["PenState", ...]]:
"""This is not adhering to the Windows Pen Implementation state machine
but we should expect the kernel to behave properly, mostly for historical
reasons."""
@@ -306,7 +210,7 @@ class Pen(object):
}
@staticmethod
- def tolerated_transitions_with_invert() -> Dict[str, Tuple[PenState, ...]]:
+ def tolerated_transitions_with_invert() -> Dict[str, Tuple["PenState", ...]]:
"""This is the second half of the Windows Pen Implementation state machine:
we now have Invert and Erase bits, so move in/out or proximity with the intend
to erase.
@@ -321,7 +225,7 @@ class Pen(object):
}
@staticmethod
- def broken_transitions() -> Dict[str, Tuple[PenState, ...]]:
+ def broken_transitions() -> Dict[str, Tuple["PenState", ...]]:
"""Those tests are definitely not part of the Windows specification.
However, a half broken device might export those transitions.
For example, a pen that has the eraser button might wobble between
@@ -359,6 +263,102 @@ class Pen(object):
}
+class Pen(object):
+ def __init__(self, x, y):
+ self.x = x
+ self.y = y
+ self.tipswitch = False
+ self.tippressure = 15
+ self.azimuth = 0
+ self.inrange = False
+ self.width = 10
+ self.height = 10
+ self.barrelswitch = False
+ self.invert = False
+ self.eraser = False
+ self.x_tilt = 0
+ self.y_tilt = 0
+ self.twist = 0
+ self._old_values = None
+ self.current_state = None
+
+ def _restore(self):
+ if self._old_values is not None:
+ for i in [
+ "x",
+ "y",
+ "tippressure",
+ "azimuth",
+ "width",
+ "height",
+ "twist",
+ "x_tilt",
+ "y_tilt",
+ ]:
+ setattr(self, i, getattr(self._old_values, i))
+
+ def move_to(self, state):
+ # fill in the previous values
+ if self.current_state == PenState.PEN_IS_OUT_OF_RANGE:
+ self._restore()
+
+ print(f"\n *** pen is moving to {state} ***")
+
+ if state == PenState.PEN_IS_OUT_OF_RANGE:
+ self._old_values = copy.copy(self)
+ self.x = 0
+ self.y = 0
+ self.tipswitch = False
+ self.tippressure = 0
+ self.azimuth = 0
+ self.inrange = False
+ self.width = 0
+ self.height = 0
+ self.invert = False
+ self.eraser = False
+ self.x_tilt = 0
+ self.y_tilt = 0
+ self.twist = 0
+ elif state == PenState.PEN_IS_IN_RANGE:
+ self.tipswitch = False
+ self.inrange = True
+ self.invert = False
+ self.eraser = False
+ elif state == PenState.PEN_IS_IN_CONTACT:
+ self.tipswitch = True
+ self.inrange = True
+ self.invert = False
+ self.eraser = False
+ elif state == PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT:
+ self.tipswitch = False
+ self.inrange = True
+ self.invert = True
+ self.eraser = False
+ elif state == PenState.PEN_IS_ERASING:
+ self.tipswitch = False
+ self.inrange = True
+ self.invert = True
+ self.eraser = True
+
+ self.current_state = state
+
+ def __assert_axis(self, evdev, axis, value):
+ if (
+ axis == libevdev.EV_KEY.BTN_TOOL_RUBBER
+ and evdev.value[libevdev.EV_KEY.BTN_TOOL_RUBBER] is None
+ ):
+ return
+
+ assert (
+ evdev.value[axis] == value
+ ), f"assert evdev.value[{axis}] ({evdev.value[axis]}) != {value}"
+
+ def assert_expected_input_events(self, evdev):
+ assert evdev.value[libevdev.EV_ABS.ABS_X] == self.x
+ assert evdev.value[libevdev.EV_ABS.ABS_Y] == self.y
+ assert self.current_state == PenState.from_evdev(evdev)
+
+
class PenDigitizer(base.UHIDTestDevice):
def __init__(
self,
@@ -486,7 +486,7 @@ class BaseTest:
@pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
@pytest.mark.parametrize(
"state_list",
- [pytest.param(v, id=k) for k, v in Pen.legal_transitions().items()],
+ [pytest.param(v, id=k) for k, v in PenState.legal_transitions().items()],
)
def test_valid_pen_states(self, state_list, scribble):
"""This is the first half of the Windows Pen Implementation state machine:
@@ -498,7 +498,7 @@ class BaseTest:
@pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
@pytest.mark.parametrize(
"state_list",
- [pytest.param(v, id=k) for k, v in Pen.tolerated_transitions().items()],
+ [pytest.param(v, id=k) for k, v in PenState.tolerated_transitions().items()],
)
def test_tolerated_pen_states(self, state_list, scribble):
"""This is not adhering to the Windows Pen Implementation state machine
@@ -515,7 +515,7 @@ class BaseTest:
"state_list",
[
pytest.param(v, id=k)
- for k, v in Pen.legal_transitions_with_invert().items()
+ for k, v in PenState.legal_transitions_with_invert().items()
],
)
def test_valid_invert_pen_states(self, state_list, scribble):
@@ -535,7 +535,7 @@ class BaseTest:
"state_list",
[
pytest.param(v, id=k)
- for k, v in Pen.tolerated_transitions_with_invert().items()
+ for k, v in PenState.tolerated_transitions_with_invert().items()
],
)
def test_tolerated_invert_pen_states(self, state_list, scribble):
@@ -553,7 +553,7 @@ class BaseTest:
@pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
@pytest.mark.parametrize(
"state_list",
- [pytest.param(v, id=k) for k, v in Pen.broken_transitions().items()],
+ [pytest.param(v, id=k) for k, v in PenState.broken_transitions().items()],
)
def test_tolerated_broken_pen_states(self, state_list, scribble):
"""Those tests are definitely not part of the Windows specification.
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 06/12] selftests/hid: tablets: move move_to function to PenDigitizer
2023-11-29 15:24 [PATCH 00/12] selftests/hid: tablets fixes Benjamin Tissoires
` (4 preceding siblings ...)
2023-11-29 15:24 ` [PATCH 05/12] selftests/hid: tablets: move the transitions to PenState Benjamin Tissoires
@ 2023-11-29 15:24 ` Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 07/12] selftests/hid: tablets: do not set invert when the eraser is used Benjamin Tissoires
` (5 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
We can easily subclass PenDigitizer for introducing firmware bugs when
subclassing Pen is harder.
Move move_to from Pen to PenDigitizer so we get that ability
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 97 ++++++++++++------------
1 file changed, 50 insertions(+), 47 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index 18961758e4aa..44a004ca69d1 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -282,7 +282,7 @@ class Pen(object):
self._old_values = None
self.current_state = None
- def _restore(self):
+ def restore(self):
if self._old_values is not None:
for i in [
"x",
@@ -297,50 +297,8 @@ class Pen(object):
]:
setattr(self, i, getattr(self._old_values, i))
- def move_to(self, state):
- # fill in the previous values
- if self.current_state == PenState.PEN_IS_OUT_OF_RANGE:
- self._restore()
-
- print(f"\n *** pen is moving to {state} ***")
-
- if state == PenState.PEN_IS_OUT_OF_RANGE:
- self._old_values = copy.copy(self)
- self.x = 0
- self.y = 0
- self.tipswitch = False
- self.tippressure = 0
- self.azimuth = 0
- self.inrange = False
- self.width = 0
- self.height = 0
- self.invert = False
- self.eraser = False
- self.x_tilt = 0
- self.y_tilt = 0
- self.twist = 0
- elif state == PenState.PEN_IS_IN_RANGE:
- self.tipswitch = False
- self.inrange = True
- self.invert = False
- self.eraser = False
- elif state == PenState.PEN_IS_IN_CONTACT:
- self.tipswitch = True
- self.inrange = True
- self.invert = False
- self.eraser = False
- elif state == PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT:
- self.tipswitch = False
- self.inrange = True
- self.invert = True
- self.eraser = False
- elif state == PenState.PEN_IS_ERASING:
- self.tipswitch = False
- self.inrange = True
- self.invert = True
- self.eraser = True
-
- self.current_state = state
+ def backup(self):
+ self._old_values = copy.copy(self)
def __assert_axis(self, evdev, axis, value):
if (
@@ -384,6 +342,51 @@ class PenDigitizer(base.UHIDTestDevice):
continue
self.fields = [f.usage_name for f in r]
+ def move_to(self, pen, state):
+ # fill in the previous values
+ if pen.current_state == PenState.PEN_IS_OUT_OF_RANGE:
+ pen.restore()
+
+ print(f"\n *** pen is moving to {state} ***")
+
+ if state == PenState.PEN_IS_OUT_OF_RANGE:
+ pen.backup()
+ pen.x = 0
+ pen.y = 0
+ pen.tipswitch = False
+ pen.tippressure = 0
+ pen.azimuth = 0
+ pen.inrange = False
+ pen.width = 0
+ pen.height = 0
+ pen.invert = False
+ pen.eraser = False
+ pen.x_tilt = 0
+ pen.y_tilt = 0
+ pen.twist = 0
+ elif state == PenState.PEN_IS_IN_RANGE:
+ pen.tipswitch = False
+ pen.inrange = True
+ pen.invert = False
+ pen.eraser = False
+ elif state == PenState.PEN_IS_IN_CONTACT:
+ pen.tipswitch = True
+ pen.inrange = True
+ pen.invert = False
+ pen.eraser = False
+ elif state == PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT:
+ pen.tipswitch = False
+ pen.inrange = True
+ pen.invert = True
+ pen.eraser = False
+ elif state == PenState.PEN_IS_ERASING:
+ pen.tipswitch = False
+ pen.inrange = True
+ pen.invert = True
+ pen.eraser = True
+
+ pen.current_state = state
+
def event(self, pen):
rs = []
r = self.create_report(application=self.cur_application, data=pen)
@@ -462,7 +465,7 @@ class BaseTest:
cur_state = PenState.PEN_IS_OUT_OF_RANGE
p = Pen(50, 60)
- p.move_to(PenState.PEN_IS_OUT_OF_RANGE)
+ uhdev.move_to(p, PenState.PEN_IS_OUT_OF_RANGE)
events = self.post(uhdev, p)
self.validate_transitions(cur_state, p, evdev, events)
@@ -475,7 +478,7 @@ class BaseTest:
events = self.post(uhdev, p)
self.validate_transitions(cur_state, p, evdev, events)
assert len(events) >= 3 # X, Y, SYN
- p.move_to(state)
+ uhdev.move_to(p, state)
if scribble and state != PenState.PEN_IS_OUT_OF_RANGE:
p.x += 1
p.y -= 1
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 07/12] selftests/hid: tablets: do not set invert when the eraser is used
2023-11-29 15:24 [PATCH 00/12] selftests/hid: tablets fixes Benjamin Tissoires
` (5 preceding siblings ...)
2023-11-29 15:24 ` [PATCH 06/12] selftests/hid: tablets: move move_to function to PenDigitizer Benjamin Tissoires
@ 2023-11-29 15:24 ` Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 08/12] selftests/hid: tablets: set initial data for tilt/twist Benjamin Tissoires
` (4 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
Turns out that the chart from Microsoft is not exactly what I got here:
when the rubber is used, and is touching the surface, invert can (should)
be set to 0...
[0] https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index 44a004ca69d1..f93dfbb2a3e5 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -382,7 +382,7 @@ class PenDigitizer(base.UHIDTestDevice):
elif state == PenState.PEN_IS_ERASING:
pen.tipswitch = False
pen.inrange = True
- pen.invert = True
+ pen.invert = False
pen.eraser = True
pen.current_state = state
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 08/12] selftests/hid: tablets: set initial data for tilt/twist
2023-11-29 15:24 [PATCH 00/12] selftests/hid: tablets fixes Benjamin Tissoires
` (6 preceding siblings ...)
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 ` Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 09/12] selftests/hid: tablets: add variants of states with buttons Benjamin Tissoires
` (3 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
Avoids getting a null event when these usages are set
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index f93dfbb2a3e5..83f6501fe984 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -276,9 +276,9 @@ class Pen(object):
self.barrelswitch = False
self.invert = False
self.eraser = False
- self.x_tilt = 0
- self.y_tilt = 0
- self.twist = 0
+ self.xtilt = 1
+ self.ytilt = 1
+ self.twist = 1
self._old_values = None
self.current_state = None
@@ -292,8 +292,8 @@ class Pen(object):
"width",
"height",
"twist",
- "x_tilt",
- "y_tilt",
+ "xtilt",
+ "ytilt",
]:
setattr(self, i, getattr(self._old_values, i))
@@ -361,8 +361,8 @@ class PenDigitizer(base.UHIDTestDevice):
pen.height = 0
pen.invert = False
pen.eraser = False
- pen.x_tilt = 0
- pen.y_tilt = 0
+ pen.xtilt = 0
+ pen.ytilt = 0
pen.twist = 0
elif state == PenState.PEN_IS_IN_RANGE:
pen.tipswitch = False
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 09/12] selftests/hid: tablets: add variants of states with buttons
2023-11-29 15:24 [PATCH 00/12] selftests/hid: tablets fixes Benjamin Tissoires
` (7 preceding siblings ...)
2023-11-29 15:24 ` [PATCH 08/12] selftests/hid: tablets: set initial data for tilt/twist Benjamin Tissoires
@ 2023-11-29 15:24 ` Benjamin Tissoires
2023-12-01 5:41 ` Peter Hutterer
2023-11-29 15:24 ` [PATCH 10/12] selftests/hid: tablets: convert the primary button tests Benjamin Tissoires
` (2 subsequent siblings)
11 siblings, 1 reply; 16+ messages in thread
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
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):
+ 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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 09/12] selftests/hid: tablets: add variants of states with buttons
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
0 siblings, 0 replies; 16+ messages in thread
From: Peter Hutterer @ 2023-12-01 5:41 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Benjamin Tissoires, Shuah Khan, linux-input,
linux-kselftest, linux-kernel
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 10/12] selftests/hid: tablets: convert the primary button tests
2023-11-29 15:24 [PATCH 00/12] selftests/hid: tablets fixes Benjamin Tissoires
` (8 preceding siblings ...)
2023-11-29 15:24 ` [PATCH 09/12] selftests/hid: tablets: add variants of states with buttons Benjamin Tissoires
@ 2023-11-29 15:24 ` 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
11 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
We get more descriptive in what we are doing, and also get more
information of what is actually being tested. Instead of having a non
exhaustive button changes that are semi-randomly done, we can describe
all the states we want to test.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 165 ++++++++++-------------
1 file changed, 69 insertions(+), 96 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index 80269d1a0f0a..440a87170db1 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -302,6 +302,55 @@ class PenState(Enum):
),
}
+ @staticmethod
+ def legal_transitions_with_primary_button() -> Dict[str, Tuple["PenState", ...]]:
+ """We revisit the Windows Pen Implementation state machine:
+ we now have a primary button.
+ """
+ return {
+ "hover-button": (PenState.PEN_IS_IN_RANGE_WITH_BUTTON,),
+ "hover-button -> out-of-range": (
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_OUT_OF_RANGE,
+ ),
+ "in-range -> button-press": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ ),
+ "in-range -> button-press -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ "in-range -> touch -> button-press -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_CONTACT_WITH_BUTTON,
+ PenState.PEN_IS_IN_CONTACT,
+ ),
+ "in-range -> touch -> button-press -> release -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_CONTACT_WITH_BUTTON,
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ "in-range -> button-press -> touch -> release -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_IN_CONTACT_WITH_BUTTON,
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ "in-range -> button-press -> touch -> button-release -> release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_IN_CONTACT_WITH_BUTTON,
+ PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ }
+
@staticmethod
def tolerated_transitions() -> Dict[str, Tuple["PenState", ...]]:
"""This is not adhering to the Windows Pen Implementation state machine
@@ -645,7 +694,10 @@ class BaseTest:
@pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
@pytest.mark.parametrize(
"state_list",
- [pytest.param(v, id=k) for k, v in PenState.tolerated_transitions().items()],
+ [
+ pytest.param(v, id=k)
+ for k, v in PenState.tolerated_transitions().items()
+ ],
)
def test_tolerated_pen_states(self, state_list, scribble):
"""This is not adhering to the Windows Pen Implementation state machine
@@ -653,6 +705,22 @@ class BaseTest:
reasons."""
self._test_states(state_list, scribble)
+ @pytest.mark.skip_if_uhdev(
+ lambda uhdev: "Barrel Switch" not in uhdev.fields,
+ "Device not compatible, missing Barrel Switch usage",
+ )
+ @pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
+ @pytest.mark.parametrize(
+ "state_list",
+ [
+ pytest.param(v, id=k)
+ for k, v in PenState.legal_transitions_with_primary_button().items()
+ ],
+ )
+ def test_valid_primary_button_pen_states(self, state_list, scribble):
+ """Rework the transition state machine by adding the primary button."""
+ self._test_states(state_list, scribble)
+
@pytest.mark.skip_if_uhdev(
lambda uhdev: "Invert" not in uhdev.fields,
"Device not compatible, missing Invert usage",
@@ -710,101 +778,6 @@ class BaseTest:
state machine."""
self._test_states(state_list, scribble)
- @pytest.mark.skip_if_uhdev(
- lambda uhdev: "Barrel Switch" not in uhdev.fields,
- "Device not compatible, missing Barrel Switch usage",
- )
- def test_primary_button(self):
- """Primary button (stylus) pressed, reports as pressed even while hovering.
- Actual reporting from the device: hid=TIPSWITCH,BARRELSWITCH,INRANGE (code=TOUCH,STYLUS,PEN):
- { 0, 0, 1 } <- hover
- { 0, 1, 1 } <- primary button pressed
- { 0, 1, 1 } <- liftoff
- { 0, 0, 0 } <- leaves
- """
-
- uhdev = self.uhdev
- evdev = uhdev.get_evdev()
-
- p = Pen(50, 60)
- p.inrange = True
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOOL_PEN, 1) in events
- assert evdev.value[libevdev.EV_ABS.ABS_X] == 50
- assert evdev.value[libevdev.EV_ABS.ABS_Y] == 60
- assert not evdev.value[libevdev.EV_KEY.BTN_STYLUS]
-
- p.barrelswitch = True
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_STYLUS, 1) in events
-
- p.x += 1
- p.y -= 1
- events = self.post(uhdev, p)
- assert len(events) == 3 # X, Y, SYN
- assert libevdev.InputEvent(libevdev.EV_ABS.ABS_X, 51) in events
- assert libevdev.InputEvent(libevdev.EV_ABS.ABS_Y, 59) in events
-
- p.barrelswitch = False
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_STYLUS, 0) in events
-
- p.inrange = False
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOOL_PEN, 0) in events
-
- @pytest.mark.skip_if_uhdev(
- lambda uhdev: "Barrel Switch" not in uhdev.fields,
- "Device not compatible, missing Barrel Switch usage",
- )
- def test_contact_primary_button(self):
- """Primary button (stylus) pressed, reports as pressed even while hovering.
- Actual reporting from the device: hid=TIPSWITCH,BARRELSWITCH,INRANGE (code=TOUCH,STYLUS,PEN):
- { 0, 0, 1 } <- hover
- { 0, 1, 1 } <- primary button pressed
- { 1, 1, 1 } <- touch-down
- { 1, 1, 1 } <- still touch, scribble on the screen
- { 0, 1, 1 } <- liftoff
- { 0, 0, 0 } <- leaves
- """
-
- uhdev = self.uhdev
- evdev = uhdev.get_evdev()
-
- p = Pen(50, 60)
- p.inrange = True
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOOL_PEN, 1) in events
- assert evdev.value[libevdev.EV_ABS.ABS_X] == 50
- assert evdev.value[libevdev.EV_ABS.ABS_Y] == 60
- assert not evdev.value[libevdev.EV_KEY.BTN_STYLUS]
-
- p.barrelswitch = True
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_STYLUS, 1) in events
-
- p.tipswitch = True
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOUCH, 1) in events
- assert evdev.value[libevdev.EV_KEY.BTN_STYLUS]
-
- p.x += 1
- p.y -= 1
- events = self.post(uhdev, p)
- assert len(events) == 3 # X, Y, SYN
- assert libevdev.InputEvent(libevdev.EV_ABS.ABS_X, 51) in events
- assert libevdev.InputEvent(libevdev.EV_ABS.ABS_Y, 59) in events
-
- p.tipswitch = False
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOUCH, 0) in events
-
- p.barrelswitch = False
- p.inrange = False
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOOL_PEN, 0) in events
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_STYLUS, 0) in events
-
class GXTP_pen(PenDigitizer):
def event(self, pen):
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 11/12] selftests/hid: tablets: add a secondary barrel switch test
2023-11-29 15:24 [PATCH 00/12] selftests/hid: tablets fixes Benjamin Tissoires
` (9 preceding siblings ...)
2023-11-29 15:24 ` [PATCH 10/12] selftests/hid: tablets: convert the primary button tests Benjamin Tissoires
@ 2023-11-29 15:24 ` Benjamin Tissoires
2023-11-29 15:24 ` [PATCH 12/12] selftests/hid: tablets: be stricter for some transitions Benjamin Tissoires
11 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
Some tablets report 2 barrel switches. We better test those too.
Use the same transistions description from the primary button tests.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 67 ++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index 440a87170db1..f24cf2e168a4 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -351,6 +351,56 @@ class PenState(Enum):
),
}
+ @staticmethod
+ def legal_transitions_with_secondary_button() -> Dict[str, Tuple["PenState", ...]]:
+ """We revisit the Windows Pen Implementation state machine:
+ we now have a secondary button.
+ Note: we don't looks for 2 buttons interactions.
+ """
+ return {
+ "hover-button": (PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,),
+ "hover-button -> out-of-range": (
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_OUT_OF_RANGE,
+ ),
+ "in-range -> button-press": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ ),
+ "in-range -> button-press -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ "in-range -> touch -> button-press -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_CONTACT,
+ ),
+ "in-range -> touch -> button-press -> release -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ "in-range -> button-press -> touch -> release -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ "in-range -> button-press -> touch -> button-release -> release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ }
+
@staticmethod
def tolerated_transitions() -> Dict[str, Tuple["PenState", ...]]:
"""This is not adhering to the Windows Pen Implementation state machine
@@ -429,6 +479,7 @@ class Pen(object):
self.width = 10
self.height = 10
self.barrelswitch = False
+ self.secondarybarrelswitch = False
self.invert = False
self.eraser = False
self.xtilt = 1
@@ -721,6 +772,22 @@ class BaseTest:
"""Rework the transition state machine by adding the primary button."""
self._test_states(state_list, scribble)
+ @pytest.mark.skip_if_uhdev(
+ lambda uhdev: "Secondary Barrel Switch" not in uhdev.fields,
+ "Device not compatible, missing Secondary Barrel Switch usage",
+ )
+ @pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
+ @pytest.mark.parametrize(
+ "state_list",
+ [
+ pytest.param(v, id=k)
+ for k, v in PenState.legal_transitions_with_secondary_button().items()
+ ],
+ )
+ def test_valid_secondary_button_pen_states(self, state_list, scribble):
+ """Rework the transition state machine by adding the secondary button."""
+ self._test_states(state_list, scribble)
+
@pytest.mark.skip_if_uhdev(
lambda uhdev: "Invert" not in uhdev.fields,
"Device not compatible, missing Invert usage",
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 12/12] selftests/hid: tablets: be stricter for some transitions
2023-11-29 15:24 [PATCH 00/12] selftests/hid: tablets fixes Benjamin Tissoires
` (10 preceding siblings ...)
2023-11-29 15:24 ` [PATCH 11/12] selftests/hid: tablets: add a secondary barrel switch test Benjamin Tissoires
@ 2023-11-29 15:24 ` Benjamin Tissoires
2023-12-01 5:50 ` Peter Hutterer
11 siblings, 1 reply; 16+ messages in thread
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
To accommodate for legacy devices, we rely on the last state of a
transition to be valid:
for example when we test PEN_IS_OUT_OF_RANGE to PEN_IS_IN_CONTACT,
any "normal" device that reports an InRange bit would insert a
PEN_IS_IN_RANGE state between the 2.
This is of course valid, but this solution prevents to detect false
releases emitted by some firmware:
when pressing an "eraser mode" button, they might send an extra
PEN_IS_OUT_OF_RANGE that we may want to filter.
So define 2 sets of transitions: one that is the ideal behavior, and
one that is OK, it won't break user space, but we have serious doubts
if we are doing the right thing. And depending on the test, either
ask only for valid transitions, or tolerate weird ones.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 122 +++++++++++++++++++----
1 file changed, 104 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index f24cf2e168a4..625dd9dcb935 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -109,7 +109,7 @@ class PenState(Enum):
return cls((touch, tool, button))
- def apply(self, events) -> "PenState":
+ def apply(self, events, strict) -> "PenState":
if libevdev.EV_SYN.SYN_REPORT in events:
raise ValueError("EV_SYN is in the event sequence")
touch = self.touch
@@ -148,13 +148,97 @@ class PenState(Enum):
button = None
new_state = PenState((touch, tool, button))
- assert (
- new_state in self.valid_transitions()
- ), f"moving from {self} to {new_state} is forbidden"
+ if strict:
+ assert (
+ new_state in self.valid_transitions()
+ ), f"moving from {self} to {new_state} is forbidden"
+ else:
+ assert (
+ new_state in self.historical_tolerated_transitions()
+ ), f"moving from {self} to {new_state} is forbidden"
return new_state
def valid_transitions(self) -> Tuple["PenState", ...]:
+ """Following the state machine in the URL above.
+
+ Note that those transitions are from the evdev point of view, not HID"""
+ if self == PenState.PEN_IS_OUT_OF_RANGE:
+ 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,
+ )
+
+ 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,
+ )
+
+ if self == PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT:
+ return (
+ PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT,
+ PenState.PEN_IS_OUT_OF_RANGE,
+ PenState.PEN_IS_ERASING,
+ )
+
+ if self == PenState.PEN_IS_ERASING:
+ return (
+ PenState.PEN_IS_ERASING,
+ PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT,
+ )
+
+ 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,
+ )
+
+ 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,
+ )
+
+ return tuple()
+
+ def historical_tolerated_transitions(self) -> Tuple["PenState", ...]:
"""Following the state machine in the URL above, with a couple of addition
for skipping the in-range state, due to historical reasons.
@@ -678,10 +762,12 @@ class BaseTest:
self.debug_reports(r, uhdev, events)
return events
- def validate_transitions(self, from_state, pen, evdev, events):
+ def validate_transitions(self, from_state, pen, evdev, events, allow_intermediate_states):
# check that the final state is correct
pen.assert_expected_input_events(evdev)
+ state = from_state
+
# check that the transitions are valid
sync_events = []
while libevdev.InputEvent(libevdev.EV_SYN.SYN_REPORT) in events:
@@ -691,12 +777,12 @@ class BaseTest:
events = events[idx + 1 :]
# now check for a valid transition
- from_state = from_state.apply(sync_events)
+ state = state.apply(sync_events, not allow_intermediate_states)
if events:
- from_state = from_state.apply(sync_events)
+ state = state.apply(sync_events, not allow_intermediate_states)
- def _test_states(self, state_list, scribble):
+ def _test_states(self, state_list, scribble, allow_intermediate_states):
"""Internal method to test against a list of
transition between states.
state_list is a list of PenState objects
@@ -711,7 +797,7 @@ class BaseTest:
p = Pen(50, 60)
uhdev.move_to(p, PenState.PEN_IS_OUT_OF_RANGE)
events = self.post(uhdev, p)
- self.validate_transitions(cur_state, p, evdev, events)
+ self.validate_transitions(cur_state, p, evdev, events, allow_intermediate_states)
cur_state = p.current_state
@@ -720,14 +806,14 @@ class BaseTest:
p.x += 1
p.y -= 1
events = self.post(uhdev, p)
- self.validate_transitions(cur_state, p, evdev, events)
+ self.validate_transitions(cur_state, p, evdev, events, allow_intermediate_states)
assert len(events) >= 3 # X, Y, SYN
uhdev.move_to(p, state)
if scribble and state != PenState.PEN_IS_OUT_OF_RANGE:
p.x += 1
p.y -= 1
events = self.post(uhdev, p)
- self.validate_transitions(cur_state, p, evdev, events)
+ self.validate_transitions(cur_state, p, evdev, events, allow_intermediate_states)
cur_state = p.current_state
@pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
@@ -740,7 +826,7 @@ class BaseTest:
we don't have Invert nor Erase bits, so just move in/out-of-range or proximity.
https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
"""
- self._test_states(state_list, scribble)
+ self._test_states(state_list, scribble, False)
@pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
@pytest.mark.parametrize(
@@ -754,7 +840,7 @@ class BaseTest:
"""This is not adhering to the Windows Pen Implementation state machine
but we should expect the kernel to behave properly, mostly for historical
reasons."""
- self._test_states(state_list, scribble)
+ self._test_states(state_list, scribble, True)
@pytest.mark.skip_if_uhdev(
lambda uhdev: "Barrel Switch" not in uhdev.fields,
@@ -770,7 +856,7 @@ class BaseTest:
)
def test_valid_primary_button_pen_states(self, state_list, scribble):
"""Rework the transition state machine by adding the primary button."""
- self._test_states(state_list, scribble)
+ self._test_states(state_list, scribble, False)
@pytest.mark.skip_if_uhdev(
lambda uhdev: "Secondary Barrel Switch" not in uhdev.fields,
@@ -786,7 +872,7 @@ class BaseTest:
)
def test_valid_secondary_button_pen_states(self, state_list, scribble):
"""Rework the transition state machine by adding the secondary button."""
- self._test_states(state_list, scribble)
+ self._test_states(state_list, scribble, False)
@pytest.mark.skip_if_uhdev(
lambda uhdev: "Invert" not in uhdev.fields,
@@ -806,7 +892,7 @@ class BaseTest:
to erase.
https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
"""
- self._test_states(state_list, scribble)
+ self._test_states(state_list, scribble, False)
@pytest.mark.skip_if_uhdev(
lambda uhdev: "Invert" not in uhdev.fields,
@@ -826,7 +912,7 @@ class BaseTest:
to erase.
https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
"""
- self._test_states(state_list, scribble)
+ self._test_states(state_list, scribble, True)
@pytest.mark.skip_if_uhdev(
lambda uhdev: "Invert" not in uhdev.fields,
@@ -843,7 +929,7 @@ class BaseTest:
For example, a pen that has the eraser button might wobble between
touching and erasing if the tablet doesn't enforce the Windows
state machine."""
- self._test_states(state_list, scribble)
+ self._test_states(state_list, scribble, True)
class GXTP_pen(PenDigitizer):
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 12/12] selftests/hid: tablets: be stricter for some transitions
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
0 siblings, 0 replies; 16+ messages in thread
From: Peter Hutterer @ 2023-12-01 5:50 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Benjamin Tissoires, Shuah Khan, linux-input,
linux-kselftest, linux-kernel
On Wed, Nov 29, 2023 at 04:24:37PM +0100, Benjamin Tissoires wrote:
> To accommodate for legacy devices, we rely on the last state of a
> transition to be valid:
> for example when we test PEN_IS_OUT_OF_RANGE to PEN_IS_IN_CONTACT,
> any "normal" device that reports an InRange bit would insert a
> PEN_IS_IN_RANGE state between the 2.
>
> This is of course valid, but this solution prevents to detect false
> releases emitted by some firmware:
> when pressing an "eraser mode" button, they might send an extra
> PEN_IS_OUT_OF_RANGE that we may want to filter.
>
> So define 2 sets of transitions: one that is the ideal behavior, and
> one that is OK, it won't break user space, but we have serious doubts
> if we are doing the right thing. And depending on the test, either
> ask only for valid transitions, or tolerate weird ones.
>
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
> tools/testing/selftests/hid/tests/test_tablet.py | 122 +++++++++++++++++++----
> 1 file changed, 104 insertions(+), 18 deletions(-)
>
> diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
> index f24cf2e168a4..625dd9dcb935 100644
> --- a/tools/testing/selftests/hid/tests/test_tablet.py
> +++ b/tools/testing/selftests/hid/tests/test_tablet.py
> @@ -109,7 +109,7 @@ class PenState(Enum):
>
> return cls((touch, tool, button))
>
> - def apply(self, events) -> "PenState":
> + def apply(self, events, strict) -> "PenState":
fwiw, if you're doing type annotations anyway, it'd be nice to do them
for args as well, `strict: bool` in this case.
> if libevdev.EV_SYN.SYN_REPORT in events:
> raise ValueError("EV_SYN is in the event sequence")
> touch = self.touch
> @@ -148,13 +148,97 @@ class PenState(Enum):
> button = None
>
> new_state = PenState((touch, tool, button))
> - assert (
> - new_state in self.valid_transitions()
> - ), f"moving from {self} to {new_state} is forbidden"
> + if strict:
> + assert (
> + new_state in self.valid_transitions()
> + ), f"moving from {self} to {new_state} is forbidden"
> + else:
> + assert (
> + new_state in self.historical_tolerated_transitions()
> + ), f"moving from {self} to {new_state} is forbidden"
>
> return new_state
>
> def valid_transitions(self) -> Tuple["PenState", ...]:
> + """Following the state machine in the URL above.
> +
> + Note that those transitions are from the evdev point of view, not HID"""
> + if self == PenState.PEN_IS_OUT_OF_RANGE:
> + 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,
> + )
> +
> + 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,
> + )
> +
> + if self == PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT:
> + return (
> + PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT,
> + PenState.PEN_IS_OUT_OF_RANGE,
> + PenState.PEN_IS_ERASING,
> + )
> +
> + if self == PenState.PEN_IS_ERASING:
> + return (
> + PenState.PEN_IS_ERASING,
> + PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT,
> + )
> +
> + 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,
> + )
> +
> + 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,
> + )
> +
> + return tuple()
> +
> + def historical_tolerated_transitions(self) -> Tuple["PenState", ...]:
s/historically/ to be grammatically correct, I guess.
> """Following the state machine in the URL above, with a couple of addition
> for skipping the in-range state, due to historical reasons.
>
> @@ -678,10 +762,12 @@ class BaseTest:
> self.debug_reports(r, uhdev, events)
> return events
>
> - def validate_transitions(self, from_state, pen, evdev, events):
> + def validate_transitions(self, from_state, pen, evdev, events, allow_intermediate_states):
> # check that the final state is correct
> pen.assert_expected_input_events(evdev)
>
> + state = from_state
> +
> # check that the transitions are valid
> sync_events = []
> while libevdev.InputEvent(libevdev.EV_SYN.SYN_REPORT) in events:
> @@ -691,12 +777,12 @@ class BaseTest:
> events = events[idx + 1 :]
>
> # now check for a valid transition
> - from_state = from_state.apply(sync_events)
> + state = state.apply(sync_events, not allow_intermediate_states)
>
> if events:
> - from_state = from_state.apply(sync_events)
> + state = state.apply(sync_events, not allow_intermediate_states)
>
> - def _test_states(self, state_list, scribble):
> + def _test_states(self, state_list, scribble, allow_intermediate_states):
> """Internal method to test against a list of
> transition between states.
> state_list is a list of PenState objects
> @@ -711,7 +797,7 @@ class BaseTest:
> p = Pen(50, 60)
> uhdev.move_to(p, PenState.PEN_IS_OUT_OF_RANGE)
> events = self.post(uhdev, p)
> - self.validate_transitions(cur_state, p, evdev, events)
> + self.validate_transitions(cur_state, p, evdev, events, allow_intermediate_states)
>
> cur_state = p.current_state
>
> @@ -720,14 +806,14 @@ class BaseTest:
> p.x += 1
> p.y -= 1
> events = self.post(uhdev, p)
> - self.validate_transitions(cur_state, p, evdev, events)
> + self.validate_transitions(cur_state, p, evdev, events, allow_intermediate_states)
> assert len(events) >= 3 # X, Y, SYN
> uhdev.move_to(p, state)
> if scribble and state != PenState.PEN_IS_OUT_OF_RANGE:
> p.x += 1
> p.y -= 1
> events = self.post(uhdev, p)
> - self.validate_transitions(cur_state, p, evdev, events)
> + self.validate_transitions(cur_state, p, evdev, events, allow_intermediate_states)
> cur_state = p.current_state
>
> @pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
> @@ -740,7 +826,7 @@ class BaseTest:
> we don't have Invert nor Erase bits, so just move in/out-of-range or proximity.
> https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
> """
> - self._test_states(state_list, scribble)
> + self._test_states(state_list, scribble, False)
not everyone's cup of tea but code like this becomes more immediately
readable as:
self._test_states(state_list, scribble, allow_intermediate_states=False)
Anyway, this looks good to me (esp the intention) and is
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Cheers,
Peter
>
> @pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
> @pytest.mark.parametrize(
> @@ -754,7 +840,7 @@ class BaseTest:
> """This is not adhering to the Windows Pen Implementation state machine
> but we should expect the kernel to behave properly, mostly for historical
> reasons."""
> - self._test_states(state_list, scribble)
> + self._test_states(state_list, scribble, True)
>
> @pytest.mark.skip_if_uhdev(
> lambda uhdev: "Barrel Switch" not in uhdev.fields,
> @@ -770,7 +856,7 @@ class BaseTest:
> )
> def test_valid_primary_button_pen_states(self, state_list, scribble):
> """Rework the transition state machine by adding the primary button."""
> - self._test_states(state_list, scribble)
> + self._test_states(state_list, scribble, False)
>
> @pytest.mark.skip_if_uhdev(
> lambda uhdev: "Secondary Barrel Switch" not in uhdev.fields,
> @@ -786,7 +872,7 @@ class BaseTest:
> )
> def test_valid_secondary_button_pen_states(self, state_list, scribble):
> """Rework the transition state machine by adding the secondary button."""
> - self._test_states(state_list, scribble)
> + self._test_states(state_list, scribble, False)
>
> @pytest.mark.skip_if_uhdev(
> lambda uhdev: "Invert" not in uhdev.fields,
> @@ -806,7 +892,7 @@ class BaseTest:
> to erase.
> https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
> """
> - self._test_states(state_list, scribble)
> + self._test_states(state_list, scribble, False)
>
> @pytest.mark.skip_if_uhdev(
> lambda uhdev: "Invert" not in uhdev.fields,
> @@ -826,7 +912,7 @@ class BaseTest:
> to erase.
> https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
> """
> - self._test_states(state_list, scribble)
> + self._test_states(state_list, scribble, True)
>
> @pytest.mark.skip_if_uhdev(
> lambda uhdev: "Invert" not in uhdev.fields,
> @@ -843,7 +929,7 @@ class BaseTest:
> For example, a pen that has the eraser button might wobble between
> touching and erasing if the tablet doesn't enforce the Windows
> state machine."""
> - self._test_states(state_list, scribble)
> + self._test_states(state_list, scribble, True)
>
>
> class GXTP_pen(PenDigitizer):
>
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread