-
Notifications
You must be signed in to change notification settings - Fork 60
feat: Skip waiting for the first connection to the key #203
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
feat: Skip waiting for the first connection to the key #203
Conversation
Test Results: Windows 2 files 2 suites 5s ⏱️ Results for commit 5a53d2d. |
Test Results: Ubuntu 2 files 2 suites 5s ⏱️ Results for commit 5a53d2d. |
Test Results: MacOS 2 files 2 suites 5s ⏱️ Results for commit 5a53d2d. |
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.
PR Overview
This PR fixes an issue where the first connection experiences an unnecessary delay by changing the early-return condition in the reclaim timeout logic.
- Updated the condition in ConnectionFactory.cs to skip waiting when the current transport is either already active or is Transport.None.
- Adjusted the integration test in ReclaimTimeoutTests.cs to verify that the first connection completes quickly.
Reviewed Changes
| File | Description |
|---|---|
| Yubico.YubiKey/src/Yubico/YubiKey/ConnectionFactory.cs | Updated condition to ensure _device.LastActiveTransport is set when it was previously None, enabling immediate connection. |
| Yubico.YubiKey/tests/integration/Yubico/YubiKey/ReclaimTimeoutTests.cs | Adjusted the test to assert that the first connection is fast (under 100ms), reflecting the intended change. |
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
Description
There is a 100ms/3s delay on the first connection. The change is in waitForReclaimTimeout, Possible fix is to change the early-return check from in ConnectionFactory.cs, line 158.
Inside the if statement, _device.LastActiveTransport is updated to prevent it from remaining None indefinitely, which could lead to unexpected transport behavior.
Activity
Fixes: # <YESDK-1432>
Type of change
How has this been tested?
The test case public void SwitchingBetweenTransports_ForcesThreeSecondWait() has been updated to reflect this change. Now, the first connection no longer waits for three seconds, reducing the test duration from approximately 10s to 6s.
Checklist:
dotnet formatto format my code