Location Provider keeps showing after HIGH_POWER updates are removed

[tobrun]

Description

Location provider icon in status bar keeps showing after removing HIGH_POWER location updates while using two lost api clients.

Steps to Reproduce

Have 2 components with each one lost api client. The first component is started when the application is created and requests NO_POWER updates as long as the application lives. The second component is started for a specific activity and requests HIGH_POWER updates. When that activity is destroyed we remove the location request but the location provider keeps showing. Only killing the application resolves this.

LMK if you need more information.

Lost & Android Version

LOST 3.0.0, Android API 25

[tobrun]

cc @sarahlensing

[msmollin]

Thanks for the write up @tobrun! We'll look into this today.

[sarahsnow1]

This is a side-effect of code we overlooked when we migrated Lost to support multiple clients. We are currently pretty aggressive when it comes to requesting updates (we request them with the highest frequency among all client requests) and we don't remove them in cases like above (see reference).

I'll circle back to implementing a fix for this in a couple days (after we get an upcoming Android SDK release out). Thanks for the clear issue!

[zugaldia]

@sarahlensing @msmollin Thank you for the quick 👀 and update on this issue. Are there any workarounds that we could use on our end meanwhile to remove those location requests?

[sarahsnow1]

@zugaldia You could disconnect and reconnect the NO_POWER client when the HIGH_POWER one disconnects

[tobrun]

@sarahlensing seeing the same behaviour with that in place.
If needed I can whip up a small test application to reproduce.

[ecgreb]

A quick look at the logic in FusionEngine to register for location updates makes me think there should be an else block here to disable GPS updates if no interval is currently set.

[sarahsnow1]

@tobrun When you "disconnect and reconnect" the NO_POWER client are you removing location updates requested by that client?

So, something like this:

noPowerClient.connect();

//noPowerClient onConnected
LocationRequest request = LocationRequest.create()
                .setPriority(LocationRequest.PRIORITY_NO_POWER)
                .setFastestInterval(0)
                .setSmallestDisplacement(0)
                .setInterval(1000);
LocationServices.FusedLocationApi.requestLocationUpdates(noPowerClient, request, noPowerListener);

highPriorityClient.connect();

// highPriorityClient onConnected
LocationRequest request = LocationRequest.create()
                .setPriority(LocationRequest.PRIORITY_HIGH_ACCURACY)
                .setFastestInterval(0)
                .setSmallestDisplacement(0)
                .setInterval(1000); 

LocationServices.FusedLocationApi.requestLocationUpdates(highPriorityClient, request, highPriorityListener);

//highPriorityClient shutdown
LocationServices.FusedLocationApi.removeLocationUpdates(highPriorityClient, highPriorityListener);

//noPower shutdown
LocationServices.FusedLocationApi.removeLocationUpdates(noPowerClient, noPowerListener);

//noPower restart
highPriorityClient.connect();

@ecgreb I'm going down the path of splitting LocationEngine#setRequest into addRequest and removeRequest and leaving enable/disable alone (bc there isn't a way to remove updates on a priority-based level with LocationManager). removeRequest will disable the engine and then re-enable it if there are still requests left. There will be a PR soon for review, I'd love your eyes on it

[ecgreb]

@sarahlensing sounds good!

[tobrun]

@sarahlensing looks like you've identified the root issue, happy to help test a potential fix. Re. the workaround, while this could resolve the issue within our SDK, it will not resolve the issue of an end developer using his own LOST client.

[sarahsnow1]

@tobrun There is a 3.0.1-SNAPSHOT deployed now, so if you want to test, just pull that down. A PR is also up, feel free to give feedback!

[tobrun]

Initial tests aren't showing the issue in OP, with mapbox/mapbox-gl-native#9142 we're addressing the issue, we'd like to do a release candidate to verify the issues is no longer present and help our devs verify it on their end, do you have any plans for a patch release that we could include?

[sarahsnow1]

Great!

A patch release is planned for the beginning of next week.

[sarahsnow1]

3.0.1 release is out https://github.com/mapzen/lost/releases/tag/lost-3.0.1

Thanks for everyone's help and patience