Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@robert-ancell
Copy link
Contributor

@robert-ancell robert-ancell commented May 8, 2020

No description provided.

@auto-assign auto-assign bot requested a review from flar May 8, 2020 01:57
@robert-ancell robert-ancell marked this pull request as draft May 8, 2020 01:57
@robert-ancell robert-ancell changed the title Linux shell fl method channel Add FlMethodChannel, FlMethodCodec and FlStandardMethodCodec May 8, 2020
@robert-ancell robert-ancell force-pushed the linux-shell-fl-method-channel branch 3 times, most recently from b17dcb6 to e84c529 Compare May 8, 2020 02:58
@robert-ancell
Copy link
Contributor Author

From review in #18118:

  • "How do you send a not-implemented response?"
  • "This, and/or some of the other methods in this class, would benefit from some high-level documentation about how method calls work (see other platforms for examples). Right now it seems like you already need to know how method channels work conceptually (E.g., that you'll get one of three possible outcomes to a method call) to use this class."
  • Regarding response envelopes - "Another option would be to return a value here, and do something like DecodeAndProcessResponseEnvelope in the C++ code for the envelope decoding case. (I don't really love that solution either though.)"

@robert-ancell robert-ancell force-pushed the linux-shell-fl-method-channel branch 5 times, most recently from 0023879 to 4ae8db4 Compare May 9, 2020 07:37
@robert-ancell
Copy link
Contributor Author

I think I've come up with a better solution for the response encoding, the usage is now something like this:

static void method_response_cb(GObject *object, GAsyncResult *result, gpointer user_data) {
  g_autoptr(GError) error = NULL;
  g_autoptr(FlMethodResponse) response = fl_method_channel_invoke_finish (FL_METHOD_CHANNEL(object), result, &error);
  if (response == NULL) {
    g_warning ("Failed to call method: %s", error->message);
    return;
  }

  FlValue *result = fl_method_response_get_result (response, &error);
  if (result == NULL) {
    g_warning ("Method call returned error %s", error->message);
    return;
  }
  
  do_something_with_result (result);
}

int main(int argc, char **argv)
{
  ...
  
  g_autoptr(FlMethodChannel) channel = fl_method_channel_new (fl_engine_get_binary_messenger (engine), "foo", FL_METHOD_CODEC(fl_standard_method_codec_new ()));

  g_autoptr(FlValue) args = fl_value_new_int(42);
  fl_method_channel_invoke (channel, "Foo", args, NULL, method_response_cb, NULL);
}

If you want to get the error details, the callback now looks like:

static void method_response_cb(GObject *object, GAsyncResult *result, gpointer user_data) {
  g_autoptr(GError) error = NULL;
  g_autoptr(FlMethodResponse) response = fl_method_channel_invoke_finish (FL_METHOD_CHANNEL(object), result, &error);
  if (response == NULL) {
    g_warning ("Failed to call method: %s", error->message);
    return;
  }

  if (FL_IS_METHOD_RESPONSE_SUCCESS (response) {
    do_something_with_result (fl_method_response_success_get_result (FL_METHOD_RESPONSE_SUCCESS (response)));
  } else if (FL_IS_METHOD_RESPONSE_ERROR (response)) {
    g_warning ("Method call returned error %s: %s",
               fl_method_response_error_get_code (FL_METHOD_RESPONSE_ERROR (response));
               fl_method_response_error_get_message (FL_METHOD_RESPONSE_ERROR (response)));
  }
}

I think this is a reasonable balance between making the simple case relatively easy to do, but still be able to access the detail if necessary.

@robert-ancell
Copy link
Contributor Author

@stuartmorgan I haven't been able to work out what a "not implemented" response is. The standard codec has two codes for responses (success and error) and the JSON codec either returns "[<result>]" or "[<code>,<method>,<args>]". Is not implemented just an error code?

@robert-ancell robert-ancell force-pushed the linux-shell-fl-method-channel branch from 4ae8db4 to e587db3 Compare May 9, 2020 07:53
@robert-ancell robert-ancell marked this pull request as ready for review May 9, 2020 07:54
@auto-assign auto-assign bot requested a review from liyuqian May 9, 2020 07:54
@robert-ancell
Copy link
Contributor Author

Ah, is not implemented when you received a zero length response?

@robert-ancell robert-ancell force-pushed the linux-shell-fl-method-channel branch 6 times, most recently from a638433 to 573cad5 Compare May 10, 2020 21:51
@robert-ancell
Copy link
Contributor Author

I'm unsure if we want to implement FlOptionalMethodChannel - it seems like this is because different shells may have missing methods, but the Dart side should be more predictable.

@robert-ancell robert-ancell deleted the linux-shell-fl-method-channel branch May 21, 2020 00:53
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 21, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 21, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 22, 2020
wandyers pushed a commit to wandyers/engine that referenced this pull request May 23, 2020
…ethodCodec (flutter#18220)

* Add FlMethodChannel, FlMethodCall, FlMethodResponse and FlMethodCodec

* Add FlJsonMethodCodec

* Add FlStandardMethodCodec
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 23, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 26, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 26, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 26, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 26, 2020
liyuqian added a commit to liyuqian/flutter that referenced this pull request May 26, 2020
Roll Engine from 9ce1e5c to 1a83498 (69 revisions)

flutter/engine@9ce1e5c...1a83498

2020-05-26 hterkelsen@users.noreply.github.com Revert "Update CanvasKit to 0.15.0 (flutter#18570)" (flutter/engine#18600)
2020-05-26 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from TSEOq... to hr-HZ... (flutter/engine#18594)
2020-05-26 skia-flutter-autoroll@skia.org Roll Dart SDK from 65113fd73dec to 9e3b0289197d (2 revisions) (flutter/engine#18593)
2020-05-26 skia-flutter-autoroll@skia.org Roll Skia from b6d158aaf3dd to e4b4ca1050b9 (1 revision) (flutter/engine#18592)
2020-05-26 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from DzG49... to pLp67... (flutter/engine#18591)
2020-05-24 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from Oz016... to TSEOq... (flutter/engine#18590)
2020-05-23 skia-flutter-autoroll@skia.org Roll Skia from afd8a6c6ae87 to b6d158aaf3dd (3 revisions) (flutter/engine#18588)
2020-05-22 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from 99Z4_... to Oz016... (flutter/engine#18582)
2020-05-22 skia-flutter-autoroll@skia.org Roll Skia from ec31488ace66 to afd8a6c6ae87 (1 revision) (flutter/engine#18581)
2020-05-22 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from Kvwlr... to DzG49... (flutter/engine#18580)
2020-05-22 skia-flutter-autoroll@skia.org Roll Skia from 80abb89c3632 to ec31488ace66 (1 revision) (flutter/engine#18579)
2020-05-22 skia-flutter-autoroll@skia.org Roll Skia from da90c3765908 to 80abb89c3632 (4 revisions) (flutter/engine#18577)
2020-05-22 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from _zNmv... to 99Z4_... (flutter/engine#18576)
2020-05-22 skia-flutter-autoroll@skia.org Roll Skia from 317dce5c81c0 to da90c3765908 (1 revision) (flutter/engine#18575)
2020-05-22 skia-flutter-autoroll@skia.org Roll Dart SDK from 4ee57c08e0a6 to 65113fd73dec (3 revisions) (flutter/engine#18574)
2020-05-22 skia-flutter-autoroll@skia.org Roll Skia from 67e21a19259b to 317dce5c81c0 (3 revisions) (flutter/engine#18573)
2020-05-22 hterkelsen@users.noreply.github.com Update CanvasKit to 0.15.0 (flutter/engine#18570)
2020-05-21 skia-flutter-autoroll@skia.org Roll Skia from 3d52abc84667 to 67e21a19259b (1 revision) (flutter/engine#18568)
2020-05-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 396b5fb7a97d to 4ee57c08e0a6 (6 revisions) (flutter/engine#18567)
2020-05-21 robert.ancell@canonical.com Add fl_value_to_string (flutter/engine#18540)
2020-05-21 skia-flutter-autoroll@skia.org Roll Skia from 1e63279156d6 to 3d52abc84667 (5 revisions) (flutter/engine#18566)
2020-05-21 chris@bracken.jp null-annotate semantics.dart (flutter/engine#18553)
2020-05-21 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from ciqRH... to Kvwlr... (flutter/engine#18565)
2020-05-21 skia-flutter-autoroll@skia.org Roll Skia from b37105ea6cca to 1e63279156d6 (9 revisions) (flutter/engine#18564)
2020-05-21 gw280@google.com Implement WriteAtomically using write/fsync on all platforms, and enable (flutter/engine#18320)
2020-05-21 skia-flutter-autoroll@skia.org Roll Dart SDK from ecce58c1e354 to 396b5fb7a97d (2 revisions) (flutter/engine#18560)
2020-05-21 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from RNByJ... to _zNmv... (flutter/engine#18558)
2020-05-21 st.krwlng@gmail.com [profiling] Memory Profiling support for iOS (flutter/engine#18516)
2020-05-21 skia-flutter-autoroll@skia.org Roll Skia from 22636205ce92 to b37105ea6cca (2 revisions) (flutter/engine#18557)
2020-05-21 skia-flutter-autoroll@skia.org Roll Dart SDK from d551980ac9fc to ecce58c1e354 (2 revisions) (flutter/engine#18556)
2020-05-21 skia-flutter-autoroll@skia.org Roll Dart SDK from bf7e9d13d730 to d551980ac9fc (6 revisions) (flutter/engine#18551)
2020-05-21 skia-flutter-autoroll@skia.org Roll Skia from 4c0578632217 to 22636205ce92 (4 revisions) (flutter/engine#18550)
2020-05-21 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from ThMeW... to ciqRH... (flutter/engine#18549)
2020-05-21 skia-flutter-autoroll@skia.org Roll Skia from 67ff541ac116 to 4c0578632217 (1 revision) (flutter/engine#18547)
2020-05-21 xster@google.com Let run_tests.py just stream output (flutter/engine#18534)
2020-05-21 robert.ancell@canonical.com Add FlKeyEventPlugin (flutter/engine#18313)
2020-05-21 mehmetf@users.noreply.github.com Add tests for StandardMethodCodec (flutter/engine#18521)
2020-05-21 skia-flutter-autoroll@skia.org Roll Skia from f83baf230c69 to 67ff541ac116 (1 revision) (flutter/engine#18545)
2020-05-21 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from 3o9aQ... to RNByJ... (flutter/engine#18543)
2020-05-21 dworsham@google.com fuchsia: Fix runtime_tests and shell_tests (flutter/engine#18492)
2020-05-21 skia-flutter-autoroll@skia.org Roll Skia from d2dc8ddcdf5e to f83baf230c69 (2 revisions) (flutter/engine#18539)
2020-05-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 7aa8656d1dd8 to bf7e9d13d730 (21 revisions) (flutter/engine#18538)
2020-05-21 robert.ancell@canonical.com Add FlMethodChannel, FlMethodCodec, FlStandardMethodCodec and FlJsonMethodCodec (flutter/engine#18220)
2020-05-20 ferhat@gmail.com [web] Fix arc rendering when it starts a new sub path. (flutter/engine#18535)
2020-05-20 garyq@google.com Send platformResolvedLocale from iOS embedder (flutter/engine#18519)
2020-05-20 dkwingsmt@users.noreply.github.com System mouse cursor: macOS (flutter/engine#18131)
...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants