Sync logic adjustments #474
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves the nearby geofence syncing logic to avoid unnecessarily removing and re-adding geofences at each track call. The changes implement a diff-based approach to only modify geofences and notifications when they actually differ from what's already monitored.
- Replaces array-based registered notifications with set-based storage to avoid duplicates
- Implements comparison methods to determine if regions and notifications are equivalent
- Refactors syncing logic to only add/remove geofences and notifications when necessary
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| RadarSDK/RadarState.m | Changes registered notifications storage from array to set to prevent duplicates |
| RadarSDK/RadarNotificationHelper.m | Fixes indentation and refactors notification removal into separate method |
| RadarSDK/RadarNotificationHelper.h | Adds method declarations for new notification helper functions |
| RadarSDK/RadarLocationManager.m | Implements diff-based geofence and notification syncing with comparison methods |
| RadarSDK/RadarAPIClient.m | Fixes incorrect parameter name in notification diff handling |
| RadarSDK.xcodeproj/project.pbxproj | Downgrades Swift version from 6.0 to 5.0 |
| Example/Pulse/ExportOptions.plist | Adds new export configuration file |
| Example/Pulse/DistributionSummary.plist | Adds new distribution summary file |
| Example/Example/MainView.swift | Adds new SwiftUI view for debugging monitored regions and notifications |
| Example/Example/AppDelegate.swift | Updates example app to support SwiftUI interface and removes test key |
| Example/Example.xcodeproj/project.pbxproj | Adds MainView.swift to project build phases |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| //radarInitializeOptions.autoLogNotificationConversions = true | ||
| //radarInitializeOptions.autoHandleNotificationDeepLinks = true | ||
| Radar.initialize(publishableKey: "prj_test_pk_0000000000000000000000000000000000000000", options: radarInitializeOptions ) | ||
| Radar.initialize(publishableKey: "prj_test_pk_", options: radarInitializeOptions ) |
There was a problem hiding this comment.
The publishable key appears to be incomplete or a placeholder. Ensure this is not accidentally committed to version control with a real key.
KennyHuRadar
left a comment
KennyHuRadar
left a comment
There was a problem hiding this comment.
So from my understanding, we are trying to move away from the re-registering of CSGNs per track to improve reliability?
Why does the monitoring for nearby geofences also need to change?
|
prefer #492 |
closed in favor of #492
Improve nearby geofence syncing logic
only remove/add when necessary to avoid removing and re-adding nearby geofences at each track.