Skip to content

feat: report unsatisfied location settings - #146

Open
pinpong wants to merge 4 commits into
devfrom
feat/android-location-settings-errors
Open

pinpong wants to merge 4 commits into
devfrom
feat/android-location-settings-errors

Conversation

@pinpong

@pinpong pinpong commented Sep 19, 2026

Copy link
Copy Markdown
Owner

Pull request

Please ensure this PR targets the dev branch and follows the project conventions.
CI already runs linting, formatting, and build checks automatically.


Before submitting

  • This PR targets the dev branch (not main)
  • Commit messages follow the semantic-release format
  • No debug logs or sensitive data included

Summary

Short description of what this PR changes or adds.


Type of change

  • Feature
  • Fix
  • Refactor
  • Internal / CI
  • Documentation

Scope

  • Android
  • iOS
  • JS
  • Example App
  • Docs

Comment thread android/src/main/java/com/rngooglemapsplus/LocationHandler.kt
Comment thread android/src/main/java/com/rngooglemapsplus/LocationHandler.kt Outdated
Comment thread android/src/main/java/com/rngooglemapsplus/LocationHandler.kt Outdated
@pinpong pinpong changed the title feat(android): report unsatisfied location settings feat: report unsatisfied location settings Sep 19, 2026
@pinpong

pinpong commented Sep 19, 2026

Copy link
Copy Markdown
Owner Author

@lorenc-tomasz thanks for the review.

@pinpong

pinpong commented Sep 19, 2026

Copy link
Copy Markdown
Owner Author

Reworked the android part and found an initialization issue that's solved within this PR as well.

.getSettingsClient(context)
.checkLocationSettings(settingsRequest)
.addOnFailureListener { ex ->
if (callback !== locationCallback) return@addOnFailureListener

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice :) Using the callback to identify the session fixes the stop / start and configuration change cases. But I think that there is still a race when two checks run in the same session, though.

For example, start() begins one check and onLocationAvailability(false) begins another. The newer check succeeds, then the older one fails. Both have the same callback, so we still send the old error to JS even though the latest check passed.

Could we give each check its own ID as well?

private var settingsCheckId = 0L

private fun checkLocationSettings(callback: LocationCallback) {
  if (!isActive || callback !== locationCallback) return
  val checkId = ++settingsCheckId

  if (!LocationManagerCompat.isLocationEnabled(locationManager)) {
    onError?.invoke(RNLocationErrorCode.SETTINGS_NOT_SATISFIED)
    return
  }
  val request = locationRequest ?: return
  val settingsRequest =
    LocationSettingsRequest
      .Builder()
      .addLocationRequest(request)
      .build()

  LocationServices
    .getSettingsClient(context)
    .checkLocationSettings(settingsRequest)
    .addOnFailureListener { ex ->
      if (!isActive || callback !== locationCallback || checkId != settingsCheckId) {
        return@addOnFailureListener
      }
      onError?.invoke(ex.toLocationErrorCode(context))
    }
}

This keeps your session check and ignores results from an older check in that session. The counter belongs to each handler, so stopping or updating one map won't affect another map.

What do you think?

}
}

override fun onLocationAvailability(availability: LocationAvailability) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It's rather edge case but I'm wondering what will happen if location availability is already false when the user switches location off?

For example, the device can't get a position, but its settings are still valid. The user then disables location. I'm think that availability could remain false and we might not receive another callback to trigger settings check.

The documentation for onLocationAvailability says it reports changes in location data availability and describes LocationAvailability as an estimate that may be inaccurate. It doesn't promise a callback for every settings change as Clients should treat LocationAvailability as a best guess that is not necessarily accurate and should not be relied upon.. I didn't test it on device so it's just my concern based on that docs.

Based on that, could we also listen for location mode/provider changes while this handler is active? We can keep the session ownership you added and use the check ID from my other comment.

import android.content.BroadcastReceiver
import android.content.IntentFilter
import androidx.core.content.ContextCompat

private var locationSettingsReceiver: BroadcastReceiver? = null

private fun registerLocationSettingsReceiver(callback: LocationCallback) {
  if (locationSettingsReceiver != null) return
  val receiver =
    object : BroadcastReceiver() {
      override fun onReceive(context: Context?, intent: Intent?) {
        when (intent?.action) {
          LocationManager.MODE_CHANGED_ACTION,
          LocationManager.PROVIDERS_CHANGED_ACTION,
          -> checkLocationSettings(callback)
        }
      }
    }
  val filter =
    IntentFilter().apply {
      addAction(LocationManager.MODE_CHANGED_ACTION)
      addAction(LocationManager.PROVIDERS_CHANGED_ACTION)
    }
  ContextCompat.registerReceiver(
    context,
    receiver,
    filter,
    ContextCompat.RECEIVER_NOT_EXPORTED,
  )
  locationSettingsReceiver = receiver
}

private fun unregisterLocationSettingsReceiver() {
  val receiver = locationSettingsReceiver ?: return
  locationSettingsReceiver = null
  context.unregisterReceiver(receiver)
}

In start():

isActive = true
registerLocationSettingsReceiver(callback)
checkLocationSettings(callback)

Then stop():

fun stop() {
  if (!isActive) return
  isActive = false
  unregisterLocationSettingsReceiver()
  val callback = locationCallback ?: return
  fusedLocationClientProviderClient.removeLocationUpdates(callback)
  fusedLocationClientProviderClient.flushLocations()
  locationCallback = null
}

We can keep onLocationAvailability as another reason to check. Each map registers and removes its own receiver, so stopping one map doesn't stop the others from receiving settings changes.

What do you think?

Comment thread ios/LocationHandler.swift

func start() {
guard !isActive else { return }
guard manager.authorizationStatus != .denied || CLLocationManager.locationServicesEnabled() else {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This handles location services being off when the map starts. Could we also cover the user switching them off while the map is already active?

I reproduced this on the iPadOS 26.5. With Settings and the app in separate windows side by side, switching off Location Services did not send didEnterBackground. The handler reported PERMISSION_DENIED, even though locationServicesEnabled() was false.

We don't currently handle the authorization change callback, and didFailWithError turns every .denied error into permissionDenied. The check in start() doesn't help here because the location session is already active.

The example code below uses the same distinction in both callbacks. It also remembers the last authorization error so we don't report it twice, and resumes updates if access returns while the map is still active.

// isActive now means that the view wants location updates, even while
// settings or permissions temporarily prevent CLLocationManager from running.
private var lastAuthorizationError: RNLocationErrorCode?

private func currentAuthorizationError() -> RNLocationErrorCode? {
  switch manager.authorizationStatus {
  case .denied:
    return CLLocationManager.locationServicesEnabled()
      ? .permissionDenied : .settingsNotSatisfied
  case .restricted:
    return .permissionDenied
  default:
    return nil
  }
}

private func reportAuthorizationError(_ code: RNLocationErrorCode) {
  manager.stopUpdatingLocation()
  guard lastAuthorizationError != code else { return }
  lastAuthorizationError = code
  onError?(code)
}

func start() {
  guard !isActive else { return }
  isActive = true

  if let code = currentAuthorizationError() {
    reportAuthorizationError(code)
    return
  }

  manager.requestLocation()
  manager.startUpdatingLocation()
}

func stop() {
  guard isActive else { return }
  isActive = false
  lastAuthorizationError = nil
  manager.stopUpdatingLocation()
}

func locationManagerDidChangeAuthorization(_ manager: CLLocationManager) {
  guard isActive else { return }

  if let code = currentAuthorizationError() {
    reportAuthorizationError(code)
    return
  }

  // Resume only if this view still wants updates and an authorization error
  // previously stopped them. Ignore the ordinary initial delegate callback.
  guard lastAuthorizationError != nil else { return }
  lastAuthorizationError = nil
  manager.requestLocation()
  manager.startUpdatingLocation()
}

func locationManager(
  _ manager: CLLocationManager,
  didFailWithError error: Error
) {
  guard isActive else { return }
  guard let clError = error as? CLError else {
    onError?(.internalError)
    return
  }

  if clError.code == .denied {
    let code: RNLocationErrorCode = CLLocationManager.locationServicesEnabled()
      ? .permissionDenied : .settingsNotSatisfied
    reportAuthorizationError(code)
    return
  }

  onError?(clError.code.toRNLocationErrorCode)
}

Here, isActive means the view still wants location updates, even while settings temporarily prevent them. stop() clears that state, so a detached map won't restart when permissions change.

What do you think?

@lorenc-tomasz

Copy link
Copy Markdown
Collaborator

@lorenc-tomasz thanks for the review.

I’m always happy to help if you feel it would be helpful :)

Reworked the android part and found an initialization issue that's solved within this PR as well.

Nice :)

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