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

Unload the backends when not in use #23

Closed
whitequark opened this issue Jan 24, 2015 · 7 comments
Closed

Unload the backends when not in use #23

whitequark opened this issue Jan 24, 2015 · 7 comments

Comments

@whitequark
Copy link

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
Copy link
Contributor

n76 commented Jan 24, 2015

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
Copy link
Author

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

n76 [email protected] 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
Copy link
Author

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 [email protected] 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
Copy link
Contributor

n76 commented Jan 24, 2015

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
Copy link
Author

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
Copy link
Member

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
Copy link
Member

Should be fixed with 1.2.2

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

No branches or pull requests

3 participants