-
Notifications
You must be signed in to change notification settings - Fork 137
[PM-39361] fix: Restart card scanner after camera interruption or reopen #2847
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,14 @@ struct CardScannerWrapperView: View { | |
| /// Drives `startScanning()`/`stopScanning()` via the SwiftUI view lifecycle. | ||
| @SwiftUI.State private var isScanning = false | ||
|
|
||
| /// Counts how many stop-then-restart cycles have been attempted this session. | ||
| /// Capped at 2 to avoid infinite loops; resets on each `.onAppear`. | ||
| @SwiftUI.State private var scannerRetryCount = 0 | ||
|
|
||
| /// Dismisses the sheet when the scanner gives up after exhausting retries. | ||
| @Environment(\.dismiss) private var dismiss | ||
| /// Used to restart scanning when the app returns to the foreground after a camera interruption. | ||
| @Environment(\.scenePhase) private var scenePhase | ||
|
|
||
| var body: some View { | ||
| NavigationView { | ||
|
|
@@ -35,11 +42,15 @@ struct CardScannerWrapperView: View { | |
| scanner: scanner, | ||
| onLinesUpdated: onLinesUpdated, | ||
| isScanning: $isScanning, | ||
| onScannerUnavailable: restartScanning, | ||
| ) | ||
| .padding(.horizontal, 12) | ||
| .padding(.top, 12) | ||
| .padding(.bottom, 35) | ||
| .onAppear { isScanning = true } | ||
| .onAppear { | ||
| scannerRetryCount = 0 | ||
| isScanning = true | ||
| } | ||
| .onDisappear { isScanning = false } | ||
| } | ||
| .navigationTitle(Localizations.scanCard) | ||
|
|
@@ -51,6 +62,28 @@ struct CardScannerWrapperView: View { | |
| } | ||
| } | ||
| .navigationViewStyle(.stack) | ||
| .onChange(of: scenePhase) { newPhase in | ||
| if newPhase == .active, isScanning { | ||
| restartScanning() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // MARK: Private | ||
|
|
||
| /// Stops scanning, waits 300 ms for the camera to fully release, then restarts. | ||
| /// After two retries the sheet is dismissed so the user is never left with a blank screen. | ||
| private func restartScanning() { | ||
| guard scannerRetryCount < 2 else { | ||
| dismiss() | ||
| return | ||
| } | ||
| scannerRetryCount += 1 | ||
| isScanning = false | ||
| Task { @MainActor in | ||
| try? await Task.sleep(for: .milliseconds(300)) | ||
| isScanning = true | ||
| } | ||
| } | ||
|
Comment on lines
+65
to
87
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Details and fix
Trace:
After two normal app-switches the sheet closes on the next foreground even though the camera is fine. A successful restart also never decrements the counter, so each recovery permanently shrinks the budget available for a later real failure. Consider resetting |
||
| } | ||
|
|
||
|
|
@@ -75,6 +108,10 @@ struct CardScannerView: UIViewControllerRepresentable { | |
| /// When `true`, scanning is active; when `false`, scanning is stopped. | ||
| @Binding var isScanning: Bool | ||
|
|
||
| /// Called when `startScanning()` throws or the scanner becomes unavailable at runtime | ||
| /// (e.g. camera interrupted). The wrapper uses this to schedule a stop-then-restart cycle. | ||
| var onScannerUnavailable: (() -> Void)? | ||
|
|
||
| // MARK: Static methods for UIViewControllerRepresentable | ||
|
|
||
| /// Stops scanning and clears the delegate when SwiftUI removes this representable from the hierarchy, | ||
|
|
@@ -103,7 +140,7 @@ struct CardScannerView: UIViewControllerRepresentable { | |
| // MARK: UIViewControllerRepresentable | ||
|
|
||
| func makeCoordinator() -> Coordinator { | ||
| Coordinator(onLinesUpdated: onLinesUpdated) | ||
| Coordinator(onLinesUpdated: onLinesUpdated, onScannerUnavailable: onScannerUnavailable) | ||
| } | ||
|
|
||
| func makeUIViewController(context: Context) -> DataScannerViewController { | ||
|
|
@@ -114,7 +151,11 @@ struct CardScannerView: UIViewControllerRepresentable { | |
|
|
||
| func updateUIViewController(_ uiViewController: DataScannerViewController, context: Context) { | ||
| if isScanning { | ||
| try? uiViewController.startScanning() | ||
| do { | ||
| try uiViewController.startScanning() | ||
| } catch { | ||
| DispatchQueue.main.async { context.coordinator.onScannerUnavailable?() } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. β Is the dispatch here just to avoid updating the view in the middle of a view update? |
||
| } | ||
| } else { | ||
| uiViewController.stopScanning() | ||
| } | ||
|
|
@@ -139,10 +180,15 @@ extension CardScannerView { | |
|
|
||
| let onLinesUpdated: ([String]) -> Void | ||
|
|
||
| /// Forwarded from `CardScannerView.onScannerUnavailable`; called when `startScanning()` | ||
| /// fails or the camera is interrupted at runtime. | ||
| var onScannerUnavailable: (() -> Void)? | ||
|
|
||
| // MARK: Initialization | ||
|
|
||
| init(onLinesUpdated: @escaping ([String]) -> Void) { | ||
| init(onLinesUpdated: @escaping ([String]) -> Void, onScannerUnavailable: (() -> Void)?) { | ||
| self.onLinesUpdated = onLinesUpdated | ||
| self.onScannerUnavailable = onScannerUnavailable | ||
| } | ||
|
|
||
| // MARK: DataScannerViewControllerDelegate | ||
|
|
@@ -173,6 +219,13 @@ extension CardScannerView { | |
| updateLines(from: allItems) | ||
| } | ||
|
|
||
| func dataScanner( | ||
| _ dataScanner: DataScannerViewController, | ||
| becameUnavailableWithError error: DataScannerViewController.ScanningUnavailable, | ||
| ) { | ||
| onScannerUnavailable?() | ||
| } | ||
|
|
||
| // MARK: Private Helpers | ||
|
|
||
| private func updateLines(from items: [RecognizedItem]) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import VisionKit | ||
| import XCTest | ||
|
|
||
| @testable import BitwardenShared | ||
|
|
||
| // MARK: - CardScannerViewTests | ||
|
|
||
| /// Tests for `CardScannerView.Coordinator`. | ||
| /// | ||
| /// `CardScannerWrapperView`'s retry/scene-phase logic is driven by SwiftUI `@State` and cannot | ||
| /// be meaningfully exercised without a live view host, so it is not covered here. | ||
| /// | ||
| @available(iOS 16.0, *) | ||
| class CardScannerViewTests: BitwardenTestCase { | ||
| // MARK: Tests | ||
|
|
||
| /// `dataScanner(_:becameUnavailableWithError:)` calls `onScannerUnavailable` when the scanner | ||
| /// reports an `unsupported` error. | ||
| func test_becameUnavailableWithError_callsOnScannerUnavailable_unsupported() { | ||
| var callbackInvoked = false | ||
| let subject = CardScannerView.Coordinator( | ||
| onLinesUpdated: { _ in }, | ||
| onScannerUnavailable: { callbackInvoked = true }, | ||
| ) | ||
|
|
||
| subject.dataScanner(CardScannerView.makeScanner(), becameUnavailableWithError: .unsupported) | ||
|
|
||
| XCTAssertTrue(callbackInvoked) | ||
| } | ||
|
|
||
| /// `dataScanner(_:becameUnavailableWithError:)` calls `onScannerUnavailable` when the scanner | ||
| /// reports a `cameraRestricted` error. | ||
| func test_becameUnavailableWithError_callsOnScannerUnavailable_cameraRestricted() { | ||
| var callbackInvoked = false | ||
| let subject = CardScannerView.Coordinator( | ||
| onLinesUpdated: { _ in }, | ||
| onScannerUnavailable: { callbackInvoked = true }, | ||
| ) | ||
|
|
||
| subject.dataScanner(CardScannerView.makeScanner(), becameUnavailableWithError: .cameraRestricted) | ||
|
|
||
| XCTAssertTrue(callbackInvoked) | ||
| } | ||
|
|
||
| /// `dataScanner(_:becameUnavailableWithError:)` does not crash when `onScannerUnavailable` is nil. | ||
| func test_becameUnavailableWithError_nilCallback_doesNotCrash() { | ||
| let subject = CardScannerView.Coordinator( | ||
| onLinesUpdated: { _ in }, | ||
| onScannerUnavailable: nil, | ||
| ) | ||
|
|
||
| subject.dataScanner(CardScannerView.makeScanner(), becameUnavailableWithError: .unsupported) | ||
| } | ||
| } |
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.
π€ Does the change of
scenePhasetrigger for you? I think the scene phase might only work if you're using SwiftUI navigation, not UIKit. You might need to observe the notification center notifications instead.