Skip to content
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

indoors #370

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

indoors #370

wants to merge 9 commits into from

Conversation

gregsadetsky
Copy link

No description provided.

@gregsadetsky gregsadetsky marked this pull request as draft July 25, 2024 18:30
@gregsadetsky gregsadetsky changed the title WIP indoors indoors Jul 30, 2024
@gregsadetsky gregsadetsky marked this pull request as ready for review July 30, 2024 14:03
Copy link
Contributor

@lmeier lmeier left a comment

Choose a reason for hiding this comment

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

some naming ideas and style nits

Comment on lines +47 to +50
<key>NSBluetoothAlwaysUsageDescription</key>
<string>Please enable Bluetooth for indoors location</string>
<key>NSBluetoothPeripheralUsageDescription</key>
<string>Please enable Bluetooth for indoors location</string>
Copy link
Contributor

@lmeier lmeier Aug 6, 2024

Choose a reason for hiding this comment

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

note: @KennyHuRadar's Info.plist will obviate the need for this

@@ -1086,6 +1089,12 @@ logConversionWithNotification
units:(RadarRouteUnits)units
completionHandler:(RadarRouteMatrixCompletionHandler)completionHandler NS_SWIFT_NAME(getMatrix(origins:destinations:mode:units:completionHandler:));

#pragma mark - Indoors

+ (void)doIndoorSurvey:(NSString *)placeLabel
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
+ (void)doIndoorSurvey:(NSString *)placeLabel
+ (void)surveyIndoors:(NSString *)placeLabel

or something of the like is more in-line with our naming conventions.

RadarEventTypeUserFailedFraud NS_SWIFT_NAME(userFailedFraud)
RadarEventTypeUserFailedFraud NS_SWIFT_NAME(userFailedFraud),
/// `user.indoor_location`
RadarEventTypeIndoorLocation NS_SWIFT_NAME(indoorLocation)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want a new event type here? I think instead we're likely to prefer something like an optional originalLocation on locations

@@ -154,6 +154,8 @@ typedef NS_ENUM(NSInteger, RadarTrackingOptionsSyncLocations) {
*/
@property (nonatomic, assign) BOOL beacons;

@property (nonatomic, assign) BOOL indoors;
Copy link
Contributor

Choose a reason for hiding this comment

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

Bigger philosophical naming question: we might want to use the techniques here in contexts that aren't actually indoors (e.g. open air malls, around the covered perimeter of buildings, etc) — would we potentially want to name this something like precise?

@@ -173,13 +173,15 @@ - (void)trackWithLocation:(CLLocation *_Nonnull)location
source:(RadarLocationSource)source
replayed:(BOOL)replayed
beacons:(NSArray<RadarBeacon *> *_Nullable)beacons
indoorsWhereAmIScan:(NSString *)indoorsWhereAmIScan
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
indoorsWhereAmIScan:(NSString *)indoorsWhereAmIScan
indoorsScan:(NSString *)indoorsScan

or maybe environmentScan?

Comment on lines +306 to +309
// in indoors mode, override accuracy
RadarTrackingOptions *options = [Radar getTrackingOptions];
if(options.indoors){
locationAccuracyNumber = [NSNumber numberWithFloat:10.0];
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want to do this server side. Best to not just throw away the data point

RadarTrackingOptions *options = [Radar getTrackingOptions];
BOOL wasStopped = [RadarState stopped];
BOOL stopped = NO;

BOOL force = (source == RadarLocationSourceForegroundLocation || source == RadarLocationSourceManualLocation || source == RadarLocationSourceBeaconEnter ||
source == RadarLocationSourceBeaconExit || source == RadarLocationSourceVisitArrival);

if(options.indoors) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if(options.indoors) {
if (options.indoors) {

[[RadarIndoorSurvey sharedInstance] start:@"WHEREAMI"
forLength:WHERE_AM_I_DURATION_SECONDS
withKnownLocation:location
isWhereAmIScan:YES
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
isWhereAmIScan:YES
isEnvironmentScan:YES

or isIndoorsScan potentially?

@@ -898,11 +949,11 @@ - (void)sendLocation:(CLLocation *)location stopped:(BOOL)stopped source:(RadarL
[[RadarBeaconManager sharedInstance] rangeBeaconUUIDs:beaconUUIDs
completionHandler:^(RadarStatus status, NSArray<RadarBeacon *> *_Nullable beacons) {
if (status != RadarStatusSuccess || !beacons) {
callTrackAPI(nil);
maybeIndoorSurveyThenCallTrackAPI(nil);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
maybeIndoorSurveyThenCallTrackAPI(nil);
maybeSurveyIndoorsThenCallTrackAPI(nil);

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