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 d85fc8fc..59416e23 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 @@ -199,14 +199,24 @@ class AprsIsClient( } } - /** 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()) + /** + * Interpret whatever the server sent back after a packet. + * + * Shares AprsLogin's judgement rather than keeping its own, because the two disagreed in a way + * that mattered: treating any leading `#` as harmless meant `# Port full` and `# Login by user + * not allowed` - both of which mean the server is about to drop us - were reported as a + * successful send. APRS-IS does not acknowledge position reports, so a harmless comment still + * counts as sent; anything the server says that is not harmless does not. + */ + private fun classifyAck(response: String?): Pair { + if (response == null) return Pair(false, "connection closed by server") + return when (val verdict = AprsLogin.parse(response)) { + // A keepalive or identification comment: no verdict, so the write stands. + null -> Pair(true, "sent") + is AprsLogin.Outcome.Rejected -> Pair(false, verdict.detail) + // A late login verdict is not about this packet, and loginOutcome already carries it. + else -> Pair(true, "sent") + } } fun disconnect() { 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 index 122560ad..8d995dff 100644 --- 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 @@ -32,7 +32,9 @@ class AprsIsClientSocketTest { 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() + private val chatter: List = emptyList(), + /** Sent right after the first packet arrives, to exercise the ack read. */ + private val afterPacket: String? = null ) : AutoCloseable { private val server = ServerSocket(0) private val ready = CountDownLatch(1) @@ -69,12 +71,17 @@ class AprsIsClientSocketTest { loginResponse?.let { out.print(it + "\r\n"); out.flush() } ready.countDown() // Read lines but never acknowledge, exactly as APRS-IS treats positions. + var firstPacket = true while (true) { val line = input.readLine() ?: break synchronized(received) { received += line rawAfterLogin.append(line) } + if (firstPacket) { + firstPacket = false + afterPacket?.let { out.print(it + "\r\n"); out.flush() } + } } } } @@ -245,6 +252,41 @@ class AprsIsClientSocketTest { } } + /** + * A server saying it is about to drop us must not read as a successful send. + * + * The ack read used to treat any leading `#` as harmless chatter, so `# Port full` - which + * means the server is closing the connection - was reported as sent. It now shares the login + * parser's judgement, so only a greeting or keepalive counts as harmless. + */ + @Test + fun `a server refusal after the write is not reported as sent`() { + FakeServer(afterPacket = "# Port full").use { server -> + server.start() + val c = client(server.port) + c.connect() + assertTrue(server.awaitLogin()) + val result = c.sendPacket("TEST>APRS,TCPIP*:=0000.00N/00000.00E>x") + c.disconnect() + assertEquals("a server refusal must fail the report", false, result?.first) + } + } + + /** The real keepalive must still count as sent, since APRS-IS never acknowledges a position. */ + @Test + fun `a keepalive after the write still counts as sent`() { + val keepalive = "# aprsc 2.1.21-gbfc2090 25 Aug 2026 16:41:07 GMT T2UK 1.2.3.4:14580" + FakeServer(afterPacket = keepalive).use { server -> + server.start() + val c = client(server.port) + c.connect() + assertTrue(server.awaitLogin()) + val result = c.sendPacket("TEST>APRS,TCPIP*:=0000.00N/00000.00E>x") + c.disconnect() + assertEquals("a keepalive must not fail the report", true, result?.first) + } + } + /** Sending after the server has gone must report failure, not success. */ @Test fun `a send after the server closes is reported as failed`() {