Looking particularly for feedback on using an empty path to signify a file error.

What are the other options? In general I would expect either a typed result with meta data or thrown exception to handle (in java). If there was going to be a string return value and neither of the others then I would expect null not empty.

I agree with you. I think I got caught up in the confusion of resulting with an error versus sending a camera error to the Dart stream. It's my understanding now (based on the current plugin) that we should send a camera error to the stream if an error occurs while an operation is ongoing. Otherwise, we can throw the error directly from the native side. I changed it to that here.

Let me know what you think. Also @bparrishMines will this work well with the wrapping pattern? I want to make sure we throw errors consistently from here on out.

Yea I agree that this is the ideal way to handle it and is how we handle it for the webview_flutter implementations as well. The native code can just return the exception in result.error and then the Dart platform implementation can handle converting it to a CameraException.

Why make this a method instead of just calling ImageCapture.Builder() were you need it?

For injecting a mock for testing

Should there be an error or warning if a flashMode is passed that is larger than an int? Is there a more typesafe variable we can use here like an enum?

https://developer.android.com/reference/androidx/camera/core/ImageCapture#FLASH_MODE_ON()
The values are not large enough to be a long and in your test you are converting from an int to a long. Why use longs at all?

Longs are used in the code generated by pigeon. That's the only reason I'm using them here

What are the other options? In general I would expect either a typed result with meta data or thrown exception to handle (in java). If there was going to be a string return value and neither of the others then I would expect null not empty.

@xizhang this is the Dart implementation for taking a picture. Do you have any feedback on binding the ImageCapture use case to take picture, and then, immediately unbinding it, as seen below? Wondering if this is best performance-wise. cc @gmackall

As I mentioned in the sync meeting. We should leave ImageCapture always bound.

I think something like this would work: Preview, ImageAnalysis and ImageCapture are always bound, since the combination is always supported by the camera hardware. VideoCapture is lazy loaded: it's only bound when the user start recording Video.

The reason to special case VideoCapture is that VideoCapture is not always supported by camera hardware. When the device doesn't support it, CameraX supports it with OpenGL which will introduce an additional cost. We should avoid that cost if necessary.

I really like this implementation of this method, but I have a few notes to add.

This is returning a String despite the output being a OutputFileResult. I'm assuming this is done because it only has one method, getSavedUri. But when a library returns a class with a single method, there is a pretty good chance they intend to add more at a later date and they don't want to introduce a breaking change.

This is also missing the OutputFileOptions.

But I would guess that the tedious process of wrapping native apis contributed to creating a simpler method. And that's still on me to improve it. So, I would suggest including in the documentation that this method doesn't completely wrap the method from ImageCapture and serves as a convenience method that was created for Dart.

I didn't wrap those since the goal right now is to reach feature parity, and it was simpler to configure taking a picture the same as the current plugin on the Java side than wrap something we aren't likely to change soon.

I think you make a good point though. I'll leave a comment, and I'm definitely open to coming back to wrap those classes later if needed. Should be straightforward to change.

@reidbaker @gmackall @bparrishMines To address @xizhang's comment, I did some significant refactoring in this method if any of you are willing to take a look.

The main change is that I configured the Preview and ImageCapture use cases here and bind them to the lifecycle together.

@reidbaker @gmackall @bparrishMines The other major change I made is not relying on class variables to know if a UseCase has been bound or not. I wrapped isBound to achieve that.