Repository navigation
Conversation
|
|
||
| ## Proposed solution | ||
|
|
||
| Keep `torchEnabled` as the **requested** torch mode, which is how the app sees it: `CameraValue.flashMode` stays `torch` across `setDescription`. Then **apply that request again every time the plugin gets a new CameraX `Camera`**, skipping cameras that have no flash unit. Reset the request on `dispose`, because a new `CameraController` starts from default settings. |
There was a problem hiding this comment.
Sometimes there is more than a front/back camera. Should we consider saving a mapping of the camera to the torch state to account for this versus a single bool?
| ## Open questions | ||
|
|
||
| > [!IMPORTANT] | ||
| > **1. Expected behavior.** The report says "after step 4, the torch does not turn back **off**". I've assumed you meant "**on**", because you expect it to come back on and keep its state from step 2. Is that right? |
reidbaker
left a comment
reidbaker
left a comment
There was a problem hiding this comment.
Pr description should be updated to say what model/effort and harness you used.
…ching cameras CameraX turns the torch off whenever a camera becomes inactive, but the plugin cached torch mode as enabled and never re-applied it to the newly bound camera, so the torch stayed off after switching cameras and further requests to enable it were no-ops. Re-enable the torch whenever the bound camera changes if torch mode was requested and the camera has a flash unit (exposed via CameraInfo.hasFlashUnit), and reset flash state on dispose.
# Conflicts: # packages/camera/camera_android_camerax/CHANGELOG.md # packages/camera/camera_android_camerax/pubspec.yaml
camsim99
left a comment
camsim99
left a comment
There was a problem hiding this comment.
Two major pieces of feedback to help improve our agent: (1) if there is no value to testing something on hardware, write a unit test for it rather than an integration test. (2) If adding convenience methods/wrappers, aim to improve the entire suite rather than only considering the newly added tests.
(2) might be a hot take so open to pushback 🤷♀️ maybe it can be part of the implementation plan to be approved/rejected before implementation.
| await newController.dispose(); | ||
| }); | ||
|
|
||
| testWidgets('Torch mode is retained after switching cameras', (WidgetTester tester) async { |
There was a problem hiding this comment.
I'm not convinced this should be an integration test. We aren't using adb or anything to check the flash is on/off, so it looks equivalent to a unit test.
| /// CameraX returns a new [Camera] each time use cases are bound, so each | ||
| /// camera switch in these tests is modeled by a separate [MockCamera]. As | ||
| /// in CameraX, enabling the torch on a camera without a flash unit fails. | ||
| MockCamera createMockCamera({ |
There was a problem hiding this comment.
We now have a lot of different methods to customize mock objects and I'm worried about this file bloating more than it already has. If agents add methods like this, I think they should try to do it effectively by, for example, designing these methods so that as many tests as possible can be refactored to use it to hopefully strengthen the entire group of tests.
Continuation of #12302..
Restores the state of a camera when that camera was previously initialized. For example, if you initialize camera A and start the torch, then initialize camera B, then switch back to A, the torch will turn off when camera B is started but then turn back on when switching back to camera A.
Fixes 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.
The implementation plan used to create this PR can be found at commit: 2e222ee. It was created using Claude Opus 5.5 with high effort in the Antigravity IDE.
For more information see the project proposal doc go/flutter-project-one-shot or the working doc for this specific attempt go/flutter-project-one-shot-torch.
Pre-Review Checklist
[shared_preferences]///).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-assistbot 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.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2