microg / UnifiedNlp

Alternative network location provider for Android, with plugin interface to easily integrate third-party location providers.
978 stars 210 forks source link

Unload the backends when not in use #23

Closed whitequark closed 9 years ago

whitequark commented 9 years ago

I'm currently looking at https://github.com/rtreffer/LocalGSMLocationProvider. In onOpen, it registers a GSM event listener, which will cause quite a lot of wakeups, even when nothing is requesting the location data. In fact, this will happen even if the location services (or just network location services) are turned off.

Can you modify UnifiedNlp so that it calls onClose on location providers when location services are disabled or unneeded? I especially dislike the former issue.

n76 commented 9 years ago

I just did a "git pull" on @rtreffer LocalGSMLocationProvider and I don't see where it is registering a GPS listener. Shouldn't the following in the project directory find it?

grep -r -I requestLocationUpdates src

It does register to be notified of changes in mobile connectivity which could make sense and should be pretty low cost as in all modes other than "airplane" that should already be on.

whitequark commented 9 years ago

Sorry, I typoed... I mean GSM of course

n76 notifications@github.com wrote:

I just did a "git pull" on @rtreffer LocalGSMLocationProvider and I don't see where it is registering a GPS listener. Shouldn't the following in the project directory find it?

grep -r -I requestLocationUpdates src

It does register to be notified of changes in mobile connectivity which could make sense and should be pretty low cost as in all modes other than "airplane" that should already be on.

— Reply to this email directly or view it on GitHub.

whitequark commented 9 years ago

Do you think that the cost of waking it up and performing a database lookup are negligible? I'm not so sure, but I can see that being the case.

n76 notifications@github.com wrote:

I just did a "git pull" on @rtreffer LocalGSMLocationProvider and I don't see where it is registering a GPS listener. Shouldn't the following in the project directory find it?

grep -r -I requestLocationUpdates src

It does register to be notified of changes in mobile connectivity which could make sense and should be pretty low cost as in all modes other than "airplane" that should already be on.

— Reply to this email directly or view it on GitHub.

n76 commented 9 years ago

Now you are going to make me go back and look at the code again. :)

I believe that the backend is using LruCache to save the most recent 1000 or so database lookups. My experience on the backends I have contributed to or written is that using LruCache that way is a very good way to reduce overhead. The other thing I found that reduces overhead is building the database file with an index. Between the two of those, the equivalent backends I use on my phone never seem to lag and I don't see the backends in the power usage display of the battery setting.

Not sure if you are using a database with indices generated or not. Depends on how you build it. If you are using one of my scripts there will be indices and they make the database quite a bit larger.

whitequark commented 9 years ago

It uses a binary search index in essence, yes. I think this answers the power consumption part.

However I think you should still close the provider when the location services are disabled. Alternatively UnifiedNlp should not rebroadcast the locations. It will match the user expectation of, well, not updating anything else with your most recent location.

mar-v-in commented 9 years ago

onOpen and onClose are intended to be called when the provider is enabled/disabled in settings. There was a bug in this code, so I removed it for testing. Apparently I forgot to re-add it. I'll fix this for the next release.

mar-v-in commented 9 years ago

Should be fixed with 1.2.2