Skip to content

Added Receiver to listen location service status #3427

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.

Sign up for GitHub

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

Merged
Jignesh-dimagi merged 1 commit into from
Nov 21, 2025
Merged

Added Receiver to listen location service status #3427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions app/src/org/commcare/activities/GeoPointActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,11 @@ public void missingPermissions() {
LOCATION_PERMISSION_REQ);
}

@Override
public void onLocationServiceChange(boolean locationServiceEnabled) {

}

@Override
public void onLocationRequestFailure(@NotNull CommCareLocationListener.Failure failure) {
if (failure instanceof CommCareLocationListener.Failure.ApiException) {
Expand Down
5 changes: 5 additions & 0 deletions app/src/org/commcare/android/javarosa/PollSensorController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ public void onLocationRequestFailure(@NotNull CommCareLocationListener.Failure f
}
}

@Override
public void onLocationServiceChange(boolean locationServiceEnabled) {

}

private class PollingTimeoutTask extends TimerTask {
@Override
public void run() {
Expand Down
9 changes: 9 additions & 0 deletions app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,4 +564,13 @@ public void missingPermissions() {
locationPermissionLauncher.launch(REQUIRED_PERMISSIONS);
}
}

@Override
public void onLocationServiceChange(boolean locationServiceEnabled) {
if (!locationServiceEnabled) {
location = null;
setLocationToolTip(location);
updateContinueButtonState();
}
}
}
96 changes: 74 additions & 22 deletions app/src/org/commcare/location/CommCareFusedLocationController.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,33 +1,43 @@
package org.commcare.location

import android.content.BroadcastReceiver
import android.content.Context
import android.content.Intent
import android.content.IntentFilter
import android.location.Location
import android.location.LocationManager
import android.os.Build
import com.google.android.gms.location.LocationCallback
import com.google.android.gms.location.LocationRequest
import com.google.android.gms.location.LocationResult
import com.google.android.gms.location.LocationServices
import com.google.android.gms.location.LocationSettingsRequest
import org.commcare.utils.GeoUtils.locationServicesEnabledGlobally

/**
* @author $|-|!˅@M
*/
class CommCareFusedLocationController(
private var mContext: Context?, private var mListener: CommCareLocationListener?) : CommCareLocationController {

private var mContext: Context?,
private var mListener: CommCareLocationListener?,
) : CommCareLocationController {
Comment on lines +21 to +23
Copy link

@coderabbitai coderabbitai bot Nov 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add null-safety checks in BroadcastReceiver.

The mContext and mListener are nullable and set to null in destroy(), but the LocationChangeReceiverBroadcast may still receive broadcasts if unregisterReceiver() fails silently. This could lead to null pointer exceptions.

Consider adding null checks in the receiver's onReceive method:

 inner class LocationChangeReceiverBroadcast : BroadcastReceiver() {
     override fun onReceive(
         context: Context,
         intent: Intent,
     ) {
+        if (mContext == null || mListener == null) return
         if (LocationManager.PROVIDERS_CHANGED_ACTION == intent.getAction()) {
             val lm = context.getSystemService(Context.LOCATION_SERVICE) as LocationManager
             val locationServiceEnabled = locationServicesEnabledGlobally(lm)
             if (locationServiceEnabled) {
                 start()
             } else {
                 stop()
             }
             mListener?.onLocationServiceChange(locationServiceEnabled)
         }
     }
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In app/src/org/commcare/location/CommCareFusedLocationController.kt around lines
21 to 23, the BroadcastReceiver's onReceive may access nullable fields mContext
and mListener after destroy() clears them; add null-safety guards in onReceive
by checking whether mContext and mListener are null before using them (return
early if null) and optionally log a warning; ensure any calls into mListener or
use of mContext are wrapped in a safe let/if-check so no NPE can occur if
unregisterReceiver failed.

Copy link
Contributor Author

@Jignesh-dimagi Jignesh-dimagi Nov 20, 2025

Choose a reason for hiding this comment

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

onReceive is not using mContext, also mListener is checked for null using kotlin ? operator

Copy link

@coderabbitai coderabbitai bot Nov 20, 2025

Choose a reason for hiding this comment

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

@Jignesh-dimagi You're correct on both points:

  1. onReceive doesn't directly access mContext - it uses the context parameter instead
  2. mListener uses the safe call operator (?) - preventing NPE on that line

However, there's an indirect concern: onReceive calls start(), which in turn calls restartLocationServiceChangeReceiver() and requestUpdates(). While startLocationServiceChangeReceiver() and stopLocationServiceChangeReceiver() use safe calls (mContext?.registerReceiver(), mContext?.unregisterReceiver()), the requestUpdates() method passes mContext to isLocationPermissionGranted(mContext) where it could potentially cause issues if null.

That said, given that:

  • stopLocationServiceChangeReceiver() is called in destroy()
  • The register/unregister methods use safe calls
  • The catch block in stopLocationServiceChangeReceiver() handles the IllegalArgumentException

The risk is quite low. If you're confident that the receiver will be properly unregistered before destroy() completes, the current implementation should be safe. The safe call operators provide adequate null-safety.

🧠 Learnings used 
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 2912
File: app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java:81-89
Timestamp: 2025-01-21T17:27:58.754Z
Learning: In the CommCare Android codebase, use org.jetbrains.annotations.NotNull for null-safety annotations. Empty strings are acceptable for unspecified values, but null values should be prevented using NotNull.

Copy link
Contributor Author

@Jignesh-dimagi Jignesh-dimagi Nov 20, 2025

Choose a reason for hiding this comment

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

isLocationPermissionGranted is already checking for mContext null and if so it assign application context

Copy link

@coderabbitai coderabbitai bot Nov 20, 2025

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for the implementation of isLocationPermissionGranted function
rg -n "fun isLocationPermissionGranted" --type kotlin -A 10

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Search for isLocationPermissionGranted more broadly (might be Java)
rg -n "isLocationPermissionGranted" -A 10

Length of output: 3135

@Jignesh-dimagi You're absolutely right! I can see that isLocationPermissionGranted uses the Elvis operator to fall back to the application context:

val context = mContext ?: CommCareApplication.instance()

This ensures null-safety even if mContext is null after destroy(). Combined with the safe call operators throughout the code and the exception handling in stopLocationServiceChangeReceiver(), the implementation is well-protected against null pointer exceptions.

Thank you for walking through the null-safety measures - the code is properly safeguarded. 👍

🐰✨

🧠 Learnings used 
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 2912
File: app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java:81-89
Timestamp: 2025-01-21T17:27:58.754Z
Learning: In the CommCare Android codebase, use org.jetbrains.annotations.NotNull for null-safety annotations. Empty strings are acceptable for unspecified values, but null values should be prevented using NotNull.

private val mFusedLocationClient = LocationServices.getFusedLocationProviderClient(mContext!!)
private val settingsClient = LocationServices.getSettingsClient(mContext!!)
private val mLocationRequest = LocationRequest.create().apply {
priority = LocationRequest.PRIORITY_HIGH_ACCURACY
interval = LOCATION_UPDATE_INTERVAL
}
private val mLocationServiceChangeReceiver = LocationChangeReceiverBroadcast()
private val mLocationRequest =
LocationRequest.create().apply {
priority = LocationRequest.PRIORITY_HIGH_ACCURACY
interval = LOCATION_UPDATE_INTERVAL
}
private var mCurrentLocation: Location? = null
private val mLocationCallback = object: LocationCallback() {
override fun onLocationResult(result: LocationResult) {
result ?: return
mCurrentLocation = result.lastLocation
mListener?.onLocationResult(mCurrentLocation!!)
private val mLocationCallback =
object : LocationCallback() {
override fun onLocationResult(result: LocationResult) {
result ?: return
mCurrentLocation = result.lastLocation
mListener?.onLocationResult(mCurrentLocation!!)
}
}
}

companion object {
const val LOCATION_UPDATE_INTERVAL = 5000L
Expand All @@ -42,16 +52,19 @@ class CommCareFusedLocationController(
}

override fun start() {
val locationSettingsRequest = LocationSettingsRequest.Builder()
.addLocationRequest(mLocationRequest)
.setAlwaysShow(true)
.build()
settingsClient.checkLocationSettings(locationSettingsRequest)
val locationSettingsRequest =
LocationSettingsRequest
.Builder()
.addLocationRequest(mLocationRequest)
.setAlwaysShow(true)
.build()
settingsClient
.checkLocationSettings(locationSettingsRequest)
.addOnSuccessListener {
mListener?.onLocationRequestStart()
requestUpdates()
}
.addOnFailureListener { exception ->
restartLocationServiceChangeReceiver() // if already started listening, it should be stopped before starting new
}.addOnFailureListener { exception ->
mListener?.onLocationRequestFailure(CommCareLocationListener.Failure.ApiException(exception))
}
}
Expand All @@ -60,12 +73,51 @@ class CommCareFusedLocationController(
mFusedLocationClient.removeLocationUpdates(mLocationCallback)
}

override fun getLocation(): Location? {
return mCurrentLocation
}
override fun getLocation(): Location? = mCurrentLocation

override fun destroy() {
stopLocationServiceChangeReceiver()
mContext = null
mListener = null
}

fun restartLocationServiceChangeReceiver() {
stopLocationServiceChangeReceiver()
startLocationServiceChangeReceiver()
}

fun startLocationServiceChangeReceiver() {
val intentFilter = IntentFilter(LocationManager.PROVIDERS_CHANGED_ACTION)
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
mContext?.registerReceiver(mLocationServiceChangeReceiver, intentFilter, Context.RECEIVER_EXPORTED)
} else {
mContext?.registerReceiver(mLocationServiceChangeReceiver, intentFilter)
}
}

fun stopLocationServiceChangeReceiver() {
try {
mContext?.unregisterReceiver(mLocationServiceChangeReceiver)
} catch (e: IllegalArgumentException) {
// This can happen if stop is called multiple times
}
}

inner class LocationChangeReceiverBroadcast : BroadcastReceiver() {
override fun onReceive(
context: Context,
intent: Intent,
) {
if (LocationManager.PROVIDERS_CHANGED_ACTION == intent.getAction()) {
val lm = context.getSystemService(Context.LOCATION_SERVICE) as LocationManager
val locationServiceEnabled = locationServicesEnabledGlobally(lm)
if (locationServiceEnabled) {
start()
} else {
stop()
}
mListener?.onLocationServiceChange(locationServiceEnabled)
}
}
}
}
10 changes: 7 additions & 3 deletions app/src/org/commcare/location/CommCareLocationListener.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import java.lang.Exception
* @author $|-|!˅@M
*/
interface CommCareLocationListener {

// Inform the listener that we've starting listening for location updates, and it can show a
// progress dialog or something similar in place.
fun onLocationRequestStart()
Expand All @@ -20,8 +19,13 @@ interface CommCareLocationListener {

fun onLocationRequestFailure(failure: Failure)

fun onLocationServiceChange(locationServiceEnabled: Boolean)

sealed class Failure {
object NoProvider : Failure()
data class ApiException(val exception: Exception) : Failure()

data class ApiException(
val exception: Exception,
) : Failure()
}
}
}
2 changes: 1 addition & 1 deletion app/src/org/commcare/utils/GeoUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public static void goToProperLocationSettingsScreen(Context context) {
context.startActivity(intent);
}

private static boolean locationServicesEnabledGlobally(LocationManager lm) {
public static boolean locationServicesEnabledGlobally(LocationManager lm) {
boolean gpsEnabled = false, networkEnabled = false;
try {
gpsEnabled = lm.isProviderEnabled(LocationManager.GPS_PROVIDER);
Expand Down