Location Services: don't reset desiredAccuracy on every update/error

[omnikron]

See #7680.

On iOS the RCTLocationObserver delegate is overriding desiredAccuracy every time CLLocationManager calls didUpdateLocations or didFailWithError. desiredAccuracy is reset to
RCT_DEFAULT_LOCATION_ACCURACY (100 meters) This effectively makes it impossible for a react-native app to use any location accuracy other than the default.

This commit simply removes the code which resets the desired accuracy, as there seems to be no rationale for doing so. The reset code was added as part of a large general
change so the original intention is unclear from the history. If somebody can explain it to me, I'm happy to rework this PR accordingly.

Changelog:

[iOS] [Fixed] - Location Services accuracy constantly reset to default of 100 meters.

Test Plan:

We've been using this in production at FATMAP for nearly a year in a fork of React Native. It works. I'm unclear on how to actually test this in code, so would appreciate any tips.

[omnikron]

Well, this is an unlikely coincidence – since I started working on this PR a few hours ago a very similar one has been opened (#23207). Happy to unify if it makes sense to do so, although I'm not sure if there is 100% overlap.

[hramos]

That one is not yet merged, so I think it's fine to continue working on this one for the time being.

[hramos]

Did you observe the issue mentioned in the comment, "otherwise update accuracy will force triggering didUpdateLocations, watchPosition would keeping receiving location updates, even there's no location changes." in your fork?

[omnikron]

@hramos there's quite a lot to unpick here, so bear with me! This issue is absolutely critical for us (our users are often in mountainous back country, so precise GPS user location is critical for safety as well as usability) which is why we've been maintaining our fork for 11 months.

Did you observe the issue mentioned in the comment, "otherwise update accuracy will force triggering didUpdateLocations, watchPosition would keeping receiving location updates, even there's no location changes."

I dispute that this is a bug that needed fixing here, so much as an application issue. When setting enableHighAccuracy and starting watchPosition the device will indeed send location updates even if you don't move. This is because the GPS signal will vary/reflect and send minute changes to very high (centimeter) accuracy, even when the device is not moving. However, my take is that this should be handled in application code, not in location services themselves. If you don't need this kind of very high accuracy, then you should simply disable enableHighAccuracy and go back to the default of 100m accuracy, which is probably fine for most 'what's near here' services.

It gets more complicated however, as the comment you mentioned was added in this commit which just adds this check:

 if (ABS(_locationManager.desiredAccuracy - RCT_DEFAULT_LOCATION_ACCURACY) > 0.000001)

to me this check alone makes absolutely zero sense. "If desiredAccuracy is not default accuracy then set desiredAccuracy to default accuracy" – what? I feel like I must be missing something, but I can't figure it out.

However, the actual code that I dispute –

_locationManager.desiredAccuracy = RCT_DEFAULT_LOCATION_ACCURACY;

– is added previously in this commit where the comment is simply 'Reset location accuracy'. I don't understand this either – when requesting a high accuracy position from the device, the device sends the first position with high accuracy (which will often be cached from earlier), and then this code resets the accuracy to 100m again, so all future positional updates will be wildly inaccurate!

My interpretation of this is that it has been added in error. If I am missing something then I'd love to know what it is, because as it stands enableHighAccuracy simply doesn't work for iOS devices in React Native.

[hramos]

Thanks for the explanation! I agree this should be handled in product code.

[facebook-github-bot]

@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[cpojer]

Retrying because we had an import tooling issue.

[react-native-bot]

@omnikron merged commit bbcb97a into facebook:master.