-
Notifications
You must be signed in to change notification settings - Fork 215
Reduce traffic during pastes #829
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
base: dev
Are you sure you want to change the base?
Conversation
There are still some issues to be addressed. Firstly, the macro is being sent on the RPC HID channel and if it takes too long, the RPC HID decides it's timed-out. We need to either move it back to the plain WebRPC channel (so it doesn't attempt timeout work), or we need to allow some sort of timeout indicator. As it is, the timeout fires and forces the key status to clear, even though the playback continues in another thread. I've moved the macros to a distinct queue so we could likely just not let those timeout? Second, if someone pastes a large chunk of text then we end up with a REALLY big RPC call, so it doesn't look like the paste has even been acknowledged until the network call is complete... e.g. the entire set of packets has been sent. If we try to chunk up the macro into multiple messages, we run into problems because if we don't wait for the chunks IN ORDER to finish then we end up resetting the keyboard and paste state. |
bc2953d
to
51d80be
Compare
This is still not complete.
|
b3b8614
to
5c1c665
Compare
5c1c665
to
832106c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR reduces network traffic during paste operations by implementing better queue management and timeout handling for HID RPC messages. It introduces token-based macro cancellation, suspends keyboard state messages during macro execution, and allows more flexible delay configurations.
Key changes:
- Replaced single keyboard macro cancellation with token-based tracking for multiple concurrent macros
- Added timeout support to HID queue messages with different limits per message type
- Suspended keyboard down state messages during macro execution to reduce traffic
- Reduced minimum delay between keystrokes from 50ms to 0ms
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
webrtc.go | Refactored HID queue initialization and message handling with timeout support |
web.go | Updated to use new token-based macro cancellation function |
ui/src/hooks/useKeyboard.ts | Minor formatting cleanup removing extra blank lines |
ui/src/hooks/useHidRpc.ts | Updated cancel message to include empty token parameter |
ui/src/hooks/hidRpc.ts | Added UUID support and token-based cancellation messages |
ui/src/components/popovers/PasteModal.tsx | Reduced minimum delay from 50ms to 0ms and updated validation |
ui/package.json | Added uuid dependency for token generation |
jsonrpc.go | Implemented token-based macro tracking with concurrent execution support |
internal/usbgadget/hid_keyboard.go | Added message suspension functionality and improved keyboard report descriptor |
internal/hidrpc/message.go | Enhanced message string representations and added token state support |
internal/hidrpc/hidrpc.go | Added queue indices and timeout configurations per message type |
hidrpc.go | Updated message handling for token-based operations and improved logging |
cloud.go | Updated to use new token-based macro cancellation function |
Files not reviewed (1)
- ui/package-lock.json: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
d1df48e
to
6c84a31
Compare
Suspend KeyDownMessages while processing a macro. Make sure we don't emit huge debugging traces. Allow 30 seconds for RPC to finish (not ideal) Reduced default delay between keys (and allow as low as 0) Move the HID keyboard descriptor LED state as it seems to interfere with boot mode Run paste/macros in background on their own queue and return a token for cancellation. Fixed error in length check for macro key state. Removed redundant clear operation. Use Once instead of init() Add a time limit for each message type/queue.
6c84a31
to
61ee27e
Compare
Suspend KeyDownMessages while processing a macro.
Make sure we don't emit huge debugging traces.
Allow 30 seconds for RPC to finish (not ideal)
Reduced default delay between keys (and allow as low as 0)