From 887f8094b3352127d1bc68f768774e97acf4e7fa Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 6 Dec 2023 11:45:52 +0100 Subject: [PATCH 01/16] selftests/hid: vmtest.sh: update vm2c and container 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. Reviewed-by: Peter Hutterer Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231206-wip-selftests-v2-1-c0350c2f5986@kernel.org Signed-off-by: Benjamin Tissoires --- 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 4da48bf6b328a..301b4e1623368 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" From 46bc0277c2507338d98e166826afec330962309e Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 6 Dec 2023 11:45:53 +0100 Subject: [PATCH 02/16] selftests/hid: vmtest.sh: allow finer control on the build steps 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. Reviewed-by: Peter Hutterer Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231206-wip-selftests-v2-2-c0350c2f5986@kernel.org Signed-off-by: Benjamin Tissoires --- 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 301b4e1623368..db534e9099a8a 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 <] -- [] +Usage: $0 [-j N] [-s] [-b] [-d ] -- [] 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 only the 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 "$@" From 110292a77f7c8d11eaa472e65f6a2084b25be2db Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 6 Dec 2023 11:45:54 +0100 Subject: [PATCH 03/16] selftests/hid: base: allow for multiple skip_if_uhdev 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 Reviewed-by: Peter Hutterer Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231206-wip-selftests-v2-3-c0350c2f5986@kernel.org Signed-off-by: Benjamin Tissoires --- 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 1305cfc9646e0..5d9c26dfc4605 100644 --- a/tools/testing/selftests/hid/tests/base.py +++ b/tools/testing/selftests/hid/tests/base.py @@ -238,8 +238,7 @@ def context(self, new_uhdev, request): 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): From b5edacf79c8e1990200f9392e6354583298e8765 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 6 Dec 2023 11:45:55 +0100 Subject: [PATCH 04/16] selftests/hid: tablets: remove unused class Looks like this is a leftover Reviewed-by: Peter Hutterer Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231206-wip-selftests-v2-4-c0350c2f5986@kernel.org Signed-off-by: Benjamin Tissoires --- 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 303ffff9ee8dc..cd9c1269afa6a 100644 --- a/tools/testing/selftests/hid/tests/test_tablet.py +++ b/tools/testing/selftests/hid/tests/test_tablet.py @@ -133,10 +133,6 @@ def valid_transitions(self) -> Tuple["PenState", ...]: return tuple() -class Data(object): - pass - - class Pen(object): def __init__(self, x, y): self.x = x From d52f52069fed639113b1014cde5eff8902d3064d Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 6 Dec 2023 11:45:56 +0100 Subject: [PATCH 05/16] selftests/hid: tablets: move the transitions to PenState 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. Reviewed-by: Peter Hutterer Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231206-wip-selftests-v2-5-c0350c2f5986@kernel.org Signed-off-by: Benjamin Tissoires --- .../selftests/hid/tests/test_tablet.py | 215 +++++++++--------- 1 file changed, 109 insertions(+), 106 deletions(-) diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py index cd9c1269afa6a..ddf28c2450460 100644 --- a/tools/testing/selftests/hid/tests/test_tablet.py +++ b/tools/testing/selftests/hid/tests/test_tablet.py @@ -132,104 +132,8 @@ def valid_transitions(self) -> Tuple["PenState", ...]: 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 @@ def legal_transitions() -> Dict[str, Tuple[PenState, ...]]: } @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 @@ def legal_transitions_with_invert() -> Dict[str, Tuple[PenState, ...]]: } @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 @@ def tolerated_transitions() -> Dict[str, Tuple[PenState, ...]]: } @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 @@ def tolerated_transitions_with_invert() -> Dict[str, Tuple[PenState, ...]]: } @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 @@ def broken_transitions() -> Dict[str, Tuple[PenState, ...]]: } +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 @@ def _test_states(self, state_list, scribble): @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,10 @@ def test_valid_pen_states(self, state_list, scribble): @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 +518,7 @@ def test_tolerated_pen_states(self, state_list, scribble): "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 +538,7 @@ def test_valid_invert_pen_states(self, state_list, scribble): "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 +556,7 @@ def test_tolerated_invert_pen_states(self, state_list, scribble): @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. From 881ccc36b42670ebbd5fb32a79b239ce1136e84d Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 6 Dec 2023 11:45:57 +0100 Subject: [PATCH 06/16] selftests/hid: tablets: move move_to function to PenDigitizer 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 Reviewed-by: Peter Hutterer Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231206-wip-selftests-v2-6-c0350c2f5986@kernel.org Signed-off-by: Benjamin Tissoires --- .../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 ddf28c2450460..27260dc02cc4d 100644 --- a/tools/testing/selftests/hid/tests/test_tablet.py +++ b/tools/testing/selftests/hid/tests/test_tablet.py @@ -282,7 +282,7 @@ def __init__(self, x, y): 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 @@ def _restore(self): ]: 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 @@ def __init__( 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 @@ def _test_states(self, state_list, scribble): 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 @@ def _test_states(self, state_list, scribble): 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 From d8d7aa2266a7957e24cf05392b2b899be1031ed4 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 6 Dec 2023 11:45:58 +0100 Subject: [PATCH 07/16] selftests/hid: tablets: do not set invert when the eraser is used 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 Reviewed-by: Peter Hutterer Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231206-wip-selftests-v2-7-c0350c2f5986@kernel.org Signed-off-by: Benjamin Tissoires --- 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 27260dc02cc4d..cb3955bf0ec58 100644 --- a/tools/testing/selftests/hid/tests/test_tablet.py +++ b/tools/testing/selftests/hid/tests/test_tablet.py @@ -382,7 +382,7 @@ def move_to(self, pen, state): 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 From e08e493ff9619daab9c11e61a80c604895a4d671 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 6 Dec 2023 11:45:59 +0100 Subject: [PATCH 08/16] selftests/hid: tablets: set initial data for tilt/twist Avoids getting a null event when these usages are set Reviewed-by: Peter Hutterer Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231206-wip-selftests-v2-8-c0350c2f5986@kernel.org Signed-off-by: Benjamin Tissoires --- 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 cb3955bf0ec58..0ddf82695ed91 100644 --- a/tools/testing/selftests/hid/tests/test_tablet.py +++ b/tools/testing/selftests/hid/tests/test_tablet.py @@ -276,9 +276,9 @@ def __init__(self, x, y): 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 @@ def restore(self): "width", "height", "twist", - "x_tilt", - "y_tilt", + "xtilt", + "ytilt", ]: setattr(self, i, getattr(self._old_values, i)) @@ -361,8 +361,8 @@ def move_to(self, pen, state): 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 From 83912f83fabcb5086613e6382756750390ed48aa Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 6 Dec 2023 11:46:00 +0100 Subject: [PATCH 09/16] selftests/hid: tablets: define the elements of PenState This introduces a little bit more readability by not using the raw values but a dedicated Enum Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231206-wip-selftests-v2-9-c0350c2f5986@kernel.org Signed-off-by: Benjamin Tissoires --- .../selftests/hid/tests/test_tablet.py | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py index 0ddf82695ed91..cec65294c9ecb 100644 --- a/tools/testing/selftests/hid/tests/test_tablet.py +++ b/tools/testing/selftests/hid/tests/test_tablet.py @@ -13,40 +13,52 @@ import libevdev import logging import pytest -from typing import Dict, Tuple +from typing import Dict, Optional, Tuple logger = logging.getLogger("hidtools.test.tablet") +class BtnTouch(Enum): + """Represents whether the BTN_TOUCH event is set to True or False""" + + DOWN = True + UP = False + + +class ToolType(Enum): + PEN = libevdev.EV_KEY.BTN_TOOL_PEN + RUBBER = libevdev.EV_KEY.BTN_TOOL_RUBBER + + 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) + PEN_IS_OUT_OF_RANGE = BtnTouch.UP, None + PEN_IS_IN_RANGE = BtnTouch.UP, ToolType.PEN + PEN_IS_IN_CONTACT = BtnTouch.DOWN, ToolType.PEN + PEN_IS_IN_RANGE_WITH_ERASING_INTENT = BtnTouch.UP, ToolType.RUBBER + PEN_IS_ERASING = BtnTouch.DOWN, ToolType.RUBBER - def __init__(self, touch, tool): + def __init__(self, touch: BtnTouch, tool: Optional[ToolType]): self.touch = touch self.tool = tool @classmethod def from_evdev(cls, evdev) -> "PenState": - touch = bool(evdev.value[libevdev.EV_KEY.BTN_TOUCH]) + touch = BtnTouch(evdev.value[libevdev.EV_KEY.BTN_TOUCH]) tool = None if ( evdev.value[libevdev.EV_KEY.BTN_TOOL_RUBBER] and not evdev.value[libevdev.EV_KEY.BTN_TOOL_PEN] ): - tool = libevdev.EV_KEY.BTN_TOOL_RUBBER + tool = ToolType(libevdev.EV_KEY.BTN_TOOL_RUBBER) elif ( evdev.value[libevdev.EV_KEY.BTN_TOOL_PEN] and not evdev.value[libevdev.EV_KEY.BTN_TOOL_RUBBER] ): - tool = libevdev.EV_KEY.BTN_TOOL_PEN + tool = ToolType(libevdev.EV_KEY.BTN_TOOL_PEN) elif ( evdev.value[libevdev.EV_KEY.BTN_TOOL_PEN] or evdev.value[libevdev.EV_KEY.BTN_TOOL_RUBBER] @@ -68,7 +80,7 @@ def apply(self, events) -> "PenState": if touch_found: raise ValueError(f"duplicated BTN_TOUCH in {events}") touch_found = True - touch = bool(ev.value) + touch = BtnTouch(ev.value) elif ev in ( libevdev.InputEvent(libevdev.EV_KEY.BTN_TOOL_PEN), libevdev.InputEvent(libevdev.EV_KEY.BTN_TOOL_RUBBER), @@ -77,7 +89,7 @@ def apply(self, events) -> "PenState": raise ValueError(f"duplicated BTN_TOOL_* in {events}") tool_found = True if ev.value: - tool = ev.code + tool = ToolType(ev.code) else: tool = None From 74452d6329be73dd2f5562005e5eef2c7bda7c5b Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 6 Dec 2023 11:46:01 +0100 Subject: [PATCH 10/16] selftests/hid: tablets: add variants of states with buttons 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. Reviewed-by: Peter Hutterer Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231206-wip-selftests-v2-10-c0350c2f5986@kernel.org Signed-off-by: Benjamin Tissoires --- .../selftests/hid/tests/test_tablet.py | 173 ++++++++++++++++-- 1 file changed, 160 insertions(+), 13 deletions(-) diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py index cec65294c9ecb..a77534f23c75b 100644 --- a/tools/testing/selftests/hid/tests/test_tablet.py +++ b/tools/testing/selftests/hid/tests/test_tablet.py @@ -30,25 +30,72 @@ class ToolType(Enum): RUBBER = libevdev.EV_KEY.BTN_TOOL_RUBBER +class BtnPressed(Enum): + """Represents whether a button is pressed on the stylus""" + + PRIMARY_PRESSED = libevdev.EV_KEY.BTN_STYLUS + SECONDARY_PRESSED = libevdev.EV_KEY.BTN_STYLUS2 + + 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 = BtnTouch.UP, None - PEN_IS_IN_RANGE = BtnTouch.UP, ToolType.PEN - PEN_IS_IN_CONTACT = BtnTouch.DOWN, ToolType.PEN - PEN_IS_IN_RANGE_WITH_ERASING_INTENT = BtnTouch.UP, ToolType.RUBBER - PEN_IS_ERASING = BtnTouch.DOWN, ToolType.RUBBER + We extend it with the various buttons when we need to check them. + """ - def __init__(self, touch: BtnTouch, tool: Optional[ToolType]): + PEN_IS_OUT_OF_RANGE = BtnTouch.UP, None, None + PEN_IS_IN_RANGE = BtnTouch.UP, ToolType.PEN, None + PEN_IS_IN_RANGE_WITH_BUTTON = BtnTouch.UP, ToolType.PEN, BtnPressed.PRIMARY_PRESSED + PEN_IS_IN_RANGE_WITH_SECOND_BUTTON = ( + BtnTouch.UP, + ToolType.PEN, + BtnPressed.SECONDARY_PRESSED, + ) + PEN_IS_IN_CONTACT = BtnTouch.DOWN, ToolType.PEN, None + PEN_IS_IN_CONTACT_WITH_BUTTON = ( + BtnTouch.DOWN, + ToolType.PEN, + BtnPressed.PRIMARY_PRESSED, + ) + PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON = ( + BtnTouch.DOWN, + ToolType.PEN, + BtnPressed.SECONDARY_PRESSED, + ) + PEN_IS_IN_RANGE_WITH_ERASING_INTENT = BtnTouch.UP, ToolType.RUBBER, None + PEN_IS_IN_RANGE_WITH_ERASING_INTENT_WITH_BUTTON = ( + BtnTouch.UP, + ToolType.RUBBER, + BtnPressed.PRIMARY_PRESSED, + ) + PEN_IS_IN_RANGE_WITH_ERASING_INTENT_WITH_SECOND_BUTTON = ( + BtnTouch.UP, + ToolType.RUBBER, + BtnPressed.SECONDARY_PRESSED, + ) + PEN_IS_ERASING = BtnTouch.DOWN, ToolType.RUBBER, None + PEN_IS_ERASING_WITH_BUTTON = ( + BtnTouch.DOWN, + ToolType.RUBBER, + BtnPressed.PRIMARY_PRESSED, + ) + PEN_IS_ERASING_WITH_SECOND_BUTTON = ( + BtnTouch.DOWN, + ToolType.RUBBER, + BtnPressed.SECONDARY_PRESSED, + ) + + def __init__(self, touch: BtnTouch, tool: Optional[ToolType], button: Optional[BtnPressed]): self.touch = touch self.tool = tool + self.button = button @classmethod def from_evdev(cls, evdev) -> "PenState": touch = BtnTouch(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] @@ -65,7 +112,17 @@ def from_evdev(cls, evdev) -> "PenState": ): 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: @@ -74,6 +131,8 @@ def apply(self, events) -> "PenState": 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): @@ -88,12 +147,22 @@ def apply(self, events) -> "PenState": if tool_found: raise ValueError(f"duplicated BTN_TOOL_* in {events}") tool_found = True - if ev.value: - tool = ToolType(ev.code) - else: - tool = None + tool = ToolType(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" @@ -109,14 +178,20 @@ def valid_transitions(self) -> Tuple["PenState", ...]: 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, ) @@ -124,6 +199,8 @@ def valid_transitions(self) -> Tuple["PenState", ...]: 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, ) @@ -142,6 +219,38 @@ def valid_transitions(self) -> Tuple["PenState", ...]: 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 @@ -376,26 +485,64 @@ def move_to(self, pen, state): 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 From 1f01537ef17efec97a8936c62e4ffa704cab7c06 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 6 Dec 2023 11:46:02 +0100 Subject: [PATCH 11/16] selftests/hid: tablets: convert the primary button tests 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. Reviewed-by: Peter Hutterer Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231206-wip-selftests-v2-11-c0350c2f5986@kernel.org Signed-off-by: Benjamin Tissoires --- .../selftests/hid/tests/test_tablet.py | 160 +++++++----------- 1 file changed, 65 insertions(+), 95 deletions(-) diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py index a77534f23c75b..20a7a7fdcd9d2 100644 --- a/tools/testing/selftests/hid/tests/test_tablet.py +++ b/tools/testing/selftests/hid/tests/test_tablet.py @@ -317,6 +317,55 @@ def legal_transitions_with_invert() -> Dict[str, Tuple["PenState", ...]]: ), } + @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 @@ -671,6 +720,22 @@ def test_tolerated_pen_states(self, state_list, scribble): 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", @@ -728,101 +793,6 @@ def test_tolerated_broken_pen_states(self, state_list, scribble): 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): From 76df1f72fb258cc7da6eff5dd8db95f4e2856517 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 6 Dec 2023 11:46:03 +0100 Subject: [PATCH 12/16] selftests/hid: tablets: add a secondary barrel switch test Some tablets report 2 barrel switches. We better test those too. Use the same transistions description from the primary button tests. Reviewed-by: Peter Hutterer Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231206-wip-selftests-v2-12-c0350c2f5986@kernel.org Signed-off-by: Benjamin Tissoires --- .../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 20a7a7fdcd9d2..a82db66264c59 100644 --- a/tools/testing/selftests/hid/tests/test_tablet.py +++ b/tools/testing/selftests/hid/tests/test_tablet.py @@ -366,6 +366,56 @@ def legal_transitions_with_primary_button() -> Dict[str, Tuple["PenState", ...]] ), } + @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 @@ -444,6 +494,7 @@ def __init__(self, x, y): self.width = 10 self.height = 10 self.barrelswitch = False + self.secondarybarrelswitch = False self.invert = False self.eraser = False self.xtilt = 1 @@ -736,6 +787,22 @@ 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: "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", From ab9b82909e9baa5b559d6457487525cfd4df2738 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 6 Dec 2023 11:46:04 +0100 Subject: [PATCH 13/16] selftests/hid: tablets: be stricter for some transitions 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. Reviewed-by: Peter Hutterer Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231206-wip-selftests-v2-13-c0350c2f5986@kernel.org Signed-off-by: Benjamin Tissoires --- .../selftests/hid/tests/test_tablet.py | 132 +++++++++++++++--- 1 file changed, 113 insertions(+), 19 deletions(-) diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py index a82db66264c59..9374bd7524efa 100644 --- a/tools/testing/selftests/hid/tests/test_tablet.py +++ b/tools/testing/selftests/hid/tests/test_tablet.py @@ -13,7 +13,7 @@ import libevdev import logging import pytest -from typing import Dict, Optional, Tuple +from typing import Dict, List, Optional, Tuple logger = logging.getLogger("hidtools.test.tablet") @@ -124,7 +124,7 @@ def from_evdev(cls, evdev) -> "PenState": return cls((touch, tool, button)) - def apply(self, events) -> "PenState": + def apply(self, events: List[libevdev.InputEvent], strict: bool) -> "PenState": if libevdev.EV_SYN.SYN_REPORT in events: raise ValueError("EV_SYN is in the event sequence") touch = self.touch @@ -163,13 +163,97 @@ def apply(self, events) -> "PenState": 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.historically_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 historically_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. @@ -693,10 +777,14 @@ def post(self, uhdev, pen): 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: @@ -706,12 +794,12 @@ def validate_transitions(self, from_state, pen, evdev, events): 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 @@ -726,7 +814,9 @@ def _test_states(self, state_list, scribble): 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 @@ -735,14 +825,18 @@ def _test_states(self, state_list, scribble): 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"]) @@ -755,7 +849,7 @@ def test_valid_pen_states(self, state_list, scribble): 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, allow_intermediate_states=False) @pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"]) @pytest.mark.parametrize( @@ -769,7 +863,7 @@ def test_tolerated_pen_states(self, state_list, scribble): """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, allow_intermediate_states=True) @pytest.mark.skip_if_uhdev( lambda uhdev: "Barrel Switch" not in uhdev.fields, @@ -785,7 +879,7 @@ def test_tolerated_pen_states(self, state_list, scribble): ) 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, allow_intermediate_states=False) @pytest.mark.skip_if_uhdev( lambda uhdev: "Secondary Barrel Switch" not in uhdev.fields, @@ -801,7 +895,7 @@ def test_valid_primary_button_pen_states(self, state_list, scribble): ) 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, allow_intermediate_states=False) @pytest.mark.skip_if_uhdev( lambda uhdev: "Invert" not in uhdev.fields, @@ -821,7 +915,7 @@ def test_valid_invert_pen_states(self, state_list, scribble): 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, allow_intermediate_states=False) @pytest.mark.skip_if_uhdev( lambda uhdev: "Invert" not in uhdev.fields, @@ -841,7 +935,7 @@ def test_tolerated_invert_pen_states(self, state_list, scribble): 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, allow_intermediate_states=True) @pytest.mark.skip_if_uhdev( lambda uhdev: "Invert" not in uhdev.fields, @@ -858,7 +952,7 @@ def test_tolerated_broken_pen_states(self, state_list, scribble): 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, allow_intermediate_states=True) class GXTP_pen(PenDigitizer): From ed5bc56cedca19ac429533aa527bce5287ab0a5a Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 6 Dec 2023 11:46:05 +0100 Subject: [PATCH 14/16] selftests/hid: fix mypy complains No code change, only typing information added/ignored Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231206-wip-selftests-v2-14-c0350c2f5986@kernel.org Signed-off-by: Benjamin Tissoires --- tools/testing/selftests/hid/tests/base.py | 4 ++-- tools/testing/selftests/hid/tests/test_tablet.py | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/testing/selftests/hid/tests/base.py b/tools/testing/selftests/hid/tests/base.py index 5d9c26dfc4605..51433063b2270 100644 --- a/tools/testing/selftests/hid/tests/base.py +++ b/tools/testing/selftests/hid/tests/base.py @@ -14,7 +14,7 @@ from hidtools.device.base_device import BaseDevice, EvdevMatch, SysfsFile from pathlib import Path -from typing import Final +from typing import Final, List, Tuple logger = logging.getLogger("hidtools.test.base") @@ -155,7 +155,7 @@ class TestUhid(object): # if any module is not available (not compiled), the test will skip. # Each element is a tuple '(kernel driver name, kernel module)', # for example ("playstation", "hid-playstation") - kernel_modules = [] + kernel_modules: List[Tuple[str, str]] = [] def assertInputEventsIn(self, expected_events, effective_events): effective_events = effective_events.copy() diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py index 9374bd7524efa..dc8b0fe9e7f36 100644 --- a/tools/testing/selftests/hid/tests/test_tablet.py +++ b/tools/testing/selftests/hid/tests/test_tablet.py @@ -87,9 +87,9 @@ class PenState(Enum): ) def __init__(self, touch: BtnTouch, tool: Optional[ToolType], button: Optional[BtnPressed]): - self.touch = touch - self.tool = tool - self.button = button + self.touch = touch # type: ignore + self.tool = tool # type: ignore + self.button = button # type: ignore @classmethod def from_evdev(cls, evdev) -> "PenState": @@ -122,7 +122,7 @@ def from_evdev(cls, evdev) -> "PenState": if tool is None: button = None - return cls((touch, tool, button)) + return cls((touch, tool, button)) # type: ignore def apply(self, events: List[libevdev.InputEvent], strict: bool) -> "PenState": if libevdev.EV_SYN.SYN_REPORT in events: @@ -162,7 +162,7 @@ def apply(self, events: List[libevdev.InputEvent], strict: bool) -> "PenState": if tool is None: button = None - new_state = PenState((touch, tool, button)) + new_state = PenState((touch, tool, button)) # type: ignore if strict: assert ( new_state in self.valid_transitions() From f556aa957df8cb3e98af0f54bf1fa65f59ae47a3 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 6 Dec 2023 11:46:06 +0100 Subject: [PATCH 15/16] selftests/hid: fix ruff linter complains rename ambiguous variables l, r, and m, and ignore the return values of uhdev.get_evdev() and uhdev.get_slot() Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231206-wip-selftests-v2-15-c0350c2f5986@kernel.org Signed-off-by: Benjamin Tissoires --- tools/testing/selftests/hid/tests/test_mouse.py | 14 +++++++------- .../selftests/hid/tests/test_wacom_generic.py | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tools/testing/selftests/hid/tests/test_mouse.py b/tools/testing/selftests/hid/tests/test_mouse.py index fd2ba62e783ab..66daf7e5975ca 100644 --- a/tools/testing/selftests/hid/tests/test_mouse.py +++ b/tools/testing/selftests/hid/tests/test_mouse.py @@ -52,13 +52,13 @@ def create_report(self, x, y, buttons=None, wheels=None, reportID=None): :param reportID: the numeric report ID for this report, if needed """ if buttons is not None: - l, r, m = buttons - if l is not None: - self.left = l - if r is not None: - self.right = r - if m is not None: - self.middle = m + left, right, middle = buttons + if left is not None: + self.left = left + if right is not None: + self.right = right + if middle is not None: + self.middle = middle left = self.left right = self.right middle = self.middle diff --git a/tools/testing/selftests/hid/tests/test_wacom_generic.py b/tools/testing/selftests/hid/tests/test_wacom_generic.py index f92fe8e02c1bf..49186a27467e6 100644 --- a/tools/testing/selftests/hid/tests/test_wacom_generic.py +++ b/tools/testing/selftests/hid/tests/test_wacom_generic.py @@ -909,7 +909,7 @@ def test_confidence_false(self): Ensure that the confidence bit being set to false should not result in a touch event. """ uhdev = self.uhdev - evdev = uhdev.get_evdev() + _evdev = uhdev.get_evdev() t0 = test_multitouch.Touch(1, 50, 100) t0.confidence = False @@ -917,6 +917,6 @@ def test_confidence_false(self): events = uhdev.next_sync_events() self.debug_reports(r, uhdev, events) - slot = self.get_slot(uhdev, t0, 0) + _slot = self.get_slot(uhdev, t0, 0) - assert not events \ No newline at end of file + assert not events From da2c1b861065b452590d75a1e2f5ee9b396fef92 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Thu, 7 Dec 2023 13:22:39 +0100 Subject: [PATCH 16/16] selftests/hid: fix failing tablet button tests An overlook from commit 74452d6329be ("selftests/hid: tablets: add variants of states with buttons"), where I don't use the Enum... Fixes: 74452d6329be ("selftests/hid: tablets: add variants of states with buttons") Acked-by: Jiri Kosina Link: https://lore.kernel.org/r/20231207-b4-wip-selftests-v1-1-c4e13fe04a70@kernel.org Signed-off-by: Benjamin Tissoires --- tools/testing/selftests/hid/tests/test_tablet.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py index dc8b0fe9e7f36..903f19f7cbe9f 100644 --- a/tools/testing/selftests/hid/tests/test_tablet.py +++ b/tools/testing/selftests/hid/tests/test_tablet.py @@ -115,7 +115,7 @@ def from_evdev(cls, evdev) -> "PenState": # 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 + button = BtnPressed(b) # the kernel tends to insert an EV_SYN once removing the tool, so # the button will be released after @@ -155,7 +155,7 @@ def apply(self, events: List[libevdev.InputEvent], strict: bool) -> "PenState": if button_found: raise ValueError(f"duplicated BTN_STYLUS* in {events}") button_found = True - button = ev.code if ev.value else None + button = BtnPressed(ev.code) if ev.value else None # the kernel tends to insert an EV_SYN once removing the tool, so # the button will be released after