Feature 25

[carlosolimpio]

No description provided.

[viniciusalbuquerque]

This part is confusing. You're using the callback from the requestLocationUpdates to call the onLocationChanged function that defines another callback to call your callback. I don't really know now how to work with LocationServices so I don't know if you need both of "requestLocationUpdates" and "lasLocation.addOnSuccessListener" and I think you shouldn't define a listener inside the function that is called when the onLocationResult is called.

Make sure to read the documentation to have a better understanding of what each of these things do.
https://developers.google.com/android/reference/com/google/android/gms/location/FusedLocationProviderClient.html

[psteiger]

Vinicius is partially right in my humble opinion. Allow me to explain:

requestLocationUpdates is for requesting further location updates, as configured by locationRequest object. As you coded it, location updates gotten from mFusedLocationProviderClient.requestLocationUpdates go to a locationCallback, and then you send to your onLocationChanged, which is correct

What's wrong is calling mFusedLocationProviderClient.lastLocation inside the onLocationChanged callback: a means to get the last known location (almost) immediately if it's available. Then you can save the location and do whatever you want on onLocationChanged

So this is the architecture:

mFusedLocationProviderClient.requestLocationUpdates _______.
mFusedLocationProviderClient.lastLocation _________________|____>    onLocationChanged 

To ways of getting the location, one single-shot, one recurring, both processed by onLocationChanged, where you should do what you want with the gotten location.

[psteiger]

e.g.

        mFusedLocationProviderClient.lastLocation.addOnSuccessListener { location: Location ->
		onLocationChanged(location)
        }

        mLocationRequest.interval = UPDATE_INTERVAL
        mLocationRequest.fastestInterval = FASTEST_INTERVAL

        var builder = LocationSettingsRequest.Builder()
        builder.addLocationRequest(mLocationRequest)
        var locationSettingsRequest = builder.build()

        //checks if location settings are satisfied
        var settingsClient = LocationServices.getSettingsClient(mContext)
        settingsClient.checkLocationSettings(locationSettingsRequest)

        mFusedLocationProviderClient = LocationServices.getFusedLocationProviderClient(mContext)
        mFusedLocationProviderClient.requestLocationUpdates(mLocationRequest, object : LocationCallback() {
            override fun onLocationResult(locationResult: LocationResult?) {
                onLocationChanged(locationResult.location)
            }

        }, Looper.myLooper())
    }

    private fun onLocationChanged(l: Location) {
        locationListener.onLocationChanged(location)
    }

[viniciusalbuquerque]

Remove this comments

[psteiger]

prefer .apply{} in kotlin

mLocationRequest.apply {
    priority = tal
    interval = tal
    ...
}

[psteiger]

Vinicius is partially right in my humble opinion. Allow me to explain:

requestLocationUpdates is for requesting further location updates, as configured by locationRequest object. As you coded it, location updates gotten from mFusedLocationProviderClient.requestLocationUpdates go to a locationCallback, and then you send to your onLocationChanged, which is correct

What's wrong is calling mFusedLocationProviderClient.lastLocation inside the onLocationChanged callback: a means to get the last known location (almost) immediately if it's available. Then you can save the location and do whatever you want on onLocationChanged

So this is the architecture:

mFusedLocationProviderClient.requestLocationUpdates _______.
mFusedLocationProviderClient.lastLocation _________________|____>    onLocationChanged 

To ways of getting the location, one single-shot, one recurring, both processed by onLocationChanged, where you should do what you want with the gotten location.

[psteiger]

consider making private

[psteiger]

e.g.

        mFusedLocationProviderClient.lastLocation.addOnSuccessListener { location: Location ->
		onLocationChanged(location)
        }

        mLocationRequest.interval = UPDATE_INTERVAL
        mLocationRequest.fastestInterval = FASTEST_INTERVAL

        var builder = LocationSettingsRequest.Builder()
        builder.addLocationRequest(mLocationRequest)
        var locationSettingsRequest = builder.build()

        //checks if location settings are satisfied
        var settingsClient = LocationServices.getSettingsClient(mContext)
        settingsClient.checkLocationSettings(locationSettingsRequest)

        mFusedLocationProviderClient = LocationServices.getFusedLocationProviderClient(mContext)
        mFusedLocationProviderClient.requestLocationUpdates(mLocationRequest, object : LocationCallback() {
            override fun onLocationResult(locationResult: LocationResult?) {
                onLocationChanged(locationResult.location)
            }

        }, Looper.myLooper())
    }

    private fun onLocationChanged(l: Location) {
        locationListener.onLocationChanged(location)
    }

[psteiger]

If you don't nullify the mContext on the onDetach() callback, it will leak.

[psteiger]

Please nullify the Context at onDetach, or else it's a leak

[psteiger]

why not private val mContext?