From 1d30367ddc82e072c72a387a7d95de7a41518394 Mon Sep 17 00:00:00 2001 From: Phil Oliver <3497406+poliver@users.noreply.github.com> Date: Wed, 17 Sep 2025 11:10:43 -0400 Subject: [PATCH] Decouple `ConnectionsScreen` from `UiViewModel` (#3126) --- .../geeksville/mesh/android/prefs/UiPrefs.kt | 37 ++++++++-- .../java/com/geeksville/mesh/model/UIState.kt | 39 ++++------- .../mesh/navigation/ConnectionsNavigation.kt | 8 +-- .../geeksville/mesh/service/MeshService.kt | 22 ++++++ .../main/java/com/geeksville/mesh/ui/Main.kt | 2 +- .../mesh/ui/connections/ConnectionsScreen.kt | 41 +++++------- .../ui/connections/ConnectionsViewModel.kt | 67 +++++++++++++++++++ 7 files changed, 153 insertions(+), 63 deletions(-) create mode 100644 app/src/main/java/com/geeksville/mesh/ui/connections/ConnectionsViewModel.kt diff --git a/app/src/main/java/com/geeksville/mesh/android/prefs/UiPrefs.kt b/app/src/main/java/com/geeksville/mesh/android/prefs/UiPrefs.kt index 74e81c183..de00c88e1 100644 --- a/app/src/main/java/com/geeksville/mesh/android/prefs/UiPrefs.kt +++ b/app/src/main/java/com/geeksville/mesh/android/prefs/UiPrefs.kt @@ -22,6 +22,10 @@ import androidx.appcompat.app.AppCompatDelegate import androidx.core.content.edit import com.geeksville.mesh.model.NodeSortOption import com.geeksville.mesh.util.LanguageUtils +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import java.util.concurrent.ConcurrentHashMap interface UiPrefs { var lang: String @@ -36,12 +40,31 @@ interface UiPrefs { var showIgnored: Boolean var showQuickChat: Boolean - fun shouldProvideNodeLocation(nodeNum: Int?): Boolean + fun shouldProvideNodeLocation(nodeNum: Int): StateFlow - fun setShouldProvideNodeLocation(nodeNum: Int?, value: Boolean) + fun setShouldProvideNodeLocation(nodeNum: Int, value: Boolean) } class UiPrefsImpl(private val prefs: SharedPreferences) : UiPrefs { + + // Maps nodeNum to a flow for the for the "provide-location-nodeNum" pref + private val provideNodeLocationFlows = ConcurrentHashMap>() + + private val sharedPreferencesListener = + SharedPreferences.OnSharedPreferenceChangeListener { sharedPreferences, key -> + // Check if the changed key is one of our node location keys + provideNodeLocationFlows.keys.forEach { nodeNum -> + if (key == provideLocationKey(nodeNum)) { + val newValue = sharedPreferences.getBoolean(key, false) + provideNodeLocationFlows[nodeNum]?.tryEmit(newValue) + } + } + } + + init { + prefs.registerOnSharedPreferenceChangeListener(sharedPreferencesListener) + } + override var lang: String by PrefDelegate(prefs, "lang", LanguageUtils.SYSTEM_DEFAULT) override var theme: Int by PrefDelegate(prefs, "theme", AppCompatDelegate.MODE_NIGHT_FOLLOW_SYSTEM) override var appIntroCompleted: Boolean by PrefDelegate(prefs, "app_intro_completed", false) @@ -54,12 +77,14 @@ class UiPrefsImpl(private val prefs: SharedPreferences) : UiPrefs { override var showIgnored: Boolean by PrefDelegate(prefs, "show-ignored", false) override var showQuickChat: Boolean by PrefDelegate(prefs, "show-quick-chat", false) - override fun shouldProvideNodeLocation(nodeNum: Int?): Boolean = - prefs.getBoolean(provideLocationKey(nodeNum), false) + override fun shouldProvideNodeLocation(nodeNum: Int): StateFlow = provideNodeLocationFlows + .getOrPut(nodeNum) { MutableStateFlow(prefs.getBoolean(provideLocationKey(nodeNum), false)) } + .asStateFlow() - override fun setShouldProvideNodeLocation(nodeNum: Int?, value: Boolean) { + override fun setShouldProvideNodeLocation(nodeNum: Int, value: Boolean) { prefs.edit { putBoolean(provideLocationKey(nodeNum), value) } + provideNodeLocationFlows[nodeNum]?.tryEmit(value) } - private fun provideLocationKey(nodeNum: Int?) = "provide-location-$nodeNum" + private fun provideLocationKey(nodeNum: Int) = "provide-location-$nodeNum" } diff --git a/app/src/main/java/com/geeksville/mesh/model/UIState.kt b/app/src/main/java/com/geeksville/mesh/model/UIState.kt index 07e96951f..20c77ef3e 100644 --- a/app/src/main/java/com/geeksville/mesh/model/UIState.kt +++ b/app/src/main/java/com/geeksville/mesh/model/UIState.kt @@ -79,6 +79,7 @@ import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapNotNull @@ -333,15 +334,6 @@ constructor( private val _showQuickChat = MutableStateFlow(uiPrefs.showQuickChat) val showQuickChat: StateFlow = _showQuickChat - private val _hasShownNotPairedWarning = MutableStateFlow(uiPrefs.hasShownNotPairedWarning) - - val hasShownNotPairedWarning: StateFlow = _hasShownNotPairedWarning.asStateFlow() - - fun suppressNoPairedWarning() { - _hasShownNotPairedWarning.value = true - uiPrefs.hasShownNotPairedWarning = true - } - fun toggleShowIgnored() = toggle(_showIgnored) { uiPrefs.showIgnored = it } fun toggleShowQuickChat() = toggle(_showQuickChat) { uiPrefs.showQuickChat = it } @@ -818,26 +810,21 @@ constructor( if (config.lora != newConfig.lora) setConfig(newConfig) } - fun refreshProvideLocation() { - viewModelScope.launch { setProvideLocation(getProvidePref()) } - } - - private fun getProvidePref(): Boolean = uiPrefs.shouldProvideNodeLocation(myNodeNum) - - private val _provideLocation = MutableStateFlow(getProvidePref()) val provideLocation: StateFlow - get() = _provideLocation.asStateFlow() + get() = + myNodeInfo + .flatMapLatest { myNodeEntity -> + // When myNodeInfo changes, set up emissions for the "provide-location-nodeNum" pref. + if (myNodeEntity == null) { + flowOf(false) + } else { + uiPrefs.shouldProvideNodeLocation(myNodeEntity.myNodeNum) + } + } + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), false) fun setProvideLocation(value: Boolean) { - viewModelScope.launch { - uiPrefs.setShouldProvideNodeLocation(myNodeNum, value) - _provideLocation.value = value - if (value) { - meshService?.startProvideLocation() - } else { - meshService?.stopProvideLocation() - } - } + myNodeNum?.let { uiPrefs.setShouldProvideNodeLocation(it, value) } } fun setOwner(name: String) { diff --git a/app/src/main/java/com/geeksville/mesh/navigation/ConnectionsNavigation.kt b/app/src/main/java/com/geeksville/mesh/navigation/ConnectionsNavigation.kt index be5e81c32..65d69cba7 100644 --- a/app/src/main/java/com/geeksville/mesh/navigation/ConnectionsNavigation.kt +++ b/app/src/main/java/com/geeksville/mesh/navigation/ConnectionsNavigation.kt @@ -25,16 +25,11 @@ import androidx.navigation.compose.composable import androidx.navigation.navDeepLink import androidx.navigation.navigation import com.geeksville.mesh.model.BluetoothViewModel -import com.geeksville.mesh.model.UIViewModel import com.geeksville.mesh.ui.connections.ConnectionsScreen import com.geeksville.mesh.ui.settings.radio.components.LoRaConfigScreen /** Navigation graph for for the top level ConnectionsScreen - [ConnectionsRoutes.Connections]. */ -fun NavGraphBuilder.connectionsGraph( - navController: NavHostController, - uiViewModel: UIViewModel, - bluetoothViewModel: BluetoothViewModel, -) { +fun NavGraphBuilder.connectionsGraph(navController: NavHostController, bluetoothViewModel: BluetoothViewModel) { @Suppress("ktlint:standard:max-line-length") navigation(startDestination = ConnectionsRoutes.Connections) { composable( @@ -45,7 +40,6 @@ fun NavGraphBuilder.connectionsGraph( val parentEntry = remember(backStackEntry) { navController.getBackStackEntry(ConnectionsRoutes.ConnectionsGraph) } ConnectionsScreen( - uiViewModel = uiViewModel, bluetoothViewModel = bluetoothViewModel, radioConfigViewModel = hiltViewModel(parentEntry), onNavigateToSettings = { navController.navigate(SettingsRoutes.Settings()) }, diff --git a/app/src/main/java/com/geeksville/mesh/service/MeshService.kt b/app/src/main/java/com/geeksville/mesh/service/MeshService.kt index 341835539..b5bcb3f40 100644 --- a/app/src/main/java/com/geeksville/mesh/service/MeshService.kt +++ b/app/src/main/java/com/geeksville/mesh/service/MeshService.kt @@ -60,6 +60,7 @@ import com.geeksville.mesh.android.GeeksvilleApplication import com.geeksville.mesh.android.Logging import com.geeksville.mesh.android.hasLocationPermission import com.geeksville.mesh.android.prefs.MeshPrefs +import com.geeksville.mesh.android.prefs.UiPrefs import com.geeksville.mesh.concurrent.handledLaunch import com.geeksville.mesh.copy import com.geeksville.mesh.database.MeshLogRepository @@ -99,6 +100,8 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.catch +import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import java.util.Random @@ -147,6 +150,8 @@ class MeshService : @Inject lateinit var meshPrefs: MeshPrefs + @Inject lateinit var uiPrefs: UiPrefs + private val tracerouteStartTimes = ConcurrentHashMap() companion object : Logging { @@ -334,6 +339,23 @@ class MeshService : radioConfigRepository.moduleConfigFlow.onEach { moduleConfig = it }.launchIn(serviceScope) radioConfigRepository.channelSetFlow.onEach { channelSet = it }.launchIn(serviceScope) radioConfigRepository.serviceAction.onEach(::onServiceAction).launchIn(serviceScope) + radioConfigRepository.myNodeInfo + .flatMapLatest { myNodeEntity -> + // When myNodeInfo changes, set up emissions for the "provide-location-nodeNum" pref. + if (myNodeEntity == null) { + flowOf(false) + } else { + uiPrefs.shouldProvideNodeLocation(myNodeEntity.myNodeNum) + } + } + .onEach { shouldProvideNodeLocation -> + if (shouldProvideNodeLocation) { + startLocationRequests() + } else { + stopLocationRequests() + } + } + .launchIn(serviceScope) loadSettings() // Load our last known node DB diff --git a/app/src/main/java/com/geeksville/mesh/ui/Main.kt b/app/src/main/java/com/geeksville/mesh/ui/Main.kt index c2975f923..fd0993051 100644 --- a/app/src/main/java/com/geeksville/mesh/ui/Main.kt +++ b/app/src/main/java/com/geeksville/mesh/ui/Main.kt @@ -379,7 +379,7 @@ fun MainScreen( nodesGraph(navController, uiViewModel = uIViewModel) mapGraph(navController, uiViewModel = uIViewModel) channelsGraph(navController, uiViewModel = uIViewModel) - connectionsGraph(navController, uiViewModel = uIViewModel, bluetoothViewModel) + connectionsGraph(navController, bluetoothViewModel) settingsGraph(navController, uiViewModel = uIViewModel) } } diff --git a/app/src/main/java/com/geeksville/mesh/ui/connections/ConnectionsScreen.kt b/app/src/main/java/com/geeksville/mesh/ui/connections/ConnectionsScreen.kt index 62bee4257..aa31e4a9d 100644 --- a/app/src/main/java/com/geeksville/mesh/ui/connections/ConnectionsScreen.kt +++ b/app/src/main/java/com/geeksville/mesh/ui/connections/ConnectionsScreen.kt @@ -40,7 +40,6 @@ import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.livedata.observeAsState import androidx.compose.runtime.mutableStateOf @@ -62,8 +61,6 @@ import com.geeksville.mesh.model.BTScanModel import com.geeksville.mesh.model.BluetoothViewModel import com.geeksville.mesh.model.DeviceListEntry import com.geeksville.mesh.model.Node -import com.geeksville.mesh.model.UIViewModel -import com.geeksville.mesh.navigation.ConfigRoute import com.geeksville.mesh.navigation.Route import com.geeksville.mesh.navigation.SettingsRoutes import com.geeksville.mesh.navigation.getNavRouteFrom @@ -94,7 +91,7 @@ fun String?.isIPAddress(): Boolean = if (Build.VERSION.SDK_INT < Build.VERSION_C @Suppress("CyclomaticComplexMethod", "LongMethod", "MagicNumber", "ModifierMissing", "ComposableParamOrder") @Composable fun ConnectionsScreen( - uiViewModel: UIViewModel = hiltViewModel(), + connectionsViewModel: ConnectionsViewModel = hiltViewModel(), scanModel: BTScanModel = hiltViewModel(), bluetoothViewModel: BluetoothViewModel = hiltViewModel(), radioConfigViewModel: RadioConfigViewModel = hiltViewModel(), @@ -103,14 +100,15 @@ fun ConnectionsScreen( onConfigNavigate: (Route) -> Unit, ) { val radioConfigState by radioConfigViewModel.radioConfigState.collectAsStateWithLifecycle() - val config by uiViewModel.localConfig.collectAsStateWithLifecycle() + val config by connectionsViewModel.localConfig.collectAsStateWithLifecycle() val currentRegion = config.lora.region val scrollState = rememberScrollState() val scanStatusText by scanModel.errorText.observeAsState("") - val connectionState by uiViewModel.connectionState.collectAsStateWithLifecycle(ConnectionState.DISCONNECTED) + val connectionState by + connectionsViewModel.connectionState.collectAsStateWithLifecycle(ConnectionState.DISCONNECTED) val scanning by scanModel.spinner.collectAsStateWithLifecycle(false) val context = LocalContext.current - val info by uiViewModel.myNodeInfo.collectAsStateWithLifecycle() + val info by connectionsViewModel.myNodeInfo.collectAsStateWithLifecycle() val selectedDevice by scanModel.selectedNotNullFlow.collectAsStateWithLifecycle() val bluetoothEnabled by bluetoothViewModel.enabled.collectAsStateWithLifecycle(false) val regionUnset = @@ -182,10 +180,12 @@ fun ConnectionsScreen( Column( modifier = Modifier.fillMaxSize().verticalScroll(scrollState).height(IntrinsicSize.Max).padding(16.dp), ) { - val isConnected by uiViewModel.isConnectedStateFlow.collectAsState(false) - val ourNode by uiViewModel.ourNodeInfo.collectAsState() + val ourNode by connectionsViewModel.ourNodeInfo.collectAsStateWithLifecycle() - AnimatedVisibility(visible = isConnected, modifier = Modifier.padding(bottom = 16.dp)) { + AnimatedVisibility( + visible = connectionState.isConnected(), + modifier = Modifier.padding(bottom = 16.dp), + ) { Column { ourNode?.let { node -> Text( @@ -207,10 +207,10 @@ fun ConnectionsScreen( } } - val setRegionText = stringResource(id = R.string.set_your_region) + /*val setRegionText = stringResource(id = R.string.set_your_region) val actionText = stringResource(id = R.string.action_go) - LaunchedEffect(isConnected && regionUnset && selectedDevice != "m") { - if (isConnected && regionUnset && selectedDevice != "m") { + LaunchedEffect(connectionState.isConnected() && regionUnset && selectedDevice != "m") { + if (connectionState.isConnected() && regionUnset && selectedDevice != "m") { uiViewModel.showSnackBar( text = setRegionText, actionLabel = actionText, @@ -220,7 +220,7 @@ fun ConnectionsScreen( }, ) } - } + }*/ var selectedDeviceType by remember { mutableStateOf(DeviceType.BLE) } LaunchedEffect(selectedDevice) { @@ -263,18 +263,13 @@ fun ConnectionsScreen( } } - LaunchedEffect(ourNode) { - if (ourNode != null) { - uiViewModel.refreshProvideLocation() - } - } - Spacer(modifier = Modifier.height(16.dp)) // Warning Not Paired - val hasShownNotPairedWarning by uiViewModel.hasShownNotPairedWarning.collectAsStateWithLifecycle() + val hasShownNotPairedWarning by + connectionsViewModel.hasShownNotPairedWarning.collectAsStateWithLifecycle() val showWarningNotPaired = - !isConnected && + !connectionState.isConnected() && !hasShownNotPairedWarning && bleDevices.none { it is DeviceListEntry.Ble && it.bonded } if (showWarningNotPaired) { @@ -286,7 +281,7 @@ fun ConnectionsScreen( ) Spacer(modifier = Modifier.height(16.dp)) - LaunchedEffect(Unit) { uiViewModel.suppressNoPairedWarning() } + LaunchedEffect(Unit) { connectionsViewModel.suppressNoPairedWarning() } } } } diff --git a/app/src/main/java/com/geeksville/mesh/ui/connections/ConnectionsViewModel.kt b/app/src/main/java/com/geeksville/mesh/ui/connections/ConnectionsViewModel.kt new file mode 100644 index 000000000..3c2a27244 --- /dev/null +++ b/app/src/main/java/com/geeksville/mesh/ui/connections/ConnectionsViewModel.kt @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2025 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package com.geeksville.mesh.ui.connections + +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope +import com.geeksville.mesh.LocalOnlyProtos.LocalConfig +import com.geeksville.mesh.android.prefs.UiPrefs +import com.geeksville.mesh.database.NodeRepository +import com.geeksville.mesh.database.entity.MyNodeEntity +import com.geeksville.mesh.model.Node +import com.geeksville.mesh.repository.datastore.RadioConfigRepository +import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.stateIn +import javax.inject.Inject + +@HiltViewModel +class ConnectionsViewModel +@Inject +constructor( + private val radioConfigRepository: RadioConfigRepository, + private val nodeRepository: NodeRepository, + private val uiPrefs: UiPrefs, +) : ViewModel() { + val localConfig: StateFlow = + radioConfigRepository.localConfigFlow.stateIn( + viewModelScope, + SharingStarted.WhileSubscribed(5_000L), + LocalConfig.getDefaultInstance(), + ) + + val connectionState + get() = radioConfigRepository.connectionState + + val myNodeInfo: StateFlow + get() = nodeRepository.myNodeInfo + + val ourNodeInfo: StateFlow + get() = nodeRepository.ourNodeInfo + + private val _hasShownNotPairedWarning = MutableStateFlow(uiPrefs.hasShownNotPairedWarning) + val hasShownNotPairedWarning: StateFlow = _hasShownNotPairedWarning.asStateFlow() + + fun suppressNoPairedWarning() { + _hasShownNotPairedWarning.value = true + uiPrefs.hasShownNotPairedWarning = true + } +}