-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Added iOS location plugin to showcase background execution APIs #656
Conversation
2a49dba
to
54781cb
Compare
|
||
@Override | ||
public void onMethodCall(MethodCall call, Result result) { | ||
if (call.method.equals("getPlatformVersion")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this return result.notImplemented() unconditionally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't even think to look at the Android stuff. Thanks!
/// This is an example of a callback for LocationBackgroundPlugin's | ||
/// `startMonitoringLocation`. A callback can be defined anywhere in an | ||
/// application's code, but cannot be from another program. | ||
class Foo { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's have a better name than Foo =) Something that hints at what one might do with location data. Maybe "LocationMonitor" or ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was left over from testing, so I'll go with your suggestion :)
// isolates since they do not share memory. | ||
uiSendPort = IsolateNameServer.lookupPortByName(kLocationPluginPortName); | ||
} | ||
uiSendPort?.send(location.toJson()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens when the foreground isolate closes the port?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed this so the port lookup is done on each event since there's really no other way to tell that an isolate port has been closed from the SendPort. Thoughts?
} | ||
|
||
// Listen on the port for location updates from our background callback. | ||
_foregroundPort.listen((dynamic message) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When does this port get closed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This port should stay open the life of the UI, but I've updated this logic to remove the port from the IsolateNameServer
on close.
5dd6c5d
to
358aa9d
Compare
bc11f5c
to
60930dd
Compare
# Flutter Background Execution Sample - LocationBackgroundPlugin | ||
|
||
A example Flutter plugin that showcases background execution using iOS location services. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessarily in this PR, but it would be good to fill this out with brief installation and usage examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack
|
||
@Override | ||
public void onMethodCall(MethodCall call, Result result) { | ||
result.notImplemented(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you had plans to implement the Android side, you might add a TODO here that points to the API to use and explains the strategy a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
flutter_test: | ||
sdk: flutter | ||
|
||
location_background_plugin: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a regular dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably. Not sure how that ended up there.
|
||
#import <CoreLocation/CoreLocation.h> | ||
|
||
@implementation LocationBackgroundPlugin { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As with the android_alarm_manager plugin, I think this file needs detailed comments so that 3p plugin developers know how to hook into the background execution features of the engine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
No description provided.