[camerax] Update README with plugin overview by camsim99 · Pull Request #3891 · flutter/packages

camsim99 · 2023-05-03T01:01:20Z

Updates the README of the camera_android_camerax plugin with an overview and instructions as to how to use the plugin. Also adds CONTRIBUTING.md for further information on how to contribute to the plugin.

Pre-launch Checklist

bparrishMines

In webview_flutter, we added what lines to run to generate new apis and mocks. See: https://pub.dev/packages/webview_flutter_android#contributing. These were added as a response to feedback from external contributors.

bparrishMines · 2023-05-03T17:50:36Z

+currently uses to implement `camera`. Each of these classes uses an `InstanceManager`
+(implementation in `instance_manager.dart`) to manage objects that are created by
+the plugin implementation that map to objects of the same type created on the native
+side. This plugin uses [`pigeon`][12] to communicate with native code, so each of


Here is where you could include the location of the pigeon file as well as the exact command line to run. See: https://pub.dev/packages/webview_flutter_android#contributing

Ideally we should actually move that to a CONTRIBUTING.md; it's not relevant to plugin clients.

Oh, this is in the section you already suggested moving. Sorry, I wasn't looking at the full context when I replied at first.

stuartmorgan-g · 2023-05-03T18:45:07Z

+currently uses to implement `camera`. Each of these classes uses an `InstanceManager`
+(implementation in `instance_manager.dart`) to manage objects that are created by
+the plugin implementation that map to objects of the same type created on the native
+side. This plugin uses [`pigeon`][12] to communicate with native code, so each of


Oh, this is in the section you already suggested moving. Sorry, I wasn't looking at the full context when I replied at first.

bparrishMines

LGTM with a couple nits

reidbaker · 2023-05-05T02:20:27Z

@@ -0,0 +1,46 @@
+# Contributing to camera\_android\_camerax
+
+## Plugin structure


not a blocker but I think this should start with an overview of the architecture @bparrishMines designed. Maybe linking the design doc or if not that then a 1 paragraph summary describing the structure.

reidbaker · 2023-05-05T02:21:59Z

+regenerate the communication layer by running
+`dart run pigeon --input pigeons/camerax_library.dart` from the plugin root.
+
+On the native side in `android/src/main/java/io/flutter/plugins/camerax/`, you'll


Consider saying java/kotlin or android. Native means enough different things to different people that we should minimize the usage when a more accurate word choice is available.

reidbaker · 2023-05-05T02:22:52Z

+`dart run pigeon --input pigeons/camerax_library.dart` from the plugin root.
+
+On the native side in `android/src/main/java/io/flutter/plugins/camerax/`, you'll
+find the Host API and Flutter API implementations of the same classes wrapped with


It would probably be helpful to articulate what host api and Flutter api mean in this context of native (or java ;) code.

reidbaker · 2023-05-05T02:28:45Z

+in the native code map to objects created on the Dart side, and thus, are also
+managed by an `InstanceManager` (implementation in `InstanceManager.java`).
+
+If you need to access any Android classes to contribute to this plugin, you should


I understand what this line through 30 is saying but I think it mixes architecture and instructions in a way that muddles what the author should use. If the architecture philosophy is include above then I think this can be shortened to "If CameraX or other Android classes do not have a duplicately named implementation in lib/src then follow the for more detail on how to add them.

reidbaker · 2023-05-05T02:30:07Z

+you need are not wrapped or you need to access any methods not wrapped in a class,
+you must take the additional steps to wrap them to maintain the structure of this plugin.
+
+For more information on the approach of wrapping native libraries for plugins, please


Having read though this I think a bunch of the information only makes sense after understanding the architecture. A plain english version should be at the top along with a link to the design document.

Oh and @bparrishMines to help people ask questions and spread this pattern or even just write about it you should come up with a unique name for the pattern.

camsim99 · 2023-05-08T17:18:24Z

@@ -0,0 +1,46 @@
+# Contributing to camera\_android\_camerax
+
+## Plugin structure


camsim99 · 2023-05-08T17:18:55Z

+described above to add them.
+
+For more information on the approach of wrapping native libraries for plugins,
+please see the [design document][2] or contact [@bparrishmines][6] on any


@bparrishMines is mentioning you like this okay with you?

We don't typically include references to our github accounts in the plugin. I think having https://github.com/flutter/packages/blob/main/CODEOWNERS should suffice. It also makes it easier to maintain since I will be added to all camera plugin PRs anyways.
Alternatively you could link to the ecosystem discord channel instead. Then anyone who is able to answer a question will be notified.

I like both of those suggestions. In general it is a good idea not try not to encourage contacting a person. It is not great for the person or the one reaching out. (imagine someone 12h timezone offset from @bparrishMines 3 years from now)

…s#3891)

flutter/packages@4800d65...1f91710 2023-05-09 43054281+camsim99@users.noreply.github.com [camerax] Update README with plugin overview (flutter/packages#3891) 2023-05-09 jmccandless@google.com Fix nullability in docs (flutter/packages#3876) 2023-05-08 stuartmorgan@google.com [tools] Check version ranges when pathifying deps (flutter/packages#3943) 2023-05-08 stuartmorgan@google.com [quick_actions] Update minimum iOS version to 11 (flutter/packages#3946) 2023-05-08 stuartmorgan@google.com [shared_preferences] Update minimum iOS version to 11 (flutter/packages#3945) 2023-05-08 stuartmorgan@google.com [path_provider] Update minimum iOS version to 11 (flutter/packages#3944) 2023-05-08 34871572+gmackall@users.noreply.github.com Adding setup for TestInstanceManagerHostApi in zoom and exposure tests (flutter/packages#3947) 2023-05-08 engine-flutter-autoroll@skia.org Roll Flutter from 43ac23b to 4ed1c92 (16 revisions) (flutter/packages#3942) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Updates the README of the `camera_android_camerax` plugin with an overview and instructions as to how to use the plugin. Also adds `CONTRIBUTING.md` for further information on how to contribute to the plugin.

camsim99 added 4 commits May 1, 2023 16:46

Merge remote-tracking branch 'upstream/main' into camx_occ
dec3d69

Merge remote-tracking branch 'upstream/main'
0e0333b

Start writing readme
886e049

Finish draft
cd1f2a7

github-actionsBot added p: camera platform-android labels May 3, 2023

camsim99 added 2 commits May 2, 2023 18:02

Merge remote-tracking branch 'upstream/main' into camx_readme
a86eeed

Undo unecessary changes
b7e3cf6

github-actionsBot removed the platform-android label May 3, 2023

camsim99 added 5 commits May 2, 2023 18:05

Undo changelog change
cdf2e67

Second draft
597f9d7

Draft 3
f862d81

Draft 4
0585bfa

Merge remote-tracking branch 'upstream/main' into camx_readme
f443d28

camsim99 marked this pull request as ready for review May 3, 2023 16:46

camsim99 requested a review from bparrishMines May 3, 2023 16:46

bparrishMines reviewed May 3, 2023
View reviewed changes

camsim99 requested review from gmackall, reidbaker and stuartmorgan-g May 3, 2023 17:57

stuartmorgan-g reviewed May 3, 2023
View reviewed changes

camsim99 added 2 commits May 3, 2023 13:58

Address first reviews
e36d363

Nits
dfa4173

camsim99 commented May 3, 2023
View reviewed changes

Comment threadpackages/camera/camera_android_camerax/CONTRIBUTING.md

camsim99 requested review from bparrishMines and stuartmorgan-g May 3, 2023 21:01

Merge remote-tracking branch 'upstream/main' into camx_readme
b54433a

bparrishMines approved these changes May 4, 2023
View reviewed changes

Comment threadpackages/camera/camera_android_camerax/CONTRIBUTING.md
Comment threadpackages/camera/camera_android_camerax/CONTRIBUTING.md Outdated
Comment threadpackages/camera/camera_android_camerax/CONTRIBUTING.md Outdated

address review
7370a5e

reidbaker reviewed May 5, 2023
View reviewed changes

camsim99 added 3 commits May 8, 2023 10:09

Address review
8343829

Update changelog
426f33b

self review
93afe6a

camsim99 commented May 8, 2023
View reviewed changes

camsim99 requested a review from reidbaker May 8, 2023 17:19

reidbaker approved these changes May 8, 2023
View reviewed changes

camsim99 added 2 commits May 8, 2023 14:37

Merge remote-tracking branch 'upstream/main' into camx_readme
f59896a

Add discord link
d0278e9

camsim99 added the autosubmit Merge PR when tree becomes green via auto submit App label May 8, 2023

camsim99 mentioned this pull request May 9, 2023
[camerax] Publish plugin #3896
Merged
11 tasks

auto-submitBot merged commit 1f91710 into flutter:mainMay 9, 2023

engine-flutter-autoroll mentioned this pull request May 9, 2023
Roll Packages from 4800d6512ebb to 1f917101d486 (8 revisions) flutter/flutter#126352
Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 9, 2023
1f91710[camerax] Update README with plugin overview (flutter/package…
3849192
…s#3891)

reidbaker mentioned this pull request May 12, 2023
[url_launcher] Set broadcast reciever visability as required by target api 34 #3973
Merged
11 tasks

This was referenced Jul 7, 2023
[Tool] New tool to download android dependencies #4408
Merged
[camera] tree fix format 2023 05 12 #4450
Closed
[tooling] mark infra steps as infra steps in yaml files #4473
Merged

reidbaker mentioned this pull request Jul 26, 2023
[Tooling] Add google owned cache for dependencies as an option in ci #4567
Merged
11 tasks

reidbaker mentioned this pull request Aug 4, 2023
[All] Expand artifact hub to all plugins #4645
Merged
11 tasks

camsim99 mentioned this pull request Aug 29, 2023
[camerax] Update README to explain plugin structure flutter/flutter#120103
Closed

		@@ -0,0 +1,46 @@
		# Contributing to camera\_android\_camerax

		## Plugin structure

Uh oh!

Conversation

camsim99 commented May 3, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

bparrishMines left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bparrishMines left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

camsim99 commented May 3, 2023•
edited
Loading