-
Notifications
You must be signed in to change notification settings - Fork 246
fix: hidRPC handshake packet should be only sent once #969
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
Conversation
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 refactors the HID RPC handshake mechanism to ensure the handshake packet is sent only once per connection. The changes move handshake logic from a React useEffect hook (which could trigger multiple times) to a dedicated doRpcHidHandshake function called once during channel creation.
- Extracted handshake logic into a standalone
doRpcHidHandshakefunction with retry logic and timeouts - Added tslog library for structured logging throughout the HID RPC module
- Removed redundant handshake triggers from useEffect and event handlers that caused duplicate handshakes
- Configured Vite to tree-shake
logger.debugcalls in production builds
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/vite.config.ts | Added logger.debug to the esbuild pure array for production builds to remove debug logs |
| ui/src/routes/devices.$id.tsx | Imported and called doRpcHidHandshake during channel setup; added setRpcHidProtocolVersion to dependencies |
| ui/src/hooks/useHidRpc.ts | Extracted handshake logic to doRpcHidHandshake function with retry/timeout logic; replaced console logging with tslog logger; removed old handshake triggering code from useEffect |
| ui/package.json | Added tslog ^4.10.2 dependency |
| ui/package-lock.json | Added tslog package lock entry |
Files not reviewed (1)
- ui/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6196bfd to
a9a495a
Compare
a9a495a to
f7b51fb
Compare
IDisposable
left a comment
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.
Just a little suggestion, not required.
fixes #806