Skip to content

Conversation

KennyHuRadar
Copy link
Contributor

@KennyHuRadar KennyHuRadar commented Sep 10, 2025

No description provided.

@KennyHuRadar KennyHuRadar marked this pull request as ready for review September 10, 2025 20:42
@Copilot Copilot AI review requested due to automatic review settings September 10, 2025 20:42
Copy link
Contributor

@Copilot Copilot AI left a 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 adds more debug logging to geofence monitoring operations and conditionally monitors nearby geofences based on whether they have notification text, while bumping the version from 3.23.2 to 3.23.3-beta.2.

  • Adds debug logging for pending notifications, monitored geofences, and region monitoring errors
  • Changes geofence monitoring to be conditional based on notification text metadata
  • Updates version numbers across multiple configuration files

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

File Description
RadarSDK/RadarNotificationHelper.m Adds debug logging for found pending notifications
RadarSDK/RadarLocationManager.m Restructures geofence monitoring logic to be conditional and adds debug logging
RadarUtils.m Updates SDK version string to beta version
Various .podspec and project files Updates version numbers to 3.23.3-beta.2 or 3.23.3

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@KennyHuRadar KennyHuRadar requested a review from lmeier September 10, 2025 20:50
@KennyHuRadar
Copy link
Contributor Author

not sure if we want to merge this, (prob not)
just looking for eyes before we cut a beta.

@KennyHuRadar KennyHuRadar changed the title add more logging, conditionally monitor nearby geofences and bump ver… [DO NOT MERGE] add more logging for CSGN lifecycle Sep 10, 2025

- (void)removeSyncedGeofences {
for (CLRegion *region in self.locationManager.monitoredRegions) {
[[RadarLogger sharedInstance] logWithLevel:RadarLogLevelDebug message:[NSString stringWithFormat:@"Encountering monitored geofence | identifier = %@", region.identifier]];
Copy link
Contributor

Choose a reason for hiding this comment

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

this and line 255 of notification helper would be too noisy in logs. but the monitoringDidFail logging should be brought in to the main sdk certainly. is there a registeringNotificationDidFail as well? would be good

[[RadarLogger sharedInstance] logWithLevel:RadarLogLevelDebug
message:[NSString stringWithFormat:@"Synced geofence | latitude = %f; longitude = %f; radius = %f; identifier = %@",
center.coordinate.latitude, center.coordinate.longitude, radius, identifier]];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah this is fine for costco, but in general I think even if a geofence has notification, it still should be tracked by regions unless specifically marked not to be track. The super simple example is if I have a store and a geofence around it, I make a notification campaign attached to my store geofence. I'd want the notification to get pushed, but still want the region to create a user location on the dashboard.

Comment on lines +1222 to +1225
- (void)locationManager:(CLLocationManager *)manager monitoringDidFailForRegion:(CLRegion *)region withError:(NSError *)error {
[[RadarLogger sharedInstance] logWithLevel:RadarLogLevelDebug message:[NSString stringWithFormat:@"CLLocation manager region monitoring error | error = %@", error]];
[[RadarDelegateHolder sharedInstance] didFailWithStatus:RadarStatusErrorLocation];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep this.

@KennyHuRadar KennyHuRadar changed the title [DO NOT MERGE] add more logging for CSGN lifecycle Add more logging for CSGN lifecycle Sep 11, 2025
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.

3 participants