[codex] Remove SwiftLint disable directives#357
Open
riderx wants to merge 6 commits into
Open
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
Warning Review limit reached
More reviews will be available in 14 minutes and 53 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR reorganizes and extends the iOS CameraPreview plugin into modular extensions: session discovery and outputs, device input/swap/cleanup, photo/video capture and processing (EXIF/GPS/overlays), preview rendering and layout (aspect/grid/webview), device controls (flash/torch/zoom/focus), gesture handling, barcode scanning, AV delegates and plugin lifecycle/permission/location integration. ChangesCamera Preview Plugin Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
|
Contributor
Beta npm buildMaintainers can publish this PR to npm for fast testing. Comment The workflow will:
Security note: beta publish is only enabled for branches inside this repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 27
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+Delegates.swift:
- Line 188: Remove the stray debug print statements (e.g., print("down") and the
similar prints around lines 188/192/196) from the
CameraController+Delegates.swift delegate handlers in CameraController; either
delete them or wrap them behind a debug-only flag (e.g., `#if` DEBUG) so
production logs aren’t cluttered, updating the orientation/observer delegate
methods where those prints occur to use the debug guard or be removed entirely.
- Line 230: In CameraController+Delegates.swift replace the explicit initializer
call in the return statement (currently using UIImage.init(cgImage: newCGImage,
scale: 1, orientation: .up)) with the shorthand type initializer
(UIImage(cgImage: newCGImage, scale: 1, orientation: .up)) to satisfy SwiftLint;
update the return inside the method that constructs the image so it uses
UIImage(...) instead of UIImage.init(...).
- Around line 204-215: The mirrored-orientation branch is discarding
CGAffineTransform results by assigning to `_`; update the mirrored cases in
CameraController+Delegates.swift so each transform operation is applied to the
existing transform (e.g., transform = transform.translatedBy(...) and transform
= transform.scaledBy(...)) instead of discarding them, ensuring the flipped
transforms for .upMirrored, .downMirrored, .leftMirrored, and .rightMirrored are
actually applied before use.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+Discovery.swift:
- Around line 41-44: The loop in CameraController+Discovery.swift iterates over
cameras and evaluates camera.isVirtualDevice ? camera.constituentDevices.count :
1 but discards the result (dead code); either remove this unused expression or
replace it with the intended behavior — e.g., use the value (accumulate a device
count, assign to a variable, or log it) inside the for-in loop — referencing the
symbols cameras, camera.isVirtualDevice, and camera.constituentDevices to locate
the code and implement the fix.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+FlashZoomFocus.swift:
- Around line 11-24: The code force-unwraps frontCamera and rearCamera when
mapping currentCameraPosition (and similarly in getHorizontalFov, setFlashMode,
setTorchMode), risking a crash; update these to use safe optional binding
instead: switch on currentCameraPosition and assign currentCamera using if-let
(or guard-let) to safely unwrap self.frontCamera/self.rearCamera, and if nil
throw CameraControllerError.noCamerasAvailable (or return an appropriate error)
before using the device; apply the same pattern inside getHorizontalFov,
setFlashMode, and setTorchMode so none of those functions force-unwrap
frontCamera/rearCamera.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+Gestures.swift:
- Around line 14-30: The tap-to-focus path uses previewLayer to compute
devicePoint, but when previewLayer is nil devicePoint is nil and the code falls
back to 0,0; update the handler (the block that computes devicePoint and calls
showFocusIndicator) to early-return (or skip configuring device focus/exposure)
when previewLayer is nil: show the visual indicator if desired (using
showFocusIndicator and disableFocusIndicator) but do not set
device.focusPointOfInterest or device.exposurePointOfInterest using a nil
devicePoint; additionally log a warning that previewLayer is unavailable so
focus configuration is skipped. Ensure you change the logic around previewLayer,
devicePoint and the device.lockForConfiguration/setting focus sections so they
only run when devicePoint is non-nil.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+ImageProcessing.swift:
- Around line 10-37: The addGPSMetadata(to:location:) function currently
discards the created JPEG with GPS metadata; change its signature to return
Data? (or Data) and at the end return destData as Data if
CGImageDestinationFinalize succeeds (nil on failure). Update the call site in
handlePhotoCaptureResult (in CameraController+PhotoCapture.swift) to accept the
returned Data and use it as the image payload (replace the previously used
image.jpegData or original image data) so the GPS-embedded JPEG is actually
stored/sent; keep the rest of the metadata construction (gpsDict, destination
creation) as-is and only change the return and caller usage.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+PhotoCapture.swift:
- Line 10: The
captureImage(width:height:quality:gpsLocation:embedTimestamp:embedLocation:photoQualityPrioritization:completion:)
function currently ignores the quality parameter; store that quality Float on
the PhotoCaptureContext (e.g., add a quality property to PhotoCaptureContext and
set it where you create the context in captureImage) and then use that context.
Apply the saved quality when encoding the final image (where UIImage/CGImage is
converted to JPEG/Data) by passing context.quality as the compressionQuality to
UIImageJPEGRepresentation / jpegData(compressionQuality:) (or equivalent
encoder) so the provided quality affects final JPEG output; update any call
sites that construct PhotoCaptureContext to carry the new property.
- Around line 126-128: The call to addGPSMetadata(to:location:) currently
discards the JPEG data it creates so GPS metadata is never embedded; change
addGPSMetadata(to:location:) in CameraController+ImageProcessing.swift to return
the modified Data (e.g., -> Data?) and then update the call site in
CameraController+PhotoCapture.swift to assign the returned Data back to the
image variable (or otherwise pass that Data into createImageDataWithExif in
CameraPreview+Capture.swift). Ensure any other callers of addGPSMetadata are
updated to accept the returned Data or handle nil, and remove the void-only
implementation to avoid the discarded-result bug.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+Session.swift:
- Around line 45-49: Replace the fallthrough usage in the switch that sets
connection.videoOrientation so that .unknown and `@unknown` default are handled
explicitly together (remove "fallthrough" and put both selectors in the same
case branch) — there are two occurrences where connection.videoOrientation =
.portrait is used, so update both switch blocks to use "case .unknown, `@unknown`
default:" and set connection.videoOrientation = .portrait inside that branch.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+Switching.swift:
- Around line 128-132: The current code calls
targetDevice.unlockForConfiguration() unconditionally after using try?
targetDevice.lockForConfiguration(), which can crash if the lock failed; update
the block in CameraController+Switching.swift to only unlock when the lock
succeeds by using a do { try targetDevice.lockForConfiguration(); defer {
targetDevice.unlockForConfiguration() } ... } catch { /* handle or ignore */ }
pattern (or check that try? targetDevice.lockForConfiguration() != nil) around
the isFocusModeSupported(.continuousAutoFocus) / focusMode assignment so
unlockForConfiguration() is only called when lockForConfiguration() succeeded.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+Video.swift:
- Around line 77-92: The stopRecording method currently calls completion
immediately after fileVideoOutput.stopRecording(), which can return an
incomplete file; instead add a stored property on CameraController like
videoRecordingCompletion: ((URL?, Error?) -> Void)?; modify stopRecording to set
this property and call fileVideoOutput.stopRecording() without invoking the
completion, and implement fileOutput(_:didFinishRecordingTo:from:error:) in your
delegate (CameraController+Delegates) to capture the stored
videoRecordingCompletion, clear it, and invoke it with the final outputFileURL
and error so the completion fires only after finalization.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraPreview`+Capture.swift:
- Around line 377-384: The current code in CameraPreview+Capture.swift quietly
does nothing when imageData is nil because imageData?.write(to:) is a no-op;
update the capture completion block to explicitly check that imageData is
non-nil before attempting to write: if imageData is nil, call.call.reject(...)
with a clear error message and return; otherwise getTempFilePath(), try writing
imageData (non-optional) and on success call.resolve(["value":
fileUrl.absoluteString]) and on failure call.reject with the caught error.
Reference the optional imageData variable, getTempFilePath(), and the
call.resolve/call.reject calls to find where to add the nil-check and improved
error handling.
- Around line 96-98: The variables embedTimestamp and embedLocationRequested in
CameraPreview+Capture.swift are using redundant optional coalescing (the
trailing "?? false") even though call.getBool("...", false) already returns a
non-optional Bool; remove the "?? false" from the assignments to embedTimestamp
and embedLocationRequested and keep the effectiveEmbedLocation computation as
effectiveEmbedLocation = (withExifLocation ?? false) && embedLocationRequested
to preserve behavior.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraPreview`+DeviceControls.swift:
- Around line 35-81: Add the same isInitialized guard used in
getZoom/setZoom/getFlashMode to the three methods getSupportedFlashModes,
getHorizontalFov, and setFlashMode: at the top of each method check if
self.isInitialized is true, and if not call.reject with a clear "Camera not
initialized" (or the project's standard init error message) and return; this
prevents using self.cameraController before initialization and mirrors the
existing pattern used elsewhere.
- Around line 83-106: Both startRecordVideo and stopRecordVideo call
cameraController without checking initialization; add the same isInitialized()
guard used by other control methods: at the top of startRecordVideo(_:) and
stopRecordVideo(_:) call isInitialized() and if it returns false immediately
call.reject("Camera not initialized") (or the existing init error message) and
return, otherwise proceed to call cameraController.captureVideo() /
cameraController.stopRecording(...). Ensure the guard prevents any
cameraController interaction when not initialized and that you return after
reject to avoid executing subsequent code.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraPreview`+Lifecycle.swift:
- Around line 102-120: The current use of DispatchQueue.main.sync in the
teardown block (where self.cameraController.removeGridOverlay(),
previewView.removeFromSuperview(), webView restoration,
cameraController.cleanup(), NotificationCenter removal,
stopOrientationNotificationsIfNeeded(), and isInitialized/isInitializing toggles
occur) can deadlock if called from the main thread (e.g., start(_:) on
Capacitor). Change the synchronization to be main-thread-safe by either: (a)
replacing DispatchQueue.main.sync with DispatchQueue.main.async, or (b) guarding
with Thread.isMainThread and executing the teardown inline when already on the
main thread, otherwise dispatch async; apply this change around the block
referencing DispatchQueue.main.sync, self.previewView, self.webView,
self.cameraController, and the NotificationCenter call.
- Around line 301-303: The code registers
UIDevice.orientationDidChangeNotification twice:
startOrientationNotificationsIfNeeded() already adds an observer for
handleOrientationChange (called from handleCameraPrepared), and the block in
CameraPreview+Lifecycle.swift adds a second observer that calls rotated when
rotateWhenOrientationChanged is true; remove the duplicate
NotificationCenter.addObserver call (or change it to use the same handler) so
orientation notifications are only registered once—update the
rotateWhenOrientationChanged branch to rely on
startOrientationNotificationsIfNeeded()/handleOrientationChange or consolidate
rotated into that single handler (references:
startOrientationNotificationsIfNeeded, handleOrientationChange, rotated,
rotateWhenOrientationChanged).
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraPreview`+Location.swift:
- Around line 100-102: The locationManager(_:didFailWithError:) currently only
logs errors and never signals callers; update it to invoke the stored
locationCompletion with a failure result (or nil location and the error) so
getCurrentLocation callers do not hang, then stop/cleanup the CLLocationManager
and nil-out locationCompletion; ensure the completion is dispatched on the main
queue and set to nil after invoking to avoid double-calls (refer to
locationManager(_:didFailWithError:), locationCompletion, and
getCurrentLocation).
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraPreview`+PreviewOptions.swift:
- Around line 32-56: The else branch in rawSetAspectRatio() is dead because posX
and posY are unconditionally set to -1; either stop forcing auto-centering or
remove the unreachable manual-positioning branch: modify rawSetAspectRatio() to
either (A) remove the else block that computes availableWidth/availableHeight
from posX/posY and only use the auto-center logic, or (B) move the lines
self.posX = -1 and self.posY = -1 behind a conditional so manual positioning can
be honored; update references to availableWidth and availableHeight accordingly
(functions/variables: rawSetAspectRatio, posX, posY, availableWidth,
availableHeight).
- Around line 99-108: call.resolve() is currently called immediately after
DispatchQueue.main.async, causing a race where the grid overlay
(cameraController.removeGridOverlay / cameraController.addGridOverlay) may not
be applied before the promise resolves; move the call.resolve() into the
DispatchQueue.main.async closure and invoke it after performing the grid update
on self.previewView (i.e., after calling removeGridOverlay or addGridOverlay
based on gridMode) so the promise resolves only once the UI update has been
scheduled on the main thread.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraPreview`+StopPermissions.swift:
- Around line 100-106: The code redundantly calls
NotificationCenter.default.removeObserver(self) and then again removes the
specific UIDevice.orientationDidChangeNotification observer; remove the specific
call so only NotificationCenter.default.removeObserver(self) remains. In the
same block ensure you still check and clear
isGeneratingDeviceOrientationNotifications and call
UIDevice.current.endGeneratingDeviceOrientationNotifications() when true,
leaving NotificationCenter.default.removeObserver(self) followed by the existing
UIDevice notification teardown to keep the restartable plugin cleanup intact.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraPreview`+WebView.swift:
- Around line 188-199: In mapAudioPermission, remove the fallthrough and combine
the .undetermined and `@unknown` default cases into a single case that returns
"prompt"; update the switch on AVAudioSession.RecordPermission in the
mapAudioPermission(_:) function so it handles .granted -> "granted", .denied ->
"denied", and a combined case for .undetermined and `@unknown` default ->
"prompt".
- Around line 176-187: The function mapAuthorizationStatus uses a fallthrough
for .notDetermined to reach the `@unknown` default; remove the fallthrough and
combine those cases instead. Update mapAuthorizationStatus so that case
.notDetermined and `@unknown` default are declared together (e.g., case
.notDetermined, `@unknown` default:) and return "prompt", and delete the
fallthrough line to satisfy SwiftLint and simplify the switch.
- Line 67: Remove the unused timing call by deleting the stray "_ =
CFAbsoluteTimeGetCurrent()" invocation in CameraPreview+WebView.swift (inside
the CameraPreview+WebView extension/method where it appears); ensure no other
logic depends on its side effects and run tests/build to confirm nothing else
references that timing value.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraPreview`+ZoomOrientation.swift:
- Around line 91-100: The comment for known identifiers and the set used in
isProModelSupportingOptical2x() are out of sync: the comment includes
iPhone18,1/18,2 (17 Pro/17 Pro Max) but supportedIdentifiers stops at
iPhone17,1/17,2; update the code to be consistent by either adding "iPhone18,1"
and "iPhone18,2" to the supportedIdentifiers Set if those models should support
optical 2x, or remove the iPhone18 entries from the comment if they should not
be considered supported; make the change within the
isProModelSupportingOptical2x() definition so the comment and the
supportedIdentifiers Set match.
In `@ios/Sources/CapgoCameraPreviewPlugin/Plugin.swift`:
- Line 37: The exposed mutable state and callbacks (isInitializing,
isInitialized, waitingForLocation, isPresentingPermissionAlert,
permissionCompletion, locationCompletion) must be confined to a single execution
context; make these properties private again and ensure all reads/writes occur
on the main queue (or a dedicated serial queue) — specifically, in
CameraPreview+Location.swift, dispatch to DispatchQueue.main before
accessing/clearing locationCompletion inside
locationManager(_:didUpdateLocations:), and in CameraPreview+Lifecycle.swift
dispatch to DispatchQueue.main when mutating isInitializing/hasResolvedStartCall
in start(_:). Also either remove/privatize unused backgroundSession or
document/protect its access. Ensure every lifecycle/location/capture path
marshals to the same queue so handlers are resolved only on that queue.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3afda4d7-a4ed-4b74-91a6-d3c1a712241f
📒 Files selected for processing (26)
ios/Sources/CapgoCameraPreviewPlugin/CameraController+Barcode.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+Delegates.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+DeviceSwap.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+Discovery.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+Exposure.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+FlashZoomFocus.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+Gestures.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+ImageOverlay.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+ImageProcessing.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+PhotoCapture.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+Preview.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+Session.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+Switching.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+Video.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController.swiftios/Sources/CapgoCameraPreviewPlugin/CameraPreview+Capture.swiftios/Sources/CapgoCameraPreviewPlugin/CameraPreview+DeviceControls.swiftios/Sources/CapgoCameraPreviewPlugin/CameraPreview+Exposure.swiftios/Sources/CapgoCameraPreviewPlugin/CameraPreview+GalleryLayout.swiftios/Sources/CapgoCameraPreviewPlugin/CameraPreview+Lifecycle.swiftios/Sources/CapgoCameraPreviewPlugin/CameraPreview+Location.swiftios/Sources/CapgoCameraPreviewPlugin/CameraPreview+PreviewOptions.swiftios/Sources/CapgoCameraPreviewPlugin/CameraPreview+StopPermissions.swiftios/Sources/CapgoCameraPreviewPlugin/CameraPreview+WebView.swiftios/Sources/CapgoCameraPreviewPlugin/CameraPreview+ZoomOrientation.swiftios/Sources/CapgoCameraPreviewPlugin/Plugin.swift
…t-disables # Conflicts: # ios/Sources/CapgoCameraPreviewPlugin/Plugin.swift
Contributor
|
Actionable comments posted: 0 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
ios/Sources/CapgoCameraPreviewPlugin/CameraController+PhotoCapture.swift (1)
10-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
qualityparameter is unused.The
quality: Floatparameter on line 10 is never stored inPhotoCaptureContext(lines 49-56) or used during photo capture/encoding. If quality should affect JPEG compression, it needs to be stored and applied during final image encoding.🐛 Proposed fix to store and use quality
struct PhotoCaptureContext { let width: Int? let height: Int? + let quality: Float let gpsLocation: CLLocation? let embedTimestamp: Bool let embedLocation: Bool let completion: (UIImage?, Data?, [AnyHashable: Any]?, Error?) -> Void }Then update the context creation:
let captureContext = PhotoCaptureContext( width: width, height: height, + quality: quality, gpsLocation: gpsLocation, embedTimestamp: embedTimestamp, embedLocation: embedLocation, completion: completion )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+PhotoCapture.swift around lines 10 - 23, The captureImage function ignores the quality: Float parameter—add a quality property to the PhotoCaptureContext type, pass the quality into the context when creating captureContext in captureImage, and then apply that quality value when encoding the final UIImage to JPEG (e.g., use jpegData(compressionQuality:) or equivalent) so the provided quality controls JPEG compression during the final image/data creation; update PhotoCaptureContext and any photo-finalization code that converts UIImage→Data to read and use captureContext.quality.ios/Sources/CapgoCameraPreviewPlugin/CameraController+ImageProcessing.swift (1)
10-37:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winGPS metadata result is discarded — metadata never embedded.
addGPSMetadatacreates JPEG data with GPS metadata (destData) but returnsVoid, discarding the result. This function performs work that has no effect. Either return the data and use it in the caller, or remove this dead code since GPS embedding appears to be handled elsewhere.🐛 Proposed fix to return the GPS-embedded data
- func addGPSMetadata(to image: UIImage, location: CLLocation) { + func addGPSMetadata(to image: UIImage, location: CLLocation) -> Data? { guard let jpegData = image.jpegData(compressionQuality: 1.0), let source = CGImageSourceCreateWithData(jpegData as CFData, nil), - let uti = CGImageSourceGetType(source) else { return } + let uti = CGImageSourceGetType(source) else { return nil } var metadata = CGImageSourceCopyPropertiesAtIndex(source, 0, nil) as? [String: Any] ?? [:] // ... existing GPS dict construction ... let destData = NSMutableData() - guard let destination = CGImageDestinationCreateWithData(destData, uti, 1, nil) else { return } + guard let destination = CGImageDestinationCreateWithData(destData, uti, 1, nil) else { return nil } CGImageDestinationAddImageFromSource(destination, source, 0, metadata as CFDictionary) - CGImageDestinationFinalize(destination) + guard CGImageDestinationFinalize(destination) else { return nil } + return destData as Data }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+ImageProcessing.swift around lines 10 - 37, The addGPSMetadata(to:location:) function builds a JPEG with GPS metadata into destData but returns Void, so the resulting GPS-embedded data is discarded; change the API to return the new Data (or Data?) and wire it back to callers: update the function signature addGPSMetadata(to:location:) to return Data? (or optionally throw) containing destData as Data after CGImageDestinationFinalize, ensure callers of CameraController.addGPSMetadata use the returned Data to replace the original image bytes (or propagate it to wherever embedding is expected), and remove dead-paths that ignore destData; keep references to CGImageDestinationCreateWithData, CGImageDestinationAddImageFromSource, and CGImageDestinationFinalize when extracting/returning the Data.ios/Sources/CapgoCameraPreviewPlugin/CameraController+Video.swift (1)
77-92:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftRace condition: completion called before video file is finalized.
AVCaptureFileOutput.stopRecording()is asynchronous—the file isn't complete until the delegate'sfileOutput(_:didFinishRecordingTo:from:error:)is invoked. Calling the completion immediately on line 91 can return an incomplete/corrupted file URL and silently ignores any finalization errors.Store the completion handler and invoke it from the delegate method instead.
🐛 Proposed fix
Add a property to store the pending completion (in CameraController properties):
var videoRecordingCompletion: ((URL?, Error?) -> Void)?Refactor
stopRecording:func stopRecording(completion: `@escaping` (URL?, Error?) -> Void) { guard let captureSession = self.captureSession, captureSession.isRunning else { completion(nil, CameraControllerError.captureSessionIsMissing) return } guard let fileVideoOutput = self.fileVideoOutput else { completion(nil, CameraControllerError.fileVideoOutputNotFound) return } + // Store completion to invoke from delegate + self.videoRecordingCompletion = completion + // Stop recording video fileVideoOutput.stopRecording() - - // Return the video file URL in the completion handler - completion(self.videoFileURL, nil) }In
CameraController+Delegates.swift, invoke the stored completion:func fileOutput(_ output: AVCaptureFileOutput, didFinishRecordingTo outputFileURL: URL, from connections: [AVCaptureConnection], error: Error?) { let completion = self.videoRecordingCompletion self.videoRecordingCompletion = nil completion?(outputFileURL, error) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+Video.swift around lines 77 - 92, The stopRecording implementation currently calls completion immediately which races with AVCaptureFileOutput finalization; add a stored property on CameraController like videoRecordingCompletion: ((URL?, Error?) -> Void)? and in stopRecording assign the passed completion to that property (instead of calling it), call fileVideoOutput.stopRecording(), and then invoke and clear videoRecordingCompletion from the AVCaptureFileOutput delegate method fileOutput(_:didFinishRecordingTo:from:error:) so the completion is only called with the finalized outputFileURL and any error.ios/Sources/CapgoCameraPreviewPlugin/CameraController+Delegates.swift (1)
200-210:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMirrored image orientations are not corrected.
The mirrored cases (
.upMirrored,.downMirrored,.leftMirrored,.rightMirrored) now fall through tobreak, which means mirrored images will not be properly flipped. The previous review correctly identified that the transform operations were being discarded with_ =, but the fix should have been to apply the transforms totransform, not to remove them entirely.🐛 Proposed fix to apply mirroring transforms
// Flip image one more time if needed to, this is to prevent flipped image switch imageOrientation { case .upMirrored, .downMirrored: - break + transform = transform.translatedBy(x: size.width, y: 0) + transform = transform.scaledBy(x: -1, y: 1) case .leftMirrored, .rightMirrored: - break + transform = transform.translatedBy(x: size.height, y: 0) + transform = transform.scaledBy(x: -1, y: 1) case .up, .down, .left, .right: break `@unknown` default: break }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+Delegates.swift around lines 200 - 210, The mirrored image orientations in the imageOrientation switch are incorrectly ignored; update the cases .upMirrored, .downMirrored, .leftMirrored and .rightMirrored to apply the appropriate CGAffineTransform operations to the existing transform variable (rather than discarding with `_ =` or using break), so the mirror flips are accumulated into transform before it is used; locate the switch on imageOrientation in CameraController+Delegates.swift and modify those mirrored cases to update transform (e.g., concatenate a scale or translation as needed) instead of leaving them as no-ops.ios/Sources/CapgoCameraPreviewPlugin/CameraPreview+PreviewOptions.swift (1)
88-98:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
call.resolve()called before async grid update completes.
call.resolve()on line 97 executes immediately after dispatching the async block, before the grid overlay update on lines 90-94 completes. If the caller expects the grid to be updated when the promise resolves, this is a race condition.🐛 Proposed fix
// Update grid overlay DispatchQueue.main.async { if gridMode == "none" { self.cameraController.removeGridOverlay() } else { self.cameraController.addGridOverlay(to: self.previewView, gridMode: gridMode) } + call.resolve() } - - call.resolve() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/Sources/CapgoCameraPreviewPlugin/CameraPreview`+PreviewOptions.swift around lines 88 - 98, call.resolve() is being invoked before the async DispatchQueue.main.async block that updates the grid overlay finishes, creating a race; move the resolve into the dispatched closure so the promise resolves only after the UI update completes. Specifically, inside the closure that calls cameraController.removeGridOverlay() or cameraController.addGridOverlay(to: self.previewView, gridMode: gridMode), call call.resolve() after those calls (ensuring it runs on the main queue), so the caller gets resolved state only once the grid update is done.
♻️ Duplicate comments (4)
ios/Sources/CapgoCameraPreviewPlugin/CameraController+PhotoCapture.swift (1)
126-128:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winGPS metadata is computed but never applied.
addGPSMetadatareturnsVoid(seeCameraController+ImageProcessing.swift), so this call has no effect. The GPS-embedded image data is discarded, andphotoDatapassed to the completion on line 138 is the original data without GPS metadata.🐛 Proposed fix (after fixing addGPSMetadata to return Data?)
- if let location = context.gpsLocation { - self.addGPSMetadata(to: image, location: location) - } + var imageDataWithGPS: Data? = photoData + if let location = context.gpsLocation { + imageDataWithGPS = self.addGPSMetadata(to: image, location: location) ?? photoData + } // ... processing ... - context.completion(finalImage, photoData, metadata, nil) + context.completion(finalImage, imageDataWithGPS, metadata, nil)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+PhotoCapture.swift around lines 126 - 128, The GPS bytes computed by addGPSMetadata are never applied because addGPSMetadata currently returns Void; update the flow so the processed image data with GPS is used before completing: change addGPSMetadata (in CameraController+ImageProcessing.swift) to return the modified Data (or provide a variant that accepts and returns Data), then in CameraController+PhotoCapture.swift replace the call self.addGPSMetadata(to: image, location: location) with an assignment like image = self.addGPSMetadata(image, location: location) (or call the returning variant) so that photoData passed to the completion is the GPS-embedded data.ios/Sources/CapgoCameraPreviewPlugin/CameraPreview+Location.swift (1)
99-101:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLocation failure not propagated to pending operations.
When location acquisition fails,
locationManager(_:didFailWithError:)only logs the error. ThelocationCompletionhandler is never called, potentially leavinggetCurrentLocationcallers waiting indefinitely.🐛 Proposed fix
public func locationManager(_ manager: CLLocationManager, didFailWithError error: Error) { print("[CameraPreview] locationManager didFailWithError: \(error.localizedDescription)") + manager.stopUpdatingLocation() + if let completion = locationCompletion { + completion(nil) + locationCompletion = nil + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/Sources/CapgoCameraPreviewPlugin/CameraPreview`+Location.swift around lines 99 - 101, The locationManager(_:didFailWithError:) currently only logs the error and never notifies pending callers; update this handler to invoke the pending locationCompletion used by getCurrentLocation (e.g., call locationCompletion with a failure/error result or pass the error and nil location), then clear/set locationCompletion to nil and stop/cleanup the CLLocationManager (stopUpdatingLocation/remove delegate) to avoid leaving callers waiting or the manager running. Ensure you reference the locationManager(_:didFailWithError:) method and the locationCompletion property when applying the fix.ios/Sources/CapgoCameraPreviewPlugin/CameraPreview+Lifecycle.swift (1)
310-313:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDuplicate observer registration for orientation changes.
startOrientationNotificationsIfNeeded()(called on line 244 viahandleCameraPrepared) already registers forUIDevice.orientationDidChangeNotificationwithhandleOrientationChangeselector. Lines 311-313 register a second observer with therotatedselector. This causes duplicate handling of orientation changes.🐛 Proposed fix - remove duplicate registration
- // Setup observers for device rotation and app state changes - if self.rotateWhenOrientationChanged == true { - NotificationCenter.default.addObserver(self, selector: `#selector`(CameraPreview.rotated), name: UIDevice.orientationDidChangeNotification, object: nil) - } + // Setup observers for app state changes (orientation observer already added in startOrientationNotificationsIfNeeded) NotificationCenter.default.addObserver(self, selector: `#selector`(CameraPreview.appDidBecomeActive), name: UIApplication.didBecomeActiveNotification, object: nil)Alternatively, consolidate
handleOrientationChangeandrotatedinto a single handler.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/Sources/CapgoCameraPreviewPlugin/CameraPreview`+Lifecycle.swift around lines 310 - 313, The code registers for UIDevice.orientationDidChangeNotification twice (startOrientationNotificationsIfNeeded() already adds an observer with selector handleOrientationChange, and the block in handleCameraPrepared adds another with selector rotated), causing duplicate callbacks; remove the duplicate NotificationCenter.addObserver call that registers rotated (or consolidate rotated into handleOrientationChange and remove the extra addObserver) so only one observer is registered for UIDevice.orientationDidChangeNotification and ensure rotated logic is invoked from the single handler.ios/Sources/CapgoCameraPreviewPlugin/CameraController+Gestures.swift (1)
14-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTap-to-focus silently targets (0,0) when previewLayer is nil.
When
previewLayeris nil,devicePointbecomes nil and the fallback?? CGPoint(x: 0, y: 0)sets focus to the top-left corner. The focus indicator still appears at the tap location, creating a mismatch. Consider returning early whenpreviewLayeris unavailable.🛡️ Proposed guard clause
`@objc` func handleTap(_ tap: UITapGestureRecognizer) { guard let device = self.currentCameraPosition == .rear ? rearCamera : frontCamera else { return } + guard let previewLayer = self.previewLayer else { return } let point = tap.location(in: tap.view) - let devicePoint = self.previewLayer?.captureDevicePointConverted(fromLayerPoint: point) - let focusPoint = devicePoint ?? CGPoint(x: 0, y: 0) + let focusPoint = previewLayer.captureDevicePointConverted(fromLayerPoint: point)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+Gestures.swift around lines 14 - 21, The tap-to-focus code falls back to CGPoint(x:0,y:0) when previewLayer is nil, causing the focus target to be top-left while the UI indicator is shown at the tap; change the logic in the gesture handler (referencing previewLayer, devicePoint, tap, showFocusIndicator, disableFocusIndicator) to guard that previewLayer and its captureDevicePointConverted(fromLayerPoint:) succeed and return early if they don't — if you still want the visual indicator, call showFocusIndicator(at: point, in: view) before returning; otherwise just return so no mismatch occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraPreview`+Lifecycle.swift:
- Around line 11-12: start(_:) calls startOnMain directly from an arbitrary
thread; ensure the plugin entry point always runs on the main thread by
dispatching to the main queue before invoking startOnMain so
prepareForCameraStart's validations/state checks run on the main thread (use a
Thread.isMainThread check and DispatchQueue.main.async/asyncIfNeeded to call
startOnMain(call) when not on the main thread); update the start(_:)
implementation to perform this dispatch and keep startOnMain and
prepareForCameraStart unchanged.
- Around line 266-280: The AVCaptureDevice.requestAccess(for: .video) completion
runs on an arbitrary queue, but the granted branch calls beginStart() directly;
move the beginStart() call onto the main thread (e.g., dispatch to
DispatchQueue.main.async) inside the requestAccess completion to match the
denied path behavior and ensure thread-safe UI/camera preparation; update the
completion in CameraPreview+Lifecycle.swift so the requestAccess granted branch
invokes beginStart() on the main queue and keep
handleDenied(authorizationStatus) unchanged.
---
Outside diff comments:
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+Delegates.swift:
- Around line 200-210: The mirrored image orientations in the imageOrientation
switch are incorrectly ignored; update the cases .upMirrored, .downMirrored,
.leftMirrored and .rightMirrored to apply the appropriate CGAffineTransform
operations to the existing transform variable (rather than discarding with `_ =`
or using break), so the mirror flips are accumulated into transform before it is
used; locate the switch on imageOrientation in CameraController+Delegates.swift
and modify those mirrored cases to update transform (e.g., concatenate a scale
or translation as needed) instead of leaving them as no-ops.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+ImageProcessing.swift:
- Around line 10-37: The addGPSMetadata(to:location:) function builds a JPEG
with GPS metadata into destData but returns Void, so the resulting GPS-embedded
data is discarded; change the API to return the new Data (or Data?) and wire it
back to callers: update the function signature addGPSMetadata(to:location:) to
return Data? (or optionally throw) containing destData as Data after
CGImageDestinationFinalize, ensure callers of CameraController.addGPSMetadata
use the returned Data to replace the original image bytes (or propagate it to
wherever embedding is expected), and remove dead-paths that ignore destData;
keep references to CGImageDestinationCreateWithData,
CGImageDestinationAddImageFromSource, and CGImageDestinationFinalize when
extracting/returning the Data.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+PhotoCapture.swift:
- Around line 10-23: The captureImage function ignores the quality: Float
parameter—add a quality property to the PhotoCaptureContext type, pass the
quality into the context when creating captureContext in captureImage, and then
apply that quality value when encoding the final UIImage to JPEG (e.g., use
jpegData(compressionQuality:) or equivalent) so the provided quality controls
JPEG compression during the final image/data creation; update
PhotoCaptureContext and any photo-finalization code that converts UIImage→Data
to read and use captureContext.quality.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+Video.swift:
- Around line 77-92: The stopRecording implementation currently calls completion
immediately which races with AVCaptureFileOutput finalization; add a stored
property on CameraController like videoRecordingCompletion: ((URL?, Error?) ->
Void)? and in stopRecording assign the passed completion to that property
(instead of calling it), call fileVideoOutput.stopRecording(), and then invoke
and clear videoRecordingCompletion from the AVCaptureFileOutput delegate method
fileOutput(_:didFinishRecordingTo:from:error:) so the completion is only called
with the finalized outputFileURL and any error.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraPreview`+PreviewOptions.swift:
- Around line 88-98: call.resolve() is being invoked before the async
DispatchQueue.main.async block that updates the grid overlay finishes, creating
a race; move the resolve into the dispatched closure so the promise resolves
only after the UI update completes. Specifically, inside the closure that calls
cameraController.removeGridOverlay() or cameraController.addGridOverlay(to:
self.previewView, gridMode: gridMode), call call.resolve() after those calls
(ensuring it runs on the main queue), so the caller gets resolved state only
once the grid update is done.
---
Duplicate comments:
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+Gestures.swift:
- Around line 14-21: The tap-to-focus code falls back to CGPoint(x:0,y:0) when
previewLayer is nil, causing the focus target to be top-left while the UI
indicator is shown at the tap; change the logic in the gesture handler
(referencing previewLayer, devicePoint, tap, showFocusIndicator,
disableFocusIndicator) to guard that previewLayer and its
captureDevicePointConverted(fromLayerPoint:) succeed and return early if they
don't — if you still want the visual indicator, call showFocusIndicator(at:
point, in: view) before returning; otherwise just return so no mismatch occurs.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraController`+PhotoCapture.swift:
- Around line 126-128: The GPS bytes computed by addGPSMetadata are never
applied because addGPSMetadata currently returns Void; update the flow so the
processed image data with GPS is used before completing: change addGPSMetadata
(in CameraController+ImageProcessing.swift) to return the modified Data (or
provide a variant that accepts and returns Data), then in
CameraController+PhotoCapture.swift replace the call self.addGPSMetadata(to:
image, location: location) with an assignment like image =
self.addGPSMetadata(image, location: location) (or call the returning variant)
so that photoData passed to the completion is the GPS-embedded data.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraPreview`+Lifecycle.swift:
- Around line 310-313: The code registers for
UIDevice.orientationDidChangeNotification twice
(startOrientationNotificationsIfNeeded() already adds an observer with selector
handleOrientationChange, and the block in handleCameraPrepared adds another with
selector rotated), causing duplicate callbacks; remove the duplicate
NotificationCenter.addObserver call that registers rotated (or consolidate
rotated into handleOrientationChange and remove the extra addObserver) so only
one observer is registered for UIDevice.orientationDidChangeNotification and
ensure rotated logic is invoked from the single handler.
In `@ios/Sources/CapgoCameraPreviewPlugin/CameraPreview`+Location.swift:
- Around line 99-101: The locationManager(_:didFailWithError:) currently only
logs the error and never notifies pending callers; update this handler to invoke
the pending locationCompletion used by getCurrentLocation (e.g., call
locationCompletion with a failure/error result or pass the error and nil
location), then clear/set locationCompletion to nil and stop/cleanup the
CLLocationManager (stopUpdatingLocation/remove delegate) to avoid leaving
callers waiting or the manager running. Ensure you reference the
locationManager(_:didFailWithError:) method and the locationCompletion property
when applying the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4712ac65-4ef6-4e26-951a-9ec84e019311
📒 Files selected for processing (10)
ios/Sources/CapgoCameraPreviewPlugin/CameraController+Delegates.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+Gestures.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+ImageProcessing.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+PhotoCapture.swiftios/Sources/CapgoCameraPreviewPlugin/CameraController+Video.swiftios/Sources/CapgoCameraPreviewPlugin/CameraPreview+DeviceControls.swiftios/Sources/CapgoCameraPreviewPlugin/CameraPreview+Lifecycle.swiftios/Sources/CapgoCameraPreviewPlugin/CameraPreview+Location.swiftios/Sources/CapgoCameraPreviewPlugin/CameraPreview+PreviewOptions.swiftios/Sources/CapgoCameraPreviewPlugin/CameraPreview+ZoomOrientation.swift
💤 Files with no reviewable changes (1)
- ios/Sources/CapgoCameraPreviewPlugin/CameraPreview+DeviceControls.swift
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
riderx
dismissed
coderabbitai[bot]’s stale review
Resolved by a646e8d; CodeRabbit recheck passed and no unresolved review threads remain.
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
1 participant
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.
What
swiftlint:disabledirectives from the iOS camera preview implementation.Why
How
Plugin.swiftandCameraController.swiftas state/core entry files.CameraPreview+...andCameraController+...extensions.Testing
bun run fmtbun run lintbun run verifyNot Tested
AI-assisted change, verified locally with the repository checks above.
Summary by CodeRabbit