Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Paste text asynchronously #1558

Closed
wants to merge 3 commits into from
Closed

Conversation

jdeanwallace
Copy link
Contributor

@jdeanwallace jdeanwallace commented Aug 8, 2023

Resolves #1026
Dependent on #1557

Writing individual keystrokes to the HID interface is slow, resulting in API requests to paste large amounts of text to timeout. This PR writes multiple keystrokes to the HID interface at once, and does so asynchronously.

Review on CodeApprove

@jdeanwallace jdeanwallace changed the base branch from master to text-to-hid-keystroke August 8, 2023 12:59
@mtlynch
Copy link
Contributor

mtlynch commented Aug 9, 2023

I'd like to take a step back and rethink how we resolve #1026.

I know that this is still in draft, but it seems like we've created a significant amount of churn moving code from JS to Python when it seems like the underlying problem is that the backend is not staying responsive to HTTP/websockets requests when it's processing a large number of sequential HID messages. I'd like to solve that underlying problem before we get too deep into the JS -> Python migration because it doesn't seem like this fix strictly depends on the migration.

My concern with this PR is:

  • Implementation details about app/hid/keyboard.py are bleeding out into api.py
    • Stuff like multiprocessing and bytearray feel too low level for API-level logic.
  • We're only fixing the lockup issue in the case of paste, but couldn't the same thing happen if the user types too quickly?

Is there a way we can fix #1026 in the app/hid modules? It's okay if we have to adjust the semantics to add a param like release_after or something, but I don't want to bleed out things like buffers or multiprocessing.

I'd like to solve the threading issue first, because if we fix that, then maybe the whole chain starting from #1545 becomes unnecessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pastes stop after dozens to hundreds of characters
2 participants