diff --git a/app/src/main/java/com/rtbishop/look4sat/AprsForegroundService.kt b/app/src/main/java/com/rtbishop/look4sat/AprsForegroundService.kt index 03dc41ca..472f2bff 100644 --- a/app/src/main/java/com/rtbishop/look4sat/AprsForegroundService.kt +++ b/app/src/main/java/com/rtbishop/look4sat/AprsForegroundService.kt @@ -91,10 +91,13 @@ class AprsForegroundService : Service() { AprsStore.saveLastReport(this, report.ok, report.detail) updateNotification(cfg) // Report result always surfaces: success = short Toast, failure = long Toast + reason - val msg = if (report.ok) { - getString(R.string.aprs_toast_ok) - } else { - getString(R.string.aprs_toast_fail, report.detail) + // An unverified login needs its own message: the write succeeded, so a bare + // failure notice would send the operator looking at their network when the + // problem is the passcode - and APRS-IS is dropping every packet meanwhile. + val msg = when { + report.ok -> getString(R.string.aprs_toast_ok) + !report.verified -> getString(R.string.aprs_toast_unverified) + else -> getString(R.string.aprs_toast_fail, report.detail) } runCatching { Toast.makeText(this, msg, diff --git a/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsIsClient.kt b/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsIsClient.kt index 7bb9e189..01e88479 100644 --- a/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsIsClient.kt +++ b/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsIsClient.kt @@ -1,16 +1,23 @@ package com.rtbishop.look4sat.core.data.aprs +import com.rtbishop.look4sat.core.domain.aprs.AprsLogin import com.rtbishop.look4sat.core.domain.aprs.AprsPacket import java.io.BufferedReader +import java.io.IOException import java.io.InputStreamReader import java.io.OutputStreamWriter import java.io.PrintWriter import java.net.InetSocketAddress import java.net.Socket +import java.net.SocketTimeoutException /** - * APRS-IS TCP client (reverse-ported from APRSdroid TcpUploader.scala). - * Plain-text protocol: one login line + one packet per line; 30 s reconnect after drop. + * APRS-IS TCP client. Plain text: the server greets, the client logs in, then one packet per line. + * + * Two things here decide whether the operator can trust the app at all. A packet sent on a dead + * socket must not report success, and a login the server refused to verify must not look like a + * working connection - an unverified client stays connected while the server silently drops + * everything it sends. */ class AprsIsClient( private val host: String, @@ -27,86 +34,178 @@ class AprsIsClient( private var reader: BufferedReader? = null private val lock = Any() - val isConnected: Boolean - get() = synchronized(lock) { socket?.isConnected == true && !socket!!.isClosed } + /** + * What the server said about this login, or null before a login has been attempted. + * + * Kept as state rather than only thrown, because [AprsLogin.Outcome.Unverified] is not a + * connection error: the socket is up and writes succeed. The operator has to be told, or + * they will watch reports "succeed" for hours while nothing reaches the network. + */ + @Volatile + var loginOutcome: AprsLogin.Outcome? = null + private set - /** Connect + login (synchronous/blocking; call from a background thread) */ + val isConnected: Boolean + get() = synchronized(lock) { socket?.isConnected == true && socket?.isClosed == false } + + /** True when the server verified the passcode, so packets from this client are accepted. */ + val isVerified: Boolean get() = loginOutcome is AprsLogin.Outcome.Verified + + /** + * True only when the server told us it did NOT verify the login. + * + * Distinct from `!isVerified` on purpose. [AprsLogin.Outcome.Unverified] means the server + * said so and really is discarding our packets. [AprsLogin.Outcome.Unknown] means we could + * not recognise its answer - the packets may well be landing - so blaming the operator's + * passcode for that would send them to fix something that is not broken. + */ + val isRefusedByServer: Boolean get() = loginOutcome is AprsLogin.Outcome.Unverified + + /** + * Connect and log in. Blocking; call from a background thread. + * + * Throws when the connection cannot be made or the server rejected the login outright. + * A login the server accepted but did not verify returns normally and leaves + * [loginOutcome] as [AprsLogin.Outcome.Unverified] for the caller to surface. + */ @Throws(Exception::class) fun connect() { disconnect() + loginOutcome = null val s = Socket() try { - s.connect(InetSocketAddress(host, port), 30_000) - s.soTimeout = timeoutSec * 1000 + s.connect(InetSocketAddress(host, port), CONNECT_MS) s.tcpNoDelay = true synchronized(lock) { socket = s writer = PrintWriter(OutputStreamWriter(s.getOutputStream(), Charsets.ISO_8859_1), true) reader = BufferedReader(InputStreamReader(s.getInputStream(), Charsets.ISO_8859_1), 256) } - // Login line - val login = AprsPacket.formatLogin(callsign, ssid, passcode, version) + filter - writer?.println(login) - // Read the login response (aprsc replies # logresp ... verified/unverified) - runCatching { - s.soTimeout = 8000 - val resp = reader?.readLine() - if (resp != null && (resp.contains("Invalid", ignoreCase = true) || - resp.contains("unverified", ignoreCase = true))) { - throw IllegalArgumentException(resp.trim()) - } - // Restore timeout - s.soTimeout = timeoutSec * 1000 + s.soTimeout = LOGIN_MS + // The spec has the client log in AFTER the server's identification line, so read the + // greeting first. Anything starting with # is a comment and may be skipped. + readGreeting(s)?.let { refusal -> + // The server refused before we even logged in. Previously this verdict was + // computed and then overwritten by readLoginResponse, so the branch was a lie. + loginOutcome = refusal + throw IllegalArgumentException(refusal.detail) } + val login = AprsLogin.line(callsign, ssid, passcode, version, filter) + writer?.print(login) + writer?.print(CRLF) + writer?.flush() + loginOutcome = readLoginResponse() + s.soTimeout = timeoutSec * 1000 + val outcome = loginOutcome + if (outcome is AprsLogin.Outcome.Rejected) throw IllegalArgumentException(outcome.detail) } catch (e: Exception) { - // Close the local socket before re-throwing, so it does not leak when - // an exception is raised after s.connect() but before socket = s. - // Otherwise periodic reconnect attempts (AprsReporter every 1–60 min) - // accumulate leaked fds until the process cannot open any more files. + // Close the local socket before re-throwing so it does not leak when the failure + // lands after connect() but before the field assignment - otherwise periodic + // reconnects accumulate file descriptors until no more can be opened. runCatching { s.close() } + synchronized(lock) { + writer = null + reader = null + socket = null + } throw e } } /** - * Sends one APRS packet (one line) and tries to read the server ack. - * Returns null=failed to send; Pair(ok, detail)=result (server error text lives in detail) + * Consume the server's greeting comment, returning a refusal when it is not one. + * + * Absence is tolerated because some servers send none, but on a short probe window rather + * than the full login timeout: waiting LOGIN_MS for a greeting that will never come cost + * eight seconds on every single connect to such a server. + */ + private fun readGreeting(socket: Socket): AprsLogin.Outcome.Rejected? { + val previous = socket.soTimeout + return try { + socket.soTimeout = GREETING_MS + val line = reader?.readLine() ?: return null + // A server that opens with anything but a comment is refusing us. + if (line.startsWith("#")) null else AprsLogin.Outcome.Rejected(line.trim()) + } catch (ignored: IOException) { + null + } finally { + runCatching { socket.soTimeout = previous } + } + } + + /** + * Read lines until the login verdict arrives, skipping keepalive comments. + * + * Bounded by the read timeout, so an unresponsive server cannot hang the caller. + */ + private fun readLoginResponse(): AprsLogin.Outcome { + // Bounded by a deadline, not a line count: comments are free to skip, and a chatty + // server that sent six of them before its verdict used to exhaust a fixed budget and + // turn an accepted login into Unknown - telling the operator their passcode was wrong + // when it had just been accepted. + val deadline = System.currentTimeMillis() + LOGIN_MS + while (System.currentTimeMillis() < deadline) { + val line = try { + reader?.readLine() + } catch (timeout: SocketTimeoutException) { + return AprsLogin.Outcome.Unknown("no response within ${LOGIN_MS}ms") + } catch (failure: IOException) { + return AprsLogin.Outcome.Rejected(failure.message ?: "login read failed") + } ?: return AprsLogin.Outcome.Rejected("connection closed during login") + AprsLogin.parse(line)?.let { return it } + } + return AprsLogin.Outcome.Unknown("no login response recognised") + } + + /** + * Send one packet and report what happened. + * + * Returns null when there is no connection to write to. Otherwise a pair of whether the + * packet went out and a detail string for the operator. */ fun sendPacket(packetLine: String): Pair? { synchronized(lock) { val w = writer ?: return null - w.println(packetLine) + // Reading the response inside the same lock: disconnect() may run concurrently and + // null these fields, and reading outside the lock raced with that - the read could + // hit a just-closed socket and be reported as a successful send. + // CRLF explicitly rather than println: the spec requires "TNC2 format terminated by + // a carriage return, line feed sequence", and println emits the platform separator, + // a bare LF on Android. An earlier draft of this method sent no terminator at all, + // which leaves the server's line reader waiting forever while every send reports + // success - exactly the failure this class exists to prevent. + w.print(packetLine) + w.print(CRLF) + w.flush() if (w.checkError()) return Pair(false, "write failed") - // Read the server response inside the same lock: disconnect() (called - // concurrently from stop()/reconnect on another thread) nulls - // writer/reader/socket and closes them. Reading outside the lock raced - // with that: the response read could hit a just-closed socket and the - // swallowing runCatching reported Pair(true,"OK") for a packet that - // never left, or read through a stale reference. Serialising keeps - // the read on the connection this thread just wrote to. The 3 s read - // timeout bounds how long a concurrent disconnect waits. - return runCatching { - val s = socket ?: return@runCatching Pair(true, "OK") - val oldTimeout = s.soTimeout - s.soTimeout = 3000 - try { - val resp = reader?.readLine() - if (resp != null && (resp.contains("Invalid", ignoreCase = true) || - resp.contains("error", ignoreCase = true))) { - Pair(false, resp.trim()) - } else { - Pair(true, if (resp.isNullOrBlank()) "OK" else resp.trim()) - } - } finally { - s.soTimeout = oldTimeout - } - }.getOrElse { Pair(true, "OK") } + val s = socket ?: return Pair(false, "not connected") + val previousTimeout = s.soTimeout + return try { + s.soTimeout = ACK_MS + classifyAck(reader?.readLine()) + } catch (timeout: SocketTimeoutException) { + // Silence is the normal case: APRS-IS does not acknowledge a position report, so + // nothing arriving means the line went out and the server had nothing to say. + // Telling this apart from a broken connection is the point of this method - the + // previous version treated EVERY exception as success, so a dead socket reported + // "sent OK" and made every real failure invisible, including a rejected login. + Pair(true, "sent") + } catch (failure: IOException) { + Pair(false, failure.message ?: "read failed") + } finally { + runCatching { s.soTimeout = previousTimeout } + } } } - /** Read one line (server response; throws on timeout) */ - fun readLine(): String? { - return reader?.readLine() + /** Interpret whatever the server sent back after a packet. */ + private fun classifyAck(response: String?): Pair = when { + response == null -> Pair(false, "connection closed by server") + // Comments are keepalives and server chatter, not a verdict on this packet. + response.startsWith("#") -> Pair(true, "sent") + response.contains("Invalid", ignoreCase = true) || + response.contains("error", ignoreCase = true) -> Pair(false, response.trim()) + else -> Pair(true, response.trim()) } fun disconnect() { @@ -119,4 +218,16 @@ class AprsIsClient( socket = null } } + + private companion object { + /** Line terminator the protocol requires, independent of the platform's own. */ + const val CRLF = "\r\n" + + const val CONNECT_MS = 30_000 + const val LOGIN_MS = 8_000 + const val ACK_MS = 3_000 + + /** Probe window for the greeting, short because its absence is legitimate. */ + const val GREETING_MS = 2_000 + } } diff --git a/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsReporter.kt b/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsReporter.kt index 7cb185f7..669e27d5 100644 --- a/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsReporter.kt +++ b/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsReporter.kt @@ -34,7 +34,17 @@ data class AprsReport( val timestamp: Long, val packet: String, val ok: Boolean, - val detail: String + val detail: String, + /** + * False only when the server told us it did not verify the login. + * + * Carried separately from [ok] because the two are independent: a refused client's writes + * still succeed, so the packet leaves the phone and looks sent, while aprsc discards every + * one of them. Without surfacing it the operator can watch reports succeed for hours with + * nothing reaching the network. A login whose response we simply could not parse leaves this + * true, since the packets may be landing and the passcode is not at fault. + */ + val verified: Boolean = true ) /** Report scheduler (periodic + manual trigger); connection management lives in the foreground service */ @@ -100,15 +110,22 @@ class AprsReporter( val pos = positionProvider() val packetLine = buildPositionPacket(cfg, pos?.first, pos?.second) val result = c.sendPacket(packetLine) - val ok = result?.first == true + val sent = result?.first == true val detail = result?.second ?: "no connection" - onReport(AprsReport(System.currentTimeMillis(), packetLine, ok, detail)) + // A write that succeeded on a login the server refused to verify is not a delivered + // packet: aprsc takes it and drops it, which is what let every real failure hide. + // Only an explicit refusal counts against us though - a login whose response we + // could not parse may be working fine, and blaming the passcode for that would send + // the operator to fix something that is not broken. + val refused = c.isRefusedByServer + val ok = sent && !refused + onReport(AprsReport(System.currentTimeMillis(), packetLine, ok, detail, !refused)) if (ok) onState(AprsState.Connected) else onState(AprsState.Error) } catch (e: Exception) { runCatching { client?.disconnect() } client = null onState(AprsState.Error) - onReport(AprsReport(System.currentTimeMillis(), "", false, e.message ?: "error")) + onReport(AprsReport(System.currentTimeMillis(), "", false, e.message ?: "error", false)) } } diff --git a/core/data/src/test/java/com/rtbishop/look4sat/core/data/aprs/AprsIsClientSocketTest.kt b/core/data/src/test/java/com/rtbishop/look4sat/core/data/aprs/AprsIsClientSocketTest.kt new file mode 100644 index 00000000..0678e84f --- /dev/null +++ b/core/data/src/test/java/com/rtbishop/look4sat/core/data/aprs/AprsIsClientSocketTest.kt @@ -0,0 +1,233 @@ +package com.rtbishop.look4sat.core.data.aprs + +import java.io.BufferedReader +import java.io.InputStreamReader +import java.io.PrintWriter +import java.net.ServerSocket +import java.net.Socket +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import kotlin.concurrent.thread +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Socket-level tests against a stand-in APRS-IS server. + * + * These exist because the pure-logic tests could not catch the failures that matter here. A draft + * of [AprsIsClient.sendPacket] once wrote the packet with no line terminator at all: every send + * reported success, the server received a single unterminated stream, and nothing anywhere went + * red. APRS-IS is a line protocol and does not acknowledge position reports, so silence is the + * normal case - which means only a test that reads the bytes off a real socket can tell a + * delivered packet from a lost one. + */ +class AprsIsClientSocketTest { + + /** + * A minimal APRS-IS server. Greets, answers the login as instructed, then records whatever + * lines arrive without acknowledging them, which is what the real network does. + */ + private class FakeServer( + private val greeting: String? = "# aprsc 2.1.19-g730c5c0", + private val loginResponse: String? = "# logresp TEST verified, server FAKE", + private val chatter: List = emptyList() + ) : AutoCloseable { + private val server = ServerSocket(0) + private val ready = CountDownLatch(1) + + /** + * The accepted connection. Held because closing the ServerSocket only stops it listening + * - an established connection survives, so a test that wants a dead peer has to close + * this one. Getting that wrong made a correct implementation look broken. + */ + @Volatile + private var peer: Socket? = null + + val port: Int get() = server.localPort + + /** Every complete line the client sent after logging in. */ + val received = mutableListOf() + + /** Raw bytes of the client's traffic, so a missing terminator is visible. */ + val rawAfterLogin = StringBuilder() + + @Volatile + var loginLine: String? = null + + fun start() { + thread(isDaemon = true) { + runCatching { + server.accept().use { client -> + peer = client + val out = PrintWriter(client.getOutputStream(), true) + val input = BufferedReader(InputStreamReader(client.getInputStream())) + greeting?.let { out.print(it + "\r\n"); out.flush() } + loginLine = input.readLine() + chatter.forEach { out.print(it + "\r\n"); out.flush() } + loginResponse?.let { out.print(it + "\r\n"); out.flush() } + ready.countDown() + // Read lines but never acknowledge, exactly as APRS-IS treats positions. + while (true) { + val line = input.readLine() ?: break + synchronized(received) { + received += line + rawAfterLogin.append(line) + } + } + } + } + ready.countDown() + } + } + + fun awaitLogin(): Boolean = ready.await(5, TimeUnit.SECONDS) + + fun lines(): List = synchronized(received) { received.toList() } + + /** Close the established connection, so the client is talking to a dead peer. */ + fun dropClient() { + runCatching { peer?.close() } + } + + override fun close() { + runCatching { server.close() } + } + } + + private fun client(port: Int, passcode: Int = 12345) = AprsIsClient( + host = "127.0.0.1", + port = port, + callsign = "TEST", + ssid = "", + passcode = passcode, + version = "Look4Sat-test" + ) + + /** + * The regression that motivated this file. Two packets must arrive as two lines; without a + * terminator they concatenate into one stream the server can never parse, while both sends + * report success. + */ + @Test + fun `each packet arrives as its own line`() { + FakeServer().use { server -> + server.start() + val c = client(server.port) + c.connect() + assertTrue(server.awaitLogin()) + val first = c.sendPacket("TEST>APRS,TCPIP*:=0000.00N/00000.00E>one") + val second = c.sendPacket("TEST>APRS,TCPIP*:=0000.00N/00000.00E>two") + Thread.sleep(300) + c.disconnect() + + assertEquals(true, first?.first) + assertEquals(true, second?.first) + val lines = server.lines() + assertEquals("both packets must reach the server as separate lines", 2, lines.size) + assertTrue(lines[0].endsWith(">one")) + assertTrue(lines[1].endsWith(">two")) + } + } + + /** The login line has to be terminated too, or the server never reads it. */ + @Test + fun `the server receives a complete login line`() { + FakeServer().use { server -> + server.start() + val c = client(server.port) + c.connect() + assertTrue(server.awaitLogin()) + c.disconnect() + assertEquals("user TEST pass 12345 vers Look4Sat-test", server.loginLine) + } + } + + /** A verified login is recognised and lets reports count as delivered. */ + @Test + fun `a verified login is not reported as refused`() { + FakeServer().use { server -> + server.start() + val c = client(server.port) + c.connect() + assertTrue(server.awaitLogin()) + assertTrue(c.isVerified) + assertFalse(c.isRefusedByServer) + c.disconnect() + } + } + + /** + * The case the rewrite exists for: the server accepts the connection, the write succeeds, + * and every packet is discarded. The client must say so rather than report success. + */ + @Test + fun `an unverified login is flagged while the connection stays up`() { + FakeServer(loginResponse = "# logresp TEST unverified, server FAKE").use { server -> + server.start() + val c = client(server.port, passcode = -1) + c.connect() + assertTrue(server.awaitLogin()) + assertFalse("unverified must not read as verified", c.isVerified) + assertTrue("the server explicitly refused", c.isRefusedByServer) + // Not a connection error: a receive-only login is legitimate and stays connected. + assertTrue(c.isConnected) + c.disconnect() + } + } + + /** + * A chatty server used to exhaust a fixed line budget, turning an accepted login into + * Unknown and telling the operator their passcode was wrong when it had been accepted. + */ + @Test + fun `keepalive chatter before the verdict does not hide it`() { + val chatter = List(8) { "# keepalive $it" } + FakeServer(chatter = chatter).use { server -> + server.start() + val c = client(server.port) + c.connect() + assertTrue(server.awaitLogin()) + assertTrue("the verdict must be found past the comments", c.isVerified) + c.disconnect() + } + } + + /** + * A server that sends no greeting is legitimate, and must not cost the full login window on + * every connect - that was eight seconds per attempt. + */ + @Test + fun `a server without a greeting connects promptly`() { + FakeServer(greeting = null).use { server -> + server.start() + val c = client(server.port) + val started = System.currentTimeMillis() + c.connect() + assertTrue(server.awaitLogin()) + val elapsed = System.currentTimeMillis() - started + c.disconnect() + assertTrue("connect took ${elapsed}ms, expected well under the login window", + elapsed < 6_000) + } + } + + /** Sending after the server has gone must report failure, not success. */ + @Test + fun `a send after the server closes is reported as failed`() { + val server = FakeServer() + server.start() + val c = client(server.port) + c.connect() + assertTrue(server.awaitLogin()) + // Closing the ServerSocket alone would leave this connection alive. + server.dropClient() + Thread.sleep(200) + // The first write may still land in the socket buffer; by the second the loss is certain. + c.sendPacket("TEST>APRS,TCPIP*:=0000.00N/00000.00E>one") + val second = c.sendPacket("TEST>APRS,TCPIP*:=0000.00N/00000.00E>two") + c.disconnect() + assertEquals("a send on a dead connection must not report success", false, second?.first) + } +} diff --git a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsLogin.kt b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsLogin.kt new file mode 100644 index 00000000..3ee48a57 --- /dev/null +++ b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsLogin.kt @@ -0,0 +1,116 @@ +/* + * Look4Sat. Amateur radio satellite tracker and pass predictor. + * Copyright (C) 2019-2026 Arty Bishop and contributors. + * + * 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.rtbishop.look4sat.core.domain.aprs + +/** + * The APRS-IS login line and the verdict the server returns for it. + * + * Kept apart from the socket so it can be tested: whether a login was verified decides whether + * anything this app sends reaches the network, and that distinction used to be made by a + * substring check inside a runCatching whose result was discarded, so it could not fail loudly. + * + * Format per aprs-is.net/connecting.aspx: + * `user mycall[-ss] pass passcode [vers softwarename softwarevers [filter ...]]` + */ +object AprsLogin { + + /** What the server decided about a login attempt. */ + sealed interface Outcome { + + /** The passcode matched the callsign. Packets from this client are accepted. */ + data class Verified(val callsign: String) : Outcome + + /** + * The server accepted the connection but did not verify the login. + * + * Not an error at the socket level, which is exactly why it needs surfacing: writes keep + * succeeding while the server discards every packet. A receive-only login (passcode -1) + * lands here legitimately. + */ + data class Unverified(val callsign: String) : Outcome + + /** The server refused the login outright. */ + data class Rejected(val detail: String) : Outcome + + /** Nothing recognisable arrived. The connection may still work; we simply do not know. */ + data class Unknown(val detail: String) : Outcome + } + + /** Passcode value that asks for a receive-only connection. */ + const val RECEIVE_ONLY_PASSCODE = -1 + + /** + * Build the login line. + * + * [version] must not contain a space: the server splits `vers` into a software name and a + * version, so an embedded space shifts every field after it. Any space becomes a hyphen + * rather than silently corrupting the line. + */ + fun line( + callsign: String, + ssid: String, + passcode: Int, + version: String, + filter: String = "" + ): String { + val callSsid = AprsPacket.formatCallSsid(callsign, ssid) + val safeVersion = version.trim().replace(' ', '-').ifEmpty { "Look4Sat" } + val base = "user $callSsid pass $passcode vers $safeVersion" + val trimmedFilter = filter.trim() + return if (trimmedFilter.isEmpty()) base else "$base $trimmedFilter" + } + + /** + * Interpret one line of server output, or null when it carries no verdict. + * + * Returning null for keepalives and identification comments lets the caller keep reading + * rather than treating the first comment it sees as an answer. + * + * aprsc answers `# logresp CALL verified, server X` or `# logresp CALL unverified, server X`. + * Note that "unverified" contains "verified", so the negative has to be tested first - a + * naive contains("verified") reports every rejected login as accepted. + */ + fun parse(line: String): Outcome? { + val trimmed = line.trim() + if (trimmed.isEmpty()) return null + if (!trimmed.startsWith("#")) { + // A non-comment line during login is the server objecting in plain text. + return Outcome.Rejected(trimmed) + } + val lower = trimmed.lowercase() + if (!lower.contains("logresp")) { + // Server identification and keepalive comments carry no verdict. + return null + } + val callsign = callsignFrom(trimmed) + return when { + lower.contains("unverified") -> Outcome.Unverified(callsign) + lower.contains("verified") -> Outcome.Verified(callsign) + lower.contains("invalid") || lower.contains("error") -> Outcome.Rejected(trimmed) + else -> Outcome.Unknown(trimmed) + } + } + + /** The callsign token in `# logresp CALL verified, ...`, or empty when absent. */ + private fun callsignFrom(response: String): String { + val tokens = response.removePrefix("#").trim().split(Regex("\\s+")) + val index = tokens.indexOfFirst { it.equals("logresp", ignoreCase = true) } + if (index < 0) return "" + return tokens.getOrNull(index + 1)?.trimEnd(',') ?: "" + } +} diff --git a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsPacket.kt b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsPacket.kt index 3b1d4c0f..63ed3497 100644 --- a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsPacket.kt +++ b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsPacket.kt @@ -22,12 +22,6 @@ object AprsPacket { return hash and 0x7FFF } - /** Login line: user CALL-SSID pass XXXX vers XXXX */ - fun formatLogin(callsign: String, ssid: String, passcode: Int, version: String): String { - val callSsid = formatCallSsid(callsign, ssid) - return "user $callSsid pass $passcode vers $version" - } - /** Callsign-SSID join (BG7NTA + 5 -> BG7NTA-5) */ fun formatCallSsid(callsign: String, ssid: String): String { if (ssid.isNullOrEmpty()) return callsign diff --git a/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/aprs/AprsLoginTest.kt b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/aprs/AprsLoginTest.kt new file mode 100644 index 00000000..bafee14d --- /dev/null +++ b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/aprs/AprsLoginTest.kt @@ -0,0 +1,122 @@ +package com.rtbishop.look4sat.core.domain.aprs + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The verified/unverified distinction is the whole point of these tests: an unverified client + * stays connected and its writes keep succeeding while the server discards every packet, so + * getting this wrong means the operator watches reports "succeed" for hours with nothing landing. + */ +class AprsLoginTest { + + @Test + fun `builds the line the specification asks for`() { + assertEquals( + "user BG7NTA-5 pass 12345 vers Look4Sat-4.6.0", + AprsLogin.line("BG7NTA", "5", 12345, "Look4Sat-4.6.0") + ) + } + + /** No SSID means no hyphen; the spec says never to write -0 explicitly. */ + @Test + fun `omits the ssid when there is none`() { + assertEquals( + "user BG7NTA pass 12345 vers Look4Sat", + AprsLogin.line("BG7NTA", "", 12345, "Look4Sat") + ) + } + + /** + * The server splits `vers` into name and version, so a space inside it shifts every field + * after that point. The shipped value was "Look4Sat 4.5.4", which parsed correctly only by + * luck - and only because nothing followed it. + */ + @Test + fun `a space in the version cannot shift the following fields`() { + val line = AprsLogin.line("BG7NTA", "5", 12345, "Look4Sat 4.6.0") + assertEquals("user BG7NTA-5 pass 12345 vers Look4Sat-4.6.0", line) + // Six tokens: user, callsign, pass, passcode, vers, version. A space inside the version + // would make seven and push the server's field parsing one place along. + assertEquals(6, line.split(" ").size) + } + + @Test + fun `appends a filter when one is given`() { + val line = AprsLogin.line("BG7NTA", "5", 12345, "Look4Sat", "r/33.25/-96.5/50") + assertEquals("user BG7NTA-5 pass 12345 vers Look4Sat r/33.25/-96.5/50", line) + } + + @Test + fun `receive-only logins use the documented passcode`() { + val line = AprsLogin.line("BG7NTA", "", AprsLogin.RECEIVE_ONLY_PASSCODE, "Look4Sat") + assertEquals("user BG7NTA pass -1 vers Look4Sat", line) + } + + /** + * "unverified" contains "verified", so the negative has to be tested first. A naive + * contains("verified") reports every rejected login as accepted - which is precisely the + * failure this replaces. + */ + @Test + fun `unverified is not mistaken for verified`() { + val outcome = AprsLogin.parse("# logresp BG7NTA unverified, server T2SYDNEY") + assertEquals(AprsLogin.Outcome.Unverified("BG7NTA"), outcome) + } + + @Test + fun `a verified login is recognised with its callsign`() { + val outcome = AprsLogin.parse("# logresp BG7NTA-5 verified, server T2SYDNEY") + assertEquals(AprsLogin.Outcome.Verified("BG7NTA-5"), outcome) + } + + /** + * Server identification and keepalive comments must return null so the caller keeps reading. + * Treating the first comment as an answer would leave every login Unknown. + */ + @Test + fun `comments without a verdict are skipped`() { + assertNull(AprsLogin.parse("# aprsc 2.1.19-g730c5c0")) + assertNull(AprsLogin.parse("# Tue Aug 25 08:00:00 UTC 2026")) + assertNull(AprsLogin.parse("")) + assertNull(AprsLogin.parse(" ")) + } + + /** A non-comment line during login is the server objecting in plain text. */ + @Test + fun `a plain text line during login is a rejection`() { + val outcome = AprsLogin.parse("Invalid callsign format") + assertTrue(outcome is AprsLogin.Outcome.Rejected) + assertEquals("Invalid callsign format", (outcome as AprsLogin.Outcome.Rejected).detail) + } + + /** A logresp with no recognisable verdict is Unknown, not silently Verified. */ + @Test + fun `an unrecognised logresp is not treated as success`() { + val outcome = AprsLogin.parse("# logresp BG7NTA something-new, server X") + assertTrue(outcome is AprsLogin.Outcome.Unknown) + } + + @Test + fun `case does not change the verdict`() { + assertEquals( + AprsLogin.Outcome.Unverified("BG7NTA"), + AprsLogin.parse("# LOGRESP BG7NTA UNVERIFIED, server X") + ) + assertEquals( + AprsLogin.Outcome.Verified("BG7NTA"), + AprsLogin.parse("# LogResp BG7NTA Verified, server X") + ) + } + + /** The callsign is read back so the UI can show whose login the server acknowledged. */ + @Test + fun `reads the callsign even when the line is oddly spaced`() { + assertEquals( + AprsLogin.Outcome.Verified("BG7NTA-9"), + AprsLogin.parse("# logresp BG7NTA-9 verified, server X") + ) + } +} diff --git a/core/presentation/src/main/res/values-id/strings.xml b/core/presentation/src/main/res/values-id/strings.xml index 3b826607..a0754bdb 100644 --- a/core/presentation/src/main/res/values-id/strings.xml +++ b/core/presentation/src/main/res/values-id/strings.xml @@ -38,6 +38,7 @@ Hitung kode sandi APRS: laporan terkirim APRS: laporan gagal - %1$s + APRS-IS tidak memverifikasi passcode Anda. Laporan Anda tidak diteruskan - periksa tanda panggil dan passcode di Pengaturan. APRS belum dikonfigurasi - isi panggilan dulu Laporan terakhir: %1$s %2$s OK diff --git a/core/presentation/src/main/res/values-in/strings.xml b/core/presentation/src/main/res/values-in/strings.xml index ee735352..59135a42 100644 --- a/core/presentation/src/main/res/values-in/strings.xml +++ b/core/presentation/src/main/res/values-in/strings.xml @@ -38,6 +38,7 @@ Hitung kode sandi APRS: laporan terkirim APRS: laporan gagal - %1$s + APRS-IS tidak memverifikasi passcode Anda. Laporan Anda tidak diteruskan - periksa tanda panggil dan passcode di Pengaturan. APRS belum dikonfigurasi - isi panggilan dulu Laporan terakhir: %1$s %2$s OK diff --git a/core/presentation/src/main/res/values-tr/strings.xml b/core/presentation/src/main/res/values-tr/strings.xml index 8590119e..8fcb7c10 100644 --- a/core/presentation/src/main/res/values-tr/strings.xml +++ b/core/presentation/src/main/res/values-tr/strings.xml @@ -39,6 +39,7 @@ Parolayı hesapla APRS: rapor gönderildi APRS: rapor başarısız - %1$s + APRS-IS passcode bilginizi doğrulamadı. Raporlarınız iletilmiyor - Ayarlar bölümünde çağrı işareti ve passcode bilgisini kontrol edin. APRS yapılandırılmadı - önce çağrı girin Son rapor: %1$s %2$s OK diff --git a/core/presentation/src/main/res/values-zh/strings.xml b/core/presentation/src/main/res/values-zh/strings.xml index bce6c5cb..40603af6 100644 --- a/core/presentation/src/main/res/values-zh/strings.xml +++ b/core/presentation/src/main/res/values-zh/strings.xml @@ -40,6 +40,7 @@ 计算验证码 APRS: 上报成功 APRS: 上报失败 - %1$s + APRS-IS 未验证你的 passcode, 上报不会被转发 - 请在设置里检查呼号与 passcode。 APRS 未配置,请先填呼号 上次上报: %1$s %2$s 成功 diff --git a/core/presentation/src/main/res/values/strings.xml b/core/presentation/src/main/res/values/strings.xml index bc765fc6..bbe29515 100644 --- a/core/presentation/src/main/res/values/strings.xml +++ b/core/presentation/src/main/res/values/strings.xml @@ -41,6 +41,7 @@ Compute passcode APRS: report sent OK APRS: report failed - %1$s + APRS-IS did not verify your passcode. Your reports are not being passed on - check the callsign and passcode in Settings. APRS not configured - set callsign first Last report: %1$s %2$s OK