Is this ready for another review pass from@blasten ?

Is there a way we could use these PR series before merge? I've been trying for a while, but no success.

@BeMacized Does this need anything else (other than the version conflict resolution, which we can trivially resolve at the point of landing) or is it good to go?

@BeMacized Does this need anything else (other than the version conflict resolution, which we can trivially resolve at the point of landing) or is it good to go?

@stuartmorgan It is pretty much complete other than the extra unit tests you requestedhere.

I've since added tests that cover the majority of the camera class, but a few are left remaining as I have some trouble figuring out how to write them. Specificallyopen,takePicture,startVideoRecording,stopVideoRecording,setFocusMode,startPreview andstartPreviewWithimageStream are still lacking tests.
I wanted to discuss these with a colleague, but it is not possible right now because of summer holidays.

Is there a way we could use these PR series before merge? I've been trying for a while, but no success.

@renfelo You can add a temporary dependency on this PR for development as follows:

dependencies:...camera:git:url:git://github.com/Baseflow/flutter-plugins.gitref:camera-android/final-rework-implementationpath:packages/camera/camera

@BeMacized Does this need anything else (other than the version conflict resolution, which we can trivially resolve at the point of landing) or is it good to go?

@stuartmorgan It is pretty much complete other than the extra unit tests you requestedhere.

I've since added tests that cover the majority of the camera class, but a few are left remaining as I have some trouble figuring out how to write them. Specificallyopen,takePicture,startVideoRecording,stopVideoRecording,setFocusMode,startPreview andstartPreviewWithimageStream are still lacking tests.
I wanted to discuss these with a colleague, but it is not possible right now because of summer holidays.

I'm comfortable with landing this as-is if you file an issue for the remaining tests and can commit to following up on them in the near future (and as long as you feel like you've done sufficient manual testing of those cases that there are no obvious regressions). Generally we don't like to do that, but there's no question that with the tests you've already added here (plus all the unit tests in the previous 8 PRs that this will cut us over to using) this PR will significantly increase our test coverage relative to the existing implementation. Given that, and the fact that we believe this PR puts us in a much better position to fix—with test coverage—any new issues that people find in real-world usage, I think the good of doing the remaining testing in parallel rather than before landing in this particular case outweighs the downsides.

(edit: "as is" meaning without those additional tests; if there are other things you know of that the PR needs we should obviously do those things 🙂 )

@stuartmorgan As far as I am aware, the remaining tests are all that still needs to happen, so if merging it as-is works for you, it works for me.

I've filed an issue for them influtter/flutter#88685, so we should be good to go.

Alright, let's flip the switch and start getting more feedback :)

Once the post-submit publish step runs I'll let the front-line triage team know that this is out, so they can start re-testing Android camera issues.