[camera] android-rework part 9: Final implementation of camera class

[BeMacized]

This PR adds the final implementation for the Camera class that incorporates all the features from previous parts.

This is the final ninth PR in a series of pull-requests which will gradually introduce changes from PR #3651, making it easier to review the code (as discussed with @stuartmorgan).

As of right now, this PR will still be kept in draft mode, as it also contains changes it depends on from the previous parts. Once those have been merged and any changes from those have been merged back into this PR, the draft status will be removed.

Pre-launch Checklist

• I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
• I read the [Tree Hygiene] wiki page, which explains my responsibilities.
• I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart format. See [plugin_tool format])
• I signed the [CLA].
• The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
• I listed at least one issue that this PR fixes in the description above.
• I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy].
  • While technically no breaking changes have occurred, a major version bump was done due to the size of the refactor. (more info here)
• I updated CHANGELOG.md to add a description of the change.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
• All existing and new tests are passing.

[stuartmorgan]

Is this ready for another review pass from @blasten ?

[blasten]

Thanks for cleaning it up.

Generally, LGTM

[renfelo]

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

[stuartmorgan]

@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?

Obsolete

[BeMacized]

@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 requested here.

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. Specifically open, takePicture, startVideoRecording, stopVideoRecording, setFocusMode, startPreview and startPreviewWithimageStream are still lacking tests.
I wanted to discuss these with a colleague, but it is not possible right now because of summer holidays.

[BeMacized]

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.git
      ref: camera-android/final-rework-implementation
      path: packages/camera/camera

[stuartmorgan]

@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 requested here.

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. Specifically open, takePicture, startVideoRecording, stopVideoRecording, setFocusMode, startPreview and startPreviewWithimageStream 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 🙂 )

[BeMacized]

@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 in flutter/flutter#88685, so we should be good to go.

[stuartmorgan]

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

[stuartmorgan]

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.