-
Notifications
You must be signed in to change notification settings - Fork 31
Shicheng/improve sync logic 2 #492
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
base: master
Are you sure you want to change the base?
Conversation
| @available(iOS 13.0, *) | ||
| actor IOActor { | ||
| static let shared = IOActor() | ||
| } |
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.
using this Actor for thread safety, this is a background actor
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.
Pull Request Overview
This PR implements an improved geofence synchronization logic for the RadarSDK, introducing a new Swift-based location manager to handle geofence and notification management more efficiently. The changes add a feature flag (useImprovedSyncLogic) that allows toggling between the old and new synchronization approaches, with the new logic providing better tracking of registered notifications and more granular control over region monitoring.
Key Changes:
- Introduced
RadarLocationManager.swiftwith improved geofence and notification sync logic - Added
useImprovedSyncLogicfeature flag toRadarSdkConfiguration - Enhanced notification diff tracking with better logging and state management
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| RadarSDK/RadarLocationManager.swift | New Swift implementation of location manager with improved geofence/notification sync logic |
| RadarSDK/RadarState.swift | New Swift wrapper for accessing registered notifications from UserDefaults |
| RadarSDK/RadarLocationManager.m | Integration point to call new Swift implementation when feature flag is enabled |
| RadarSDK/RadarSettings.m | Clears existing geofences/notifications when transitioning to new sync logic |
| RadarSDK/RadarSdkConfiguration.m/h | Adds useImprovedSyncLogic feature flag property |
| RadarSDK/RadarState.m | Changes notification storage from NSMutableArray to NSMutableSet to prevent duplicates |
| RadarSDK/RadarNotificationHelper.m | Enhanced logging for notification diff processing |
| RadarSDK/RadarAPIClient.m | Fixed variable name from notificationsDelivered to notificationsRemaining |
| set_version.sh | Commented out version updates for RadarSDKIndoors |
| Example files | Added SwiftUI example interface and test waypoints |
Comments suppressed due to low confidence (1)
RadarSDK/RadarLocationManager.swift:1
- [nitpick] Consider using optional chaining to simplify the condition:
if sdkConfiguration.useImprovedSyncLogic && old?.useImprovedSyncLogic != true. This is more concise and handles the nil case naturally.
//
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Improve nearby geofence syncing logic
only remove/add when necessary to avoid removing and re-adding nearby geofences at each track.
Also added radar:notificationRepeats to campaignMetadata (added after original implmentation of repeats)
notification will prioritize geofence's notificationRepeats if set (since it's more granular), and fallback to campaign's notificationRepeats, then fallback to false / no-repeat