[camera_avfoundation] Pigeon swift migration - part 2 #10980

RobertOdrowaz · 2026-02-08T19:30:07Z

Migrates camera package to the Swift-based pigeon interface as the last part of flutter/flutter#119109

This PR replaces the ObjC-based pigeon interfaces with Swift-based ones, thus removing the last remaining pieces of ObjC code in the package 🎉

I know there are a lot of changes in this PR, but there is no way to split it into smaller parts.

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩² ↩³

gemini-code-assist

Code Review

This pull request completes the migration of the camera_avfoundation package to a Swift-based Pigeon interface, removing the last Objective-C-based Pigeon interfaces. The changes are extensive and primarily involve:

Replacing Objective-C generated Pigeon files with new Swift-generated ones (Messages.swift).
Updating the CameraApi implementation in CameraPlugin.swift and DefaultCamera.swift to use the new Swift types and Result-based completion handlers.
Removing the Objective-C Pigeon target and related source files.
Updating all unit and integration tests to align with the new Swift API, including changes to mock objects and assertions to handle Result and PigeonError types.
Adjusting the podspec and Package.swift files to reflect the removal of the Objective-C target.

My feedback focuses on improving the maintainability of the updated test files by reducing code duplication.

gemini-code-assist · 2026-02-08T19:33:23Z

packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraPermissionTests.swift

    permissionManager.requestCameraPermission { error in
-      XCTAssertEqual(error, expectedError)
+      XCTAssertEqual(error?.code, "CameraAccessDeniedWithoutPrompt")
+      XCTAssertEqual(
+        error?.message,
+        "User has previously denied the camera access request. Go to Settings to enable camera access."
+      )
      expectation.fulfill()
    }


There's a lot of repeated code for asserting error properties across this test file. To improve maintainability and reduce duplication, consider adding a helper function to assert the properties of a PigeonError.

For example:

private func assertPermissionError( _ error: PigeonError?, hasCode expectedCode: String, hasMessage expectedMessage: String, file: StaticString = #file, line: UInt = #line ) { XCTAssertNotNil(error, "Expected a permission error, but got nil", file: file, line: line) XCTAssertEqual(error?.code, expectedCode, file: file, line: line) XCTAssertEqual(error?.message, expectedMessage, file: file, line: line) }

You could then simplify this test case and others like it:

permissionManager.requestCameraPermission { error in assertPermissionError( error, hasCode: "CameraAccessDeniedWithoutPrompt", hasMessage: "User has previously denied the camera access request. Go to Settings to enable camera access." ) expectation.fulfill() }

Pls let me know if you're in favor or agains a helper in those cases. I remember that you were against in an earlier PR but it would be a much simpler helper here

I'm ok with either approach. Both seem readable to me.

gemini-code-assist · 2026-02-08T19:33:23Z

...s/camera/camera_avfoundation/example/ios/RunnerTests/CameraPluginDelegatingMethodTests.swift

+    cameraPlugin.lockCaptureOrientation(orientation: targetOrientation) { result in
+      switch result {
+      case .success:
+        break
+      case .failure:
+        XCTFail("Unexpected error")
+      }
      expectation.fulfill()
    }


Throughout this test file, there are many switch statements to check for a successful Result and fail the test on failure. This pattern is repeated frequently. To make the tests more concise and reduce boilerplate, you could introduce a helper function.

For example, a helper could unwrap the success value or fail the test:

func assertSuccess<T>(_ result: Result<T, any Error>, file: StaticString = #file, line: UInt = #line) -> T? { switch result { case .success(let value): return value case .failure(let error): XCTFail("Expected success, but got failure: \(error)", file: file, line: line) return nil } }

This would simplify tests that expect success. For this test, it would look like:

cameraPlugin.lockCaptureOrientation(orientation: targetOrientation) { result in _ = assertSuccess(result) expectation.fulfill() }

For tests that also check the returned value, it would be even cleaner:

// Example from another test in this file cameraPlugin.stopVideoRecording { result in if let path = assertSuccess(result) { XCTAssertEqual(path, targetPath) } expectation.fulfill() }

This seems nice cleanup that saves lots of lines

RobertOdrowaz · 2026-02-10T20:23:15Z

@hellohuanlin pls take a look at this PR when you have some time

hellohuanlin · 2026-02-10T23:17:24Z

@RobertOdrowaz Hello, this PR is 2000 LOC and all in a single commit. Could you give a tour of the change and how i can start reviewing it, if possible?

RobertOdrowaz · 2026-02-11T21:54:54Z

I know it's over 2000 LOC, but the vast majority are trivial changes:

~1300 LOC is in Messages.swift which is generated by pigeon
~600 LOC in tests which is mostly very repetitive change from something like

camera.setZoomLevel(CGFloat(1.0)) { error in
      XCTAssertNotNil(error)
      XCTAssertEqual(error?.code, "ZOOM_ERROR")
      expectation.fulfill()
    }

to

camera.setZoomLevel(CGFloat(1.0)) { result in
      switch result {
      case .failure(let error as PigeonError):
        XCTAssertEqual(error.code, "ZOOM_ERROR")
      default:
        XCTFail("Expected failure")
      }
      expectation.fulfill()
    }

because methods now pass a Result union type instead of nullable error to the completion.

~100 LOC in CameraPlugin.swift mostly changes from nullable error to a Result in completions
~140 LOC in DefaultCamera.swift mostly changes from nullable error to a Result and PigeonError instead of FlutterError.

I can split 1, 2 and 3+4 into separate commits if it will help you but IMO going file by file is effectively the same.

hellohuanlin · 2026-02-11T22:11:22Z

@RobertOdrowaz Thanks for the tour. Don't worry about splitting it. Taking a look now.

hellohuanlin

LGTM with just a few questions

hellohuanlin · 2026-02-12T21:33:17Z

packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraMethodChannelTests.swift

-      settings: FCPPlatformMediaSettings.make(
-        with: FCPPlatformResolutionPreset.medium,
+      settings: PlatformMediaSettings(
+        resolutionPreset: PlatformResolutionPreset.medium,


nit: can infer type

hellohuanlin · 2026-02-12T21:34:42Z

packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraPermissionTests.swift

    permissionManager.requestCameraPermission { error in
-      XCTAssertEqual(error, expectedError)
+      XCTAssertEqual(error?.code, "CameraAccessDeniedWithoutPrompt")
+      XCTAssertEqual(
+        error?.message,
+        "User has previously denied the camera access request. Go to Settings to enable camera access."
+      )
      expectation.fulfill()
    }


I'm ok with either approach. Both seem readable to me.

hellohuanlin · 2026-02-12T21:36:27Z

packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraPermissionTests.swift

      handler(false)
    }
    permissionManager.requestCameraPermission { error in
-      XCTAssertEqual(error, expectedError)


curious why are we changing this? I don't see any problem with this approach. Is it related to equatable?

hellohuanlin · 2026-02-12T21:38:19Z

...s/camera/camera_avfoundation/example/ios/RunnerTests/CameraPluginDelegatingMethodTests.swift

+    cameraPlugin.lockCaptureOrientation(orientation: targetOrientation) { result in
+      switch result {
+      case .success:
+        break
+      case .failure:
+        XCTFail("Unexpected error")
+      }
      expectation.fulfill()
    }


This seems nice cleanup that saves lots of lines

hellohuanlin · 2026-02-12T21:39:17Z

packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraSettingsTests.swift

-      with: testResolutionPreset,
-      framesPerSecond: NSNumber(value: testFramesPerSecond),
-      videoBitrate: NSNumber(value: testVideoBitrate),
-      audioBitrate: NSNumber(value: testAudioBitrate),


nice to see these gone

hellohuanlin · 2026-02-12T21:40:33Z

.../camera/camera_avfoundation/ios/camera_avfoundation/Sources/camera_avfoundation/Camera.swift

  /// @param messenger Nullable messenger for capturing each frame.
  func startVideoRecording(
-    completion: @escaping (_ error: FlutterError?) -> Void,
+    completion: @escaping (Result<Void, any Error>) -> Void,


why is it changing from FlutterError to any Error?

hellohuanlin · 2026-02-12T21:44:37Z

packages/camera/camera_avfoundation/ios/camera_avfoundation.podspec

-  # Combine camera_avfoundation and camera_avfoundation_objc sources into a single pod, unlike
-  # SwiftPM, where separate Swift and Objective-C targets are required.
-  s.source_files = 'camera_avfoundation/Sources/camera_avfoundation*/**/*.{h,m,swift}'
-  s.public_header_files = 'camera_avfoundation/Sources/camera_avfoundation_objc/include/**/*.h'


Migrate to Swift-based pigeon interface

c4f140a

RobertOdrowaz requested review from LouiseHsu and hellohuanlin as code owners February 8, 2026 19:30

github-actions bot added p: camera platform-ios platform-macos labels Feb 8, 2026

gemini-code-assist bot reviewed Feb 8, 2026

View reviewed changes

stuartmorgan-g added the triage-ios Should be looked at in iOS triage label Feb 10, 2026

hellohuanlin approved these changes Feb 12, 2026

View reviewed changes

[camera_avfoundation] Pigeon swift migration - part 2 #10980

Are you sure you want to change the base?

[camera_avfoundation] Pigeon swift migration - part 2 #10980

Uh oh!

Conversation

RobertOdrowaz commented Feb 8, 2026

Pre-Review Checklist

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RobertOdrowaz commented Feb 10, 2026

Uh oh!

hellohuanlin commented Feb 10, 2026

Uh oh!

RobertOdrowaz commented Feb 11, 2026

Uh oh!

hellohuanlin commented Feb 11, 2026

Uh oh!

hellohuanlin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants