Skip to content

[camera_android] Convert calls from native to dart side to Pigeon. #7857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Oct 14, 2024

Conversation

yaakovschectman
Copy link
Contributor

Part of the effort to transition to typesafe implementations for platform calls. Android analog to #6568

Part of flutter/flutter#117905

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@yaakovschectman yaakovschectman marked this pull request as ready for review October 11, 2024 20:45
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just a couple of notes.

}

handler.post(() -> deviceChannel.invokeMethod(eventType.method, args));
String errorMessage = (description == null) ? "" : description;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minimal null check as description is Nullable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you put a TODO (with an issue link) on this line to fix description being nullable and update any failing call sites? Actually fixing it is out of scope of this PR, but the fact that we need this line is a significant problem since there's no other content in the message, so if we ever actually do this, the plugin client will get absolutely no actionable information.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with two comments added.

}

handler.post(() -> deviceChannel.invokeMethod(eventType.method, args));
String errorMessage = (description == null) ? "" : description;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you put a TODO (with an issue link) on this line to fix description being nullable and update any failing call sites? Actually fixing it is out of scope of this PR, but the fact that we need this line is a significant problem since there's no other content in the message, so if we ever actually do this, the plugin client will get absolutely no actionable information.


import androidx.annotation.NonNull;

/** A convenience class for results of a Pigeon Flutter API method call that perform no action. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add this line to the comment as well; it's true of this plugin as well.

@yaakovschectman yaakovschectman merged commit ca3da1b into flutter:main Oct 14, 2024
76 checks passed
@yaakovschectman yaakovschectman deleted the camera_setup_flutterapi branch October 14, 2024 17:06
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 14, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 14, 2024
flutter/packages@67401e1...1e670f2

2024-10-14 [email protected] [image_picker] Update for non-nullable generics in Pigeon (flutter/packages#7775)
2024-10-14 [email protected] [camera_android] Convert calls from native to dart side to Pigeon. (flutter/packages#7857)
2024-10-14 [email protected] [google_maps_flutter_android] Update JSON code to Pigeon for `BitmapDescriptor`, `Cap`, and `PatternItem` (flutter/packages#7840)
2024-10-13 [email protected] [flutter_markdown] Soft wrapping in blockquotes (flutter/packages#7848)
2024-10-12 [email protected] [google_maps_flutter] Update iOS Pigeon for non-nullable generics (flutter/packages#7792)
2024-10-11 [email protected] [pigeon] Swift implementation for ProxyApis (flutter/packages#6602)
2024-10-11 [email protected] [camera_android_camerax] Remove duplicated 'report' in README.md (flutter/packages#7708)

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants