Add Background Location support & fix background GPU usage#385
Add Background Location support & fix background GPU usage#385sarahcodes100 merged 8 commits intomasterfrom
Conversation
msmollin
changed the title
| @@ -13,3 +13,9 @@ protocol ApplicationProtocol { | |||
| } | |||
|
|
|||
| extension UIApplication: ApplicationProtocol {} | |||
There was a problem hiding this comment.
Do you think we should rename this file to be more generic now that it doesnt just contain application extensions?
There was a problem hiding this comment.
Agreed, yep. Will update.
MapzenSDK/LocationManager.swift
Outdated
| */ | ||
| open func canEnableBackgroundLocationUpdates() -> Bool { | ||
| if UIApplication.shared.backgroundRefreshStatus != .available { return false } | ||
| if !isAlwaysAuthorized() { return false } |
There was a problem hiding this comment.
Can simplify these two lines to just return isAlwaysAuthorized()
There was a problem hiding this comment.
Not sure I see that? isAlwaysAuthorized() (which just checks CLLocationManager.authorizationStatus()) doesn't necessarily reflect the current backgroundRefreshStatus which is a separate setting in iOS apps. Here's Waze, for example:
MapzenSDK/MZMapViewController.swift
Outdated
| //MARK: - Application Lifecycle Observance | ||
| func observeLifecycleNotifications(notificationCenter: NotificationCenterProtocol = NotificationCenter.default) { | ||
| notificationCenter.addObserver(self, selector: #selector(applicationWillResignActive), name: .UIApplicationWillResignActive, object: nil) | ||
| notificationCenter.addObserver(self, selector: #selector(applicationWillEnterForeground), name: .UIApplicationWillEnterForeground, object: nil) |
There was a problem hiding this comment.
Why do we use UIApplicationWillEnterForeground instead of UIApplicationDidBecomeActive?
There was a problem hiding this comment.
Hmmm I think I misread UIApplicationWillEnterForeground vs DidBecomActive description in the lifecycle docs. Basically I thought that DidBecomeActive would get called when the application became active regardless of foreground or background status. This can happen if your app uses background refresh via silent push notifications. In that scenario, we don't want the GPU to start rendering.
However, re-reading the docs (https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1622956-applicationdidbecomeactive), it seems that DidBecomeActive is the appropriate time to restart GPU interaction. Will fix.
| - parameter seconds: How long the animation should last. | ||
| */ | ||
| open func animate(toPosition position: TGGeoPoint, withDuration seconds: Float) { | ||
| if isInBackground { return } |
There was a problem hiding this comment.
We would be fine without the isInBackground tests since we would let a 'pending' animation until the app is brought back from the background. Now, by adding those, we set this to be a no-op when the app is in the background. I think that it would be important to document that depending on what you end up choosing here (no-op vs pending).
There was a problem hiding this comment.
Agreed on documentation for the no-op. Not sure I follow the "pending animation" part? Do you mean on ES 0.9 ? This is still on 0.8 so allowing any calls through to Tangram-es will cause errors?
There was a problem hiding this comment.
Ah yes, this is still pending for 0.9.0 so don't mind it for now. You could consider changing this behavior when updating to the new version.
MapzenSDK/MZMapViewController.swift
Outdated
|
|
||
| /** | ||
| Resets the camera on the current location, as well as the zoom and tilt. | ||
| Resets the camera on the current location, as well as the zoom and tilt. Can return false if the app is currently backgrounded |
There was a problem hiding this comment.
Should be in the returns block?
- Renamed ApplicationExtensions file to more reflect usage - Updated location manager for more succinct logic - Changes from applicationWillEnterForeground to applicationDidBecomeActive - Updated tests
3 participants
Overview
This fixes #378 and additionally adds support for background location mode updates in the main location manager.
Proposed Changes