From f8f02dd93e9da68a5829ece53c5b9ad706688b34 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 23:52:39 +0000 Subject: [PATCH 1/2] Make LAN play work over a phone's hotspot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two phones on one phone's hotspot could not find or reach each other. Nothing was wrong with the Wi-Fi. There were three faults, and the first one is the whole bug. A hotspot has no internet behind it, and Android takes that literally: the Wi-Fi network fails its connectivity check, stays unvalidated, and mobile data remains the *default* network. Every socket opened without naming a network is therefore routed out over cellular — where 192.168.43.1 means nothing. The join socket is now bound to the Wi-Fi network before it connects, and the search for that network deliberately does not ask whether it has internet, because asking is what breaks it. Second, the manifest has asked for CHANGE_WIFI_MULTICAST_STATE since the beginning, with a comment saying discovery needs it, and nothing ever took the lock. Without it the radio drops multicast frames not addressed to this device before the system sees them, and mDNS is multicast. Both advertising and discovering now hold it. Third, and not fixable: a phone sharing its own hotspot does not reliably advertise over the tethering interface, whatever the other end does. So the host now shows where it is and the joiner can type it in. That path is offered always rather than kept back for when discovery has failed, because discovery here does not fail loudly — it just never finds anything, and a screen saying only "no games found" leaves nowhere to go. Hosts take a known port when they can get one, so what gets typed is an address and not an address and five more digits. The lobby scrolls instead of filling. Its lazy lists took the whole screen for a handful of rows and pushed the Start button off the bottom; they would have done the same to the address box. Parsing what somebody types off another phone's screen is the one pure piece of this and is tested, including that what a host displays is exactly what the other end accepts. The rest needs two phones and a hotspot, which CI does not have. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0132V9gE6DF9itz2z3Hn7BK9 --- .../prolibertate/games/net/LanTransport.kt | 36 ++++- .../prolibertate/games/net/LobbyController.kt | 41 +++++ .../prolibertate/games/net/LocalNetwork.kt | 145 ++++++++++++++++++ .../org/prolibertate/games/net/Transport.kt | 22 +++ .../org/prolibertate/games/ui/LobbyScreen.kt | 144 ++++++++++++----- .../games/net/ManualAddressTest.kt | 106 +++++++++++++ 6 files changed, 452 insertions(+), 42 deletions(-) create mode 100644 app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt create mode 100644 app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt diff --git a/app/src/main/java/org/prolibertate/games/net/LanTransport.kt b/app/src/main/java/org/prolibertate/games/net/LanTransport.kt index 506b550..941ba87 100644 --- a/app/src/main/java/org/prolibertate/games/net/LanTransport.kt +++ b/app/src/main/java/org/prolibertate/games/net/LanTransport.kt @@ -7,6 +7,9 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.isActive import kotlinx.coroutines.launch @@ -22,7 +25,9 @@ import java.util.Collections * * NSD is used rather than a UDP broadcast ping because it works across the * Wi-Fi isolation settings most home routers ship with, and because Android - * gives us the resolver for free. + * gives us the resolver for free. It is not, however, trusted to be enough: + * see [endpoint] and [LocalNetwork] for what a phone's own hotspot does to + * both discovery and routing, and for the way round it. */ class LanTransport(private val context: Context) : Transport { @@ -31,10 +36,17 @@ class LanTransport(private val context: Context) : Transport { private val nsdManager: NsdManager get() = context.getSystemService(Context.NSD_SERVICE) as NsdManager + private val multicast = MulticastGuard(context) + private var serverSocket: ServerSocket? = null private var registrationListener: NsdManager.RegistrationListener? = null private var discoveryListener: NsdManager.DiscoveryListener? = null + private val _endpoint = MutableStateFlow(null) + + /** Where this device is listening while hosting, so it can be read out. */ + val endpoint: StateFlow = _endpoint.asStateFlow() + override fun isAvailable(): Boolean = true // ----------------------------------------------------------------------- @@ -42,9 +54,15 @@ class LanTransport(private val context: Context) : Transport { // ----------------------------------------------------------------------- override fun host(displayName: String, scope: CoroutineScope): Flow = callbackFlow { - // Port 0 lets the OS pick a free port, which is then advertised. - val server = ServerSocket(0) + // The known port when it can be had, so that joining by hand needs an + // address and nothing else; anything free if something already has it. + val server = runCatching { ServerSocket(DEFAULT_HOST_PORT) } + .getOrElse { ServerSocket(0) } serverSocket = server + _endpoint.value = HostEndpoint(localIpv4Addresses(), server.localPort) + + // Advertising is mDNS, and mDNS is multicast. + multicast.acquire() val serviceInfo = NsdServiceInfo().apply { serviceName = "$SERVICE_NAME @ $displayName" @@ -82,6 +100,7 @@ class LanTransport(private val context: Context) : Transport { acceptJob.cancel() stopAdvertising() runCatching { server.close() } + _endpoint.value = null } } @@ -163,6 +182,10 @@ class LanTransport(private val context: Context) : Transport { } discoveryListener = listener + // Listening for mDNS replies needs the multicast filter held open just + // as much as answering does. + multicast.acquire() + publish() runCatching { nsdManager.discoverServices(SERVICE_TYPE, NsdManager.PROTOCOL_DNS_SD, listener) @@ -178,6 +201,11 @@ class LanTransport(private val context: Context) : Transport { override suspend fun join(host: DiscoveredHost, scope: CoroutineScope): Connection = withContext(Dispatchers.IO) { val socket = Socket() + // Before connecting, and this is the whole fix for hotspot play: a + // hotspot has no internet behind it, so Android leaves mobile data + // as the default network and an unbound socket goes out over + // cellular looking for a 192.168 address. + bindToWifi(context, socket) socket.connect(InetSocketAddress(host.address, host.port), CONNECT_TIMEOUT_MS) socket.tcpNoDelay = true StreamConnection( @@ -205,8 +233,10 @@ class LanTransport(private val context: Context) : Transport { override fun stop() { stopAdvertising() stopDiscovery() + multicast.release() runCatching { serverSocket?.close() } serverSocket = null + _endpoint.value = null } private companion object { diff --git a/app/src/main/java/org/prolibertate/games/net/LobbyController.kt b/app/src/main/java/org/prolibertate/games/net/LobbyController.kt index 22cefd2..35f5ea9 100644 --- a/app/src/main/java/org/prolibertate/games/net/LobbyController.kt +++ b/app/src/main/java/org/prolibertate/games/net/LobbyController.kt @@ -34,6 +34,8 @@ class LobbyController( val hostName: String = "", val seats: List = emptyList(), val discovered: List = emptyList(), + /** Where this device is listening while hosting, for reading out. */ + val endpoint: HostEndpoint? = null, val message: String? = null, val started: TableConfig? = null, ) @@ -91,6 +93,15 @@ class LobbyController( } } } + + // Discovery is not guaranteed — a phone sharing its own hotspot cannot + // be relied on to advertise over the tethering interface — so where the + // host is listening is put on its screen to be read out. + jobs += scope.launch { + lan.endpoint.collect { endpoint -> + _state.value = _state.value.copy(endpoint = endpoint) + } + } } private fun onClientConnected(connection: Connection) { @@ -205,6 +216,36 @@ class LobbyController( } } + /** + * Joins a host at an address somebody typed in. + * + * The way in when discovery cannot work. Two phones on one phone's hotspot + * is exactly that case: mDNS over a tethering interface is unreliable in + * both directions, and no amount of listening harder fixes it, but the + * address is right there on the host's screen and a TCP connection to it + * works perfectly well once the socket is pointed at the right network. + */ + fun joinAt(typed: String, displayName: String, peerId: String) { + val parsed = parseManualAddress(typed) + if (parsed == null) { + _state.value = _state.value.copy( + message = "That doesn't look like an address. Try 192.168.43.1", + ) + return + } + join( + host = DiscoveredHost( + id = "lan:${parsed.address}:${parsed.port}", + name = parsed.address, + kind = TransportKind.LAN, + address = parsed.address, + port = parsed.port, + ), + displayName = displayName, + peerId = peerId, + ) + } + fun join(host: DiscoveredHost, displayName: String, peerId: String) { jobs += scope.launch { _state.value = _state.value.copy(message = "Connecting to ${host.name}…") diff --git a/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt b/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt new file mode 100644 index 0000000..cb8c9f7 --- /dev/null +++ b/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt @@ -0,0 +1,145 @@ +package org.prolibertate.games.net + +import android.content.Context +import android.net.ConnectivityManager +import android.net.Network +import android.net.NetworkCapabilities +import android.net.wifi.WifiManager +import java.net.Inet4Address +import java.net.NetworkInterface +import java.net.Socket + +/** + * Finding the network the other phone is actually on. + * + * A phone's hotspot has no internet behind it, and Android takes that + * literally: the Wi-Fi network fails its captive-portal check, stays + * unvalidated, and **mobile data remains the default network**. Every socket + * this app opens without saying otherwise is therefore routed out over + * cellular, where 192.168.x.x means nothing at all. That is the whole of the + * "two phones on one phone's hotspot cannot see each other" bug: nothing is + * wrong with the Wi-Fi, the packets are simply being posted to the wrong + * network. + * + * The cure is to name the network explicitly rather than accept the default — + * which is why everything here deliberately looks for a Wi-Fi network *without* + * asking whether it has internet. Asking is what breaks it. + */ + +/** The Wi-Fi network this device is on, internet or no internet. */ +fun wifiNetwork(context: Context): Network? { + val manager = context.applicationContext + .getSystemService(Context.CONNECTIVITY_SERVICE) as? ConnectivityManager + ?: return null + @Suppress("DEPRECATION") + return manager.allNetworks.firstOrNull { network -> + val capabilities = manager.getNetworkCapabilities(network) + capabilities != null && + capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) + } +} + +/** + * Sends this socket out over the Wi-Fi rather than wherever the system would + * otherwise send it. Silent if there is no Wi-Fi network to bind to: a phone + * running the hotspot itself has no Wi-Fi *client* network, and reaches its own + * guests over the tethering interface's own route without any help. + */ +fun bindToWifi(context: Context, socket: Socket) { + val network = wifiNetwork(context) ?: return + runCatching { network.bindSocket(socket) } +} + +/** + * The addresses another device on the same Wi-Fi could reach this one at. + * + * Read off the interfaces rather than asked of [WifiManager], because the phone + * running the hotspot is not a Wi-Fi client and has no client address to give — + * its guests reach it at whatever the tethering interface is numbered, which + * only the interface list knows. + */ +fun localIpv4Addresses(): List { + val interfaces = runCatching { NetworkInterface.getNetworkInterfaces() } + .getOrNull() ?: return emptyList() + + val found = mutableListOf>() + for (nic in interfaces) { + if (runCatching { !nic.isUp || nic.isLoopback }.getOrDefault(true)) continue + for (address in nic.inetAddresses) { + if (address !is Inet4Address || address.isLoopbackAddress) continue + val text = address.hostAddress ?: continue + // Tethering interfaces first: on the phone running the hotspot that + // is the only address its guests can reach, and it is the case the + // list exists to serve. + val rank = when { + nic.name.startsWith("ap") || nic.name.startsWith("swlan") -> 0 + nic.name.startsWith("wlan") -> 1 + else -> 2 + } + found += rank to text + } + } + return found.sortedBy { it.first }.map { it.second }.distinct() +} + +/** + * Holds the Wi-Fi chip's multicast filter open. + * + * mDNS is multicast, and without this lock the radio drops multicast frames + * that are not addressed to this device before the system ever sees them. The + * manifest has asked for the permission since the beginning; nothing ever took + * the lock. + */ +class MulticastGuard(context: Context) { + + private val wifi = context.applicationContext + .getSystemService(Context.WIFI_SERVICE) as? WifiManager + + private var lock: WifiManager.MulticastLock? = null + + fun acquire() { + if (lock != null) return + lock = runCatching { + wifi?.createMulticastLock("plgames-discovery")?.apply { + setReferenceCounted(false) + acquire() + } + }.getOrNull() + } + + fun release() { + runCatching { lock?.takeIf { it.isHeld }?.release() } + lock = null + } +} + +/** An address and port typed in by hand, once it has been made sense of. */ +data class ManualAddress(val address: String, val port: Int) + +/** + * Reads what somebody typed into the "connect to" box. + * + * Accepts a bare address, since [DEFAULT_HOST_PORT] means the port is usually + * the same one and asking a person to copy five more digits off another screen + * is a good way to have them type four of them. A port may still be given + * after a colon for the case where the usual one was taken. + */ +fun parseManualAddress(typed: String, defaultPort: Int = DEFAULT_HOST_PORT): ManualAddress? { + val text = typed.trim() + if (text.isEmpty()) return null + + val colon = text.lastIndexOf(':') + if (colon < 0) return ManualAddress(text, defaultPort).takeIf { looksLikeHost(it.address) } + + val address = text.substring(0, colon).trim() + val port = text.substring(colon + 1).trim().toIntOrNull() ?: return null + if (port !in 1..65_535) return null + return ManualAddress(address, port).takeIf { looksLikeHost(it.address) } +} + +/** + * Enough of a check to catch a slip, and no more. Anything that survives this + * is handed to the socket, which is the real judge of whether it exists. + */ +private fun looksLikeHost(address: String): Boolean = + address.isNotEmpty() && address.none { it.isWhitespace() } diff --git a/app/src/main/java/org/prolibertate/games/net/Transport.kt b/app/src/main/java/org/prolibertate/games/net/Transport.kt index 8e9a6b0..cad96af 100644 --- a/app/src/main/java/org/prolibertate/games/net/Transport.kt +++ b/app/src/main/java/org/prolibertate/games/net/Transport.kt @@ -33,6 +33,28 @@ data class DiscoveredHost( val port: Int = 0, ) +/** + * Where a host is listening, for reading out loud. + * + * Discovery is not always possible — a phone sharing its own hotspot does not + * reliably advertise over the tethering interface, whatever the other end does + * — so a host has to be able to say where it is in a form somebody can type + * into the other phone. + */ +data class HostEndpoint(val addresses: List, val port: Int) { + /** The one to read out, tethering address first. */ + val primary: String? get() = addresses.firstOrNull()?.let { "$it:$port" } +} + +/** + * The port a host listens on when it can have it. + * + * Fixed rather than assigned by the system, so that joining by hand is typing + * an address and not an address and a five-digit number. A host that finds it + * taken falls back to whatever it is given and says so. + */ +const val DEFAULT_HOST_PORT = 47_654 + /** * One peer-to-peer link carrying [NetMessage]s. * diff --git a/app/src/main/java/org/prolibertate/games/ui/LobbyScreen.kt b/app/src/main/java/org/prolibertate/games/ui/LobbyScreen.kt index d31e32d..ee28ecc 100644 --- a/app/src/main/java/org/prolibertate/games/ui/LobbyScreen.kt +++ b/app/src/main/java/org/prolibertate/games/ui/LobbyScreen.kt @@ -6,24 +6,30 @@ import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.lazy.LazyColumn -import androidx.compose.foundation.lazy.items +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.text.KeyboardOptions +import androidx.compose.foundation.verticalScroll +import androidx.compose.material3.Button import androidx.compose.material3.Card import androidx.compose.material3.Divider import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.OutlinedTextField 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.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.unit.dp import org.prolibertate.games.game.engine.PlayerKind import org.prolibertate.games.game.engine.TableConfig -import org.prolibertate.games.net.DiscoveredHost import org.prolibertate.games.net.LobbyController import org.prolibertate.games.net.seatForDevice @@ -75,7 +81,13 @@ fun LobbyScreen( val title = if (route.hosting) "Hosting" else "Join a game" ScreenScaffold(title = title, onBack = onBack) { modifier -> - Column(modifier = modifier, verticalArrangement = Arrangement.spacedBy(12.dp)) { + // Scrolls rather than filling: a lazy list here took the whole screen + // and pushed the Start button — and now the address to type in — off + // the bottom of it. There are never more than a handful of rows. + Column( + modifier = modifier.verticalScroll(rememberScrollState()), + verticalArrangement = Arrangement.spacedBy(12.dp), + ) { val available = env.lobby.availableTransports() Text( text = if (available.isEmpty()) { @@ -94,7 +106,7 @@ fun LobbyScreen( if (route.hosting) { HostingBody(env.lobby, state) } else { - JoiningBody(env, state.discovered, playerName) + JoiningBody(env, state, playerName) } } } @@ -102,34 +114,47 @@ fun LobbyScreen( @Composable private fun HostingBody(lobby: LobbyController, state: LobbyController.State) { + // Where to find this phone, for when the other one cannot find it by + // itself. Over a phone's own hotspot that is the normal case rather than + // the exception, so this is stated plainly rather than tucked away. + state.endpoint?.primary?.let { where -> + Card(modifier = Modifier.fillMaxWidth()) { + Column(modifier = Modifier.padding(12.dp)) { + Text("This phone is at", style = MaterialTheme.typography.labelSmall) + Text(where, fontWeight = FontWeight.Bold) + Text( + text = "If the other phone doesn't find the game on its own — which " + + "happens when you're sharing this phone's hotspot — type that " + + "into its Join screen.", + style = MaterialTheme.typography.bodySmall, + ) + } + } + } + Text("Seats", fontWeight = FontWeight.Bold) - LazyColumn( - verticalArrangement = Arrangement.spacedBy(6.dp), - modifier = Modifier.fillMaxWidth(), - ) { - items(state.seats) { slot -> - Card(modifier = Modifier.fillMaxWidth()) { - Row( - modifier = Modifier.fillMaxWidth().padding(12.dp), - verticalAlignment = Alignment.CenterVertically, - horizontalArrangement = Arrangement.SpaceBetween, - ) { - Column { - Text("Seat ${slot.seat} — ${slot.name}") - Text( - text = when (slot.kind) { - PlayerKind.HUMAN_LOCAL -> "You" - PlayerKind.HUMAN_REMOTE -> "Joined" - PlayerKind.AI -> "Computer" - } + " · Team ${slot.team}", - style = MaterialTheme.typography.bodySmall, - ) - } - if (slot.kind == PlayerKind.HUMAN_REMOTE) { - TextButton(onClick = { - lobby.setSeatKind(slot.seat, PlayerKind.AI, "Computer ${slot.seat}") - }) { Text("Drop") } - } + state.seats.forEach { slot -> + Card(modifier = Modifier.fillMaxWidth()) { + Row( + modifier = Modifier.fillMaxWidth().padding(12.dp), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.SpaceBetween, + ) { + Column { + Text("Seat ${slot.seat} — ${slot.name}") + Text( + text = when (slot.kind) { + PlayerKind.HUMAN_LOCAL -> "You" + PlayerKind.HUMAN_REMOTE -> "Joined" + PlayerKind.AI -> "Computer" + } + " · Team ${slot.team}", + style = MaterialTheme.typography.bodySmall, + ) + } + if (slot.kind == PlayerKind.HUMAN_REMOTE) { + TextButton(onClick = { + lobby.setSeatKind(slot.seat, PlayerKind.AI, "Computer ${slot.seat}") + }) { Text("Drop") } } } } @@ -150,20 +175,21 @@ private fun HostingBody(lobby: LobbyController, state: LobbyController.State) { @Composable private fun JoiningBody( env: AppEnv, - discovered: List, + state: LobbyController.State, playerName: String, ) { + val discovered = state.discovered + if (discovered.isEmpty()) { Text("No games found yet. Make sure the host has started theirs.") Text( - text = "Both devices need to be on the same Wi-Fi network.", + text = "Both phones need to be on the same Wi-Fi — including one phone's " + + "hotspot, though over a hotspot the game often has to be joined by " + + "address rather than found.", style = MaterialTheme.typography.bodySmall, ) - return - } - - LazyColumn(verticalArrangement = Arrangement.spacedBy(6.dp)) { - items(discovered) { host -> + } else { + discovered.forEach { host -> Card( modifier = Modifier .fillMaxWidth() @@ -179,4 +205,44 @@ private fun JoiningBody( } } } + + Divider() + ManualJoin(env, playerName) +} + +/** + * Joining by address. + * + * Always offered rather than kept for when discovery has failed, because over a + * phone's hotspot discovery does not fail loudly — it just never finds + * anything, and a screen that only says "no games found" gives no way forward. + */ +@Composable +private fun ManualJoin(env: AppEnv, playerName: String) { + var typed by remember { mutableStateOf("") } + + Text("Join by address", fontWeight = FontWeight.Bold) + Text( + text = "The host's screen shows where it is. The port can be left off unless " + + "the host is showing a different one.", + style = MaterialTheme.typography.bodySmall, + ) + Row( + modifier = Modifier.fillMaxWidth(), + horizontalArrangement = Arrangement.spacedBy(8.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + OutlinedTextField( + value = typed, + onValueChange = { typed = it }, + singleLine = true, + label = { Text("192.168.43.1") }, + keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Uri), + modifier = Modifier.weight(1f), + ) + Button( + onClick = { env.lobby.joinAt(typed, playerName, env.peerId) }, + enabled = typed.isNotBlank(), + ) { Text("Connect") } + } } diff --git a/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt b/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt new file mode 100644 index 0000000..f8369da --- /dev/null +++ b/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt @@ -0,0 +1,106 @@ +package org.prolibertate.games.net + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * Reading an address somebody has typed off another phone's screen. + * + * This is the way into a game when discovery cannot work, which over a phone's + * own hotspot is the normal case — so it has to accept what a person will + * actually type, including the trailing space the keyboard adds and the port + * they will usually leave off. + */ +class ManualAddressTest { + + @Test + fun `a bare address gets the usual port`() { + assertEquals( + ManualAddress("192.168.43.1", DEFAULT_HOST_PORT), + parseManualAddress("192.168.43.1"), + ) + } + + @Test + fun `a port after a colon is used instead`() { + assertEquals( + ManualAddress("192.168.43.1", 50000), + parseManualAddress("192.168.43.1:50000"), + ) + } + + /** Typing on a phone puts spaces in places nobody intended. */ + @Test + fun `surrounding space is ignored`() { + assertEquals( + ManualAddress("192.168.1.20", DEFAULT_HOST_PORT), + parseManualAddress(" 192.168.1.20 "), + ) + assertEquals( + ManualAddress("192.168.1.20", 47654), + parseManualAddress(" 192.168.1.20 : 47654 "), + ) + } + + @Test + fun `a hostname works as well as an address`() { + assertEquals( + ManualAddress("pixel-9.local", DEFAULT_HOST_PORT), + parseManualAddress("pixel-9.local"), + ) + } + + @Test + fun `nothing typed is nothing to connect to`() { + assertNull(parseManualAddress("")) + assertNull(parseManualAddress(" ")) + } + + @Test + fun `a port that is not a number is refused rather than guessed`() { + assertNull(parseManualAddress("192.168.43.1:door")) + assertNull(parseManualAddress("192.168.43.1:")) + } + + @Test + fun `a port outside the range is refused`() { + assertNull(parseManualAddress("192.168.43.1:0")) + assertNull(parseManualAddress("192.168.43.1:70000")) + assertNull(parseManualAddress("192.168.43.1:-1")) + } + + @Test + fun `an address with a hole in it is refused`() { + assertNull(parseManualAddress(":47654")) + assertNull(parseManualAddress("192.168.43.1 20:47654")) + } + + /** + * The default is only a default. A host that found the usual port taken + * shows a different one, and that is what gets typed. + */ + @Test + fun `the default port is only used when none is given`() { + assertEquals(DEFAULT_HOST_PORT, parseManualAddress("10.0.0.5")?.port) + assertEquals(1234, parseManualAddress("10.0.0.5:1234")?.port) + } + + @Test + fun `an endpoint reads out as address and port`() { + val endpoint = HostEndpoint(listOf("192.168.43.1", "10.0.0.5"), DEFAULT_HOST_PORT) + assertEquals("192.168.43.1:$DEFAULT_HOST_PORT", endpoint.primary) + } + + @Test + fun `an endpoint with no addresses has nothing to read out`() { + assertNull(HostEndpoint(emptyList(), DEFAULT_HOST_PORT).primary) + } + + /** What the host shows must be what the joiner can type. */ + @Test + fun `what a host shows is what the other end accepts`() { + val shown = HostEndpoint(listOf("192.168.43.1"), 47654).primary!! + assertEquals(ManualAddress("192.168.43.1", 47654), parseManualAddress(shown)) + } +} From deb4acf96be4d2f00dc7f1322257364445dcfd4e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 00:25:54 +0000 Subject: [PATCH 2/2] Give the hotspot's owner a route to its guests too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Binding the join socket to the Wi-Fi network covers a phone that has joined a hotspot. It does nothing for the phone that is sharing one: that phone has no Wi-Fi *client* network, so there was nothing to bind to and the code gave up — leaving the socket on the default network, which is mobile data, and a connection to a guest going out over cellular to die. ConnectivityManager does not offer the tethering interface as a Network, so there is no network to name. Binding this end of the socket to one of our own addresses on the guest's subnet picks the route instead, which is the only lever available. The subnet test assumes a /24, which every Android hotspot hands out, and getting it wrong costs a bind that would not have helped anyway. This is the direction that was left broken: the owner of the hotspot trying to join a game hosted by one of its guests. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0132V9gE6DF9itz2z3Hn7BK9 --- .../prolibertate/games/net/LanTransport.kt | 4 +- .../prolibertate/games/net/LocalNetwork.kt | 44 ++++++++++++++++--- .../games/net/ManualAddressTest.kt | 30 +++++++++++++ 3 files changed, 69 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/org/prolibertate/games/net/LanTransport.kt b/app/src/main/java/org/prolibertate/games/net/LanTransport.kt index 941ba87..cf939ce 100644 --- a/app/src/main/java/org/prolibertate/games/net/LanTransport.kt +++ b/app/src/main/java/org/prolibertate/games/net/LanTransport.kt @@ -201,11 +201,11 @@ class LanTransport(private val context: Context) : Transport { override suspend fun join(host: DiscoveredHost, scope: CoroutineScope): Connection = withContext(Dispatchers.IO) { val socket = Socket() - // Before connecting, and this is the whole fix for hotspot play: a + // Before connecting, and this is the fix for hotspot play: a // hotspot has no internet behind it, so Android leaves mobile data // as the default network and an unbound socket goes out over // cellular looking for a 192.168 address. - bindToWifi(context, socket) + bindForTarget(context, socket, host.address) socket.connect(InetSocketAddress(host.address, host.port), CONNECT_TIMEOUT_MS) socket.tcpNoDelay = true StreamConnection( diff --git a/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt b/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt index cb8c9f7..29ce3c0 100644 --- a/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt +++ b/app/src/main/java/org/prolibertate/games/net/LocalNetwork.kt @@ -6,6 +6,8 @@ import android.net.Network import android.net.NetworkCapabilities import android.net.wifi.WifiManager import java.net.Inet4Address +import java.net.InetAddress +import java.net.InetSocketAddress import java.net.NetworkInterface import java.net.Socket @@ -40,14 +42,42 @@ fun wifiNetwork(context: Context): Network? { } /** - * Sends this socket out over the Wi-Fi rather than wherever the system would - * otherwise send it. Silent if there is no Wi-Fi network to bind to: a phone - * running the hotspot itself has no Wi-Fi *client* network, and reaches its own - * guests over the tethering interface's own route without any help. + * Points this socket at the network [target] is actually on, before it + * connects. + * + * Two different problems wearing the same hat, depending on which end of the + * hotspot this phone is. + * + * A phone that has *joined* a hotspot has a Wi-Fi network, and naming it is + * enough. A phone that is *sharing* one does not: its guests live on the + * tethering interface, and ConnectivityManager does not offer that as a + * [Network] anything can bind to — so the socket falls back to the default + * network, which is mobile data, and a connection to a guest goes out over + * cellular and dies. Binding this end to an address on the guest's own subnet + * picks the route instead, which is the only lever available. */ -fun bindToWifi(context: Context, socket: Socket) { - val network = wifiNetwork(context) ?: return - runCatching { network.bindSocket(socket) } +fun bindForTarget(context: Context, socket: Socket, target: String) { + val network = wifiNetwork(context) + if (network != null) { + runCatching { network.bindSocket(socket) } + return + } + val source = localIpv4Addresses().firstOrNull { sameSubnet(it, target) } ?: return + runCatching { socket.bind(InetSocketAddress(InetAddress.getByName(source), 0)) } +} + +/** + * Whether two IPv4 addresses are on the same /24. + * + * A guess, but a safe one here: every Android hotspot hands out a /24, and the + * only thing this decides is which of our own addresses to send from. Getting + * it wrong costs a bind that would not have helped anyway. + */ +fun sameSubnet(a: String, b: String): Boolean { + val left = a.split('.') + val right = b.split('.') + if (left.size != 4 || right.size != 4) return false + return left.subList(0, 3) == right.subList(0, 3) } /** diff --git a/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt b/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt index f8369da..755eab7 100644 --- a/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt +++ b/app/src/test/java/org/prolibertate/games/net/ManualAddressTest.kt @@ -1,7 +1,9 @@ package org.prolibertate.games.net import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test /** @@ -103,4 +105,32 @@ class ManualAddressTest { val shown = HostEndpoint(listOf("192.168.43.1"), 47654).primary!! assertEquals(ManualAddress("192.168.43.1", 47654), parseManualAddress(shown)) } + + // -- Choosing which of our own addresses to send from -------------------- + + /** + * The phone sharing the hotspot has no Wi-Fi network to bind to, so it + * picks a source address on the guest's subnet instead. Getting this wrong + * sends the connection out over mobile data. + */ + @Test + fun `addresses on one hotspot subnet match`() { + assertTrue(sameSubnet("192.168.43.1", "192.168.43.117")) + assertTrue(sameSubnet("192.168.1.20", "192.168.1.20")) + } + + @Test + fun `addresses on different subnets do not`() { + assertFalse(sameSubnet("192.168.43.1", "192.168.1.5")) + assertFalse(sameSubnet("192.168.43.1", "10.0.0.5")) + } + + /** Anything that is not a dotted quad is not a match, rather than a crash. */ + @Test + fun `nonsense is not a match`() { + assertFalse(sameSubnet("", "192.168.43.1")) + assertFalse(sameSubnet("192.168.43", "192.168.43.1")) + assertFalse(sameSubnet("pixel-9.local", "192.168.43.1")) + assertFalse(sameSubnet("192.168.43.1.7", "192.168.43.1")) + } }