pointfreeco / composable-core-location

A library that bridges the Composable Architecture and Core Location.
MIT License
110 stars 55 forks source link

take too much time to update current location #7

Closed saroar closed 1 year ago

saroar commented 3 years ago

Describe the bug I don't know what is the problem I just know the issue and send a small video clip also click any point of interest button that does not work

I run your demo app and it takes too much time to my iPhone 6s take too much time to update the current location

Expected behavior it should take not more than 1/2s or less

Screenshots

https://user-images.githubusercontent.com/8770772/124570994-0e15ed00-de50-11eb-951a-f510b98a7ce8.MP4

Environment

Additional context Add any more context about the problem here.

stephencelis commented 2 years ago

@saroar Can you reproduce the problem outside of the library in vanilla CoreLocation? Our library is a lightweight wrapper around CoreLocation so we don't imagine that it would behave differently.

mltbnz commented 2 years ago

I was facing the same issue with a long time passing to requestLocation and today did review my setup since I thought there must be a mistake. But after some benchmarking I saw a regular CLLocationManager takes the same amount of time to request the location as a ComposableCoreLocation.LocationManager which is around 10.00 s according to instruments.

func getCurrentLocation() {
  os_signpost(.begin, log: log, name: "location")
  locationManager.requestLocation()
}
......
func locationManager(_ manager: CLLocationManager, didUpdateLocations locations: [CLLocation]) {
  os_signpost(.end, log: log, name: "location")
  lastSeenLocation = locations.first
}

I used requestLocation() like in the demo app since it also the behaviour I want but the docs say

Calling it causes the location manager to obtain a location fix (which may take several seconds)

so seems like that works as expected.

mltbnz commented 2 years ago

I ended up using startUpdatingLocation(id:) since that returns a location almost immediately and combined it with a distanceFilter which also worked for me 👍

PierreCapo commented 2 years ago

@stephencelis is completely right, this code is just a wrapper around CLLocationManager. requestLocation(). And this function is indeed slow if you have not called before startUpdatingLocation(), as this stackoverflow thread explains: `https://stackoverflow.com/questions/42245571/cllocationmanager-is-slow-getting-location-swift.

It looks like though, this repository would need a pull request to add this startUpdatingLocation() in the Example folder, so that we don't have this long delay when tapping the button. I will try to see if I can do one it if I got time.

PierreCapo commented 2 years ago

Actually this is not that straightforward, because if you use startUpdatingLocation(), then you start a stream of your position which will lead to continually move your camera to the user position because since we have a stream of event, we keep entering into the didUpdateLocations case: https://github.com/pointfreeco/composable-core-location/blob/main/Examples/LocationManager/Common/AppCore.swift#L213

So to sum up:

In conclusion, if you want to retrieve the user location, I think using CLLocationManager().location?.coordinate is the best approach?

//  .currentLocationButtonTapped action
        case .authorizedAlways, .authorizedWhenInUse:
            guard let userLocation = CLLocationManager().location?.coordinate else {
                return .none
            }
            state.region = CoordinateRegion.init(center: userLocation, span: state.region.span)
            return .none

Would be nice to have the thoughts of one of the maintainer to confirm this solution is okay 👍