From 3044a054b8dff6436dfc8ed1ccbfa47f2e3773c5 Mon Sep 17 00:00:00 2001 From: Peter Johanson Date: Thu, 26 May 2022 17:00:03 +0000 Subject: [PATCH 1/8] feat(shell) Add new `key` shell command. * Allow tap/press/release of a given key position within the Zephyr shell. --- app/CMakeLists.txt | 1 + app/Kconfig | 2 + app/src/shell/CMakeLists.txt | 1 + app/src/shell/Kconfig | 8 ++++ app/src/shell/key_positions.c | 80 +++++++++++++++++++++++++++++++++++ 5 files changed, 92 insertions(+) create mode 100644 app/src/shell/CMakeLists.txt create mode 100644 app/src/shell/Kconfig create mode 100644 app/src/shell/key_positions.c diff --git a/app/CMakeLists.txt b/app/CMakeLists.txt index 793f386d..151bf835 100644 --- a/app/CMakeLists.txt +++ b/app/CMakeLists.txt @@ -91,5 +91,6 @@ target_sources(app PRIVATE src/workqueue.c) target_sources(app PRIVATE src/main.c) add_subdirectory(src/display/) +add_subdirectory(src/shell/) zephyr_cc_option(-Wfatal-errors) diff --git a/app/Kconfig b/app/Kconfig index 0dd9316a..87110d97 100644 --- a/app/Kconfig +++ b/app/Kconfig @@ -380,6 +380,8 @@ endmenu menu "Advanced" +rsource "src/shell/Kconfig" + menu "Initialization Priorities" if USB_DEVICE_STACK diff --git a/app/src/shell/CMakeLists.txt b/app/src/shell/CMakeLists.txt new file mode 100644 index 00000000..76ead89b --- /dev/null +++ b/app/src/shell/CMakeLists.txt @@ -0,0 +1 @@ +target_sources_ifdef(CONFIG_ZMK_SHELL_KEY_POSITIONS app PRIVATE key_positions.c) \ No newline at end of file diff --git a/app/src/shell/Kconfig b/app/src/shell/Kconfig new file mode 100644 index 00000000..6d242235 --- /dev/null +++ b/app/src/shell/Kconfig @@ -0,0 +1,8 @@ +menu "Shell" + +config ZMK_SHELL_KEY_POSITIONS + bool "Key Position commands" + default y if !ZMK_SPLIT || ZMK_SPLIT_ROLE_CENTRAL + depends on SHELL + +endmenu diff --git a/app/src/shell/key_positions.c b/app/src/shell/key_positions.c new file mode 100644 index 00000000..12277fa4 --- /dev/null +++ b/app/src/shell/key_positions.c @@ -0,0 +1,80 @@ + +#include +#include +#include +#include +#include + +#define HELP_NONE "[key_position]" + +static int parse_key_position(const struct shell *shell, char *pos_str) { + int position; + char *endptr; + + position = strtoul(pos_str, &endptr, 10); + + if (endptr == pos_str) { + shell_error(shell, "Enter an integer key position"); + return -EINVAL; + } + + return position; +} + +static int parse_and_raise(const struct shell *shell, char *pos_str, bool pressed) { + int position = parse_key_position(shell, pos_str); + + if (position < 0) { + return position; + } + + ZMK_EVENT_RAISE(new_zmk_position_state_changed( + (struct zmk_position_state_changed){.source = ZMK_POSITION_STATE_CHANGE_SOURCE_LOCAL, + .state = pressed, + .position = position, + .timestamp = k_uptime_get()})); + + return position; +} + +static int cmd_key_tap(const struct shell *shell, size_t argc, char **argv) { + if (argc != 2) { + return -EINVAL; + } + + parse_and_raise(shell, argv[1], true); + + k_sleep(K_MSEC(50)); + + parse_and_raise(shell, argv[1], false); + + return 0; +}; + +static int cmd_key_press(const struct shell *shell, size_t argc, char **argv) { + if (argc != 2) { + return -EINVAL; + } + + parse_and_raise(shell, argv[1], true); + + return 0; +}; + +static int cmd_key_release(const struct shell *shell, size_t argc, char **argv) { + if (argc != 2) { + return -EINVAL; + } + + parse_and_raise(shell, argv[1], false); + + return 0; +}; + +SHELL_STATIC_SUBCMD_SET_CREATE(sub_key, SHELL_CMD(tap, NULL, HELP_NONE, cmd_key_tap), + SHELL_CMD(press, NULL, HELP_NONE, cmd_key_press), + SHELL_CMD(release, NULL, HELP_NONE, cmd_key_release), + SHELL_SUBCMD_SET_END /* Array terminated. */ +); + +SHELL_CMD_REGISTER(key, &sub_key, "Key commands", NULL); \ No newline at end of file From 42169c50048c9c96b1b04d262d515ae943976b79 Mon Sep 17 00:00:00 2001 From: Peter Johanson Date: Thu, 26 May 2022 17:03:25 +0000 Subject: [PATCH 2/8] refactor: Default kscan events for native posix * By default, don't generate any events for the native posix board DTS, to allow native builds outside of the testing use case. --- app/boards/native_posix_64.overlay | 1 + 1 file changed, 1 insertion(+) diff --git a/app/boards/native_posix_64.overlay b/app/boards/native_posix_64.overlay index f8a8f700..71076f64 100644 --- a/app/boards/native_posix_64.overlay +++ b/app/boards/native_posix_64.overlay @@ -14,5 +14,6 @@ rows = <2>; columns = <2>; exit-after; + events = <>; }; }; From 255f9b73509afb09c6d9bea9a4bb373c8bc7a5bf Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 13 Nov 2023 21:15:08 -0800 Subject: [PATCH 3/8] add wait and exit commands to the shell --- app/src/shell/key_positions.c | 46 +++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/app/src/shell/key_positions.c b/app/src/shell/key_positions.c index 12277fa4..26ea96ba 100644 --- a/app/src/shell/key_positions.c +++ b/app/src/shell/key_positions.c @@ -6,25 +6,24 @@ #include #define HELP_NONE "[key_position]" +#define TAP_EVENT_SPACING K_MSEC(50) -static int parse_key_position(const struct shell *shell, char *pos_str) { - int position; +static int parse_positive_int(char *s) { char *endptr; + unsigned long value = strtoul(s, &endptr, 10); - position = strtoul(pos_str, &endptr, 10); - - if (endptr == pos_str) { - shell_error(shell, "Enter an integer key position"); + if (endptr == s) return -EINVAL; - } - - return position; + if (value > INT_MAX) + return -ERANGE; + return value; } static int parse_and_raise(const struct shell *shell, char *pos_str, bool pressed) { - int position = parse_key_position(shell, pos_str); + long position = parse_positive_int(pos_str); if (position < 0) { + shell_error(shell, "Enter an integer key position"); return position; } @@ -34,7 +33,7 @@ static int parse_and_raise(const struct shell *shell, char *pos_str, bool presse .position = position, .timestamp = k_uptime_get()})); - return position; + return 0; } static int cmd_key_tap(const struct shell *shell, size_t argc, char **argv) { @@ -44,7 +43,7 @@ static int cmd_key_tap(const struct shell *shell, size_t argc, char **argv) { parse_and_raise(shell, argv[1], true); - k_sleep(K_MSEC(50)); + k_sleep(TAP_EVENT_SPACING); parse_and_raise(shell, argv[1], false); @@ -71,10 +70,31 @@ static int cmd_key_release(const struct shell *shell, size_t argc, char **argv) return 0; }; +static int cmd_sleep(const struct shell *shell, size_t argc, char **argv) { + if (argc != 2) { + return -EINVAL; + } + + int sleep_ms = parse_positive_int(argv[1]); + + if (sleep_ms < 0) { + shell_error(shell, "Enter a positive number of milliseconds"); + return sleep_ms; + } + + k_sleep(K_MSEC(sleep_ms)); + + return 0; +}; + +static int cmd_exit(const struct shell *shell, size_t argc, char **argv) { exit(0); }; + SHELL_STATIC_SUBCMD_SET_CREATE(sub_key, SHELL_CMD(tap, NULL, HELP_NONE, cmd_key_tap), SHELL_CMD(press, NULL, HELP_NONE, cmd_key_press), SHELL_CMD(release, NULL, HELP_NONE, cmd_key_release), SHELL_SUBCMD_SET_END /* Array terminated. */ ); -SHELL_CMD_REGISTER(key, &sub_key, "Key commands", NULL); \ No newline at end of file +SHELL_CMD_REGISTER(key, &sub_key, "Key commands", NULL); +SHELL_CMD_REGISTER(sleep, NULL, "Sleep (milliseconds)", cmd_sleep); +SHELL_CMD_REGISTER(exit, NULL, "Exit", cmd_exit); \ No newline at end of file From 87c6a32ef41b2a2d54496fcdc49c0e9a549dc53d Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 13 Nov 2023 21:16:32 -0800 Subject: [PATCH 4/8] add shell configuration for the native_posix board --- app/boards/native_posix_64.overlay | 5 +++++ app/boards/native_posix_shell_test_extra.conf | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 app/boards/native_posix_shell_test_extra.conf diff --git a/app/boards/native_posix_64.overlay b/app/boards/native_posix_64.overlay index 71076f64..149218e5 100644 --- a/app/boards/native_posix_64.overlay +++ b/app/boards/native_posix_64.overlay @@ -5,6 +5,7 @@ / { chosen { zmk,kscan = &kscan; + zephyr,shell-uart = &uart0; }; kscan: kscan { @@ -17,3 +18,7 @@ events = <>; }; }; + +&uart0 { + status = "okay"; +}; \ No newline at end of file diff --git a/app/boards/native_posix_shell_test_extra.conf b/app/boards/native_posix_shell_test_extra.conf new file mode 100644 index 00000000..19f8440d --- /dev/null +++ b/app/boards/native_posix_shell_test_extra.conf @@ -0,0 +1,16 @@ +CONFIG_SERIAL=y +CONFIG_UART_NATIVE_POSIX=y +#CONFIG_NATIVE_UART_0_ON_OWN_PTY=y +CONFIG_NATIVE_UART_0_ON_STDINOUT=y + +CONFIG_SHELL=y +CONFIG_SHELL_BACKEND_SERIAL=y +CONFIG_ZMK_SHELL_KEY_POSITIONS=y +CONFIG_SHELL_BACKEND_SERIAL_RX_RING_BUFFER_SIZE=1024 + +CONFIG_SHELL_PROMPT_UART="zmk> " + +#CONFIG_SHELL_BACKEND_SERIAL_INTERRUPT_DRIVEN=n +#CONFIG_UART_INTERRUPT_DRIVEN=n +#CONFIG_UART_LINE_CTRL=n +#CONFIG_SHELL_BACKEND_SERIAL_CHECK_DTR=n \ No newline at end of file From 6677117e6b8a310246e3d24c8ef444601be1a6cf Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 13 Nov 2023 21:20:19 -0800 Subject: [PATCH 5/8] hack: don't exit from kscan-mock if there are no events We should probably move this into .conf files, or wait for Zephyr 3.4 and its EXTRA_DTC_OVERLAY_FILE. --- app/module/drivers/kscan/kscan_mock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/module/drivers/kscan/kscan_mock.c b/app/module/drivers/kscan/kscan_mock.c index 604e164c..f849fb06 100644 --- a/app/module/drivers/kscan/kscan_mock.c +++ b/app/module/drivers/kscan/kscan_mock.c @@ -55,7 +55,7 @@ static int kscan_mock_configure(const struct device *dev, kscan_callback_t callb uint32_t ev = cfg->events[data->event_index]; \ LOG_DBG("delaying next keypress: %d", ZMK_MOCK_MSEC(ev)); \ k_work_schedule(&data->work, K_MSEC(ZMK_MOCK_MSEC(ev))); \ - } else if (cfg->exit_after) { \ + } else if (cfg->exit_after && DT_INST_PROP_LEN(n, events) > 0) { \ LOG_DBG("Exiting"); \ exit(0); \ } \ From 8cd0dfe1e5d83a7d5b3bc83ea4939245af1bcf9d Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 13 Nov 2023 21:21:11 -0800 Subject: [PATCH 6/8] overhaul run-test.sh - require bash and use subroutines - write intermediate output to stderr instead of stdout - strip ANSI escape sequences from the logs - add option to run tests via ZMK shell commands --- app/run-test.sh | 131 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 98 insertions(+), 33 deletions(-) diff --git a/app/run-test.sh b/app/run-test.sh index 6935f2c8..36a127f8 100755 --- a/app/run-test.sh +++ b/app/run-test.sh @@ -1,47 +1,112 @@ -#!/bin/sh +#!/bin/bash # Copyright (c) 2020 The ZMK Contributors # SPDX-License-Identifier: MIT -if [ -z "$1" ]; then - echo "Usage: ./run-test.sh " - exit 1 -fi +function main() { + if [ -z "$1" ] || [ "$1" = "all" ]; then + path="tests" + else + path="$1" + fi -path="$1" -if [ $path = "all" ]; then - path="tests" -fi + >&2 echo "Running all tests in ./$path" -testcases=$(find $path -name native_posix_64.keymap -exec dirname \{\} \;) -num_cases=$(echo "$testcases" | wc -l) -if [ $num_cases -gt 1 ] || [ "$testcases" != "$path" ]; then - echo "" > ./build/tests/pass-fail.log - echo "$testcases" | xargs -L 1 -P ${J:-4} ./run-test.sh + testcases=$(find $path -name native_posix_64.keymap -exec dirname \{\} \;) + + :>./build/tests/pass-fail.log + echo "$testcases" | xargs -n 1 -P ${J:-4} bash -c 'run_test "$@"' _ err=$? + >&2 echo + >&2 echo "=== Test results ===" sort -k2 ./build/tests/pass-fail.log exit $err -fi +} -testcase="$path" -echo "Running $testcase:" +function run_test() { + path="$1" -west build -d build/$testcase -b native_posix_64 -- -DZMK_CONFIG="$(pwd)/$testcase" > /dev/null 2>&1 -if [ $? -gt 0 ]; then - echo "FAILED: $testcase did not build" | tee -a ./build/tests/pass-fail.log - exit 1 -fi + build_dir="build/$path" + build_log="$build_dir/build.log" -./build/$testcase/zephyr/zmk.exe | sed -e "s/.*> //" | tee build/$testcase/keycode_events_full.log | sed -n -f $testcase/events.patterns > build/$testcase/keycode_events.log -diff -auZ $testcase/keycode_events.snapshot build/$testcase/keycode_events.log -if [ $? -gt 0 ]; then - if [ -f $testcase/pending ]; then - echo "PENDING: $testcase" | tee -a ./build/tests/pass-fail.log - exit 0 + # Look for a file named "inputs.txt" in some subdirectory of the test. + # + # If none exists, assume it uses the old approach of hardcoding events in the kscan-mock driver. + inputs=$(find $path -mindepth 1 -name inputs.txt -exec dirname \{\} \;) + + extra_config="" + if [ -n "$inputs" ]; then + # replace with DEXTRA_CONFIG_FILE for Zephyr 3.4 + extra_config+="-DOVERLAY_CONFIG=boards/native_posix_shell_test_extra.conf" fi - echo "FAILED: $testcase" | tee -a ./build/tests/pass-fail.log - exit 1 -fi -echo "PASS: $testcase" | tee -a ./build/tests/pass-fail.log -exit 0 + mkdir -p $build_dir + >&2 echo "Building $path:" + west build -d $build_dir -b native_posix_64 -- -DZMK_CONFIG="$(pwd)/$path" $extra_config >"$build_log" 2>&1 + if [ $? -gt 0 ]; then + echo "FAILED: $path did not build" + >&2 printf "\t%s\n" "for details see ./$build_log" + return 1 + fi + + + function check_test_results() { + testdir="$1" + outdir="$2" + tee $outdir/keycode_events_full.log | \ + sed -e "s/.*> //" | \ + sed -e "s/\x1B\[[0-9;]*[a-zA-Z]//g" | \ + sed -n -f $testdir/events.patterns > $outdir/keycode_events.log + + >&2 diff -auZ $testdir/keycode_events.snapshot $outdir/keycode_events.log + + if [ $? -gt 0 ]; then + if [ -f $testdir/pending ]; then + echo "PENDING: $testdir" + return 0 + fi + echo "FAILED: $testdir" + return 1 + else + echo "PASS: $testdir" + return 0 + fi | tee -a ./build/tests/pass-fail.log + } + + if [ -z "$inputs" ]; then + >&2 echo "Running $path using kscan-mock:" + ./build/$path/zephyr/zmk.exe | check_test_results $path build/$path + return $? + fi + + # Add waits between each command to better simulate typing. + # + # FIXME(ecstaticmore): make this configurable + function augment_commands() { + awk '{print; print "wait 10"}' "$1/inputs.txt" + echo "exit" + } + + # This convoluted invocation is needed to work around some quirks in + # Zephyr's serial shell driver: + # + # - When stdin is closed while CONFIG_NATIVE_UART_0_ON_STDINOUT is enabled, + # reading from the UART driver returns the same byte over and over + # (instead of blocking). + # - The shell driver reads from the UART driver greedily until it blocks + # (instead of stopping on newlines or when the ring buffer is full). + # + # To work around this, we sleep inside a process substitution to keep + # the pipe open until the Zephyr executable exits. + ret=0 + for input in $inputs; do + >&2 echo "Running $path/native_posix_64.keymap with $input/inputs.txt:" + mkdir -p build/$input + ./build/$path/zephyr/zmk.exe < <(augment_commands $input; sleep 999) \ + | check_test_results $input build/$input || ret=1 + done + return $ret +} + +export -f run_test +main "$@" \ No newline at end of file From dfc1e4b4f73644cb3a6bbc2ab0ca19bc9656153f Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 13 Nov 2023 21:21:42 -0800 Subject: [PATCH 7/8] update toggle-layer test to use new framework --- app/tests/toggle-layer/early-key-release/inputs.txt | 5 +++++ .../early-key-release/native_posix_64.keymap | 9 --------- .../{behavior_keymap.dtsi => native_posix_64.keymap} | 0 app/tests/toggle-layer/normal/inputs.txt | 2 ++ app/tests/toggle-layer/normal/native_posix_64.keymap | 8 -------- 5 files changed, 7 insertions(+), 17 deletions(-) create mode 100644 app/tests/toggle-layer/early-key-release/inputs.txt delete mode 100644 app/tests/toggle-layer/early-key-release/native_posix_64.keymap rename app/tests/toggle-layer/{behavior_keymap.dtsi => native_posix_64.keymap} (100%) create mode 100644 app/tests/toggle-layer/normal/inputs.txt delete mode 100644 app/tests/toggle-layer/normal/native_posix_64.keymap diff --git a/app/tests/toggle-layer/early-key-release/inputs.txt b/app/tests/toggle-layer/early-key-release/inputs.txt new file mode 100644 index 00000000..25714e0a --- /dev/null +++ b/app/tests/toggle-layer/early-key-release/inputs.txt @@ -0,0 +1,5 @@ +key press 0 +key press 1 +key release 0 +key release 1 +key tap 0 \ No newline at end of file diff --git a/app/tests/toggle-layer/early-key-release/native_posix_64.keymap b/app/tests/toggle-layer/early-key-release/native_posix_64.keymap deleted file mode 100644 index 0a0c88ea..00000000 --- a/app/tests/toggle-layer/early-key-release/native_posix_64.keymap +++ /dev/null @@ -1,9 +0,0 @@ -#include -#include -#include -#include "../behavior_keymap.dtsi" - -&kscan { - events = ; -}; \ No newline at end of file diff --git a/app/tests/toggle-layer/behavior_keymap.dtsi b/app/tests/toggle-layer/native_posix_64.keymap similarity index 100% rename from app/tests/toggle-layer/behavior_keymap.dtsi rename to app/tests/toggle-layer/native_posix_64.keymap diff --git a/app/tests/toggle-layer/normal/inputs.txt b/app/tests/toggle-layer/normal/inputs.txt new file mode 100644 index 00000000..d7e4d969 --- /dev/null +++ b/app/tests/toggle-layer/normal/inputs.txt @@ -0,0 +1,2 @@ +key tap 1 +key tap 0 \ No newline at end of file diff --git a/app/tests/toggle-layer/normal/native_posix_64.keymap b/app/tests/toggle-layer/normal/native_posix_64.keymap deleted file mode 100644 index 97bdd179..00000000 --- a/app/tests/toggle-layer/normal/native_posix_64.keymap +++ /dev/null @@ -1,8 +0,0 @@ -#include -#include -#include -#include "../behavior_keymap.dtsi" - -&kscan { - events = ; -}; \ No newline at end of file From dab6f26a64dbcbdd6479261705b4f29a1b4395f4 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 14 Nov 2023 14:39:39 -0800 Subject: [PATCH 8/8] minor fixes --- app/run-test.sh | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/app/run-test.sh b/app/run-test.sh index 36a127f8..a24e68f1 100755 --- a/app/run-test.sh +++ b/app/run-test.sh @@ -15,7 +15,7 @@ function main() { testcases=$(find $path -name native_posix_64.keymap -exec dirname \{\} \;) :>./build/tests/pass-fail.log - echo "$testcases" | xargs -n 1 -P ${J:-4} bash -c 'run_test "$@"' _ + echo "$testcases" | xargs -I {} -P ${J:-4} bash -c 'run_test "$@"' _ {} err=$? >&2 echo >&2 echo "=== Test results ===" @@ -36,7 +36,7 @@ function run_test() { extra_config="" if [ -n "$inputs" ]; then - # replace with DEXTRA_CONFIG_FILE for Zephyr 3.4 + # replace with -DEXTRA_CONFIG_FILE for Zephyr 3.4 extra_config+="-DOVERLAY_CONFIG=boards/native_posix_shell_test_extra.conf" fi @@ -49,7 +49,6 @@ function run_test() { return 1 fi - function check_test_results() { testdir="$1" outdir="$2" @@ -79,11 +78,13 @@ function run_test() { return $? fi - # Add waits between each command to better simulate typing. + # Filters comments and blank lines from the inputs.txt file, adds a fixed + # `wait` after each command, and appends an `exit`. # - # FIXME(ecstaticmore): make this configurable + # FIXME(ecstaticmore): make the `wait` configurable. function augment_commands() { - awk '{print; print "wait 10"}' "$1/inputs.txt" + grep -v -e '^\s*#' -e '^\s*$' "$1/inputs.txt" | \ + awk '{ print; print "wait 10" }' echo "exit" } @@ -96,8 +97,9 @@ function run_test() { # - The shell driver reads from the UART driver greedily until it blocks # (instead of stopping on newlines or when the ring buffer is full). # - # To work around this, we sleep inside a process substitution to keep - # the pipe open until the Zephyr executable exits. + # This means that piping directly to the Zephyr executable causes it to + # enter an infinite loop. To work around this, we sleep inside a process + # substitution to keep the pipe open until the Zephyr executable exits. ret=0 for input in $inputs; do >&2 echo "Running $path/native_posix_64.keymap with $input/inputs.txt:"