[TIMOB-23135] Android: Modified "locationServicesEnabled" handling

[maggieaxway]

JIRA: https://jira.appcelerator.org/browse/TIMOB-23135

Summary:

• Was returning true for "passive" and "test" location providers when it shouldn't have.
  • Now only returns true if GPS or WiFi/Cellular location providers are enabled.
• Was returning false when if at least 1 location value hasn't been received yet, even if location services is enabled.
  • Not the correct behavior. Especially since it might take a minute for the GPS to get a satellite lock and acquire location data.
• Note that on Android 6.0 and above, this property will return false until location permission has been granted to the app, regardless if location services is enabled on the device.

Test:
Need manually test turning on/off the location service with the app running.

var win = Ti.UI.createWindow();

var btn = Ti.UI.createButton({
  title: 'Click Me'
});

btn.addEventListener('click', function(e) {

  console.log('Ti.Geolocation.hasLocationPermissions() ' + Ti.Geolocation.hasLocationPermissions());

  console.log('Ti.Geolocation.locationServicesEnabled ' + Ti.Geolocation.locationServicesEnabled);
  
  alert('locationServicesEnabled = ' + Ti.Geolocation.locationServicesEnabled);

  if (Ti.Geolocation.hasLocationPermissions()) {
    getCurrentPosition();

  } else {
    Ti.Geolocation.requestLocationPermissions(null, getCurrentPosition);
  }
});

function getCurrentPosition() {
  Ti.Geolocation.getCurrentPosition(function(e) {
    console.log('Ti.Geolocation.getCurrentPosition() ' + JSON.stringify(e));
  });
}

win.add(btn);
win.open();

[build]

	Messages
📖	🎉 Another contribution from our awesome community member, maggieaxway! Thanks again for helping us make Titanium SDK better. 👍
📖	💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

[garymathews]

see above

[garymathews]

We want to return true if any location provider is enabled, GPS, NETWORK etc..

return providerNames.size() > 0;

[jquick-axway]

Gary is right. We need to check all providers. His code change is the correct solution.

Also, we should check if the providerNames collection is null after calling the LocationManager.getProviders() method. If null, return false. (Odds are this isn't going to happen, but better safe than sorry. Especially on Android.)

[jquick-axway]

Gary is right. We need to check all providers. His code change is the correct solution.

Also, we should check if the providerNames collection is null after calling the LocationManager.getProviders() method. If null, return false. (Odds are this isn't going to happen, but better safe than sorry. Especially on Android.)

[jquick-axway]

Question:
Is there a reason why we're limiting this to GPS and NETWORK? Does disabling location services on a phone not disable the PASSIVE provider?

I ask because our "GeolocationModule" reads from the PASSIVE location provider.
https://github.com/appcelerator/titanium_mobile/blob/master/android/modules/geolocation/src/java/ti/modules/titanium/geolocation/GeolocationModule.java#L200

[jquick-axway]

Okay, so @maggieaxway and I just talked about this. The PASSIVE provider will still provide data, even if location services is disabled. In fact, that was a customer complaint here...
https://jira.appcelerator.org/browse/TIMOB-23135

So, the real question here is should we provide PASSIVE location data at all? The GPS and NETWORK providers are the ones we really want. I'm starting to think reverting the code like Maggie just did now and removing PASSIVE provider support might be the way to go.

@garymathews, you've been playing around with this recently, what are your thoughts? How is this compared to Google Play Service's fused location provider?

[garymathews]

@jquick-axway sounds good, could do:

return providerNames.contains(LocationManager.GPS_PROVIDER) || providerNames.contains(LocationManager.NETWORK_PROVIDER);

[maggieaxway]

So my changes approved? @garymathews @jquick-axway

[jquick-axway]

CR: Pass

[jquick-axway]

Updated PR to pass Java code formatting rules.