Fix and Improve LocationManager logic (#599)

* location manager improvements

* fix: resolve compilation errors and improve location state handling

- Fix missing imports and stray braces in LocationChannelManager
- Implement checkSystemLocationEnabled and BroadcastReceiver for location state
- Order class members for correct initialization
- Consolidate UI refresh logic in LocationChannelsSheet

* fix: guard against stale geocoding results on legacy devices

- Add isActive check after blocking Geocoder.getFromLocation call
- Prevent stale results from overwriting state after job cancellation
This commit is contained in:
callebtc
2026-01-15 11:11:56 +07:00
committed by GitHub
parent e769a03634
commit 5ec02c99fd
4 changed files with 191 additions and 169 deletions
@@ -36,7 +36,18 @@ data class GeohashChannel(
*/
sealed class ChannelID {
object Mesh : ChannelID()
data class Location(val channel: GeohashChannel) : ChannelID()
data class Location(val channel: GeohashChannel) : ChannelID() {
companion object {
fun fromPersisted(levelName: String, geohash: String): Location? {
return try {
val level = GeohashChannelLevel.valueOf(levelName)
Location(GeohashChannel(level, geohash))
} catch (_: IllegalArgumentException) {
null
}
}
}
}
/**
* Human readable name for UI.
@@ -2,6 +2,7 @@ package com.bitchat.android.geohash
import android.Manifest
import android.content.Context
import android.content.IntentFilter
import android.content.pm.PackageManager
import android.location.Geocoder
import android.location.Location
@@ -15,7 +16,10 @@ import java.util.*
import com.google.gson.Gson
import com.google.gson.JsonSyntaxException
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.stateIn
/**
* Manages location permissions, one-shot location retrieval, and computing geohash channels.
@@ -38,22 +42,40 @@ class LocationChannelManager private constructor(private val context: Context) {
// State enum matching iOS
enum class PermissionState {
NOT_DETERMINED,
DENIED,
RESTRICTED,
AUTHORIZED
}
private val locationManager: LocationManager = context.getSystemService(Context.LOCATION_SERVICE) as LocationManager
private val geocoder: Geocoder = Geocoder(context, Locale.getDefault())
private var lastLocation: Location? = null
private var refreshTimer: Job? = null
private var isGeocoding: Boolean = false
private var geocodingJob: Job? = null
private val gson = Gson()
private var dataManager: com.bitchat.android.ui.DataManager? = null
private fun checkSystemLocationEnabled(): Boolean {
return try {
locationManager.isProviderEnabled(LocationManager.GPS_PROVIDER) ||
locationManager.isProviderEnabled(LocationManager.NETWORK_PROVIDER)
} catch (_: Exception) {
false
}
}
private val locationStateReceiver = object : android.content.BroadcastReceiver() {
override fun onReceive(context: Context?, intent: android.content.Intent?) {
if (intent?.action == LocationManager.PROVIDERS_CHANGED_ACTION) {
val isEnabled = checkSystemLocationEnabled()
Log.d(TAG, "System location state changed: $isEnabled")
_systemLocationEnabled.value = isEnabled
}
}
}
// Published state for UI bindings (matching iOS @Published properties)
private val _permissionState = MutableStateFlow(PermissionState.NOT_DETERMINED)
private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate)
private val _permissionState = MutableStateFlow(PermissionState.DENIED)
val permissionState: StateFlow<PermissionState> = _permissionState
private val _availableChannels = MutableStateFlow<List<GeohashChannel>>(emptyList())
@@ -74,12 +96,30 @@ class LocationChannelManager private constructor(private val context: Context) {
private val _locationServicesEnabled = MutableStateFlow(false)
val locationServicesEnabled: StateFlow<Boolean> = _locationServicesEnabled
private val _systemLocationEnabled = MutableStateFlow(checkSystemLocationEnabled())
val systemLocationEnabled: StateFlow<Boolean> = _systemLocationEnabled
val effectiveLocationEnabled: StateFlow<Boolean> = combine(
locationServicesEnabled,
systemLocationEnabled
) { appToggle, systemToggle ->
appToggle && systemToggle
}.stateIn(
scope,
SharingStarted.Eagerly,
false
)
init {
updatePermissionState()
// Initialize DataManager and load persisted settings
dataManager = com.bitchat.android.ui.DataManager(context)
loadPersistedChannelSelection()
loadLocationServicesState()
// Register for system location changes
context.registerReceiver(locationStateReceiver, IntentFilter(LocationManager.PROVIDERS_CHANGED_ACTION))
}
// MARK: - Public API (matching iOS interface)
@@ -91,26 +131,19 @@ class LocationChannelManager private constructor(private val context: Context) {
fun enableLocationChannels() {
Log.d(TAG, "enableLocationChannels() called")
// UNIFIED FIX: Check if location services are enabled by user
if (!isLocationServicesEnabled()) {
Log.w(TAG, "Location services disabled by user - not requesting location")
if (!_locationServicesEnabled.value || !_systemLocationEnabled.value) {
Log.w(TAG, "Location services disabled (app or system) - not requesting location")
return
}
when (getCurrentPermissionStatus()) {
PermissionState.NOT_DETERMINED -> {
Log.d(TAG, "Permission not determined - user needs to grant in app settings")
_permissionState.value = PermissionState.NOT_DETERMINED
}
PermissionState.DENIED, PermissionState.RESTRICTED -> {
Log.d(TAG, "Permission denied or restricted")
_permissionState.value = PermissionState.DENIED
}
PermissionState.AUTHORIZED -> {
Log.d(TAG, "Permission authorized - requesting location")
_permissionState.value = PermissionState.AUTHORIZED
requestOneShotLocation()
}
if (getCurrentPermissionStatus() == PermissionState.AUTHORIZED) {
Log.d(TAG, "Permission authorized - requesting location")
_permissionState.value = PermissionState.AUTHORIZED
requestOneShotLocation()
} else {
Log.d(TAG, "Permission not granted")
_permissionState.value = PermissionState.DENIED
}
}
@@ -136,14 +169,10 @@ class LocationChannelManager private constructor(private val context: Context) {
}
if (!isLocationServicesEnabled()) {
Log.w(TAG, "Cannot start live refresh - location services disabled by user")
Log.w(TAG, "Cannot start live refresh - location services disabled")
return
}
// Cancel any existing timer-based refreshers
refreshTimer?.cancel()
refreshTimer = null
// Register for continuous updates from available providers
try {
if (hasLocationPermission()) {
@@ -180,8 +209,6 @@ class LocationChannelManager private constructor(private val context: Context) {
*/
fun endLiveRefresh() {
Log.d(TAG, "Ending live refresh")
refreshTimer?.cancel()
refreshTimer = null
// Unregister continuous updates listener
try {
locationManager.removeUpdates(continuousLocationListener)
@@ -235,8 +262,8 @@ class LocationChannelManager private constructor(private val context: Context) {
_locationServicesEnabled.value = true
saveLocationServicesState(true)
// If we have permission, start location operations
if (_permissionState.value == PermissionState.AUTHORIZED) {
// If we have permission and system location is on, start location operations
if (_permissionState.value == PermissionState.AUTHORIZED && systemLocationEnabled.value) {
requestOneShotLocation()
}
}
@@ -265,8 +292,11 @@ class LocationChannelManager private constructor(private val context: Context) {
/**
* Check if location services are enabled by the user
*/
/**
* Check if both the app toggle and system location are enabled
*/
fun isLocationServicesEnabled(): Boolean {
return _locationServicesEnabled.value
return _locationServicesEnabled.value && _systemLocationEnabled.value
}
// MARK: - Location Operations
@@ -451,16 +481,10 @@ class LocationChannelManager private constructor(private val context: Context) {
// MARK: - Helpers
private fun getCurrentPermissionStatus(): PermissionState {
return when {
ActivityCompat.checkSelfPermission(context, Manifest.permission.ACCESS_FINE_LOCATION) == PackageManager.PERMISSION_GRANTED -> {
PermissionState.AUTHORIZED
}
ActivityCompat.checkSelfPermission(context, Manifest.permission.ACCESS_COARSE_LOCATION) == PackageManager.PERMISSION_GRANTED -> {
PermissionState.AUTHORIZED
}
else -> {
PermissionState.DENIED // In Android, we can't distinguish between denied and not determined after first ask
}
return if (hasLocationPermission()) {
PermissionState.AUTHORIZED
} else {
PermissionState.DENIED
}
}
@@ -519,33 +543,70 @@ class LocationChannelManager private constructor(private val context: Context) {
return
}
if (isGeocoding) {
Log.d(TAG, "Already geocoding, skipping")
return
}
// Cancel any pending geocoding job to avoid race conditions
geocodingJob?.cancel()
isGeocoding = true
CoroutineScope(Dispatchers.IO).launch {
geocodingJob = scope.launch(Dispatchers.IO) {
try {
Log.d(TAG, "Starting reverse geocoding")
@Suppress("DEPRECATION")
val addresses = geocoder.getFromLocation(location.latitude, location.longitude, 1)
if (!addresses.isNullOrEmpty()) {
val address = addresses[0]
val names = namesByLevel(address)
Log.d(TAG, "Reverse geocoding result: $names")
_locationNames.value = names
if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.TIRAMISU) {
// Use new listener-based API for Android 13+
suspendCancellableCoroutine<Unit> { cont ->
try {
geocoder.getFromLocation(
location.latitude,
location.longitude,
1,
object : Geocoder.GeocodeListener {
override fun onGeocode(addresses: MutableList<android.location.Address>) {
if (!cont.isActive) return
if (addresses.isNotEmpty()) {
val address = addresses[0]
val names = namesByLevel(address)
Log.d(TAG, "Reverse geocoding result: $names")
_locationNames.value = names
} else {
Log.w(TAG, "No reverse geocoding results")
}
cont.resume(Unit) {}
}
override fun onError(errorMessage: String?) {
if (!cont.isActive) return
Log.e(TAG, "Reverse geocoding failed: $errorMessage")
cont.resume(Unit) {}
}
}
)
} catch (e: Exception) {
if (!cont.isActive) return@suspendCancellableCoroutine
Log.e(TAG, "Error initiating geocoding listener: ${e.message}")
cont.resume(Unit) {}
}
cont.invokeOnCancellation {
// Listener-based API has no explicit unregister, so we just ignore callbacks
}
}
} else {
Log.w(TAG, "No reverse geocoding results")
@Suppress("DEPRECATION")
val addresses = geocoder.getFromLocation(location.latitude, location.longitude, 1)
if (!isActive) return@launch
if (!addresses.isNullOrEmpty()) {
val address = addresses[0]
val names = namesByLevel(address)
Log.d(TAG, "Reverse geocoding result: $names")
_locationNames.value = names
} else {
Log.w(TAG, "No reverse geocoding results")
}
}
} catch (e: Exception) {
Log.e(TAG, "Reverse geocoding failed: ${e.message}")
} finally {
isGeocoding = false
if (e !is CancellationException) {
Log.e(TAG, "Reverse geocoding failed: ${e.message}")
}
}
}
}
@@ -599,18 +660,14 @@ class LocationChannelManager private constructor(private val context: Context) {
private fun saveChannelSelection(channel: ChannelID) {
try {
val channelData = when (channel) {
is ChannelID.Mesh -> {
gson.toJson(mapOf("type" to "mesh"))
}
is ChannelID.Location -> {
gson.toJson(mapOf(
"type" to "location",
"level" to channel.channel.level.name,
"precision" to channel.channel.level.precision,
"geohash" to channel.channel.geohash,
"displayName" to channel.channel.level.displayName
))
}
is ChannelID.Mesh -> gson.toJson(PersistedChannel(mesh = true))
is ChannelID.Location -> gson.toJson(
PersistedChannel(
mesh = false,
level = channel.channel.level.name,
geohash = channel.channel.geohash
)
)
}
dataManager?.saveLastGeohashChannel(channelData)
Log.d(TAG, "Saved channel selection: ${channel.displayName}")
@@ -625,46 +682,14 @@ class LocationChannelManager private constructor(private val context: Context) {
private fun loadPersistedChannelSelection() {
try {
val channelData = dataManager?.loadLastGeohashChannel()
if (channelData != null) {
val channelMap = gson.fromJson(channelData, Map::class.java) as? Map<String, Any>
if (channelMap != null) {
val channel = when (channelMap["type"] as? String) {
"mesh" -> ChannelID.Mesh
"location" -> {
val levelName = channelMap["level"] as? String
val precision = (channelMap["precision"] as? Double)?.toInt()
val geohash = channelMap["geohash"] as? String
val displayName = channelMap["displayName"] as? String
if (levelName != null && precision != null && geohash != null && displayName != null) {
try {
val level = GeohashChannelLevel.valueOf(levelName)
val geohashChannel = GeohashChannel(level, geohash)
ChannelID.Location(geohashChannel)
} catch (e: IllegalArgumentException) {
Log.w(TAG, "Invalid geohash level in persisted data: $levelName")
null
}
} else {
Log.w(TAG, "Incomplete location channel data in persistence")
null
}
}
else -> {
Log.w(TAG, "Unknown channel type in persisted data: ${channelMap["type"]}")
null
}
}
if (channel != null) {
_selectedChannel.value = channel
Log.d(TAG, "Restored persisted channel: ${channel.displayName}")
} else {
Log.d(TAG, "Could not restore persisted channel, defaulting to Mesh")
_selectedChannel.value = ChannelID.Mesh
}
if (!channelData.isNullOrBlank()) {
val persisted = gson.fromJson(channelData, PersistedChannel::class.java)
val channel = persisted?.toChannel()
if (channel != null) {
_selectedChannel.value = channel
Log.d(TAG, "Restored persisted channel: ${channel.displayName}")
} else {
Log.w(TAG, "Invalid channel data format in persistence")
Log.d(TAG, "Could not restore persisted channel, defaulting to Mesh")
_selectedChannel.value = ChannelID.Mesh
}
} else {
@@ -679,7 +704,23 @@ class LocationChannelManager private constructor(private val context: Context) {
_selectedChannel.value = ChannelID.Mesh
}
}
data class PersistedChannel(
val mesh: Boolean,
val level: String? = null,
val geohash: String? = null
) {
fun toChannel(): ChannelID? {
return if (mesh) {
ChannelID.Mesh
} else {
val levelName = level ?: return null
val gh = geohash ?: return null
ChannelID.Location.fromPersisted(levelName, gh)
}
}
}
/**
* Clear persisted channel selection (useful for testing or reset)
*/
@@ -724,6 +765,12 @@ class LocationChannelManager private constructor(private val context: Context) {
Log.d(TAG, "Cleaning up LocationChannelManager")
endLiveRefresh()
geocodingJob?.cancel()
geocodingJob = null
// Unregister receiver
try { context.unregisterReceiver(locationStateReceiver) } catch (_: Exception) {}
// Remove listeners to prevent memory leaks
try { locationManager.removeUpdates(oneShotLocationListener) } catch (_: Exception) {}
try { locationManager.removeUpdates(continuousLocationListener) } catch (_: Exception) {}
@@ -63,7 +63,9 @@ fun LocationChannelsSheet(
val availableChannels by locationManager.availableChannels.collectAsStateWithLifecycle()
val selectedChannel by locationManager.selectedChannel.collectAsStateWithLifecycle()
val locationNames by locationManager.locationNames.collectAsStateWithLifecycle()
val locationServicesEnabled by locationManager.locationServicesEnabled.collectAsStateWithLifecycle()
val appLocationEnabled by locationManager.locationServicesEnabled.collectAsStateWithLifecycle()
val systemLocationEnabled by locationManager.systemLocationEnabled.collectAsStateWithLifecycle()
val locationServicesEnabled by locationManager.effectiveLocationEnabled.collectAsStateWithLifecycle()
// Observe bookmarks state
val bookmarks by bookmarksStore.bookmarks.collectAsStateWithLifecycle()
@@ -162,24 +164,7 @@ fun LocationChannelsSheet(
verticalArrangement = Arrangement.spacedBy(4.dp)
) {
when (permissionState) {
LocationChannelManager.PermissionState.NOT_DETERMINED -> {
Button(
onClick = { locationManager.enableLocationChannels() },
colors = ButtonDefaults.buttonColors(
containerColor = standardGreen.copy(alpha = 0.12f),
contentColor = standardGreen
),
modifier = Modifier.fillMaxWidth()
) {
Text(
text = stringResource(R.string.grant_location_permission),
fontSize = 12.sp,
fontFamily = FontFamily.Monospace
)
}
}
LocationChannelManager.PermissionState.DENIED,
LocationChannelManager.PermissionState.RESTRICTED -> {
LocationChannelManager.PermissionState.DENIED -> {
Column(verticalArrangement = Arrangement.spacedBy(4.dp)) {
Text(
text = stringResource(R.string.location_permission_denied),
@@ -211,20 +196,6 @@ fun LocationChannelsSheet(
color = standardGreen
)
}
null -> {
Row(
horizontalArrangement = Arrangement.spacedBy(4.dp),
verticalAlignment = Alignment.CenterVertically
) {
CircularProgressIndicator(modifier = Modifier.size(12.dp))
Text(
text = stringResource(R.string.checking_permissions),
fontSize = 11.sp,
fontFamily = FontFamily.Monospace,
color = MaterialTheme.colorScheme.onSurface.copy(alpha = 0.7f)
)
}
}
}
}
}
@@ -562,34 +533,27 @@ fun LocationChannelsSheet(
}
}
// Lifecycle management: when presented, sample both nearby and bookmarked geohashes
// Lifecycle management: when presented, manage location updates
DisposableEffect(isPresented, permissionState, locationServicesEnabled) {
if (isPresented && permissionState == LocationChannelManager.PermissionState.AUTHORIZED && locationServicesEnabled) {
locationManager.refreshChannels()
locationManager.beginLiveRefresh()
}
onDispose {
locationManager.endLiveRefresh()
}
}
// Sampling management: update sampling when channels/bookmarks change
LaunchedEffect(isPresented, availableChannels, bookmarks) {
if (isPresented) {
if (permissionState == LocationChannelManager.PermissionState.AUTHORIZED && locationServicesEnabled) {
locationManager.refreshChannels()
locationManager.beginLiveRefresh()
}
val geohashes = (availableChannels.map { it.geohash } + bookmarks).toSet().toList()
viewModel.beginGeohashSampling(geohashes)
} else {
locationManager.endLiveRefresh()
viewModel.endGeohashSampling()
}
}
// React to permission changes
LaunchedEffect(permissionState) {
if (permissionState == LocationChannelManager.PermissionState.AUTHORIZED && locationServicesEnabled) {
locationManager.refreshChannels()
}
}
// React to location services enable/disable
LaunchedEffect(locationServicesEnabled) {
if (locationServicesEnabled && permissionState == LocationChannelManager.PermissionState.AUTHORIZED) {
locationManager.refreshChannels()
}
}
}
@Composable
@@ -38,7 +38,7 @@ fun LocationNotesButton(
val selectedLocationChannel by viewModel.selectedLocationChannel.collectAsStateWithLifecycle()
val locationManager = remember { LocationChannelManager.getInstance(context) }
val permissionState by locationManager.permissionState.collectAsStateWithLifecycle()
val locationServicesEnabled by locationManager.locationServicesEnabled.collectAsStateWithLifecycle(false)
val locationServicesEnabled by locationManager.effectiveLocationEnabled.collectAsStateWithLifecycle(false)
// Check both permission AND location services enabled
val locationPermissionGranted = permissionState == LocationChannelManager.PermissionState.AUTHORIZED