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

Simplify experimental pedometer example #2104

Merged
merged 5 commits into from
Dec 6, 2023

Conversation

liamappelbe
Copy link
Contributor

Since ffigen added support for NativeCallable.listener to its ObjC bindings, this example can be simplified. We can replace the Dart_Port logic with ObjCBlock.listener, which lets us get rid of most of the native code.

We still need a small bit of native code to retain a reference to the callback's arguments before invoking the listener, otherwise the arguments may be ref counted and deleted before the Dart side of the callback is invoked. See dart-lang/native#835

@liamappelbe
Copy link
Contributor Author

Nevermind, I think this is blocked on another small tweak to the ObjC bindings.

@liamappelbe liamappelbe closed this Dec 5, 2023
@liamappelbe
Copy link
Contributor Author

Actually I managed to fix the bug without ffigen changes. Ready for review.

@domesticmouse
Copy link
Contributor

Looks like there are CI issues:

warning - example/lib/steps_repo.dart:5:8 - Unused import: 'dart:isolate'. Try removing the import directive. - unused_import
warning - example/lib/steps_repo.dart:102:49 - The receiver can't be null, so the null-aware operator '?.' is unnecessary. Try replacing the operator '?.' with '.'. - invalid_null_aware_operator
warning - example/lib/steps_repo.dart:104:61 - The '!' will have no effect because the receiver can't be null. Try removing the '!' operator. - unnecessary_non_null_assertion

@liamappelbe
Copy link
Contributor Author

Looks like there are CI issues:

Fixed

@domesticmouse
Copy link
Contributor

PTAL @leighajarett

@domesticmouse domesticmouse merged commit f0e6da6 into flutter:main Dec 6, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants