Skip to content

FENCE-2804: Port bubble-geofence + region-removal methods to Swift#615

Open
stevepopovich wants to merge 12 commits into
masterfrom
stevepopovich/port-bubble-geofence-methods
Open

FENCE-2804: Port bubble-geofence + region-removal methods to Swift#615
stevepopovich wants to merge 12 commits into
masterfrom
stevepopovich/port-bubble-geofence-methods

Conversation

@stevepopovich

@stevepopovich stevepopovich commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Continues the RadarLocationManager ObjC→Swift migration by porting the bubble-geofence + region-removal cluster, following the established beacon-sync seam pattern.

Ported as @objc static twins on RadarLocationManagerSwift:

  • replaceBubbleGeofence(locationManager:location:radius:)
  • removeBubbleGeofence(locationManager:)
  • removeAllRegions(locationManager:)

Each ObjC method dispatches to its twin when useSwiftLocationManager is enabled and returns; both implementations coexist until the Swift port is trusted. Instance state (CLLocationManager) is passed in explicitly.

Changes

  • RadarLocationManager+Swift.swift — three Swift twins + mirrored radar_ / radar_bubble_ prefix constants.
  • RadarLocationManagerSwift.h — ObjC-visible declarations.
  • RadarLocationManager.museSwiftLocationManager dispatch shims (ObjC bodies kept for coexistence).
  • project.pbxproj — registers the new test file.
  • RadarLocationManagerSwiftRegionTests.swift — behavior tests: prefix-scoped removal, tracking-gated add, and the new region's center/radius.
  • AGENTS.md — note that make format reformats the entire repo (separate commit).

Manual test steps

Part A — Example app setup

  1. Force the Swift path on. Hardcode RadarSdkConfiguration.swift:60 so server config can't flip it mid-test:
    useSwiftLocationManager = true   // was: dict?["useSwiftLocationManager"] as? Bool ?? false
  2. Build & run on a real device (bubble geofences need real Core Location). Grant "Always" location. In the app's Logs tab, ensure debug logs are visible (Radar.setLogLevel(.debug) if needed).

Part B — Verify the Swift path (flag ON)

  1. Tap startTracking with the Responsive preset (it enables both the stopped and moving bubble geofences at a 100 m radius).
  2. On the first location update, confirm the emoji-prefixed line appears — this proves the Swift twin ran:
    • 🦅 Successfully added bubble geofence | latitude = …; longitude = …; radius = 100; identifier = radar_bubble_<uuid> (from replaceBubbleGeofence).
    • The absence of the 🦅 on this line would mean the ObjC body ran instead → fail.
  3. Move / re-evaluate: walk ≳150 m (beyond the 100 m radius) so the SDK syncs and replaces the bubble. Confirm:
    • 🦅 Removed bubble geofences (from removeBubbleGeofence), immediately followed by a new
    • 🦅 Successfully added bubble geofence | … at the updated location.

Part C — Regression check (flag OFF, ObjC path)

  1. Flip RadarSdkConfiguration.swift:60 to useSwiftLocationManager = false, rebuild, and repeat Part B.
  2. Behavior should be identical, except the same log lines now appear without the 🦅 prefix (Successfully added bubble geofence | …, Removed bubble geofences) — confirming the ObjC fallback does the same work.

Cleanup

Revert the RadarSdkConfiguration.swift hardcode before committing.

stevepopovich and others added 9 commits May 29, 2026 15:41
… twin

The existing seam tests only asserted that RadarSettings.previousTrackingOptions
was cleared after the call, which the Swift twin does unconditionally as its
last line — so they passed even if Radar.startTracking / Radar.stopTracking
were never actually invoked. Add assertions on RadarSettings.tracking and
RadarSettings.trackingOptions to verify the start/stop calls really did take
effect, and install a RadarPermissionsHelperMock for the start-tracking path
so the permission gate in RadarLocationManager.startTrackingWithOptions:
doesn't bail early in the simulator.

Also:
- Move helpers (clearState, installAuthorizedPermissions) into a new
  RadarLocationManagerSwiftTestHelpers enum so future test files can
  share them.
- Nest the suite inside RadarSerializedTests so it serializes with other
  suites that mutate global RadarSettings state.
- Expose RadarPermissionsHelperMock to Swift via the test bridging header.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Port matchBeaconIds, replaceSyncedBeacons, replaceSyncedBeaconUUIDs,
and removeSyncedBeacons to RadarLocationManagerSwift, dispatched from
RadarLocationManager.m when useSwiftLocationManager is enabled. ObjC
bodies are kept in place; both implementations coexist until the Swift
port is trusted. Swift twins that need a CLLocationManager take it as
an explicit argument; the identifier-prefix constants are mirrored as
private static let on RadarLocationManagerSwift until the ObjC bodies
are deleted.

Tests:
- TrackingCLLocationManager: CLLocationManager subclass that records
  startMonitoring / stopMonitoring / requestState and overrides
  monitoredRegions, mirroring the existing CLLocationManagerMock
  pattern.
- Extend RadarLocationManagerSwiftTestHelpers with makeBeacon and
  trackingOptions(beacons:) builders.
- New RadarLocationManagerSwiftBeaconSyncTests covering every guard
  in replaceSyncedBeacons / replaceSyncedBeaconUUIDs (useRadarModifiedBeacon,
  tracking off, options.beacons off, nil input, invalid UUID continue,
  9-region cap, happy path with requestState calls), plus removeSyncedBeacons
  prefix filtering, and one public-method routing test.
- matchBeaconIds tests live in RadarLocationManagerSwiftSeamTests since
  it's a pure function.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Trailing commas on the last item in array/dictionary literals and one
CLCircularRegion init re-wrap. CI's make format-check applies swift-format
in-place and diffs against HEAD, so these have to be committed.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Move the matchBeaconIds functionality tests out of the Seam suite and
into the beacon-sync suite, and move the replaceSyncedBeacons routing
test into the Seam suite. The Seam suite now asserts only flag cutover;
the beacon-sync suite holds all beacon behavior.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…beacon-methods

# Conflicts:
#	RadarSDK.xcodeproj/project.pbxproj
#	RadarSDKTests/RadarLocationManagerSwiftSeamTests.swift
#	RadarSDKTests/RadarLocationManagerSwiftTestHelpers.swift
Port replaceBubbleGeofence:radius:, removeBubbleGeofence, and removeAllRegions
to Swift twins on RadarLocationManagerSwift, following the existing beacon-sync
seam pattern. Each ObjC method dispatches to its twin when useSwiftLocationManager
is enabled; both implementations coexist until the Swift port is trusted.

Add RadarLocationManagerSwiftRegionTests covering prefix-scoped removal,
tracking-gated add, and the new region's center/radius.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@stevepopovich stevepopovich changed the title Port bubble-geofence + region-removal methods to Swift FENCE-2804: Port bubble-geofence + region-removal methods to Swift Jun 4, 2026
@stevepopovich stevepopovich marked this pull request as ready for review June 4, 2026 17:23
Base automatically changed from stevepopovich/port-beacon-methods to master June 4, 2026 18:44
…bubble-geofence-methods

# Conflicts:
#	RadarSDK.xcodeproj/project.pbxproj
#	RadarSDK/RadarLocationManager+Swift.swift
#	RadarSDK/RadarLocationManagerSwift.h

// Mirror of the identifier prefix constants in RadarLocationManager.m. Kept in sync by
// hand until that file is fully ported.
private static let identifierPrefix = "radar_"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we make these @objc static lets and ref them from the objective-c codebase so we don't have to keep them in sync?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

or reference them in swift via a shared header that exposes them

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this is ok. Sharing them makes it a bit more work to clean it up and I would like to keep the implementations as separate as possible. Usually I would agree with you but in this transitional case, I think it is ok to have some duplication so this file is fully testable without using obj c code

Comment thread RadarSDK/RadarLocationManager+Swift.swift Outdated
Comment thread RadarSDK/RadarLocationManager+Swift.swift

for uuid in uuids.prefix(numUUIDs) {
let identifier = "\(syncBeaconUUIDIdentifierPrefix)\(uuid)"
guard let proximityUUID = UUID(uuidString: uuid) else {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd prefer filtering before the loop instead of using continue to skip iterations.

let validUUIDs = uuids.prefix(numUUIDs).compactMap { uuid -> (String, UUID)? in
    guard let proximityUUID = UUID(uuidString: uuid) else {
	   // log here
	   return nil
   }
   (uuid, proximityUUID)
}

// Then this, or use a `.forEach` since you know you won't need to bail on any of the iterations
for (uuid, proximityUUID) in validUUIDs {
	// work
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Tomato, tomato 🙃

I feel like having having 2 loops is marginally slower and unneeded

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd have more sympathy for the performance argument if this wasn't bounded to a 9 elements 😅
But you're right, it's purely a personal preference nit - functional composition over collections reads easier to me at a glance.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'd have more sympathy for the performance argument if this wasn't bounded to a 9 elements

Good point 😆

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