Skip to content
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

[pigeon] improve documentation #66511

Closed
bsutton opened this issue Sep 24, 2020 · 8 comments · Fixed by flutter/packages#3705
Closed

[pigeon] improve documentation #66511

bsutton opened this issue Sep 24, 2020 · 8 comments · Fixed by flutter/packages#3705
Labels
p: pigeon related to pigeon messaging codegen tool P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels.

Comments

@bsutton
Copy link

bsutton commented Sep 24, 2020

I'm trying to convert an existing project to pigeon using a federated model and I'm find the documentation a little sparse.

https://pub.dev/packages?q=sounds

Googling pigeon and dart shows links like:
https://www.reddit.com/r/WTF/comments/86n6w2/so_a_pigeon_with_a_dart_in_the_head_just_landed/

:D

I've posted a question on stackoverflow but suspect that @gaaclarke is the only authority on pigeon at the moment.
https://stackoverflow.com/questions/64037971/using-dart-pigeon-in-a-federated-model

It would be good to see documentation that directly speaks to the federated model.

The documentation mentions:

Api's should be defined as an abstract class with either HostApi() or FlutterApi() as metadata. The former being for procedures that are defined on the host platform and the latter for procedures that are defined in Dart.

However this statement leaves you guessing as to exactly what this are meant for.

I'm guessing that:
HostApi defines the set of calls the dart code makes into the platform specific code.
FlutterApi defines the set of calls the platform code can make back into the dart code.

(I now see that the api doco does provide more info on these but they are important enough that I think they should be documented on the readme).

It would be useful to see a discussion around migrating an existing channel based project to pigeon.

I note there is a PR for supporting async. This suggests that async calls are not currently supported.
It also suggests that more detailed documentation around calls into the platform needs to be created to explain what a sync vs async call into the platform should look like on both sides of the interface.

It may be a little early in the dev process but I would also like to see a section listing and discussing each of the generated files and their specific use.

@gaaclarke
Copy link
Member

Thanks for the good ideas. I think the documentation can be improved, for sure. If you want to see an example of using Pigeon with federated plugins, looking at video_player is the definitive example: https://github.com/flutter/plugins/tree/master/packages/video_player

As to the answers to some of your questions:

HostApi defines the set of calls the dart code makes into the platform specific code.
FlutterApi defines the set of calls the platform code can make back into the dart code.

That's correct.

It would be useful to see a discussion around migrating an existing channel based project to pigeon.

I don't have that. I do have a working example from when I migrated video_player: flutter/plugins@c65bdd0#diff-1119d814fb92c6c6cff89707b50874ec

I note there is a PR for supporting async. This suggests that async calls are not currently supported.
It also suggests that more detailed documentation around calls into the platform needs to be created to explain what a sync vs async call into the platform should look like on both sides of the interface.

I'll try to elaborate in the README. What I meant there is that implementors of the generated interface are expected to return synchronously, in contrast to platform channels which expect you to call a callback whenever you want. There is currently a PR out for a external contributor that is implementing this. There is a workaround now by just using a HostApi and FlutterApi and callback back and forth between them.

It may be a little early in the dev process but I would also like to see a section listing and discussing each of the generated files and their specific use.

I've limited the generate code to one source file per platform. There is one .dart file for dart, one .m file for swift/objc (plus header), one .java file for java/kotlin. I plan on sticking to that. I've also tried to make the generated code readable. We'd like to have docstrings eventually in the generated code as well.

Thanks again, hopefully my answers get you moving with the issues you are facing today. If you discover something that is missing in the documentation please consider contributing back via PR.

@bsutton
Copy link
Author

bsutton commented Sep 24, 2020

Thanks for the prompt response.

The video player left me somewhat confused. It doesn't explicitly mentioned that its a federated model and then the IOS code and Android code are both in the main project which left me wondering what was going on.

@xster xster added the p: pigeon related to pigeon messaging codegen tool label Sep 24, 2020
@TahaTesser TahaTesser added documentation package flutter/packages repository. See also p: labels. labels Sep 24, 2020
@bsutton
Copy link
Author

bsutton commented Sep 24, 2020

I think it is important that the pigeon documentation talks directly to its role in a federated model.

@TytaniumDev
Copy link
Contributor

To add on to this, it would be awesome to have an example of iOS/Android code using the @FlutterApi generated code. I haven't been able to find an example anywhere, and video_player only uses @HostApi

@gaaclarke
Copy link
Member

@Tywholland You can check out this example project that uses FlutterApi in the meantime: https://github.com/gaaclarke/GiantsA2A/blob/dc504e4acb8a5f08b53341ad9e9a7afc5cbf97ad/giants/pigeons/messages.dart#L35

@glassmonkey
Copy link

Hi.
I made a simple sample app to get the communication status (e.g. wifi, mobile, lost) from native with FlutterApi.

Here's a link

https://github.com/glassmonkey/flutter_wifi

@stuartmorgan stuartmorgan added the P2 Important issues not at the top of the work list label Oct 20, 2021
@stuartmorgan
Copy link
Contributor

I think it is important that the pigeon documentation talks directly to its role in a federated model.

In particular, we should document (based on experience with video_player) that Pigeon should not be used in the interface package, only as an internal detail of per-package method channels, to avoid any Pigeon update being a potentially breaking change to the platform interface.

auto-submit bot pushed a commit to flutter/packages that referenced this issue Apr 20, 2023
This creates an initial example app covering Android (Kotlin), iOS and macOS (Swift), and Windows, showing integration of Pigeon into an app directly, without using a plugin. This serves as both a simple runnable example for clients, and as a future source for snippet extraction for documentation. It includes a simple integration test so that CI will ensure that it's actually working.

Since we can't demonstrate Java and Objective-C in the same application, we could consider adding a second example app covering those in the future.

Currently it shows only host API, and only a single trivial method, but we can expand over time as is useful for documentation.

Part of flutter/flutter#66511
auto-submit bot pushed a commit to flutter/packages that referenced this issue Jun 30, 2023
Updates README's to better reflect modern pigeon and it's usage. 
Very open to feedback on this, please let me know what could be better, clearer, or is just missing from this documentation.
I don't expect to be able to cover all uses with this, if people want to see all possibilities, I think looking into our very thorough integration tests will provide them with anything they could need (all of which is linked in the example README).

fixes flutter/flutter#66511
partial work for flutter/flutter#123851
fixes flutter/flutter#108531
fixes flutter/flutter#92641
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p: pigeon related to pigeon messaging codegen tool P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants