-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor timer code into separate utility class #53
Changes from 5 commits
7be2b1f
76581b4
8058e31
a26362a
fb2c280
974f943
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
#import <Foundation/Foundation.h> | ||
|
||
@class MMEBackgroundLocationServiceTimeoutHandler; | ||
@protocol MMEUIApplicationWrapper; | ||
|
||
@protocol MMEBackgroundLocationServiceTimeoutHandlerDelegate | ||
- (BOOL)timeoutHandlerShouldCheckForTimeout:(MMEBackgroundLocationServiceTimeoutHandler *)handler; | ||
- (void)timeoutHandlerDidTimeout:(MMEBackgroundLocationServiceTimeoutHandler *)handler; | ||
- (void)timeoutHandlerBackgroundTaskDidExpire:(MMEBackgroundLocationServiceTimeoutHandler *)handler; | ||
|
||
@end | ||
|
||
@interface MMEBackgroundLocationServiceTimeoutHandler: NSObject | ||
|
||
@property (nonatomic, weak) id<MMEBackgroundLocationServiceTimeoutHandlerDelegate> delegate; | ||
@property (nonatomic, readonly) NSTimer *timer; | ||
- (instancetype)initWithApplication:(id<MMEUIApplicationWrapper>)application; | ||
- (void)startTimer; | ||
- (void)stopTimer; | ||
@end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
#import "MMEBackgroundLocationServiceTimeoutHandler.h" | ||
#import "MMEUIApplicationWrapper.h" | ||
|
||
static const NSTimeInterval MMELocationManagerHibernationTimeout = 300.0; | ||
static const NSTimeInterval MMELocationManagerHibernationPollInterval = 5.0; | ||
|
||
@interface MMEBackgroundLocationServiceTimeoutHandler () | ||
|
||
@property (nonatomic) id<MMEUIApplicationWrapper> application; | ||
@property (nonatomic) NSDate *expiration; | ||
@property (nonatomic, readwrite) NSTimer *timer; | ||
@property (nonatomic) UIBackgroundTaskIdentifier backgroundTaskId; | ||
|
||
@end | ||
|
||
#pragma mark - MMEBackgroundLocationServiceTimeoutHandler | ||
|
||
@implementation MMEBackgroundLocationServiceTimeoutHandler | ||
|
||
- (instancetype)initWithApplication:(id<MMEUIApplicationWrapper>)application { | ||
self = [super init]; | ||
if (self) { | ||
_application = application; | ||
} | ||
return self; | ||
} | ||
|
||
- (void)timeoutAllowedCheck:(NSTimer *)timer { | ||
id<MMEBackgroundLocationServiceTimeoutHandlerDelegate> delegate = self.delegate; | ||
|
||
if (!delegate) { | ||
dispatch_async(dispatch_get_main_queue(), ^{ | ||
[self stopTimer]; | ||
}); | ||
return; | ||
} | ||
|
||
if (![delegate timeoutHandlerShouldCheckForTimeout:self]) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While this is an internal application, would it be good if we check first if the delegate already implements the method? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These aren't Since these are really internal methods, I didn't see a need to make them optional. |
||
self.expiration = [[NSDate date] dateByAddingTimeInterval:MMELocationManagerHibernationTimeout]; | ||
return; | ||
} | ||
|
||
if (!self.expiration) { | ||
return; | ||
} | ||
|
||
NSTimeInterval timeIntervalSinceTimeoutAllowed = [[NSDate date] timeIntervalSinceDate:self.expiration]; | ||
if (timeIntervalSinceTimeoutAllowed > 0) { | ||
dispatch_async(dispatch_get_main_queue(), ^{ | ||
[self stopTimer]; | ||
[delegate timeoutHandlerDidTimeout:self]; | ||
}); | ||
} | ||
} | ||
|
||
- (void)startTimer { | ||
if (self.timer) { | ||
return; | ||
} | ||
|
||
__weak __typeof__(self) weakself = self; | ||
NSAssert(self.backgroundTaskId == UIBackgroundTaskInvalid, @"Background task Id should be invalid"); | ||
self.backgroundTaskId = [self.application beginBackgroundTaskWithExpirationHandler:^{ | ||
__typeof__(self) strongSelf = weakself; | ||
|
||
[strongSelf stopTimer]; | ||
[strongSelf.delegate timeoutHandlerBackgroundTaskDidExpire:strongSelf]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as above. |
||
}]; | ||
|
||
self.expiration = [[NSDate date] dateByAddingTimeInterval:MMELocationManagerHibernationTimeout]; | ||
self.timer = [NSTimer scheduledTimerWithTimeInterval:MMELocationManagerHibernationPollInterval target:self selector:@selector(timeoutAllowedCheck:) userInfo:nil repeats:YES]; | ||
} | ||
|
||
- (void)stopTimer { | ||
if (UIBackgroundTaskInvalid != self.backgroundTaskId) { | ||
[self.application endBackgroundTask:self.backgroundTaskId]; | ||
self.backgroundTaskId = UIBackgroundTaskInvalid; | ||
} | ||
|
||
[self.timer invalidate]; | ||
self.timer = nil; | ||
self.expiration = nil; | ||
} | ||
|
||
@end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,39 +1,44 @@ | ||
#import "MMELocationManager.h" | ||
#import "MMEBackgroundLocationServiceTimeoutHandler.h" | ||
#import "MMEUIApplicationWrapper.h" | ||
#import "MMEDependencyManager.h" | ||
#import "MMEEventsService.h" | ||
#import "MMEEventsConfiguration.h" | ||
#import <CoreLocation/CoreLocation.h> | ||
|
||
static const NSTimeInterval MMELocationManagerHibernationTimeout = 300.0; | ||
static const NSTimeInterval MMELocationManagerHibernationPollInterval = 5.0; | ||
|
||
const CLLocationDistance MMELocationManagerDistanceFilter = 5.0; | ||
const CLLocationDistance MMERadiusAccuracyMax = 300.0; | ||
|
||
NSString * const MMELocationManagerRegionIdentifier = @"MMELocationManagerRegionIdentifier.fence.center"; | ||
|
||
@interface MMELocationManager () <CLLocationManagerDelegate> | ||
@interface MMELocationManager () <CLLocationManagerDelegate, MMEBackgroundLocationServiceTimeoutHandlerDelegate> | ||
|
||
@property (nonatomic) id<MMEUIApplicationWrapper> application; | ||
@property (nonatomic) CLLocationManager *locationManager; | ||
@property (nonatomic, getter=isUpdatingLocation, readwrite) BOOL updatingLocation; | ||
@property (nonatomic) NSDate *backgroundLocationServiceTimeoutAllowedDate; | ||
@property (nonatomic) NSTimer *backgroundLocationServiceTimeoutTimer; | ||
@property (nonatomic) MMEBackgroundLocationServiceTimeoutHandler *backgroundLocationServiceTimeoutTimerWrapper; | ||
@property (nonatomic) BOOL hostAppHasBackgroundCapability; | ||
@property (nonatomic) MMEEventsConfiguration *configuration; | ||
|
||
@end | ||
|
||
@implementation MMELocationManager | ||
|
||
- (void)dealloc { | ||
_locationManager.delegate = nil; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a great question. I had all the Do you know? *uh oh. |
||
[_backgroundLocationServiceTimeoutTimerWrapper stopTimer]; | ||
} | ||
|
||
- (instancetype)init { | ||
self = [super init]; | ||
if (self) { | ||
_application = [[MMEUIApplicationWrapper alloc] init]; | ||
NSArray *backgroundModes = [[NSBundle mainBundle] objectForInfoDictionaryKey:@"UIBackgroundModes"]; | ||
_hostAppHasBackgroundCapability = [backgroundModes containsObject:@"location"]; | ||
_configuration = [[MMEEventsService sharedService] configuration]; | ||
|
||
_backgroundLocationServiceTimeoutTimerWrapper = [[MMEBackgroundLocationServiceTimeoutHandler alloc] initWithApplication:_application]; | ||
_backgroundLocationServiceTimeoutTimerWrapper.delegate = self; | ||
} | ||
return self; | ||
} | ||
|
@@ -52,6 +57,10 @@ - (void)startUpdatingLocation { | |
|
||
- (void)stopUpdatingLocation { | ||
if ([self isUpdatingLocation]) { | ||
|
||
// Stop the timer | ||
[self stopBackgroundTimeoutTimer]; | ||
|
||
[self.locationManager stopUpdatingLocation]; | ||
[self.locationManager stopMonitoringSignificantLocationChanges]; | ||
[self.locationManager stopMonitoringVisits]; | ||
|
@@ -85,6 +94,15 @@ - (void)setMetricsEnabledForInUsePermissions:(BOOL)metricsEnabledForInUsePermiss | |
|
||
#pragma mark - Utilities | ||
|
||
- (void)setLocationManager:(CLLocationManager *)locationManager { | ||
if (locationManager == _locationManager) { | ||
return; | ||
} | ||
|
||
_locationManager.delegate = nil; | ||
_locationManager = locationManager; | ||
} | ||
|
||
- (void)configurePassiveLocationManager { | ||
self.locationManager.delegate = self; | ||
self.locationManager.desiredAccuracy = kCLLocationAccuracyThreeKilometers; | ||
|
@@ -132,31 +150,12 @@ - (void)startLocationServices { | |
} | ||
} | ||
|
||
- (void)timeoutAllowedCheck { | ||
if (!self.isUpdatingLocation) { | ||
return; | ||
} | ||
|
||
if (self.application.applicationState == UIApplicationStateActive || | ||
self.application.applicationState == UIApplicationStateInactive ) { | ||
[self startBackgroundTimeoutTimer]; | ||
return; | ||
} | ||
|
||
NSTimeInterval timeIntervalSinceTimeoutAllowed = [[NSDate date] timeIntervalSinceDate:self.backgroundLocationServiceTimeoutAllowedDate]; | ||
if (timeIntervalSinceTimeoutAllowed > 0) { | ||
[self.locationManager stopUpdatingLocation]; | ||
self.backgroundLocationServiceTimeoutAllowedDate = nil; | ||
if ([self.delegate respondsToSelector:@selector(locationManagerBackgroundLocationUpdatesDidTimeout:)]) { | ||
[self.delegate locationManagerBackgroundLocationUpdatesDidTimeout:self]; | ||
} | ||
} | ||
- (void)startBackgroundTimeoutTimer { | ||
[self.backgroundLocationServiceTimeoutTimerWrapper startTimer]; | ||
} | ||
|
||
- (void)startBackgroundTimeoutTimer { | ||
[self.backgroundLocationServiceTimeoutTimer invalidate]; | ||
self.backgroundLocationServiceTimeoutAllowedDate = [[NSDate date] dateByAddingTimeInterval:MMELocationManagerHibernationTimeout]; | ||
self.backgroundLocationServiceTimeoutTimer = [NSTimer scheduledTimerWithTimeInterval:MMELocationManagerHibernationPollInterval target:self selector:@selector(timeoutAllowedCheck) userInfo:nil repeats:YES]; | ||
- (void)stopBackgroundTimeoutTimer { | ||
[self.backgroundLocationServiceTimeoutTimerWrapper stopTimer]; | ||
} | ||
|
||
- (void)establishRegionMonitoringForLocation:(CLLocation *)location { | ||
|
@@ -202,10 +201,33 @@ - (void)locationManager:(CLLocationManager *)manager didVisit:(CLVisit *)visit { | |
} | ||
|
||
- (void)locationManagerDidPauseLocationUpdates:(CLLocationManager *)locationManager { | ||
// TODO: Should we stop the background timer here for completeness? | ||
// [self stopBackgroundTimeoutTimer]; | ||
|
||
if ([self.delegate respondsToSelector:@selector(locationManagerBackgroundLocationUpdatesDidAutomaticallyPause:)]) { | ||
[self.delegate locationManagerBackgroundLocationUpdatesDidAutomaticallyPause:self]; | ||
} | ||
} | ||
|
||
@end | ||
#pragma mark - MMEBackgroundLocationServiceTimeoutHandlerDelegate | ||
|
||
- (BOOL)timeoutHandlerShouldCheckForTimeout:(__unused MMEBackgroundLocationServiceTimeoutHandler *)handler { | ||
return self.isUpdatingLocation && (self.application.applicationState == UIApplicationStateBackground); | ||
} | ||
|
||
- (void)timeoutHandlerDidTimeout:(__unused MMEBackgroundLocationServiceTimeoutHandler *)handler { | ||
if ([self.delegate respondsToSelector:@selector(locationManagerBackgroundLocationUpdatesDidTimeout:)]) { | ||
[self.delegate locationManagerBackgroundLocationUpdatesDidTimeout:self]; | ||
} | ||
|
||
[self.locationManager stopUpdatingLocation]; | ||
} | ||
|
||
- (void)timeoutHandlerBackgroundTaskDidExpire:(__unused MMEBackgroundLocationServiceTimeoutHandler *)handler { | ||
// Do we need a delegate method here (i.e. do we need an event for background task expiry?) | ||
NSAssert(!handler.timer, @"Timer should be nil by this point"); | ||
} | ||
|
||
|
||
|
||
@end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hate this protocol name 😢 - any suggestions @rclee @fabian-guerra. Maybe replace
HandlerDelegate
with just one ofProtocol
,Delegate
,Handler
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MMEBackgroundLocationServiceTimeoutDelegate
works for me.