plover
plover copied to clipboard
Add config option for sleeping between key presses
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 thevalue
to thedefault
ifvalue 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
- [x] Changes have tests
- [x] News fragment added in news.d. See documentation for details
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?
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.
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.
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 https://github.com/user202729/plover/commit/f4cb21c5f6d178b06340a77c68629d370d0b5944 .
- 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)
I want this feature
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().
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"?
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".
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.
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.
When I use the feature it appears to be the same. Do you have weird behavior on your machine?
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.
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!
Any updates on merging this?
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.