Skip to content
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

feat: added ability to specify the activity type if needed #123

Merged
merged 6 commits into from
Oct 9, 2019

Conversation

dcortright
Copy link
Contributor

@dcortright dcortright commented Oct 7, 2019

The iOS activity types, CLActivityType, help the iOS determine what type of activity is being tracked and when/if the tracking should be paused based on how long the device has been stationary.

This PR allows the user to set the activity type for their specific use case. CLLocationManager defaults to CLActivityTypeOther and will continue to do so if not set by the user.

  • overloaded subscription methods with new activity type parameter
  • updated example app to include activity type selection
  • updated read.me with info on activity type
  • updated device simulator iOS version to 12
  • updated travis Xcode version to 10
  • updated target iOS pod version to 12

@dcortright
Copy link
Contributor Author

for issue: #120

@lwdupont lwdupont self-assigned this Oct 7, 2019
@dcortright
Copy link
Contributor Author

@lwdupont The CLActivityTypeAirborne is only compatible with iOS 12 and above. Should we update the travis build environment to run on iOS 12?

Asynchronously requests the current location of the device using location services, optionally delaying the timeout countdown until the user has
responded to the dialog requesting permission for this app to access location services.

@param desiredAccuracy The accuracy level desired (refers to the accuracy and recency of the location).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job on the comments/headers!! 💯

{
switch (desiredActivityType) {
case CLActivityTypeFitness:
if (self.locationManager.activityType != CLActivityTypeFitness) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it good practice to check if it's currently what you are about to set it? (maybe so that it doesn't fire up location services on the switch?)

(vs was I was expecting was just setting the activityType)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is any real reason to check it before hand. At this point you have already requested a new location reading and there is no MAX activity type. I'll go ahead and remove this redundant check.

@@ -70,8 +68,24 @@
<action selector="desiredAccuracyControlChanged:" destination="vXZ-lx-hvc" eventType="valueChanged" id="HLd-94-Htx"/>
</connections>
</segmentedControl>
<segmentedControl opaque="NO" contentMode="scaleToFill" contentHorizontalAlignment="left" contentVerticalAlignment="top" apportionsSegmentWidthsByContent="YES" segmentControlStyle="plain" selectedSegmentIndex="0" translatesAutoresizingMaskIntoConstraints="NO" id="ujK-C3-t78">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And thank you for updating the example app too!!!!! 👍 🥇

Copy link
Contributor

@lwdupont lwdupont left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are great changes, thank you @dcortright ! Just had one question for my sake, see the comment.

@lwdupont
Copy link
Contributor

lwdupont commented Oct 8, 2019

@dcortright And yes, I'd say let's go and increase Travis to build on iOS 12. Thanks.

@dcortright
Copy link
Contributor Author

@lwdupont Should I leave the pod file at iOS 10? Or bump that up to 12 as well?

@lwdupont
Copy link
Contributor

lwdupont commented Oct 8, 2019

I say got to iOS 12 too. We can bump the version when we release to something higher than a .0.1

@dcortright
Copy link
Contributor Author

@lwdupont Done! Updated .travis.yml and pod file.

@dcortright
Copy link
Contributor Author

If possible could I also get assigned to this as well as get the hacktoberfest label added? Thank you for all your help!

@lwdupont
Copy link
Contributor

lwdupont commented Oct 9, 2019

Thanks, looks great to me! :) I assigned you, and added the label too.

@lwdupont lwdupont merged commit 91869f6 into intuit:master Oct 9, 2019
@dcortright
Copy link
Contributor Author

Thanks @lwdupont! That was my first contribution to open source and it was pretty fun!

@lwdupont
Copy link
Contributor

lwdupont commented Oct 9, 2019

You did an awesome job @dcortright ! I'll look tomorrow to release this to Cocoapods too, so people can use it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants