fix: restart on ping bugs

Fixes bug where restart on ping could kill itself or not start correctly given certain settings combinations.

This change also makes auto tunneling and all or nothing service as this intuitively makes the most sense with the way the global settings are presented.

This change also makes it so users can toggle tunnel on untrusted wifi without location permissions because location permissions are only required when they go to add trusted ssids.

app/src/main/java/com/zaneschepke/wireguardautotunnel/service/foreground/autotunnel/AutoTunnelService.kt

@@ -9,7 +9,6 @@ import androidx.lifecycle.LifecycleService
 import androidx.lifecycle.lifecycleScope
 import com.wireguard.android.util.RootShell
 import com.zaneschepke.wireguardautotunnel.R
-import com.zaneschepke.wireguardautotunnel.data.domain.Settings
 import com.zaneschepke.wireguardautotunnel.data.domain.TunnelConfig
 import com.zaneschepke.wireguardautotunnel.data.repository.AppDataRepository
 import com.zaneschepke.wireguardautotunnel.module.AppShell
@@ -23,7 +22,6 @@ import com.zaneschepke.wireguardautotunnel.service.network.NetworkStatus
 import com.zaneschepke.wireguardautotunnel.service.network.WifiService
 import com.zaneschepke.wireguardautotunnel.service.notification.NotificationService
 import com.zaneschepke.wireguardautotunnel.service.tunnel.TunnelService
-import com.zaneschepke.wireguardautotunnel.service.tunnel.TunnelState
 import com.zaneschepke.wireguardautotunnel.util.Constants
 import com.zaneschepke.wireguardautotunnel.util.extensions.cancelWithMessage
 import com.zaneschepke.wireguardautotunnel.util.extensions.getCurrentWifiName
@@ -35,6 +33,7 @@ import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.collect
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.update
@@ -42,6 +41,7 @@ import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 import timber.log.Timber
 import java.net.InetAddress
+import java.util.concurrent.atomic.AtomicBoolean
 import javax.inject.Inject
 import javax.inject.Provider
 
@@ -86,11 +86,9 @@ class AutoTunnelService : LifecycleService() {
 
 	private var wakeLock: PowerManager.WakeLock? = null
 
-	private var wifiJob: Job? = null
+	private val pingTunnelRestartActive = AtomicBoolean(false)
-	private var mobileDataJob: Job? = null
+
-	private var ethernetJob: Job? = null
 	private var pingJob: Job? = null
-	private var networkEventJob: Job? = null
 
 	override fun onCreate() {
 		super.onCreate()
@@ -123,6 +121,8 @@ class AutoTunnelService : LifecycleService() {
 			}
 			startSettingsJob()
 			startVpnStateJob()
+			startNetworkJobs()
+			startPingStateJob()
 		}.onFailure {
 			Timber.e(it)
 		}
@@ -138,7 +138,6 @@ class AutoTunnelService : LifecycleService() {
 	}
 
 	override fun onDestroy() {
-		cancelAndResetNetworkJobs()
 		cancelAndResetPingJob()
 		serviceManager.autoTunnelService = CompletableDeferred()
 		super.onDestroy()
@@ -203,6 +202,16 @@ class AutoTunnelService : LifecycleService() {
 		handleNetworkEventChanges()
 	}
 
+	private fun startPingStateJob() = lifecycleScope.launch {
+		autoTunnelStateFlow.collect {
+			if (it.isPingEnabled()) {
+				pingJob.onNotRunning { pingJob = startPingJob() }
+			} else {
+				if (!pingTunnelRestartActive.get()) cancelAndResetPingJob()
+			}
+		}
+	}
+
 	private suspend fun watchForMobileDataConnectivityChanges() {
 		withContext(ioDispatcher) {
 			Timber.i("Starting mobile data watcher")
@@ -232,8 +241,8 @@ class AutoTunnelService : LifecycleService() {
 			Timber.i("Starting ping watcher")
 			runCatching {
 				do {
-					val vpnState = tunnelService.get().vpnState.value
+					val vpnState = autoTunnelStateFlow.value.vpnState
-					if (vpnState.status == TunnelState.UP) {
+					if (vpnState.status.isUp() && !autoTunnelStateFlow.value.isNoConnectivity()) {
 						if (vpnState.tunnelConfig != null) {
 							val config = TunnelConfig.configFromWgQuick(vpnState.tunnelConfig.wgQuick)
 							val results = if (vpnState.tunnelConfig.pingIp != null) {
@@ -249,7 +258,9 @@ class AutoTunnelService : LifecycleService() {
 							if (results.contains(false)) {
 								Timber.i("Restarting VPN for ping failure")
 								val cooldown = vpnState.tunnelConfig.pingCooldown
+								pingTunnelRestartActive.set(true)
 								tunnelService.get().bounceTunnel()
+								pingTunnelRestartActive.set(false)
 								delay(cooldown ?: Constants.PING_COOLDOWN)
 								continue
 							}
@@ -267,14 +278,10 @@ class AutoTunnelService : LifecycleService() {
 		Timber.i("Starting settings watcher")
 		withContext(ioDispatcher) {
 			appDataRepository.settings.getSettingsFlow().combine(
-				// ignore isActive changes to allow manual tunnel overrides
+				appDataRepository.tunnels.getTunnelConfigsFlow(),
-				appDataRepository.tunnels.getTunnelConfigsFlow().distinctUntilChanged { old, new ->
-					old.map { it.isActive } != new.map { it.isActive }
-				},
 			) { settings, tunnels ->
 				Pair(settings, tunnels)
 			}.collect { pair ->
-				manageJobsBySettings(pair.first)
 				autoTunnelStateFlow.update {
 					it.copy(
 						settings = pair.first,
@@ -292,53 +299,16 @@ class AutoTunnelService : LifecycleService() {
 				autoTunnelStateFlow.update {
 					it.copy(vpnState = state)
 				}
-				// TODO think about this
-				// What happens if we change the pinger setting while vpn is active?
-				state.tunnelConfig?.let {
-					val settings = appDataRepository.settings.getSettings()
-					if (it.isPingEnabled && !settings.isPingEnabled) {
-						pingJob.onNotRunning { pingJob = startPingJob() }
-					}
-					if (!it.isPingEnabled && !settings.isPingEnabled) {
-						cancelAndResetPingJob()
-					}
-				}
-			}
-		}
-	}
-
-	private fun manageJobsBySettings(settings: Settings) {
-		with(settings) {
-			if (isPingEnabled) {
-				pingJob.onNotRunning { pingJob = startPingJob() }
-			} else {
-				cancelAndResetPingJob()
-			}
-			if (isTunnelOnWifiEnabled || isTunnelOnEthernetEnabled || isTunnelOnMobileDataEnabled) {
-				startNetworkJobs()
-			} else {
-				cancelAndResetNetworkJobs()
 			}
 		}
 	}
 
 	private fun startNetworkJobs() {
-		wifiJob.onNotRunning {
+		Timber.i("Starting all network state jobs..")
-			Timber.i("Wifi job starting")
+		startWifiJob()
-			wifiJob = startWifiJob()
+		startEthernetJob()
-		}
+		startMobileDataJob()
-		ethernetJob.onNotRunning {
+		startNetworkEventJob()
-			ethernetJob = startEthernetJob()
-			Timber.i("Ethernet job starting")
-		}
-		mobileDataJob.onNotRunning {
-			mobileDataJob = startMobileDataJob()
-			Timber.i("Mobile data job starting")
-		}
-		networkEventJob.onNotRunning {
-			Timber.i("Network event job starting")
-			networkEventJob = startNetworkEventJob()
-		}
 	}
 
 	private fun cancelAndResetPingJob() {
@@ -346,17 +316,6 @@ class AutoTunnelService : LifecycleService() {
 		pingJob = null
 	}
 
-	private fun cancelAndResetNetworkJobs() {
-		networkEventJob?.cancelWithMessage("Network event job canceled")
-		wifiJob?.cancelWithMessage("Wifi job canceled")
-		ethernetJob?.cancelWithMessage("Ethernet job canceled")
-		mobileDataJob?.cancelWithMessage("Mobile data job canceled")
-		networkEventJob = null
-		wifiJob = null
-		ethernetJob = null
-		mobileDataJob = null
-	}
-
 	private fun emitEthernetConnected(connected: Boolean) {
 		autoTunnelStateFlow.update {
 			it.copy(
@@ -459,19 +418,16 @@ class AutoTunnelService : LifecycleService() {
 	private suspend fun handleNetworkEventChanges() {
 		withContext(ioDispatcher) {
 			Timber.i("Starting auto-tunnel network event watcher")
-			// allow manual overrides
+			// ignore vpnState emits to allow manual overrides
 			autoTunnelStateFlow.distinctUntilChanged { old, new ->
-				old.copy(vpnState = new.vpnState) == new
+				old.copy(vpnState = new.vpnState) == new || old.tunnels.map { it.isActive } != new.tunnels.map { it.isActive }
 			}.collect { watcherState ->
 				when (val event = watcherState.asAutoTunnelEvent()) {
-					is AutoTunnelEvent.Start -> {
+					is AutoTunnelEvent.Start -> tunnelService.get().startTunnel(
-						Timber.d("Start tunnel ${event.tunnelConfig?.name}")
+						event.tunnelConfig
-						tunnelService.get().startTunnel(event.tunnelConfig ?: appDataRepository.getPrimaryOrFirstTunnel())
+							?: appDataRepository.getPrimaryOrFirstTunnel(),
-					}
+					)
-					is AutoTunnelEvent.Stop -> {
+					is AutoTunnelEvent.Stop -> tunnelService.get().stopTunnel()
-						Timber.d("Stop tunnel")
-						tunnelService.get().stopTunnel()
-					}
 					AutoTunnelEvent.DoNothing -> Timber.i("Auto-tunneling: no condition met")
 				}
 			}

app/src/main/java/com/zaneschepke/wireguardautotunnel/service/foreground/autotunnel/AutoTunnelState.kt

@@ -55,7 +55,7 @@ data class AutoTunnelState(
 		return isEthernetConnected && !settings.isTunnelOnEthernetEnabled && vpnState.status.isUp()
 	}
 
-	private fun isNoConnectivity(): Boolean {
+	fun isNoConnectivity(): Boolean {
 		return !isEthernetConnected && !isWifiConnected && !isMobileDataConnected
 	}
 
@@ -96,7 +96,9 @@ data class AutoTunnelState(
 		val vpnTunnel = vpnState.tunnelConfig
 		return if (preferred != null && vpnTunnel != null) {
 			preferred.id == vpnTunnel.id
-		} else true
+		} else {
+			true
+		}
 	}
 
 	fun asAutoTunnelEvent(): AutoTunnelEvent {
@@ -120,14 +122,23 @@ data class AutoTunnelState(
 	private fun isCurrentSSIDTrusted(): Boolean {
 		return if (settings.isWildcardsEnabled) {
 			settings.trustedNetworkSSIDs.isMatchingToWildcardList(currentNetworkSSID)
-		} else settings.trustedNetworkSSIDs.contains(currentNetworkSSID)
+		} else {
+			settings.trustedNetworkSSIDs.contains(currentNetworkSSID)
+		}
 	}
 
 	private fun getTunnelWithMatchingTunnelNetwork(): TunnelConfig? {
 		return tunnels.firstOrNull {
 			if (settings.isWildcardsEnabled) {
 				it.tunnelNetworks.isMatchingToWildcardList(currentNetworkSSID)
-			} else it.tunnelNetworks.contains(currentNetworkSSID)
+			} else {
+				it.tunnelNetworks.contains(currentNetworkSSID)
 			}
 		}
 	}
+
+	fun isPingEnabled(): Boolean {
+		return settings.isPingEnabled ||
+			(vpnState.status.isUp() && vpnState.tunnelConfig != null && tunnels.first { it.id == vpnState.tunnelConfig.id }.isPingEnabled)
+	}
+}

app/src/main/java/com/zaneschepke/wireguardautotunnel/ui/screens/main/MainViewModel.kt

@@ -252,7 +252,7 @@ constructor(
 
 	fun onCopyTunnel(tunnel: TunnelConfig) = viewModelScope.launch {
 		saveTunnel(
-			TunnelConfig(name = makeTunnelNameUnique(tunnel.name), wgQuick = tunnel.wgQuick, amQuick = tunnel.amQuick)
+			TunnelConfig(name = makeTunnelNameUnique(tunnel.name), wgQuick = tunnel.wgQuick, amQuick = tunnel.amQuick),
 		)
 	}
 

app/src/main/java/com/zaneschepke/wireguardautotunnel/ui/screens/settings/autotunnel/AutoTunnelScreen.kt

@@ -72,16 +72,18 @@ fun AutoTunnelScreen(uiState: AppUiState, viewModel: AutoTunnelViewModel = hiltV
 		isBackgroundLocationGranted = fineLocationState.status.isGranted
 	}
 
-	fun onAutoTunnelWifiChecked() {
+	fun isWifiNameReadable(): Boolean {
-		if (uiState.settings.isTunnelOnWifiEnabled) viewModel.onToggleTunnelOnWifi().also { return }
+		return when {
-		when (false) {
+			!isBackgroundLocationGranted ||
-			isBackgroundLocationGranted -> showLocationDialog = true
+				!fineLocationState.status.isGranted -> {
-			fineLocationState.status.isGranted -> showLocationDialog = true
+				showLocationDialog = true
-			context.isLocationServicesEnabled() ->
+				false
-				showLocationServicesAlertDialog = true
-			else -> {
-				viewModel.onToggleTunnelOnWifi()
 			}
+			!context.isLocationServicesEnabled() -> {
+				showLocationServicesAlertDialog = true
+				false
+			}
+			else -> true
 		}
 	}
 
@@ -118,14 +120,14 @@ fun AutoTunnelScreen(uiState: AppUiState, viewModel: AutoTunnelViewModel = hiltV
 		topBar = {
 			TopNavBar(stringResource(R.string.auto_tunneling))
 		},
-	) {
+	) { padding ->
 		Column(
 			horizontalAlignment = Alignment.Start,
 			verticalArrangement = Arrangement.spacedBy(24.dp.scaledHeight(), Alignment.Top),
 			modifier =
 			Modifier
 				.fillMaxSize()
-				.padding(it)
+				.padding(padding)
 				.padding(top = 24.dp.scaledHeight())
 				.padding(horizontal = 24.dp.scaledWidth()),
 		) {
@@ -148,14 +150,12 @@ fun AutoTunnelScreen(uiState: AppUiState, viewModel: AutoTunnelViewModel = hiltV
 										enabled = !uiState.settings.isAlwaysOnVpnEnabled,
 										checked = uiState.settings.isTunnelOnWifiEnabled,
 										onClick = {
-											if (uiState.settings.isWifiNameByShellEnabled) viewModel.onToggleTunnelOnWifi().also { return@ScaledSwitch }
+											viewModel.onToggleTunnelOnWifi()
-											onAutoTunnelWifiChecked()
 										},
 									)
 								},
 								onClick = {
-									if (uiState.settings.isWifiNameByShellEnabled) viewModel.onToggleTunnelOnWifi().also { return@SelectionItem }
+									viewModel.onToggleTunnelOnWifi()
-									onAutoTunnelWifiChecked()
 								},
 							),
 							SelectionItem(
@@ -251,7 +251,9 @@ fun AutoTunnelScreen(uiState: AppUiState, viewModel: AutoTunnelViewModel = hiltV
 											uiState.settings.trustedNetworkSSIDs,
 											onDelete = viewModel::onDeleteTrustedSSID,
 											currentText = currentText,
-											onSave = viewModel::onSaveTrustedSSID,
+											onSave = { ssid ->
+												if (uiState.settings.isWifiNameByShellEnabled || isWifiNameReadable()) viewModel.onSaveTrustedSSID(ssid)
+											},
 											onValueChange = { currentText = it },
 											supporting = {
 												if (uiState.settings.isWildcardsEnabled) {

app/src/main/res/values/strings.xml

@@ -133,7 +133,7 @@
 	<string name="tunnel_required">Feature requires at least one tunnel</string>
     <string name="background_location_message">Allow all the time location permission and/or precise location is required for this feature. Please see</string>
 	<string name="app_settings">app settings</string>
-	<string name="background_location_message2">to make sure these permissions are enabled.</string>
+	<string name="background_location_message2">to make sure these permissions are enabled</string>
 	<string name="root_accepted">Root shell accepted</string>
 	<string name="set_custom_ping_ip">Set custom ping ip</string>
 	<string name="default_ping_ip">(optional, defaults to peers)</string>