diff options
author | Doug Anderson <dianders@chromium.org> | 2013-12-05 15:44:43 -0800 |
---|---|---|
committer | chrome-internal-fetch <chrome-internal-fetch@google.com> | 2013-12-10 03:26:43 +0000 |
commit | 1d24f4d661473223fd5eaf60a62ec0f1068d58f2 (patch) | |
tree | 285fa05df6755c7f304637187ccd0ee848bcd74c | |
parent | 9f40f11fe815f2b8034d1863a35c0ba1854bdcba (diff) | |
download | chrome-ec-1d24f4d661473223fd5eaf60a62ec0f1068d58f2.tar.gz |
keyboard: Improve kbpress reliability for automation
The "kbpress" command had a few issues if you wanted to reliably use
it for automation. Specifically it was not possible to guarantee how
much time would pass between the press of a key and the release of a
key. Sometimes you might press and release before the key was
officially "there" and sometimes you might get a press and hold of a
key.
Fix this:
1. Make it so that kbpress with no press/release parameter gives a
press and release (and guarantees that the press / release will
actually take effect).
2. Make it so that kbpress guarantees that when it finishes that the
key has actually been pressed or released.
BRANCH=pit
BUG=chrome-os-partner:24249
TEST=kbtype is (https://chromium-review.googlesource.com/178680) reliable
TEST=make -j32 BOARD=bds tests && make BOARD=bds runtests
TEST=Pick Ibe00a796bde7d06416889b621359671a2f68e162 and test.
Change-Id: Ia213ab2e8d8da273e3ac4876d97d5452df88f47d
Signed-off-by: Doug Anderson <dianders@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/178983
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
(cherry picked from commit 247650ecc90385417f5dcb2d60bb6ae1e5cfa32f)
Reviewed-on: https://chromium-review.googlesource.com/179325
Reviewed-by: Randall Spangler <rspangler@chromium.org>
-rw-r--r-- | common/keyboard_scan.c | 64 | ||||
-rw-r--r-- | test/kb_scan.c | 8 |
2 files changed, 64 insertions, 8 deletions
diff --git a/common/keyboard_scan.c b/common/keyboard_scan.c index d1e90c72c7..a22c43b7c7 100644 --- a/common/keyboard_scan.c +++ b/common/keyboard_scan.c @@ -28,6 +28,9 @@ #define SCAN_TIME_COUNT 32 /* Number of last scan times to track */ +/* If we're waiting for a scan to happen, we'll give it this long */ +#define SCAN_TASK_TIMEOUT_US (100 * MSEC) + #ifndef CONFIG_KEYBOARD_POST_SCAN_CLOCKS /* * Default delay in clocks; this was experimentally determined to be long @@ -87,6 +90,9 @@ static int print_state_changes; static int enable_scanning = 1; /* Must init to 1 for scanning at boot */ +/* Constantly incrementing counter of the number of times we polled */ +static volatile int kbd_polls; + static int is_scanning_enabled(void) { #ifdef CONFIG_LID_SWITCH @@ -119,6 +125,30 @@ static void print_state(const uint8_t *state, const char *msg) } /** + * Ensure that the keyboard has been scanned. + * + * Makes sure that we've fully gone through the keyboard scanning loop at + * least once. + */ +static void ensure_keyboard_scanned(int old_polls) +{ + uint64_t start_time; + + start_time = get_time().val; + + /* + * Ensure we see the poll task run. + * + * Note that the poll task is higher priority than ours so we know that + * while we're running it's not partway through a poll. That means that + * if kbd_polls changes we've gone through a whole cycle. + */ + while ((kbd_polls == old_polls) && + (get_time().val - start_time < SCAN_TASK_TIMEOUT_US)) + usleep(keyscan_config.scan_period_us); +} + +/** * Simulate a keypress. * * @param row Row of key @@ -127,15 +157,29 @@ static void print_state(const uint8_t *state, const char *msg) */ static void simulate_key(int row, int col, int pressed) { + int old_polls; + if ((simulated_key[col] & (1 << row)) == ((pressed ? 1 : 0) << row)) return; /* No change */ simulated_key[col] ^= (1 << row); + /* Keep track of polls now that we've got keys simulated */ + old_polls = kbd_polls; + print_state(simulated_key, "simulated "); /* Wake the task to handle changes in simulated keys */ task_wake(TASK_ID_KEYSCAN); + + /* + * Make sure that the keyboard task sees the key for long enough. + * That means it needs to have run and for enough time. + */ + ensure_keyboard_scanned(old_polls); + usleep(pressed ? + keyscan_config.debounce_down_us : keyscan_config.debounce_up_us); + ensure_keyboard_scanned(kbd_polls); } /** @@ -387,6 +431,8 @@ static int check_keys_changed(uint8_t *state) #endif } + kbd_polls++; + return any_pressed; } @@ -647,7 +693,7 @@ static int command_keyboard_press(int argc, char **argv) ccprintf("\t%d %d\n", i, j); } - } else if (argc == 4) { + } else if (argc == 3 || argc == 4) { int r, c, p; char *e; @@ -659,16 +705,22 @@ static int command_keyboard_press(int argc, char **argv) if (*e || r < 0 || r >= KEYBOARD_ROWS) return EC_ERROR_PARAM2; - p = strtoi(argv[3], &e, 0); - if (*e || p < 0 || p > 1) - return EC_ERROR_PARAM3; + if (argc == 3) { + /* Simulate a press and release */ + simulate_key(r, c, 1); + simulate_key(r, c, 0); + } else { + p = strtoi(argv[3], &e, 0); + if (*e || p < 0 || p > 1) + return EC_ERROR_PARAM3; - simulate_key(r, c, p); + simulate_key(r, c, p); + } } return EC_SUCCESS; } DECLARE_CONSOLE_COMMAND(kbpress, command_keyboard_press, - "[col] [row] [0 | 1]", + "[col row [0 | 1]]", "Simulate keypress", NULL); diff --git a/test/kb_scan.c b/test/kb_scan.c index a273a20466..e0e73d17f5 100644 --- a/test/kb_scan.c +++ b/test/kb_scan.c @@ -253,10 +253,14 @@ static int debounce_test(void) static int simulate_key_test(void) { + int old_count; + + old_count = fifo_add_count; host_command_simulate(1, 1, 1); - TEST_ASSERT(expect_keychange() == EC_SUCCESS); + TEST_ASSERT(fifo_add_count > old_count); + old_count = fifo_add_count; host_command_simulate(1, 1, 0); - TEST_ASSERT(expect_keychange() == EC_SUCCESS); + TEST_ASSERT(fifo_add_count > old_count); return EC_SUCCESS; } |