[camera] Implement initialize and add documentation #1838
Conversation
bparrishMines
changed the title
| /// | ||
| /// This will choose the best [CameraConfigurator] for the current device. | ||
| factory CameraController({@required CameraDescription description}) { | ||
| assert(description != null); |
There was a problem hiding this comment.
Unnecessary since its in the constructor.
| return Future<void>.value(); | ||
| } | ||
|
|
||
| _instance?.dispose(); |
There was a problem hiding this comment.
Should this await the dispose? Also, can this be tested?
There was a problem hiding this comment.
Yea it should. But, I want this method to also be usable synchronously, so I don't want to make it async. I added a Completer to the method.
Also, I added tests to make sure the first controller is disposed.
| } | ||
|
|
||
| // Keep only one active instance of CameraController. | ||
| static CameraController _instance; |
There was a problem hiding this comment.
May I offer a different view about this approach? How about not having it as a static field? If the client developer unwisely calls initialize without disposing the previous CameraController, this may be a error, but also that may be that he is trying to access more than one camera at the same time (I don't know if this is possible in the hardware layer), but seems feasible to have more than one camera open and this to be alright.
Even if there is no chance of the developer accessing more than one camera at the same time, I believe that an exception should be raised explaining that the device is busy.
There was a problem hiding this comment.
Thanks for the review!
Our reasoning to keep this functionality is because the current implementation of the plugin only allows one controller to be active at a time. It isn't documented in the plugin, but that is how it works underneath. While we refactor the platform specific code, we want to keep the interface the same and not cause many breaking changes.
The other reason for this functionality is because the CameraController will only serve as a simple to use and narrowly defined interface usable on any platform. For more complex features (like accessing multiple cameras), we will also expose access to this through another interface or require platform specific code.
4 participants
Description
To keep backwards compatibility, this implements the initialize method and CameraController maintains a singular static instance in the new interface. As per comment: #1832 (comment)
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?