Offline ramp up #404

KennyHuRadar · 2024-10-16T20:53:10Z

This PR implements offline ramp ups.

Config calls will sync down all 3 configurations of RTOs for the user, default, on-trip and in-geofence
As we sync nearby geofences, we will also store nearby geofences into RadarState
We deviate from the tech spec by not currently storing the entire RadarUser, instead we simply use the existing geofenceIds methods from RadarState to denote the geofences we are in. It should also be noted that this would affect the results reported by users using anonymous tracks, but the behavior "makes sense".
if the sendLocation network call fails, we instead use the RadarOfflineManager, written in swift, to contextualize the location lat longs.
- The RadarOfflineManager is written as a stand in for the server that has access to all the information the client has stored.
- It takes the lat long and nearby geofences to perform point in circle calculations. From those calculation, it determines if the user is in a geofence or not.
- It then takes the stored RTO configurations of the user to ramp up or ramp down as needed.
The main change in the "critical path" of the track lifecycle is that the SDK will update its RTO from config even if sendLocation returns a network error. This allows the RadarOfflineManager to ramp up or ramp down the SDK.
Since RTO needs to be turned on anyways to this feature to be relevant, we simply use RadarOfflineManager to override the RTO, we do not need an new fields to manage.

KennyHuRadar · 2024-10-17T15:12:07Z

RadarSDK/RadarGeofence.m

@@ -228,7 +228,7 @@ - (NSDictionary *)dictionaryValue {
        RadarCircleGeometry *circleGeometry = (RadarCircleGeometry *)self.geometry;
        [dict setValue:@(circleGeometry.radius) forKey:@"geometryRadius"];
        [dict setValue:[circleGeometry.center dictionaryValue] forKey:@"geometryCenter"];
-        [dict setValue:@"Circle" forKey:@"type"];
+        [dict setValue:@"circle" forKey:@"type"];


we pass down the type as circle and polygon from the server and parse it as such, however, we were setting the dict with the capitalized version, this causes issues. We need to make sure this does not impact our cross-platform SDKs.

I vaguely remember fixing this (maybe in RN, changing it to expect capitalization)

hmm it might be easier to just change the fromJson within the native SDK to accept both capitalized and non capitalized names of the shape to avoid introducing knock on effects downstream, this can help avoid introducing a breaking change.

KennyHuRadar · 2024-10-17T15:13:21Z

RadarSDK/RadarLocationManager.m

-    [RadarNotificationHelper removePendingNotificationsWithCompletionHandler: ^{
-        [RadarNotificationHelper addOnPremiseNotificationRequests:requests];
-    }];
+    if (NSClassFromString(@"XCTestCase") == nil) {


this method is now ran in our test suite, so we need to suppress this chunk in tests as test env does not have a notification manager.

lmeier · 2024-10-18T12:07:00Z

RadarSDK/RadarAPIClient.m

I think if there's a network error and replay: all is on, we should change the log level of a network error from error to info.

how deeply do we want to couple these offline server fallback behaviors with the idea of replay? Asking this as I'm already creating new flags.

…ofences

lmeier · 2024-10-24T20:41:57Z

RadarSDK/RadarSdkConfiguration.m

+    NSObject *inGeofenceTrackingOptionsObj = dict[@"inGeofenceTrackingOptions"];
+    _inGeofenceTrackingOptions = nil;
+    if (inGeofenceTrackingOptionsObj && [inGeofenceTrackingOptionsObj isKindOfClass:[NSDictionary class]]) {
+        RadarTrackingOptions *radarTrackingOptions = [RadarTrackingOptions trackingOptionsFromObject:inGeofenceTrackingOptionsObj];
+        if (radarTrackingOptions) {
+            _inGeofenceTrackingOptions = radarTrackingOptions;
+        }
+    }
+
+    NSObject *defaultTrackingOptionsObj = dict[@"defaultTrackingOptions"];
+    _defaultTrackingOptions = nil;
+    if (defaultTrackingOptionsObj && [defaultTrackingOptionsObj isKindOfClass:[NSDictionary class]]) {
+        RadarTrackingOptions *radarTrackingOptions = [RadarTrackingOptions trackingOptionsFromObject:defaultTrackingOptionsObj];
+        if (radarTrackingOptions) {
+            _defaultTrackingOptions = radarTrackingOptions;
+        }
+    } 
+
+    NSObject *onTripTrackingOptionsObj = dict[@"onTripTrackingOptions"];
+    _onTripTrackingOptions = nil;
+    if (onTripTrackingOptionsObj && [onTripTrackingOptionsObj isKindOfClass:[NSDictionary class]]) {
+        RadarTrackingOptions *radarTrackingOptions = [RadarTrackingOptions trackingOptionsFromObject:onTripTrackingOptionsObj];
+        if (radarTrackingOptions) {
+            _onTripTrackingOptions = radarTrackingOptions;
+        }
+    } 
+


See server PR comment for an alternative struct that I think we should prefer

* define cocoapods version * typo

lmeier · 2024-10-29T20:22:06Z

RadarSDK/Radar.m

-                                         }];
-                                     }
-                                 }];
+                                    if (config != nil) {


why are we removing the status check? if (status == RadarStatusSuccess)

This is one of the changes I highlighted regarding the track lifecycle. When the server track request fails, we fallback to the offline manager to act as a stand-in. In these cases, although we return a non-success status, we still receive a valid config, which the rest of the track lifecycle should process.

Therefore, instead of relying on the status code, we should check if the config is non-nil. A non-nil config only occurs either from a successful server-side track or when the offline manager handles the network error.

lmeier · 2024-10-29T20:35:11Z

RadarSDK/RadarLocationManager.m

+                                            self.sending = NO;
+
+                                            if (config != nil) {
+                                                [self updateTrackingFromMeta:config.meta];


I wonder if we should adopt the pattern of doing the check in the function that's called, like: #409

it seems like each instance of updateTrackingFromMeta in unwrapping config, I'm changing the method to updateTrackingFromConfig and capturing the `config != nil' inside the new method.

lmeier · 2024-10-29T20:47:10Z

RadarSDK/RadarOfflineManager.swift

+import CoreLocation
+
+@objc(RadarOfflineManager) class RadarOfflineManager: NSObject {
+    @objc public static func contextualizeLocation(_ location: CLLocation, completionHandler: @escaping (RadarConfig?) -> Void) {


I find this function name to be quite opaque. I feel like this is more something in the vain of updateConfigFromOfflineLocation (or something in that ballpark)

I intentionally kept it more vague, it was my understanding that this might become a place where stuff like client side event generation and trip updates may occur?

updateTrackingOptionsFromOfflineLocation

lmeier · 2024-10-30T15:42:33Z

RadarSDK/RadarSDKTests-Bridging-Header.h

Is this supposed to be empty?

its auto-generated as we introduce swift into the repo, not removing as it may come in useful when we want to directly test swift stuff in the future and it is a pain to manually add back in these autogenerated files in xcode with all the obscure linking we need to do via the UI.

lmeier · 2024-10-30T15:46:01Z

RadarSDK/RadarState.m

@@ -190,4 +193,31 @@ + (BOOL)notificationPermissionGranted {
    return [[NSUserDefaults standardUserDefaults] boolForKey:kNotificationPermissionGranted];
 }

+ (void)setNearbyGeofences:(NSArray<RadarGeofence *> *_Nullable)nearbyGeofences {


One thing that makes me mildly uncomfortable is that presently I've thought of those geofences synced as CLRegions as the "nearbyGeofences". Which if I were to preserve that understanding, I'd want this to be called something else.

Maybe I / we should be thinking of those as "monitored regions"? In which case, maybe we'd (at some point? now?) want to abstract that monitoring logic into a monitorRegions function?

per offline discussion, no change required as of now.

lmeier · 2024-10-30T15:46:24Z

RadarSDKTests/Resources/track.json

+            "type": "Point"
+         },
+         "_id": "5ca7dd72208530002b30683c",
+         "description": "S3 Test Monk's Café",


KennyHuRadar added 2 commits October 16, 2024 16:22

implement client side ramp up

27fc369

fix parsing of geofences

9206728

KennyHuRadar changed the title ~~Kenny/Offline ramp up~~ [WIP] Kenny/Offline ramp up Oct 16, 2024

KennyHuRadar added 6 commits October 16, 2024 16:57

use swift 5.0

e2efd53

change build definition of radar test to remove un-needed source files

429b4ed

remove un-needed logging, just flacky tests

1a38ecd

remove logging

e84a9f0

update pod file

61f6b8e

remove un-needed imports

eb7e5d8

KennyHuRadar commented Oct 17, 2024

View reviewed changes

KennyHuRadar added 6 commits October 17, 2024 15:45

update normal trackonce behaviour

decb2a8

clean up

6e54e28

add new swift definitions

e6e3c6a

add logger to be accessible by swift

65b9250

add flag

9142abf

put feature behind flag

3397fc6

KennyHuRadar marked this pull request as ready for review October 17, 2024 20:27

KennyHuRadar requested a review from lmeier October 17, 2024 20:28

lmeier reviewed Oct 18, 2024

View reviewed changes

KennyHuRadar and others added 10 commits October 21, 2024 09:36

bump beta version

c4e115b

Merge branch 'master' into kenny/off-line-ramp-up

e8522f3

bump to right beta version

d404d57

Update RadarSettings.m

3fd5b00

add build settings and suppress testing on builds for now

c2fb45d

add flag to project

4015e47

uncomment out the tests

c4828b8

clean up

a87cea2

remove duplicate soruce and also add some logging to adding nearby ge…

28ce949

…ofences

add back compile source

be8c1d2

KennyHuRadar added 5 commits October 21, 2024 14:54

add inside geofence log message for radar offline manager

bdfcc07

use ramp up specific json resource and improve logging in radar state

5ac6d23

make compatible with replays

c5572b3

only call when replay is unsuccessful

b6dc7fa

change json file for test

9b4f71d

lmeier reviewed Oct 24, 2024

View reviewed changes

KennyHuRadar added 2 commits October 24, 2024 16:48

change how geofenes are turned to jsons

af9e4f7

change shape of alternative tracking options

f9f874c

KennyHuRadar requested a review from lmeier October 25, 2024 17:38

KennyHuRadar and others added 6 commits October 28, 2024 10:03

Merge branch 'master' into kenny/off-line-ramp-up

aa12fd4

refactor geofenceTag and tracking option extraction logic

39af20d

disable user script in build setting in radarSDKMotion

25d1094

use xcode 16 in circle ci

6dd7481

use iphone 16 in ci

4f277fd

Merge branch 'master' into kenny/off-line-ramp-up

7f8c280

KennyHuRadar changed the title ~~[WIP] Kenny/Offline ramp up~~ Kenny/Offline ramp up Oct 28, 2024

KennyHuRadar and others added 5 commits October 28, 2024 12:49

Update RadarSDKMotion.podspec

25e455d

define cocoapods version (#410)

fc75f16

* define cocoapods version * typo

rename

f2baf95

Merge branch 'master' into kenny/off-line-ramp-up

3785164

rename

85e1abe

lmeier reviewed Oct 30, 2024

View reviewed changes

KennyHuRadar and others added 3 commits October 30, 2024 16:27

refactor updateTrackingFromConfig

761ef37

Merge branch 'master' into kenny/off-line-ramp-up

b1cae04

rename and fix typo

880cbe7

KennyHuRadar requested a review from lmeier October 30, 2024 21:06

lmeier changed the title ~~Kenny/Offline ramp up~~ Offline ramp up Nov 4, 2024

KennyHuRadar and others added 2 commits November 5, 2024 15:53

increase number of geofences synced

b6f811b

Merge branch 'master' into kenny/off-line-ramp-up

999b459

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Offline ramp up #404

Offline ramp up #404

KennyHuRadar commented Oct 16, 2024 •

edited

Loading

KennyHuRadar Oct 17, 2024 •

edited

Loading

lmeier Oct 24, 2024

KennyHuRadar Oct 24, 2024

KennyHuRadar Oct 17, 2024

lmeier Oct 18, 2024

KennyHuRadar Oct 18, 2024

lmeier Oct 24, 2024 •

edited

Loading

lmeier Oct 29, 2024

KennyHuRadar Oct 30, 2024

lmeier Oct 29, 2024

KennyHuRadar Oct 30, 2024 •

edited

Loading

lmeier Oct 29, 2024

KennyHuRadar Oct 30, 2024

KennyHuRadar Oct 30, 2024

lmeier Oct 30, 2024

KennyHuRadar Oct 30, 2024

lmeier Oct 30, 2024

KennyHuRadar Oct 30, 2024

lmeier Oct 30, 2024

Offline ramp up #404

Are you sure you want to change the base?

Offline ramp up #404

Conversation

KennyHuRadar commented Oct 16, 2024 • edited Loading

KennyHuRadar Oct 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmeier Oct 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KennyHuRadar Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KennyHuRadar commented Oct 16, 2024 •

edited

Loading

KennyHuRadar Oct 17, 2024 •

edited

Loading

lmeier Oct 24, 2024 •

edited

Loading

KennyHuRadar Oct 30, 2024 •

edited

Loading