android: health check - ping the server only when it can matter

- WITH_TUNNEL mode pings the server only once the external targets fail
  (no ping fork + DNS lookup outside the tunnel on every healthy cycle;
  an ICMP-blocking server no longer forces fast polls)
- initial battery-low state from the sticky ACTION_BATTERY_CHANGED
- non-state notifications no longer restart a pending cycle

Co-Authored-By: Claude Opus 5.5 <[email protected]>

src/frontends/android/app/src/main/java/org/strongswan/android/logic/connectivity/HealthCheckManager.kt

@@ -25,6 +25,8 @@ import android.net.ConnectivityManager
 import android.net.Network
 import android.net.NetworkCapabilities
 import android.net.NetworkRequest
+import android.os.BatteryManager
+import android.os.Build
 import android.os.Handler
 import android.os.IBinder
 import android.os.Looper
@@ -116,6 +118,9 @@ class HealthCheckManager private constructor(
 	private val cycleMutex = Mutex()
 	private var config = HealthConfig()
 	private var running = false
+	/* last state seen by onStateChanged(): VpnStateService also notifies on
+	 * error/IMC/rebind changes, which must not restart a pending cycle */
+	private var lastState: VpnStateService.State? = null
 	private var service: VpnStateService? = null
 	private var cycleJob: Job? = null
 	private var confirmJob: Job? = null
@@ -230,6 +235,7 @@ class HealthCheckManager private constructor(
 		{
 		}
 		service = null
+		lastState = null
 		resetCounters()
 	}
 
@@ -239,6 +245,8 @@ class HealthCheckManager private constructor(
 		if (config.enabled)
 		{
 			start()
+			/* re-arm with the new settings even if the state is unchanged */
+			lastState = null
 			onStateChanged()
 		}
 		else
@@ -265,6 +273,8 @@ class HealthCheckManager private constructor(
 		if (updated.enabled)
 		{
 			start()
+			/* re-arm with the new settings even if the state is unchanged */
+			lastState = null
 			onStateChanged()
 		}
 		else
@@ -284,12 +294,17 @@ class HealthCheckManager private constructor(
 		{
 			return
 		}
-		when (service?.state)
+		val state = service?.state
+		val previous = lastState
+		lastState = state
+		when (state)
 		{
 			VpnStateService.State.CONNECTED ->
 			{
-				/* let the fresh tunnel settle before the first probe */
-				scheduleCycle(INITIAL_DELAY_MS)
+				if (previous != VpnStateService.State.CONNECTED)
+				{	/* let the fresh tunnel settle before the first probe */
+					scheduleCycle(INITIAL_DELAY_MS)
+				}
 			}
 			VpnStateService.State.DISABLED ->
 			{
@@ -345,29 +360,27 @@ class HealthCheckManager private constructor(
 		}
 		val gateway = svc.profile?.gateway
 		val targets = config.targets
-		val checkServer = config.serverCheckMode != ServerCheckMode.OFF
+		val mode = config.serverCheckMode
 		val timeout = config.timeoutMs
 
-		val results = coroutineScope {
-			val serverDeferred = if (checkServer)
-			{
-				async { ConnectivityProbe.probe(ProbeTarget.SERVER, timeout, context, gateway) }
-			}
-			else
-			{
-				null
-			}
-			val external = targets.map { target ->
+		val externalResults = coroutineScope {
+			targets.map { target ->
 				async { ConnectivityProbe.probe(target, timeout, context, gateway) }
-			}
-			(listOfNotNull(serverDeferred) + external).map { it.await() }
+			}.map { it.await() }
 		}
-
-		val serverResult = results.firstOrNull { it.target == ProbeTarget.SERVER }
-		val externalResults = results.filter { it.target != ProbeTarget.SERVER }
 		val externalOk = externalResults.count { it.ok }
 		val externalTotal = externalResults.size
-		val serverDown = serverResult?.let { !it.ok }
+		/* the direct server ping (a fork of /system/bin/ping plus a DNS lookup
+		 * outside the tunnel) only runs when its result can matter: always in
+		 * ALWAYS mode, in WITH_TUNNEL mode only once the tunnel looks dead */
+		val serverDown = if (shouldProbeServer(mode, externalOk, externalTotal))
+		{
+			!ConnectivityProbe.probe(ProbeTarget.SERVER, timeout, context, gateway).ok
+		}
+		else
+		{
+			null
+		}
 
 		val decision = decide(config, serverDown, externalOk, externalTotal, serverAttempts, tunnelAttempts)
 		val healthy = serverDown != true && (externalTotal == 0 || externalOk == externalTotal)
@@ -523,9 +536,10 @@ class HealthCheckManager private constructor(
 		powerState = PowerState(
 			screenOn = pm?.isInteractive ?: true,
 			powerSaveMode = pm?.isPowerSaveMode ?: false,
-			batteryLow = false,
+			batteryLow = isBatteryLowNow(),
 		)
-		/* BATTERY_LOW/OKAY are sticky, so registration immediately reflects the current state */
+		/* BATTERY_LOW/OKAY are not sticky: the initial state comes from the
+		 * sticky ACTION_BATTERY_CHANGED, later changes from the broadcasts */
 		val filter = IntentFilter().apply {
 			addAction(Intent.ACTION_SCREEN_ON)
 			addAction(Intent.ACTION_SCREEN_OFF)
@@ -543,6 +557,29 @@ class HealthCheckManager private constructor(
 		}
 	}
 
+	private fun isBatteryLowNow(): Boolean
+	{
+		return try
+		{
+			val sticky = context.registerReceiver(null, IntentFilter(Intent.ACTION_BATTERY_CHANGED))
+				?: return false
+			if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P)
+			{
+				sticky.getBooleanExtra(BatteryManager.EXTRA_BATTERY_LOW, false)
+			}
+			else
+			{	/* the platform's default low-battery threshold */
+				val level = sticky.getIntExtra(BatteryManager.EXTRA_LEVEL, -1)
+				val scale = sticky.getIntExtra(BatteryManager.EXTRA_SCALE, -1)
+				level >= 0 && scale > 0 && level * 100 / scale <= 15
+			}
+		}
+		catch (_: Exception)
+		{
+			false
+		}
+	}
+
 	private fun unregisterPowerReceiver()
 	{
 		try
@@ -710,6 +747,16 @@ class HealthCheckManager private constructor(
 		 * @param serverAttempts reconnect decisions already taken for the server rule
 		 * @param tunnelAttempts reconnect decisions already taken for the tunnel rules
 		 */
+		/** Whether the server probe can influence [decide] this cycle. */
+		@JvmStatic
+		fun shouldProbeServer(mode: ServerCheckMode, externalOk: Int, externalTotal: Int): Boolean =
+			when (mode)
+			{
+				ServerCheckMode.OFF -> false
+				ServerCheckMode.ALWAYS -> true
+				ServerCheckMode.WITH_TUNNEL -> externalTotal > 0 && externalOk == 0
+			}
+
 		fun decide(
 			config: HealthConfig,
 			serverDown: Boolean?,

src/frontends/android/app/src/test/java/org/strongswan/android/logic/connectivity/HealthCheckManagerTest.kt

@@ -230,4 +230,16 @@ class HealthCheckManagerTest
 		assertThat(ConnectivityProbe.parsePingRtt("time<1 ms")).isEqualTo(1L)
 		assertThat(ConnectivityProbe.parsePingRtt("no time here")).isNull()
 	}
+
+	@Test
+	fun serverIsOnlyPingedWhenItCanMatter()
+	{
+		assertThat(HealthCheckManager.shouldProbeServer(ServerCheckMode.OFF, 0, 3)).isFalse()
+		assertThat(HealthCheckManager.shouldProbeServer(ServerCheckMode.ALWAYS, 3, 3)).isTrue()
+		/* WITH_TUNNEL: a working tunnel makes the server result irrelevant */
+		assertThat(HealthCheckManager.shouldProbeServer(ServerCheckMode.WITH_TUNNEL, 3, 3)).isFalse()
+		assertThat(HealthCheckManager.shouldProbeServer(ServerCheckMode.WITH_TUNNEL, 1, 3)).isFalse()
+		assertThat(HealthCheckManager.shouldProbeServer(ServerCheckMode.WITH_TUNNEL, 0, 3)).isTrue()
+		assertThat(HealthCheckManager.shouldProbeServer(ServerCheckMode.WITH_TUNNEL, 0, 0)).isFalse()
+	}
 }