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

Add rto tracking options update to trackOnce and trackVerified calls #277

Merged
merged 5 commits into from
Oct 11, 2023

Conversation

KennyHuRadar
Copy link
Contributor

We previously only updated the RTO settings with background tracking calls, this PR adds a side-effect to the trackOnce and trackVerified calls such that any updates to RTO tracking options are saved into settings.

QA:
steps:

  1. Have RTO turned off
  2. check that RTO is not on in waypoint setting:
image
  1. turn on RTO to efficient in dashboard
  2. call track once in waypoint and verify that it updated
image
  1. modify RTO settings on dashboard and verify that it is propagated to waypoint after a calling trackOnce
image
  1. turn off RTO on dashboard and call another trackonce on waypoint to verify that RTO is turned off
image
  1. repeat the previosu steps but this time use the trackverify call

Follow up question, do we wish to introduce this side effect to the track verified calls and the mock tracking calls too?

@KennyHuRadar KennyHuRadar marked this pull request as ready for review October 2, 2023 21:16
@KennyHuRadar KennyHuRadar requested a review from lmeier October 3, 2023 17:17
RadarSDK/Radar.m Outdated
@@ -161,6 +161,7 @@ + (void)trackOnceWithDesiredAccuracy:(RadarTrackingOptionsDesiredAccuracy)desire
NSArray<RadarGeofence *> *_Nullable nearbyGeofences, RadarConfig *_Nullable config, NSString *_Nullable token) {
if (status == RadarStatusSuccess) {
[[RadarLocationManager sharedInstance] replaceSyncedGeofences:nearbyGeofences];
[[RadarLocationManager sharedInstance] updateTrackingFromMeta:config.meta];
Copy link
Contributor

Choose a reason for hiding this comment

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

This config is nullable so we should check that exists before accessing config.meta

Copy link
Contributor

@lmeier lmeier left a comment

Choose a reason for hiding this comment

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

2 things:

  • address the nullable config
  • bump versions

@KennyHuRadar KennyHuRadar requested a review from lmeier October 10, 2023 20:23
@@ -161,6 +161,10 @@ + (void)trackOnceWithDesiredAccuracy:(RadarTrackingOptionsDesiredAccuracy)desire
NSArray<RadarGeofence *> *_Nullable nearbyGeofences, RadarConfig *_Nullable config, NSString *_Nullable token) {
if (status == RadarStatusSuccess) {
[[RadarLocationManager sharedInstance] replaceSyncedGeofences:nearbyGeofences];
if(config != nil){
Copy link
Contributor

Choose a reason for hiding this comment

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

text formatting is off here, see line 162 for correct spacing

Copy link
Contributor

@lmeier lmeier left a comment

Choose a reason for hiding this comment

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

once you fix the text formatting and confirm that you tested this specific version with waypoint, this is good to go

@KennyHuRadar KennyHuRadar requested a review from lmeier October 11, 2023 16:02
Copy link
Contributor

@lmeier lmeier left a comment

Choose a reason for hiding this comment

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

🚀

@KennyHuRadar KennyHuRadar merged commit ba97664 into master Oct 11, 2023
1 check passed
@KennyHuRadar KennyHuRadar deleted the experiment-tracking-waypoint branch October 11, 2023 16:59
ShiCheng-Lu pushed a commit that referenced this pull request Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants