[camera_android] Begin conversion to Pigeon #7755
Conversation
yaakovschectman
requested review from
stuartmorgan-g,
camsim99,
gmackall,
reidbaker and
a team
and removed request for
camsim99
...ages/camera/camera_android/android/src/main/java/io/flutter/plugins/camera/CameraPlugin.java
Outdated
Show resolved
Hide resolved
|
|
||
| @NonNull | ||
| @Override | ||
| public List<Messages.PlatformCameraDescription> getAvailableCameras() { |
There was a problem hiding this comment.
Looks like we have no native testing of this codepath, and in general almost no testing of MethodCallHandlerImpl.java (so, the top layer of the code this PR series will be changing, although it looks like most of the serialization details are lower down and hopefully tested) 🙁
@reidbaker How much scope creep do we want for this conversion in terms of native test backfill? Normally I would want to take this opportunity to push for getting at least minimal coverage of these codepaths with native unit tests, but that's going to be a non-trivial increase in the work and I know this is a quasi-legacy plugin. I'm okay to proceed with a Pigeon conversion based on manual testing, as that's still a net safety win (and the code in MethodCallHandlerImpl is mostly pretty simple passthrough to lower layers), but I think it should be an Android team call.
packages/camera/camera_android/android/src/main/java/io/flutter/plugins/camera/CameraUtils.java
Show resolved
Hide resolved
yaakovschectman
force-pushed
the
camera_setup_pigeon
branch
from
d455c71 to
54e3a3e
Compare
| @@ -19,13 +20,19 @@ const MethodChannel _channel = | |||
|
|
|||
| /// The Android implementation of [CameraPlatform] that uses method channels. | |||
| class AndroidCamera extends CameraPlatform { | |||
| /// Creates a new [CameraPlatform] instance. | |||
| AndroidCamera([@visibleForTesting CameraApi? hostApi]) | |||
There was a problem hiding this comment.
This should be named, not positional.
As a general rule of thumb: if something is a public API (where changing from positional to named would be a breaking change), and you are not absolutely certain that there could never be more parameters added later, don't use optional positional parameters. They don't scale at all to orthogonal parameters—and in this case that's especially true because no actual client could ever pass anything but null for this parameter, so if there were a parameter added later that was actually useful, everyone would have to pass a pointless null.
flutter/packages@9d00fb1...f1a3da2 2024-10-09 [email protected] Remove additional (harmless but annoying) native stack traces. (flutter/packages#7837) 2024-10-09 [email protected] Roll Flutter from 0917e9d to 2d45fb3 (50 revisions) (flutter/packages#7836) 2024-10-09 [email protected] [camera_android] Begin conversion to Pigeon (flutter/packages#7755) 2024-10-09 [email protected] [google_maps_flutter_android] Add `PlatformCap` pigeon type. (flutter/packages#7639) 2024-10-09 [email protected] [camerax] Revert "Explicitly remove READ_EXTERNAL_STORAGE permission" (flutter/packages#7826) 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 [email protected] 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Convert one method to use Pigeon to setup the backbone of future conversions. Android analog of #6553
Part of flutter/flutter#117905
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.