radarlabs / flutter-radar

Flutter package for Radar, the leading geofencing and location tracking platform
https://radar.io
Apache License 2.0
22 stars 13 forks source link

Fix issue where running a background isolate causes method channel not to work #63

Closed Jonny1987 closed 3 months ago

Jonny1987 commented 3 months ago

Made the method channel and call handler static so every flutter engine can access

Jonny1987 commented 3 months ago

This seems to work properly for both background and foreground without causing the issue I mentioned in https://github.com/radarlabs/flutter-radar/pull/62

A few questions I have about this PR:

1) I made the call handler static which means I needed to make all methods call from it static too, and this means I needed to make mActivity, mContext and mPermissionsRequestResult static too. Is this ok to do?

I think for mActivity it won't make a difference since this is only set in onAttachedToActivity and similar methods, which only get called in the main isolate anyway, not the background one. But I'm not sure about mContext and mPermissionsRequestResult.

2) Strangely, it doesn't work properly when these following lines are kept in onAttachedToEngine, even though they get called from initialize anyway. I'm not quite sure why, but figured that since they get called from initialize they aren't needed in onAttachedToEngine. What is the reason they were in both methods?

        Radar.setReceiver(new RadarFlutterReceiver(channel));
        Radar.setVerifiedReceiver(new RadarFlutterVerifiedReceiver(channel));

3) I removed sBackgroundFlutterEngine and sBackgroundChannel since there way no reference to them anywhere. Let me know if they were needed for some reason.

4) I see registerWith uses the channel radar_flutter_plugin of which there is no mention anywhere, instead of flutter_radar. Should this be corrected?

KennyHuRadar commented 3 months ago

Thank you for our contribution to this repo. I've gone ahead and tried out this PR and its working well for me with both foreground and background.

  1. I think it should be ok, there should only ever be one mContext and mPermissionsRequestResult. 2/3. Yeah we must had missed it, it should not be there.
  2. You are also right to point out that our registerWith method is outdated.
Jonny1987 commented 3 months ago

There is still one issue with this setup (with the event callbacks given in onLocation and similar being done in the dart code instead of the java code) which means that it doesn't play nicely with firebase messaging when using a background handler (which I imagine is pretty common):

Firebase automatically creates the background engine when a background message handler is set, but there is no way to call Radar.initialise, Radar.attachListeners and Radar.onLocation when the engine/isolate is created....it's only possible to call this in the background handler and that only gets called when a background message is received. As a result, if Radar.attachListeners is called and event callbacks are set in the main isolate, then when the app goes into the background so only the background engine created by firebase is running, this dart code does not have a method channel set (because Radar.initialise has not yet been called in this engine) or event callbacks set and so location updates are now not triggering the location callback.

In other words, it only works in background when tracking is triggered in the background isolate after a message is received, because it's then that Radar.initialise and Radar.attachListeners can be called (and then tracking triggered). But it won't work in background if tracking is started in the main isolate and a firebase background message never received.

Ideally, starting tracking in either the main isolate or background isolate should mean that callbacks are still being called when the app is closed.

I'm currently making alterations to version 3.9.1 (where the event callbacks are processed on the java side in a background engine) since I think this will be easier to work for my use case, as once Radar.initialise and Radar.attachListeners are called from the main isolate, this stored the dart method channel handler and event callbacks in shared preferences which can then be used by the background engine without it needing to call those methods again (in theory).

I did try to also store the event callbacks in shared preferences but still process them on the dart side so that only one isolate needs to call Radar.attachedListeners, however Radar.initialise would still need to be called in order to create the method channel in the background isolate. I tried to trigger this method channel creation from the java side but couldn't get it to work but will some back to this.

KennyHuRadar commented 3 months ago

Really appreciate your insight on this. I think we will still release the current version as it is as its an improvement over the previous version. However, we will come back to this issue in the near future and would really appreciate you logging this issue with us. Thank you so much!

Jonny1987 commented 3 months ago

I can't get this background location callbacks to work when the tracking is started in the foreground...I think the issue is my app gets frozen by android, which resets the Radar sdk, meaning Radar.receiver (set by Radar.setReceiver) is then null again, so event callbacks no longer work (since I can't call Radar.initialize in the dart code until a background firebase message is received).