-
Notifications
You must be signed in to change notification settings - Fork 129
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
Card scanner module - Business layer #2008
base: COIOS-826_OCR_feature
Are you sure you want to change the base?
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.
This pull request does not contain a valid label. Please add one of the following labels: ['new', 'changed', 'fixed', 'removed', 'deprecated', 'chore', 'improvement']
|
✅ No changes detectedComparing Analyzed targets: Adyen, AdyenActions, AdyenCard, AdyenCashAppPay, AdyenComponents, AdyenDelegatedAuthentication, AdyenDropIn, AdyenEncryption, AdyenSession, AdyenSwiftUI, AdyenTwint, AdyenWeChatPay |
private func configureCaptureSession() throws { | ||
captureSession.beginConfiguration() | ||
|
||
// Set up input |
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.
These comments are redundant, code speaks better
} | ||
|
||
private func configureCaptureDevice(_ device: AVCaptureDevice) { | ||
try? device.lockForConfiguration() |
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.
What happens if this fails?
@@ -46,17 +46,17 @@ class CardScannerViewController: UIViewController { | |||
addOverlayView() | |||
observeRoiLayoutChanges() | |||
|
|||
viewModel.configureSession() | |||
viewModel.viewDidLoad() |
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.
These are lifecycle methods of a UIViewController
, what is the reason to leak them to the view model layer? This approach might be fragile if we ever decide using this outside of UIKit (think of SwiftUI for example).
try? device.lockForConfiguration() | ||
|
||
// Adjust Frame Rate | ||
device.activeVideoMinFrameDuration = CMTime(value: 1, timescale: 15) // 15 fps |
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.
Should it move to Constants?
case .portrait: | ||
return .portrait | ||
case .landscapeRight: | ||
return .landscapeLeft // Camera flips the orientation |
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.
Why do we flip it for both left and right?
Summary
This PR introduces the core UI and business logic for the card scanner, following the MVVM architectural pattern.
CardScannerViewModel
AVCaptureSession
:AVCaptureDevice
:cropRegionOfInterest
.CardScannerViewController
CardScannerOverlayView
, which renders the mask and highlights the region of interest.Next Steps
CardImageParser
.Ticket
COIOS-826