[camera_android_camerax] Restore torch state between camera switches by camsim99 · Pull Request #11541 · flutter/packages

camsim99 · 2026-04-20T17:23:25Z

WIP towards fixing flutter/flutter#160956.

Normal pull request information above the line.

This pull request is an attempt to "oneshot" fixing flutter/flutter#160956. Using antigravity to execute a plan that is collaboratively worked on. The "rules" are to not allow human authored code to camera_android_camerax and to not rely on human code review feedback for code iteration.

We can add documentation, skills, mcp servers, presubmit test etc. Basically any "support infrastructure" for development is allowed but the package changes must be generated as a single pass.
When we discover that the plan is not good enough we will blow the package changes away and either modify the plan or add more support infrastructure and try again.

For more information see the working doc in go/flutter-project-one-shot-torch

…(may be equal)

…really a timing issue

reidbaker · 2026-04-20T19:34:36Z

+> - **Option B (Ideal)**: Expose `hasFlashUnit()` via Pigeon and check it before attempting to restore torch.
+>


@camsim99 what option do you prefer here.

I think this is an indication that we have not given the agent sufficient context to understand the design principles for where we put what layer of logic. That said, our goal is to one-shot package changes not one shot plans. Let's see if there is also evidence that it does not understand where to put logic when it implements code.

I think A is the right approach for fixing the issue. B wouldn't fix it on its own but I do think it identified a good improvement for the plugin which is checking hasFlashUnit before trying to turn on the torch and giving back a helpful error if so.

reidbaker · 2026-04-20T19:39:59Z

+
+---
+
+#### [MODIFY] [android_camera_camerax.dart](file:///Users/camillesimon/packages/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart)


Agents can wander across the files they have access to. I think running this under the camera_android_camerax package is probably sufficient to keep the changes scoped there. My one worry is that it tries to modify the root federated api surface.

Maybe we say "Changes must be backwards compatible and should stay in camera_android_camerax"

Also if this was a relative path I think in code review we could click it and read the file it was suggesting modifications to.

reidbaker · 2026-04-20T19:44:39Z

+
+#### [MODIFY] [android_camera_camerax.dart](file:///Users/camillesimon/packages/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart)
+
+- Add `_currentCameraDescription` instance variable to track active camera.


I do not understand why this needs a new variable. Why cant one of camera, cameraInfo or liveCameraState track which camera is "current"

I think the key to the map should be Camera2CameraInfo.from(cameraInfo).cameraId in java. Not sure what the dart equivalent is.

We need it to track the last torch state per camera. The suggested _currentCameraDescription.name is the equivalent of Camera2CameraInfo.from(cameraInfo).cameraId

reidbaker · 2026-04-20T19:49:47Z

+- Update `_currentCameraDescription` in `createCameraWithSettings` and `setDescriptionWhileRecording`.
+- Replace `torchEnabled` boolean with `Map<String, bool> _torchEnabledPerCamera = {};`.
+- Update `setFlashMode` to use `_torchEnabledPerCamera` keyed by `_currentCameraDescription.name`.
+- Modify `_enableTorchMode` to accept an optional `isRestore` boolean parameter (defaulting to `false`). If `isRestore` is true and the operation fails, the error should not be added to `cameraErrorStreamController`. `isRestore` is true when called during automatic restoration.


Reading the implementation of _enableTorchMode I dont think the input should be isRestore because in the context of that method the only thing it would decide is if an error should be thrown. The variable in _enableTorchMode should be "shouldThrowOnFailure" or something similar.

Then in the places where we call _enableTorchMode(<bool>. shouldThrowOnFailure=isRestore)

reidbaker · 2026-04-20T19:55:42Z

+
+Run tests using:
+```bash
+dart test test/android_camera_camerax_test.dart


I think this should be dart run ../../../script/tool/bin/flutter_plugin_tools.dart dart-test --package=camera_android_camerax and we should remove it from below.

reidbaker · 2026-04-20T19:56:30Z

+
+### Automated Tests
+
+I will add unit tests in `android_camera_camerax_test.dart` to verify:


There should be a test for restoring a torch mode to off in addition to restoring to on.

reidbaker · 2026-04-20T19:58:35Z

+```
+
+Additionally, run the following commands to ensure codebase health:
+```bash


@camsim99 do you think we should ask/tell the agent to make the tool call an alias to make the below commands easier to read.

Yes, good idea!

reidbaker · 2026-04-20T20:09:22Z

+
+Additionally, run the following commands to ensure codebase health:
+```bash
+# Format and Analyze


I suggest we give some guidance to the prompt on when to run each of these tools.

Here is my suggestion.

Run format and analyze after every code edit.

Run fix if errors are found in analyze before attempting any other mitigations.

Run tests after each test case added and after finishing a unit of code work.

Run gradle check after touching build.gradle.kts files

Run license check after getting new files to their final state.

When you think you are completely done run readme, version, pubspec checks.

finally run publish check.

reidbaker · 2026-04-20T20:13:30Z

+dart run ../../../script/tool/bin/flutter_plugin_tools.dart pubspec-check --packages=camera_android_camerax
+dart run ../../../script/tool/bin/flutter_plugin_tools.dart readme-check --packages=camera_android_camerax
+dart run ../../../script/tool/bin/flutter_plugin_tools.dart version-check --packages=camera_android_camerax
+```


The automated test need to include building the example app.

Not sure when you want to add this in but we can also point it to the integration tests, making sure those all pass (and thus, the example app builds). Of course, it ideally also adds an integration test.

reidbaker · 2026-04-20T20:14:15Z

+dart run ../../../script/tool/bin/flutter_plugin_tools.dart version-check --packages=camera_android_camerax
+```
+
+### Manual Verification


This needs to expanded to let the agent build the example app against an emulator.

Give the agent a command to start an emulator you have installed locally, We can worry about portability later. I also recommend we install the skill android-adb it is mit licensed.

adb shell dumpsys media.camera | grep -i "torch" can find the state of the torch.

2 considerations we might consider including in the manual verification section since I hope to run the plan from more than just your host machine.

Hardware Emulation: In the AVD Manager, ensure your emulator's camera setting is set to something other than None. If it is set to Emulated or Webcam, the system image may or may not support torch operations depending on the API level and emulator build.

Unsupported State: If the emulator simply does not support the torch, CameraCharacteristics.FLASH_INFO_AVAILABLE will return false. You can verify this by checking the dumpsys output for "flash" availability. If it is unavailable, the plugin's attempts to toggle the torch will be ignored by the system, which is a common reason for the "silent failure" you might be seeing in the issue you linked.

camsim99

High level thoughts on the plan % the manual verification section (which will require an entirely new draft from me looks like)

camsim99 · 2026-04-20T22:26:23Z

+> - **Option B (Ideal)**: Expose `hasFlashUnit()` via Pigeon and check it before attempting to restore torch.
+>


I think A is the right approach for fixing the issue. B wouldn't fix it on its own but I do think it identified a good improvement for the plugin which is checking hasFlashUnit before trying to turn on the torch and giving back a helpful error if so.

camsim99 · 2026-04-20T22:28:40Z

+- Update `_currentCameraDescription` in `createCameraWithSettings` and `setDescriptionWhileRecording`.
+- Replace `torchEnabled` boolean with `Map<String, bool> _torchEnabledPerCamera = {};`.
+- Update `setFlashMode` to use `_torchEnabledPerCamera` keyed by `_currentCameraDescription.name`.
+- Modify `_enableTorchMode` to accept an optional `isRestore` boolean parameter (defaulting to `false`). If `isRestore` is true and the operation fails, the error should not be added to `cameraErrorStreamController`. `isRestore` is true when called during automatic restoration.


camsim99 · 2026-04-20T22:29:13Z

+
+#### [MODIFY] [android_camera_camerax.dart](file:///Users/camillesimon/packages/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart)
+
+- Add `_currentCameraDescription` instance variable to track active camera.


We need it to track the last torch state per camera. The suggested _currentCameraDescription.name is the equivalent of Camera2CameraInfo.from(cameraInfo).cameraId

camsim99 · 2026-04-20T22:30:20Z

+```
+
+Additionally, run the following commands to ensure codebase health:
+```bash


Yes, good idea!

camsim99 · 2026-04-20T22:31:38Z

+dart run ../../../script/tool/bin/flutter_plugin_tools.dart pubspec-check --packages=camera_android_camerax
+dart run ../../../script/tool/bin/flutter_plugin_tools.dart readme-check --packages=camera_android_camerax
+dart run ../../../script/tool/bin/flutter_plugin_tools.dart version-check --packages=camera_android_camerax
+```


Not sure when you want to add this in but we can also point it to the integration tests, making sure those all pass (and thus, the example app builds). Of course, it ideally also adds an integration test.

reidbaker · 2026-04-22T17:41:39Z

Despite me being at next I am available to review this so we can make forward progress this week. Just ping me when it is ready for another review.

reidbaker · 2026-04-26T04:30:19Z

+#### [MODIFY] [pigeons/camerax_library.dart](file:///Users/camillesimon/packages/packages/camera/camera_android_camerax/pigeons/camerax_library.dart)
+
+- Add `bool hasFlashUnit();` to `abstract class CameraInfo`.
+- Run the Pigeon generator to update generated files.


I don't think we have to modify the plan here but I am wondering if the agents know how to use this library. I do think we should probably make sure that one of the automated verification steps will ensure that the generated files have been updated correctly and not by the agent.

reidbaker · 2026-04-26T04:30:54Z

+- Add `bool hasFlashUnit();` to `abstract class CameraInfo`.
+- Run the Pigeon generator to update generated files.
+
+#### [MODIFY] [android/src/main/java/io/flutter/plugins/camerax/CameraInfoProxyApi.java](file:///Users/camillesimon/packages/packages/camera/camera_android_camerax/android/src/main/java/io/flutter/plugins/camerax/CameraInfoProxyApi.java)


Can you remove all global paths and replace them with relative paths? Or is this the result of having strong file selectors in your ide?

I'm guessing it's the latter but not sure how to configure that differently.

reidbaker · 2026-04-26T04:36:23Z

I think this plan is good enough for a first attempt.

@kevmoo has a concept he called when the agent is "tired" I am curious if this plan will max out the context window and need to be picked upbon another session mid way through.

Check out his "rules" here. https://github.com/kevmoo/sdk/blob/socket2/conductor%2Fcheat_sheet_for_the_agent.md

Depending on how the first prompt goes we may want to incorporate some of those.

kevmoo · 2026-04-26T04:50:55Z

I think this plan is good enough for a first attempt.

@kevmoo has a concept he called when the agent is "tired" I am curious if this plan will max out the context window and need to be picked upbon another session mid way through.

Check out his "rules" here. https://github.com/kevmoo/sdk/blob/socket2/conductor%2Fcheat_sheet_for_the_agent.md

Depending on how the first prompt goes we may want to incorporate some of those.

It's basically just "context window is full" – but it is fun to talk to the agent about it.

I EXPLICITLY say "DO NOT be passive aggressive about 'are we ready to call it a day on this project?' – just tell me your context window is getting full"

camsim99 · 2026-04-29T18:45:45Z

Just pushed the first implementation. Haven't reviewed it yet but at a quick glance, I notice the following:

it did not run the package tests
it did not run format/analyze
it did not attempt to run the example app on an emulator
the agent complains that it's encountering unexpected user interaction type: not permission errors when trying to run shell commands
the agent claims it fixed 52 tests that were failing before the changes were made due to "Cannot call when within a stub response" errors.

camsim99

Left some thoughts on the implementation.

the agent complains that it's encountering unexpected user interaction type: not permission errors when trying to run shell commands

Haven't tested it though and would like to update the implementation plan to hopefully get an agent to test the code on its own 🤞 Might need to work through that difficulty with an agent.

camsim99 · 2026-04-29T23:12:51Z

 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-// Autogenerated from Pigeon (v26.1.7), do not edit directly.
+// Autogenerated from Pigeon (v26.1.5), do not edit directly.


It used an older version on pigeon? We should instruct it not to do that. Maybe it needs to run flutter pub upgrade or use pigeon from source.

I think that for features that we should use the same version of pigeon. If you agree then we need to decide how to enforce that. The simplest would be to add it to the prompt as part of the design and as part of the verification steps.

We agreed on instructing pigeon to use the same version of pigeon by:

Editing the plan to request the agent uses the same version of pigeon as before and does not downgrade the version

Add a verification step to ensure that the version of pigeon was not downgraded

camsim99 · 2026-04-29T23:16:20Z

    await liveCameraState?.removeObservers();
    liveCameraState = await cameraInfo!.getCameraState();
    await liveCameraState!.observe(_createCameraClosingObserver(cameraId));
+


This is not a "bad" place to put this logic but I would suggest moving this out to its own method because this method (_updateCameraInfoAndLiveCameraState) gets called also when a new camera use case is bound to the app lifecycle, a case where the torch shouldn't be impacted.

Do you have thoughts on how to coach that for the agent? I am coming up empty on my first pass. Maybe something like a general software development practice of only updating code paths that need the changes.

_updateCameraInfoAndLiveCameraState is called in three separate places:

when the camera is created (initializeCamera)

when the camera is switched during recording (setDescriptionWhileRecording)

and when new camera use cases (video capture, image capture, etc.) are bound to the Activity lifecycle (_bindUseCaseToLifecycle)

In the third case (_bindUseCaseToLifecycle), the torch should not have to be restored because the camera is not being switched. So, the code below isn't relevant in that case.

camsim99 · 2026-04-29T23:18:25Z

-  Future<void> _enableTorchMode(bool value) async {
+  Future<void> _enableTorchMode(
+    bool value, {
+    bool addErrorToStream = true,


If we can't restore the torch state to what's expected after switching back to a camera, I actually think sending an error would be good, so not sure we need addErrorToStream.

I think our approved plan has this error logic so this complexity might be our fault.

See: "_enableTorchMode handles failures by not adding to stream when addErrorToStream is false."

We can't identify any cases where we would want to suppress the error (perhaps initially thought this would be a good idea because we are now checking if the device has a flash). So, we can update the plan to remove addErrorToStream.

reidbaker · 2026-04-30T19:19:03Z

    await liveCameraState?.removeObservers();
    liveCameraState = await cameraInfo!.getCameraState();
    await liveCameraState!.observe(_createCameraClosingObserver(cameraId));
+


Do you have thoughts on how to coach that for the agent? I am coming up empty on my first pass. Maybe something like a general software development practice of only updating code paths that need the changes.

reidbaker · 2026-04-30T19:25:18Z

-  Future<void> _enableTorchMode(bool value) async {
+  Future<void> _enableTorchMode(
+    bool value, {
+    bool addErrorToStream = true,


I think our approved plan has this error logic so this complexity might be our fault.

See: "_enableTorchMode handles failures by not adding to stream when addErrorToStream is false."

reidbaker · 2026-04-30T19:26:49Z

-      );
+      if (addErrorToStream) {
+        cameraErrorStreamController.add(
+          e.message ?? 'The camera was unable to change torch modes.',


Error messages should include the values that we tried to set. We can probably just define this in the plan.

There is actually more that is wrong here because we expect platformException to have a message and we should print both what we were doing and what went wrong

reidbaker

I think one of the biggest failures is that analyze failed in presubmit despite our verification logic.
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8683162281373946241/+/u/Run_package_tests/analyze/stdout

Second biggest failure is that camera unit tests failed.
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8683162281652884641/+/u/Run_package_tests/Dart_unit_tests/stdout

we probably need to debug what happened in your models execution. Did we fill the context window or what caused that verification step to fail.

camsim99 · 2026-04-30T20:07:46Z

+  /// Whether or not torch flash mode has been enabled for the current camera.
+  @visibleForTesting
+  bool get torchEnabled {
+    final String name = _currentCameraDescription?.name ?? 'default';


'default' should be a constant

camsim99 · 2026-04-30T20:18:35Z

+
+    // Restore torch state if enabled for this camera.
+    if (torchEnabled) {
+      final bool hasFlash = await cameraInfo!.hasFlashUnit();


The check for the device having a flash unit should be handled in setter of the torch mode, setFlashMode.

camsim99

Review of the current testing state!

camsim99 · 2026-04-30T21:32:52Z

        pigeonInstance.getZoomState(), LiveDataSupportedType.ZOOM_STATE);
  }
+
+  @Override


A test needs to be added for this method in CameraInfoProxyTest.java.

camsim99 · 2026-04-30T22:17:18Z

+      ).thenAnswer((_) async => mockLiveCameraState);
+      when(mockCameraInfo.hasFlashUnit()).thenAnswer((_) async => true);
+
+      camera.torchEnabled = true;


Open to pushback on this, but I think we should try to avoid having visibleForTesting variables like torchEnabeld when we don't need them. I think testing the full flow of creating and initializing two cameras and switching between them to see what happens to the torch (instead of simulating it by setting torchEnabled) is more valuable.

Maybe we can ask the agent to test all possible scenarios switching between three cameras or something and give it feedback that torchEnabled should not be visible for testing.

camsim99 · 2026-04-30T22:21:48Z

+    },
+  );
+
+  test(


We should expect this case to go when we get rid of the addErrorToStream parameter in the _enableTorch method.

camsim99 · 2026-04-30T22:22:25Z

+      when(mockCameraInfo.hasFlashUnit()).thenAnswer((_) async => true);
+
+      camera.torchEnabled = true;
+


To follow up on my comment directly above, I think getting more cameras involved (3 is fine) to test more complex restoration scenarios would be a good improvement.

camsim99 · 2026-04-30T22:23:16Z

We need tests added for setDescriptionWhileRecording to ensure that the torch state is restored as expected when the camera is switched during recording.

github-actions Bot added p: camera platform-android and removed platform-android labels Apr 20, 2026

camsim99 added 11 commits April 20, 2026 10:47

rebase

4f67ef5

one camera working on emulator?

7a58f80

it works??? without switching cameras

eb00f8a

saved previewview fixes switching cameras, aspect ratio causing jump …

929a070

…(may be equal)

fix example code bug

d7c47d6

working state, need to try to remove workarounds and see if this was …

b181e26

…really a timing issue

inital state

7cd5f5e

initial plan

fe75707

second revision of implemntation plan

a6f4a41

restore android

61911c2

more cleanup of source + example

b33f6ae

camsim99 force-pushed the camx-i160956-torchstate branch from 631b962 to b33f6ae Compare April 20, 2026 17:48

reidbaker reviewed Apr 20, 2026

View reviewed changes

camsim99 commented Apr 20, 2026

View reviewed changes

implementation plan iteration 3

9aeefb3

camsim99 requested a review from reidbaker April 24, 2026 16:37

reidbaker reviewed Apr 26, 2026

View reviewed changes

first implementation attempt + walkthrough

b2ef1cb

flutter-dashboard Bot added the CICD Run CI/CD label Apr 29, 2026

github-actions Bot added the platform-android label Apr 29, 2026

camsim99 requested a review from reidbaker April 29, 2026 22:41

camsim99 commented Apr 29, 2026

View reviewed changes

reidbaker reviewed Apr 30, 2026

View reviewed changes

camsim99 commented Apr 30, 2026

View reviewed changes

		> - Option B (Ideal): Expose `hasFlashUnit()` via Pigeon and check it before attempting to restore torch.
		>


		---

		#### [MODIFY] [android_camera_camerax.dart](file:///Users/camillesimon/packages/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart)


		#### [MODIFY] [android_camera_camerax.dart](file:///Users/camillesimon/packages/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart)

		- Add `_currentCameraDescription` instance variable to track active camera.


		### Automated Tests

		I will add unit tests in `android_camera_camerax_test.dart` to verify:

+                  },
+                );
+                test(

		when(mockCameraInfo.hasFlashUnit()).thenAnswer((_) async => true);

		camera.torchEnabled = true;

Conversation

camsim99 commented Apr 20, 2026 • edited by reidbaker Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

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!

camsim99 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!

reidbaker commented Apr 22, 2026

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!

reidbaker commented Apr 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevmoo commented Apr 26, 2026

Uh oh!

camsim99 commented Apr 29, 2026

Uh oh!

camsim99 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

camsim99 commented Apr 20, 2026 •

edited by reidbaker

Loading

reidbaker commented Apr 26, 2026 •

edited

Loading

camsim99 left a comment •

edited

Loading

camsim99 Apr 30, 2026 •

edited

Loading

reidbaker left a comment •

edited

Loading