refactor: typed Trezor device-busy handling#617
Open
jvsena42 wants to merge 4 commits into
Open
Conversation
… 0.3.9 bitkit-core bump and add regression tests
Member
Author
|
Waiting for parent PR merge |
Greptile SummaryThis PR updates Trezor busy-device handling to use typed errors. The main changes are:
Confidence Score: 5/5This looks safe to merge.
|
| Filename | Overview |
|---|---|
| Bitkit/Extensions/TrezorError+Cancellation.swift | Adds typed busy-device detection with AppError unwrapping. |
| Bitkit/Managers/TrezorManager.swift | Uses typed busy detection for retry behavior and shared error presentation. |
| Bitkit/Utilities/TrezorErrorPresenter.swift | Adds the shared Trezor user-message mapper. |
| Bitkit/ViewModels/Trezor/TrezorViewModel.swift | Delegates dashboard Trezor error messages to the shared presenter. |
| BitkitTests/TrezorDeviceBusyTests.swift | Adds tests for typed busy handling and presenter output. |
Reviews (2): Last reviewed commit: "Merge branch 'chore/bump-bitkit-core-0.3..." | Re-trigger Greptile
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aea4314d7c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ge-verification failures
…bitkit-ios into chore/bump-bitkit-core-0.3.9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #589
This PR switches Trezor locked/busy detection to the typed
DeviceBusyerror that bitkit-core surfaces since the 0.3.9 bump (a locked device is now mapped fromDeviceLockedtoDeviceBusyin core), replacing the previous substring matching on stringified error text.It also consolidates the two near-duplicate Trezor error-to-message mappers — one in the manager, one in the dashboard view model — into a single shared presenter. That closes a gap where the dashboard flow showed a raw technical string for a locked device instead of the friendly "unlock it" prompt.
QA Notes
couldn't simulate the locker on emulator
Manual Tests
regression:Lock the Trezor (powered, PIN-locked) → connect from the hardware dashboard: shows the "Trezor is busy, unlock it" message rather than a raw error, and does not loop reconnecting.regression:Lock the Trezor → sign a transfer-to-spending transaction: same busy prompt; after unlocking on device, retry succeeds.regression:Trigger other Trezor errors (timeout, disconnect, cancel on device): user-facing messages are unchanged from before.Automated Checks
BitkitTests/TrezorDeviceBusyTests.swiftcovers the typed busy classification (raw and app-error-wrapped) and the shared presenter mapping to the unlock prompt.xcodebuildbuild-for-testing plusTrezorDeviceBusyTestsandTrezorCancellationTestson the iPhone 16 simulator locally: all pass.Linked Issues/Tasks
Refs #589
Screenshot / Video
cancel-and-retry.mov