-
-
Notifications
You must be signed in to change notification settings - Fork 936
Drop support for iOS < 11 #1461
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
Changes from 5 commits
aece25f
a87bd99
a20a414
388c1e7
c89e07c
682478a
8c27fc4
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 |
|---|---|---|
|
|
@@ -174,44 +174,36 @@ + (PermissionStatus)permissionStatus:(PermissionGroup)permission { | |
|
|
||
|
|
||
| + (PermissionStatus)determinePermissionStatus:(PermissionGroup)permission authorizationStatus:(CLAuthorizationStatus)authorizationStatus { | ||
| if (@available(iOS 8.0, *)) { | ||
| if (permission == PermissionGroupLocationAlways) { | ||
| switch (authorizationStatus) { | ||
| case kCLAuthorizationStatusNotDetermined: | ||
| return PermissionStatusDenied; | ||
| case kCLAuthorizationStatusRestricted: | ||
| return PermissionStatusRestricted; | ||
| case kCLAuthorizationStatusAuthorizedWhenInUse: | ||
| case kCLAuthorizationStatusDenied: | ||
| return PermissionStatusPermanentlyDenied; | ||
| case kCLAuthorizationStatusAuthorizedAlways: | ||
| return PermissionStatusGranted; | ||
| } | ||
| } | ||
|
|
||
| if (permission == PermissionGroupLocationAlways) { | ||
|
Contributor
Author
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. The diff for this method looks odd because of how the diff is computed. What I actually did is:
The last two switches could be combined together, I just did the minimal change. I'm happy to restructure it if you would prefer.
Contributor
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. Thanks for the update. It looks like
Contributor
Author
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. That would change the behavior of this method, and would require fully auditing all codepaths that lead to it to determine whether the deprecated constant is ever passed in as a parameter on iOS, since if it is adding a compile-time check would cause it to return the wrong value on iOS. Do you want to make that part of this PR, rather than changing behavior in a follow-up?
Contributor
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. No let's keep this one simple. Let's do this in another PR. |
||
| switch (authorizationStatus) { | ||
| case kCLAuthorizationStatusNotDetermined: | ||
| return PermissionStatusDenied; | ||
| case kCLAuthorizationStatusRestricted: | ||
| return PermissionStatusRestricted; | ||
| case kCLAuthorizationStatusAuthorizedWhenInUse: | ||
| case kCLAuthorizationStatusDenied: | ||
| return PermissionStatusPermanentlyDenied; | ||
| case kCLAuthorizationStatusAuthorizedWhenInUse: | ||
| case kCLAuthorizationStatusAuthorizedAlways: | ||
| return PermissionStatusGranted; | ||
| } | ||
| } | ||
|
|
||
| #pragma clang diagnostic push | ||
| #pragma clang diagnostic ignored "-Wdeprecated-declarations" | ||
|
|
||
| switch (authorizationStatus) { | ||
| case kCLAuthorizationStatusNotDetermined: | ||
| return PermissionStatusDenied; | ||
| case kCLAuthorizationStatusRestricted: | ||
| return PermissionStatusRestricted; | ||
| case kCLAuthorizationStatusDenied: | ||
| return PermissionStatusPermanentlyDenied; | ||
| case kCLAuthorizationStatusAuthorizedWhenInUse: | ||
| case kCLAuthorizationStatusAuthorizedAlways: | ||
| return PermissionStatusGranted; | ||
| } | ||
|
|
||
| #pragma clang diagnostic push | ||
| #pragma clang diagnostic ignored "-Wdeprecated-declarations" | ||
|
|
||
| switch (authorizationStatus) { | ||
| case kCLAuthorizationStatusAuthorized: | ||
| return PermissionStatusGranted; | ||
| default: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,12 +13,10 @@ - (PermissionStatus)checkPermissionStatus:(PermissionGroup)permission { | |
| } | ||
|
|
||
| - (void)checkServiceStatus:(PermissionGroup)permission completionHandler:(ServiceStatusHandler)completionHandler { | ||
| if (@available(iOS 11.0, *)) { | ||
| completionHandler([CMMotionActivityManager isActivityAvailable] | ||
| ? ServiceStatusEnabled | ||
| : ServiceStatusDisabled | ||
| ); | ||
| } | ||
| completionHandler([CMMotionActivityManager isActivityAvailable] | ||
| ? ServiceStatusEnabled | ||
| : ServiceStatusDisabled | ||
| ); | ||
|
|
||
| completionHandler(ServiceStatusDisabled); | ||
|
||
| } | ||
|
|
@@ -31,46 +29,38 @@ - (void)requestPermission:(PermissionGroup)permission completionHandler:(Permiss | |
| return; | ||
| } | ||
|
|
||
| if (@available(iOS 11.0, *)) { | ||
| CMMotionActivityManager *motionManager = [[CMMotionActivityManager alloc] init]; | ||
|
|
||
| NSDate *today = [NSDate new]; | ||
| [motionManager queryActivityStartingFromDate:today toDate:today toQueue:[NSOperationQueue mainQueue] withHandler:^(NSArray<CMMotionActivity *> *__nullable activities, NSError *__nullable error) { | ||
| PermissionStatus status = [SensorPermissionStrategy permissionStatus]; | ||
| completionHandler(status); | ||
| }]; | ||
| } else { | ||
| completionHandler(PermissionStatusDenied); | ||
| } | ||
| CMMotionActivityManager *motionManager = [[CMMotionActivityManager alloc] init]; | ||
|
|
||
| NSDate *today = [NSDate new]; | ||
| [motionManager queryActivityStartingFromDate:today toDate:today toQueue:[NSOperationQueue mainQueue] withHandler:^(NSArray<CMMotionActivity *> *__nullable activities, NSError *__nullable error) { | ||
| PermissionStatus status = [SensorPermissionStrategy permissionStatus]; | ||
| completionHandler(status); | ||
| }]; | ||
|
|
||
| } | ||
|
|
||
| + (PermissionStatus)permissionStatus { | ||
| if (@available(iOS 11.0, *)) { | ||
| CMAuthorizationStatus status = [CMMotionActivityManager authorizationStatus]; | ||
| PermissionStatus permissionStatus; | ||
|
|
||
| switch (status) { | ||
| case CMAuthorizationStatusNotDetermined: | ||
| permissionStatus = PermissionStatusDenied; | ||
| break; | ||
| case CMAuthorizationStatusRestricted: | ||
| permissionStatus = PermissionStatusRestricted; | ||
| break; | ||
| case CMAuthorizationStatusDenied: | ||
| permissionStatus = PermissionStatusPermanentlyDenied; | ||
| break; | ||
| case CMAuthorizationStatusAuthorized: | ||
| permissionStatus = PermissionStatusGranted; | ||
| break; | ||
| default: | ||
| permissionStatus = PermissionStatusGranted; | ||
| } | ||
|
|
||
| return permissionStatus; | ||
| CMAuthorizationStatus status = [CMMotionActivityManager authorizationStatus]; | ||
| PermissionStatus permissionStatus; | ||
|
|
||
| switch (status) { | ||
| case CMAuthorizationStatusNotDetermined: | ||
| permissionStatus = PermissionStatusDenied; | ||
| break; | ||
| case CMAuthorizationStatusRestricted: | ||
| permissionStatus = PermissionStatusRestricted; | ||
| break; | ||
| case CMAuthorizationStatusDenied: | ||
| permissionStatus = PermissionStatusPermanentlyDenied; | ||
| break; | ||
| case CMAuthorizationStatusAuthorized: | ||
| permissionStatus = PermissionStatusGranted; | ||
| break; | ||
| default: | ||
| permissionStatus = PermissionStatusGranted; | ||
| } | ||
|
|
||
| return PermissionStatusDenied; | ||
| return permissionStatus; | ||
| } | ||
|
|
||
| @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 left the code from here on to preserve behavior, but—with the caveat that I didn't look at the surrounding code—this looks very likely to be a pre-existing bug. It seems likely that this code was intended to be in an
else, or after areturnin theif, such that it only ran for iOS < 10. What it has actually been doing (and what this preserves) is running both sets of code on iOS 10+.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.
Good found, It looks like
CBCentralManagerStatePoweredOnis deprecated so we should remove this. I've tested this briefly and it still works nevertheless we should clean it up.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.
Okay, I've now removed the deprecated path.