Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements machine learning-based indoor positioning using beacons. The changes transition from a simple indoorScan string parameter to a full CLLocation object representing ML-predicted indoor locations, and introduce infrastructure to manage indoor models and beacon scanning.
Key changes:
- Replaced
indoorScanparameter withindoorLocationthroughout the tracking API - Added
RadarIndoorsmodule to manage ML model lifecycle and location prediction - Introduced Swift bridge utilities to invoke Objective-C methods from async Swift contexts
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| RadarSDK/RadarIndoors.swift | New core module managing indoor ML models and location predictions |
| RadarSDK/RadarAPIClient.m | Updated to send indoor ML location data in tracking requests |
| RadarSDK/RadarLocationManager.m | Integrated RadarIndoors module into location tracking flow |
| RadarSDK/RadarSwiftBridge.m | Added method invocation utility for Objective-C/Swift interop |
| RadarSDK/RadarGeofence.m | Added activeIndoorModelId field to support model selection |
| RadarSDK/RadarUserDefaults.swift | Added initialization guard to prevent repeated app group lookups |
| RadarSDK/RadarSite.swift | New model classes for site and floorplan data |
| Example/Example/DebugView.swift | New debugging interface for indoor positioning development |
| Example/Example/AppDelegate.swift | Removed delegate protocols and demo button implementations |
Comments suppressed due to low confidence (8)
RadarSDK/RadarIndoors.swift:1
- Using DispatchSemaphore to block while waiting for async Task creates a potential deadlock. The semaphore.wait() blocks the thread, but if this closure is called on the actor's executor thread or main thread, it can deadlock when the Task tries to resume on the same thread. Consider refactoring getModelData to be async or restructure to avoid blocking waits.
RadarSDK/RadarIndoors.swift:1 - TODO comment indicates incomplete error logging implementation. The error should be properly logged using the SDK's logging infrastructure.
RadarSDK/RadarIndoors.swift:1 - Multiple print statements should be replaced with proper logging using RadarLogger to maintain consistency with the SDK's logging infrastructure and provide appropriate log levels.
RadarSDK/RadarIndoors.swift:1 - Multiple print statements should be replaced with proper logging using RadarLogger to maintain consistency with the SDK's logging infrastructure and provide appropriate log levels.
RadarSDK/RadarIndoors.swift:1 - Multiple print statements should be replaced with proper logging using RadarLogger to maintain consistency with the SDK's logging infrastructure and provide appropriate log levels.
RadarSDK/RadarIndoors.swift:1 - Multiple print statements should be replaced with proper logging using RadarLogger to maintain consistency with the SDK's logging infrastructure and provide appropriate log levels.
RadarSDK/RadarIndoors.swift:1 - Multiple print statements should be replaced with proper logging using RadarLogger to maintain consistency with the SDK's logging infrastructure and provide appropriate log levels.
RadarSDK/RadarSwiftBridge.m:1 - Loop variable should use NSUInteger instead of int to match args.count type and avoid potential sign conversion issues.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Published var transform = simd_float4x4() | ||
| @Published var heading = 0.0 | ||
|
|
||
| var updated: () -> Void = {} |
There was a problem hiding this comment.
The closure property name 'updated' is ambiguous. Consider renaming to 'onFrameUpdated' or 'frameUpdateCallback' to clarify its purpose.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.