Add config option for sleeping between key presses #1132

oberien · 2020-06-20T20:22:41Z

Summary of changes

On Linux (haven't tested other OS) Firefox seems to drop some key events if they are sent too quickly. One extreme example is stroking SER to get "certificate", followed by SRER to change it to "server". In Firefox, often times the beginning "c" or "ce" of "certificate" aren't deleted, which results in "ceserver". Sometimes a backspace key is even applied after "server" has been written, resulting in "ceserve" and similar.
To work around that problem, I implemented a config option, which allows setting a timeout in milliseconds, which is applied between all emulated keys, delaying the key events so that Firefox processes them correctly in the correct order. For me about 5 milliseconds seems to be the optimum.

Open Questions

Do I need to / how should I update the .pot file?
Is changing the KeyboardEmulation.{send_backspaces,send_string} functions in some os-implementations from static to non-static a breaking API change that needs to be documented for towncrier?
To not cause config-file problems, I added an if-condition to int_option, which sets the value to the default if value is None. This results in the config option added by this PR to default to its default value. Before doing this, I got an error message during startup of plover that the config option is missing in the config file. Is this an appropriate solution, or is there a different way that I should use for config-file migration?

Pull Request Checklist

Changes have tests
News fragment added in news.d. See documentation for details

morinted · 2020-10-23T17:36:11Z

We need to test this on all platforms (I can do Mac and Windows pretty easily.) One consideration is to test a mixed-format translation like hi {#shift(t) h e r e} 👋{#shift(1)} which will cover normal key presses, Unicode, and keyboard shortcuts. I think we want the delay to be consistent between all of them?

user202729 · 2020-10-29T03:59:56Z

No problem on Linux. (including when two

Note that key_combo ({#keys}) are not delayed. This should be documented somewhere.

If the maximum is not specified, it defaults to 99. https://doc.qt.io/qt-5/qspinbox.html#maximum-prop (It would not makes a lot of sense for the value to be more than 99 anyway)

(perhaps IntOption should make this clearer with Python default parameter, or set some default that makes more sense (INT_MAX?), rather than relying on Qt's default behavior)

Nobody needs the delay value to be float, right?

It's usually not a problem, but the output is non-interruptible, even after Plover output is disabled.

user202729 · 2020-10-29T04:06:01Z

plover/oslayer/osxkeyboardcontrol.py

        event = CGEventCreateKeyboardEvent(OUTPUT_SOURCE, 0, True)
        KeyboardEmulation._set_event_string(event, c)
        CGEventPost(kCGSessionEventTap, event)
        event = CGEventCreateKeyboardEvent(OUTPUT_SOURCE, 0, False)
        KeyboardEmulation._set_event_string(event, c)
        CGEventPost(kCGSessionEventTap, event)
+        if self._time_between_key_presses != 0:
+                sleep(self._time_between_key_presses / 1000)


The indentation should be fixed.

user202729 · 2020-10-29T04:06:33Z

plover/oslayer/xkeyboardcontrol.py

@@ -1214,6 +1219,9 @@ def send_backspaces(self, number_of_backspaces):
        for x in range(number_of_backspaces):
            self._send_keycode(self._backspace_mapping.keycode,
                               self._backspace_mapping.modifiers)
+            if self._time_between_key_presses != 0:
+                sleep(self._time_between_key_presses / 1000)
+                self._display.sync()


Perhaps the sync should be before the sleep, for consistency with other platforms.

(usually the delay is small and the user won't notice this anyway?)

user202729 · 2020-10-29T04:06:44Z

plover/oslayer/xkeyboardcontrol.py

@@ -1233,6 +1241,9 @@ def send_string(self, s):
                continue
            self._send_keycode(mapping.keycode,
                               mapping.modifiers)
+            if self._time_between_key_presses != 0:
+                sleep(self._time_between_key_presses / 1000)
+                self._display.sync()


Same as above.

oberien · 2020-10-31T15:12:47Z

Thanks for the review. I fixed all your comments.

Note that key_combo ({#keys}) are not delayed

I missed that. On OSX send_key_combination calls _send_sequence, which does already have the delay. On Linux X and Windows I added the delay to the send_key_combination function as well.

If the maximum is not specified, it defaults to 99.

I did not know that. For debugging it might make sense to allow values of over 1 second (e.g. to more easily see each emulated key press). I added a maximum delay of 100'000. I guess 100 seconds between each key press should be more than enough for everyone :)

Nobody needs the delay value to be float, right?

I think 1ms between key presses should be granular enough. 1ms delay + let's say 1ms code runtime per press is 500 keys per second. That's over 80 words per second(!) assuming English with 6 keys / word on average.

Edit: CircleCI failing shouldn't be my fault.

user202729 · 2021-02-20T00:36:34Z

More about the issue.

Even with this merged in, I still get terrible result with typing characters that needs keymap modifications in Chrome (on X). Obviously, because the key map is modified right before the key is typed in.

I see there are some ways to fix that:

Wait for X milliseconds between changing the layout and pressing the key. (time before changing layout: X, time after changing layout: X)
Change the layout before waiting the X milliseconds. (time before changing layout: X, time after changing layout: 0)
Wait for X/2 milliseconds between pressing the previous key and changing the layout, and X/2 milliseconds between changing the layout and pressing the next key.
(time before changing layout: X/2, time after changing layout: X/2) -> Implemented in user202729@f4cb21c .
Make another configurable option.
(time before changing layout: X', time after changing layout: X)
Change the key map in mass as long as it fits in the table, and wait for X milliseconds after that.

Possible issues with all the options:

Confusing for the end user.
Make the delay between successive key presses uneven (only looks annoying).
Cause problem with other programs.
Make the total time to output things too slow.
Requires redesign of the algorithm. (actually I'm not entirely sure if the algorithm already does that)

petercpark · 2021-05-24T22:49:24Z

I want this feature

fourshade · 2021-06-22T02:37:19Z

Instead of a uniform sleep between events, what if we just fill a buffer with all of the events and send them out at an even pace? It would be far less erratic, and farming that task out to another thread would also prevent the current thread from being blocked by sleep().

user202729 · 2021-06-22T02:39:00Z

Instead of a uniform sleep between events, what if we just fill a buffer with all of the events and send them out at an even pace? It would be far less erratic, and farming that task out to another thread would also prevent the current thread from being blocked by sleep().

What's the difference? Isn't "uniform sleep" the same as "even pace"?

fourshade · 2021-06-22T02:43:12Z

What's the difference? Isn't "uniform sleep" the same as "even pace"?

What I mean is, instead of running "send event, sleep for x ms" every time an event comes in (and those come in unevenly), just add each event to a buffer. On another thread, we loop "if there's something in the buffer, pop the head and send it. then sleep regardless".

user202729 · 2021-06-22T02:44:43Z

What I mean is, instead of running "send event, sleep for x ms" every time an event comes in (and those come in unevenly), just add each event to a buffer. On another thread, we loop "if there's something in the buffer, pop the head and send it. then sleep regardless".

Actually, even though the (keyboard sending) thread is blocked, the behavior is exactly the same as would be in your suggested implementation.

If it wasn't, it would be a bug.

fourshade · 2021-06-22T02:51:17Z

Actually, even though the (keyboard sending) thread is blocked, the behavior is exactly the same as would be in your suggested implementation.

If it wasn't, it would be a bug.

In theory, yes, it should be exactly the same. But it may not be in practice due to the way Python handles threads and blocking. It's probably worth a shot at least.

user202729 · 2021-06-22T02:56:08Z

When I use the feature it appears to be the same. Do you have weird behavior on your machine?

fourshade · 2021-06-22T03:19:46Z

I haven't tried it on my machine; I was just thinking of a possible way to mitigate timing issues. send_string() is called by the engine's thread, and I just get a bad feeling (programmer spidey-sense?) when I see code that blocks the main thread like that. It may not matter on some or even most machines, but this bug is very machine/application sensitive.

zeramorphic · 2021-12-02T18:45:14Z

If this PR has lost some traction, I'd be happy to help with it. I've also implemented your suggestion locally, @fourshade, but at the moment I'm only running on Pop!_OS with an X server, so the quite extreme delays between stroke release and key output mean that I'm not certain if the extra thread is actually doing anything useful!

alesya-h · 2022-08-27T10:47:59Z

Any updates on merging this?

oberien · 2022-08-27T11:15:44Z

I most likely won't revisit this PR. Feel free to fork my state, rebase it, fix it and open a new PR. At that point I'll close this one.

See openstenoproject#1132 (comment)

oberien added 2 commits June 20, 2020 21:57

Add config option for sleeping between key presses

dc90c6b

Add news.d description for time between key presses

f164f96

user202729 reviewed Oct 29, 2020

View reviewed changes

Add delay to key_combos and sleep after sending keys on linux

6845784

user202729 mentioned this pull request Oct 31, 2020

Garbled Output with Later Versions of LibreOffice in Linux #1059

Closed

user202729 mentioned this pull request Jul 7, 2021

Difficulties outputting Shavian-block characters on Ubuntu #1373

Open

user202729 mentioned this pull request Sep 11, 2021

Firefox on Linux deduplicates keypresses #1408

Closed

DropDemBits added a commit to DropDemBits/plover that referenced this pull request Dec 19, 2022

oslayer/linux: add delay when switching key maps

f2a5e8e

See openstenoproject#1132 (comment)

DropDemBits mentioned this pull request Dec 19, 2022

Add Option for Inserting Delay between Key Press Events #1584

Closed

5 tasks

sammdot mentioned this pull request Sep 27, 2023

Add configurable key press delay #1633

Merged

5 tasks

sammdot added a commit to sammdot/plover that referenced this pull request Sep 27, 2023

Reimplement half-delay on X11 as covered in openstenoproject#1132

a127694

sammdot closed this in #1633 Sep 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add config option for sleeping between key presses #1132

Add config option for sleeping between key presses #1132

oberien commented Jun 20, 2020 •

edited

Loading

morinted commented Oct 23, 2020

user202729 commented Oct 29, 2020 •

edited

Loading

user202729 Oct 29, 2020

user202729 Oct 29, 2020

user202729 Oct 29, 2020

oberien commented Oct 31, 2020 •

edited

Loading

user202729 commented Feb 20, 2021 •

edited

Loading

petercpark commented May 24, 2021

fourshade commented Jun 22, 2021

user202729 commented Jun 22, 2021

fourshade commented Jun 22, 2021

user202729 commented Jun 22, 2021 •

edited

Loading

fourshade commented Jun 22, 2021

user202729 commented Jun 22, 2021

fourshade commented Jun 22, 2021

zeramorphic commented Dec 2, 2021

alesya-h commented Aug 27, 2022

oberien commented Aug 27, 2022

Add config option for sleeping between key presses #1132

Add config option for sleeping between key presses #1132

Conversation

oberien commented Jun 20, 2020 • edited Loading

Summary of changes

Open Questions

Pull Request Checklist

morinted commented Oct 23, 2020

user202729 commented Oct 29, 2020 • edited Loading

user202729 Oct 29, 2020

Choose a reason for hiding this comment

user202729 Oct 29, 2020

Choose a reason for hiding this comment

user202729 Oct 29, 2020

Choose a reason for hiding this comment

oberien commented Oct 31, 2020 • edited Loading

user202729 commented Feb 20, 2021 • edited Loading

petercpark commented May 24, 2021

fourshade commented Jun 22, 2021

user202729 commented Jun 22, 2021

fourshade commented Jun 22, 2021

user202729 commented Jun 22, 2021 • edited Loading

fourshade commented Jun 22, 2021

user202729 commented Jun 22, 2021

fourshade commented Jun 22, 2021

zeramorphic commented Dec 2, 2021

alesya-h commented Aug 27, 2022

oberien commented Aug 27, 2022

oberien commented Jun 20, 2020 •

edited

Loading

user202729 commented Oct 29, 2020 •

edited

Loading

oberien commented Oct 31, 2020 •

edited

Loading

user202729 commented Feb 20, 2021 •

edited

Loading

user202729 commented Jun 22, 2021 •

edited

Loading