From 2ff864398828f2e47d7b88344f35c80b93815f48 Mon Sep 17 00:00:00 2001 From: QIU Date: Tue, 25 Aug 2026 14:17:23 +0000 Subject: [PATCH] fix(aprs): build a legal packet, and keep beaconing when the screen locks The position line was one string template, and it broke four rules at once. No path. The specification says a client-originated packet carries TCPIP* in the path, "nothing more or less", and there was none - `CALL>APRS:=...` went out bare. No position meant 0,0. When the station QTH was unset and no GPS fix was available, `lat ?: 0.0` put the operator at 0 degrees north, 0 degrees east - a point in the Gulf of Guinea - on the global network, under their own callsign. There is no honest default for "nowhere", so AprsBeacon refuses instead and the reporter says why. A genuine 0,0 fix is still legal and still sent; the refusal is about absence. No comment sanitising. A line break typed into the status field ended the packet and started a second one from the remaining text, which an operator could trigger by pressing return. Measured: the old builder emitted two lines from one call, the second impersonating whatever callsign the text contained. Only printable ASCII survives now. No length cap. A 600-character status produced a 636-byte line against a 512-byte limit including CRLF. The comment is trimmed to whatever room is left after the header and the coordinates, bounded also by the format's own 43-character limit. Symbol handling was whatever character the operator typed first, including one that breaks the fixed-width parse. It now accepts only what the specification allows - the two table selectors and overlay characters - and falls back to the primary table. aprs.fi names symbol misconfiguration as the most common reason a station never appears on the map, so this is not cosmetic. Separately, beaconing stopped whenever the screen locked. The interval was a coroutine delay inside the reporter, and Doze suspends network access and ignores wake locks even for a foreground service: the timer fired on schedule and then could not reach the network, while the notification went on claiming the service was running. The service now books each beacon with setExactAndAllowWhileIdle, which is the only scheduling that survives Doze, and reschedules after each tick so a changed interval applies at once. If the operator has revoked exact alarms it falls back to an inexact one, which beacons late rather than not at all. The foreground service type changes from dataSync to location. dataSync is capped at six hours in any 24-hour window on recent Android and then stopped by the system, which would silently end a beacon meant to run all day; the service reads the station position and falls back to GPS, so location describes what it actually does. The interval floor becomes five minutes rather than one. This station is fixed or walking, and APRS-IS etiquette is to beacon no more often than the position changes. The packet builder moves to core:domain as pure logic, so all of this is testable without a socket - including that a comma-decimal locale cannot corrupt the coordinates, which nothing covered before. --- app/src/main/AndroidManifest.xml | 17 +- .../look4sat/AprsForegroundService.kt | 54 ++++++ .../look4sat/core/data/aprs/AprsReporter.kt | 61 +++++-- .../look4sat/core/domain/aprs/AprsBeacon.kt | 147 +++++++++++++++ .../core/domain/aprs/AprsBeaconTest.kt | 172 ++++++++++++++++++ 5 files changed, 429 insertions(+), 22 deletions(-) create mode 100644 core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsBeacon.kt create mode 100644 core/domain/src/test/java/com/rtbishop/look4sat/core/domain/aprs/AprsBeaconTest.kt diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index c3b572d8..07159276 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -15,7 +15,20 @@ - + + + + + android:foregroundServiceType="location" /> diff --git a/app/src/main/java/com/rtbishop/look4sat/AprsForegroundService.kt b/app/src/main/java/com/rtbishop/look4sat/AprsForegroundService.kt index 4e11d22b..c9e67028 100644 --- a/app/src/main/java/com/rtbishop/look4sat/AprsForegroundService.kt +++ b/app/src/main/java/com/rtbishop/look4sat/AprsForegroundService.kt @@ -1,5 +1,6 @@ package com.rtbishop.look4sat.app +import android.app.AlarmManager import android.app.Notification import android.app.NotificationChannel import android.app.NotificationManager @@ -37,6 +38,11 @@ class AprsForegroundService : Service() { const val ACTION_REPORT_NOW = AprsStore.ACTION_REPORT_NOW const val CHANNEL_ID = "aprs_service" const val NOTIF_ID = 101 + + /** Alarm-driven tick, kept separate so a manual report stays distinguishable. */ + const val ACTION_ALARM_TICK = "com.rtbishop.look4sat.APRS_ALARM_TICK" + + private const val ALARM_REQUEST = 4101 } private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO) @@ -63,6 +69,12 @@ class AprsForegroundService : Service() { override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { when (intent?.action) { ACTION_STOP -> stopReporting() + ACTION_ALARM_TICK -> { + // Woken by the exact alarm. Reporting once and then booking the next tick, rather + // than using a repeating alarm, means a changed interval takes effect at once. + if (reporter == null) startReporting() else reporter?.reportNow() + scheduleNextTick() + } ACTION_REPORT_NOW -> { if (reporter == null) { // Service not running: start it first (Toast hint when not configured) @@ -119,9 +131,12 @@ class AprsForegroundService : Service() { ) reporter = rep rep.start() + // The reporter beacons once on start; the alarm carries every one after that. + scheduleNextTick() } private fun stopReporting() { + cancelTicks() reporter?.stop() reporter = null stopForeground(STOP_FOREGROUND_REMOVE) @@ -212,4 +227,43 @@ class AprsForegroundService : Service() { reporter = null super.onDestroy() } + + /** + * Book the next beacon with an exact alarm. + * + * A coroutine delay was used before, which Doze defeats: the timer fires but network access is + * suspended and wake locks are ignored, even inside a foreground service. Only + * setExactAndAllowWhileIdle survives that, and the five-minute floor keeps this well clear of + * the system's throttle on how often such an alarm may repeat. + */ + private fun scheduleNextTick() { + val cfg = AprsStore.loadConfig(this) + if (!cfg.enabled) return + val alarms = getSystemService(Context.ALARM_SERVICE) as AlarmManager + val minutes = cfg.intervalMin.coerceAtLeast(AprsReporter.MIN_INTERVAL_MIN) + val at = System.currentTimeMillis() + minutes * 60_000L + runCatching { + val exact = Build.VERSION.SDK_INT < Build.VERSION_CODES.S || alarms.canScheduleExactAlarms() + if (exact) { + alarms.setExactAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, at, tickIntent()) + } else { + // The operator revoked exact alarms. An inexact one still beacons, just whenever + // the system decides, which beats not beaconing at all. + alarms.set(AlarmManager.RTC_WAKEUP, at, tickIntent()) + } + } + } + + private fun cancelTicks() { + val alarms = getSystemService(Context.ALARM_SERVICE) as AlarmManager + runCatching { alarms.cancel(tickIntent()) } + } + + private fun tickIntent(): PendingIntent = PendingIntent.getService( + this, + ALARM_REQUEST, + Intent(this, AprsForegroundService::class.java).setAction(ACTION_ALARM_TICK), + PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE + ) + } 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 50666e2d..a8dfdb45 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 @@ -1,13 +1,11 @@ package com.rtbishop.look4sat.core.data.aprs -import com.rtbishop.look4sat.core.domain.aprs.AprsPacket +import com.rtbishop.look4sat.core.domain.aprs.AprsBeacon import com.rtbishop.look4sat.core.domain.aprs.AprsPasscode -import com.rtbishop.look4sat.core.domain.aprs.AprsPosition import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.delay import kotlinx.coroutines.isActive import kotlinx.coroutines.launch @@ -70,12 +68,11 @@ class AprsReporter( onState(AprsState.Error) return } - job = scope.launch { - while (isActive) { - reportOnce() - delay(cfg.intervalMin.coerceAtLeast(1) * 60_000L) - } - } + // One report now; the service's exact alarm drives every one after this. The loop that + // used to live here relied on a coroutine delay, which Doze defeats - the timer fires on + // schedule and then finds network access suspended, so the beacon stopped whenever the + // screen locked while the notification still claimed it was running. + job = scope.launch { reportOnce() } } fun stop() { @@ -111,7 +108,25 @@ class AprsReporter( onState(AprsState.Connected) val pos = positionProvider() - val packetLine = buildPositionPacket(cfg, pos?.first, pos?.second) + val beacon = AprsBeacon.build( + callsign = cfg.callsign, + ssid = cfg.ssid, + latitude = pos?.first, + longitude = pos?.second, + symbolTable = cfg.symbolTable, + symbolCode = cfg.symbolCode, + comment = cfg.statusText + ) + // Nothing goes out without a position. Substituting 0,0 put this station in the Gulf + // of Guinea on the global network, under the operator's own callsign. + if (beacon is AprsBeacon.Result.Blocked) { + onState(AprsState.Error) + onReport( + AprsReport(System.currentTimeMillis(), "", false, refusalDetail(beacon.refusal)) + ) + return + } + val packetLine = (beacon as AprsBeacon.Result.Line).text val result = c.sendPacket(packetLine) val sent = result?.first == true val detail = result?.second ?: "no connection" @@ -132,15 +147,21 @@ class AprsReporter( } } - /** Build position packet: BG7NTA-5>APRS:=DDMM.MMN/DDDMM.MME' - ) - return "$source>APRS:=${pos.toUncompressedString()}${cfg.statusText}" + + /** A short reason for a refusal, for the operator's last-report line. */ + private fun refusalDetail(refusal: AprsBeacon.Refusal): String = when (refusal) { + AprsBeacon.Refusal.NoPosition -> "no position yet" + AprsBeacon.Refusal.NoCallsign -> "no callsign set" + is AprsBeacon.Refusal.ImpossiblePosition -> "position out of range" } + + + + + companion object { + + /** Floor for the reporting interval, in minutes. */ + const val MIN_INTERVAL_MIN = 5 + } + } diff --git a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsBeacon.kt b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsBeacon.kt new file mode 100644 index 00000000..1ab58232 --- /dev/null +++ b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsBeacon.kt @@ -0,0 +1,147 @@ +/* + * 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 + +/** + * Builds the position line this station puts on APRS-IS, or refuses to. + * + * Separated from the reporter so the packet can be tested without a socket. Every rule here + * comes from aprs-is.net/connecting.aspx, and each of them was being broken: + * + * - the path must be exactly `TCPIP*`, and was absent entirely + * - the line must not exceed 512 bytes including CRLF, and had no cap + * - the comment must not contain a line break, or it injects a second packet + * - a station with no position must not transmit, where 0.0 was substituted so 0 degrees north, + * 0 degrees east - a point in the Gulf of Guinea - went out under the operator's callsign + */ +object AprsBeacon { + + /** The only path a client-originated packet may carry. */ + const val PATH = "TCPIP*" + + /** Destination for a position report with no addressee. */ + const val DESTINATION = "APRS" + + /** Maximum line length including the CRLF the caller appends. */ + const val MAX_LINE_BYTES = 512 + + /** Comment limit for this position format, per the APRS specification. */ + const val MAX_COMMENT = 43 + + /** Why a beacon could not be built. */ + sealed interface Refusal { + + /** No position was available. Transmitting 0,0 would claim the Gulf of Guinea. */ + data object NoPosition : Refusal + + /** The callsign is missing, so the packet would have no valid source. */ + data object NoCallsign : Refusal + + /** Latitude or longitude outside the possible range. */ + data class ImpossiblePosition(val latitude: Double, val longitude: Double) : Refusal + } + + /** Either a line ready to send, or the reason there is none. */ + sealed interface Result { + data class Line(val text: String) : Result + data class Blocked(val refusal: Refusal) : Result + } + + /** + * Build the position line. + * + * Returns [Result.Blocked] rather than a placeholder: a beacon is a claim about where the + * operator is, and there is no honest default for "nowhere". + */ + fun build( + callsign: String, + ssid: String, + latitude: Double?, + longitude: Double?, + symbolTable: String, + symbolCode: String, + comment: String + ): Result { + if (callsign.isBlank()) return Result.Blocked(Refusal.NoCallsign) + if (latitude == null || longitude == null) return Result.Blocked(Refusal.NoPosition) + if (latitude !in -90.0..90.0 || longitude !in -180.0..180.0) { + return Result.Blocked(Refusal.ImpossiblePosition(latitude, longitude)) + } + val source = AprsPacket.formatCallSsid(callsign.trim().uppercase(), ssid.trim()) + val position = AprsPosition( + latitude = latitude, + longitude = longitude, + symbolTable = tableOf(symbolTable), + symbolCode = codeOf(symbolCode) + ) + val header = "$source>$DESTINATION,$PATH:=" + val body = position.toUncompressedString() + val room = MAX_LINE_BYTES - CRLF_BYTES - header.toByteArray().size - body.toByteArray().size + return Result.Line(header + body + sanitiseComment(comment, room)) + } + + /** + * Strip anything that would break the line, then trim to fit. + * + * A newline typed into the comment field used to end the packet early and start a second one + * from the remaining text - an injection the operator could trigger by accident. + */ + fun sanitiseComment(comment: String, room: Int = MAX_COMMENT): String { + if (room <= 0) return "" + val cleaned = comment.asSequence() + // Printable ASCII only: line breaks split the packet, and control characters have no + // meaning in a comment while being able to confuse a parser. + .filter { it.code in 0x20..0x7E } + .joinToString("") + .trim() + val limit = minOf(MAX_COMMENT, room) + return if (cleaned.length <= limit) cleaned else cleaned.take(limit) + } + + /** + * The symbol table byte, defaulting to the primary table. + * + * Must be `/`, `\` or an overlay character. It was previously whatever the operator typed + * first - any character at all, including one that breaks the fixed-width parse. aprs.fi + * names symbol misconfiguration as the most common reason a station never appears on the map. + */ + fun tableOf(entry: String): Char { + val candidate = entry.trim().firstOrNull() ?: return TABLE_PRIMARY + return when { + candidate == TABLE_PRIMARY || candidate == TABLE_ALTERNATE -> candidate + candidate.isDigit() -> candidate + candidate in 'A'..'Z' -> candidate + else -> TABLE_PRIMARY + } + } + + /** The symbol byte. Any printable character is a valid symbol; anything else is not. */ + fun codeOf(entry: String): Char { + val candidate = entry.trim().firstOrNull() ?: return DEFAULT_SYMBOL + return if (candidate.code in 0x21..0x7E) candidate else DEFAULT_SYMBOL + } + + private const val TABLE_PRIMARY = '/' + private const val TABLE_ALTERNATE = '\\' + + /** Bytes the caller adds after the line. */ + private const val CRLF_BYTES = 2 + + /** Fallback symbol. `>` is a car on the primary table - a reasonable stand-in for a phone. */ + private const val DEFAULT_SYMBOL = '>' +} diff --git a/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/aprs/AprsBeaconTest.kt b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/aprs/AprsBeaconTest.kt new file mode 100644 index 00000000..08a7cf9e --- /dev/null +++ b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/aprs/AprsBeaconTest.kt @@ -0,0 +1,172 @@ +package com.rtbishop.look4sat.core.domain.aprs + +import java.util.Locale +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * These pin the rules that decide whether a packet is legal on APRS-IS, each of which was being + * broken by the previous one-line string template. + */ +class AprsBeaconTest { + + private val original: Locale = Locale.getDefault() + + @After + fun restoreLocale() { + Locale.setDefault(original) + } + + private fun line( + latitude: Double? = 51.5, + longitude: Double? = -0.12, + callsign: String = "BG7NTA", + ssid: String = "5", + table: String = "/", + code: String = "[", + comment: String = "Look4Sat" + ): String { + val result = AprsBeacon.build(callsign, ssid, latitude, longitude, table, code, comment) + assertTrue("expected a line, got $result", result is AprsBeacon.Result.Line) + return (result as AprsBeacon.Result.Line).text + } + + /** + * The specification is explicit: a client-originated packet carries TCPIP* in the path, + * "nothing more or less". The old template emitted `CALL>APRS:=...` with no path at all. + */ + @Test + fun `the path is exactly TCPIP star`() { + val text = line() + assertTrue(text, text.startsWith("BG7NTA-5>APRS,TCPIP*:=")) + assertEquals(1, Regex(Regex.escape("TCPIP*")).findAll(text).count()) + } + + /** + * No position means no beacon. Substituting 0.0 put the station at 0N 0E - the Gulf of + * Guinea - on the global network, under the operator's own callsign. + */ + @Test + fun `a missing position refuses to build rather than claiming zero`() { + assertEquals( + AprsBeacon.Result.Blocked(AprsBeacon.Refusal.NoPosition), + AprsBeacon.build("BG7NTA", "5", null, null, "/", "[", "x") + ) + assertEquals( + AprsBeacon.Result.Blocked(AprsBeacon.Refusal.NoPosition), + AprsBeacon.build("BG7NTA", "5", 51.5, null, "/", "[", "x") + ) + assertEquals( + AprsBeacon.Result.Blocked(AprsBeacon.Refusal.NoPosition), + AprsBeacon.build("BG7NTA", "5", null, -0.12, "/", "[", "x") + ) + } + + /** A genuine 0,0 fix is legal and must still be sent - the refusal is about absence. */ + @Test + fun `an actual zero position is still a position`() { + val result = AprsBeacon.build("BG7NTA", "5", 0.0, 0.0, "/", "[", "x") + assertTrue(result is AprsBeacon.Result.Line) + } + + @Test + fun `an impossible position is refused`() { + val result = AprsBeacon.build("BG7NTA", "5", 91.0, 0.0, "/", "[", "x") + assertEquals( + AprsBeacon.Result.Blocked(AprsBeacon.Refusal.ImpossiblePosition(91.0, 0.0)), + result + ) + } + + @Test + fun `no callsign means no packet`() { + assertEquals( + AprsBeacon.Result.Blocked(AprsBeacon.Refusal.NoCallsign), + AprsBeacon.build("", "5", 51.5, -0.12, "/", "[", "x") + ) + assertEquals( + AprsBeacon.Result.Blocked(AprsBeacon.Refusal.NoCallsign), + AprsBeacon.build(" ", "5", 51.5, -0.12, "/", "[", "x") + ) + } + + /** + * A newline in the comment ended the packet early and started a second one from the rest. + * The operator could trigger that by pressing return in a text field. + */ + @Test + fun `a line break in the comment cannot split the packet`() { + val text = line(comment = "hello\r\nCALL>APRS,TCPIP*:=injected") + // Only the line break matters. The text of a second packet surviving inside the comment + // is harmless - without a terminator the server reads one line, and a comment is free to + // contain any printable characters the operator likes. + assertFalse(text, text.contains('\n')) + assertFalse(text, text.contains('\r')) + assertEquals("must remain a single line", 1, text.lines().size) + } + + @Test + fun `control characters are stripped from the comment`() { + val text = line(comment = "a\tb\u0000c") + assertTrue(text, text.endsWith("abc")) + } + + /** The line must fit in 512 bytes including the CRLF the client appends. */ + @Test + fun `an over-long comment is trimmed to keep the line legal`() { + val text = line(comment = "x".repeat(600)) + assertTrue("line was ${text.toByteArray().size} bytes", text.toByteArray().size + 2 <= 512) + } + + /** The comment limit for this format is 43 characters. */ + @Test + fun `the comment respects the format's own limit`() { + assertEquals(AprsBeacon.MAX_COMMENT, AprsBeacon.sanitiseComment("y".repeat(100)).length) + } + + /** + * Symbol misconfiguration is, per aprs.fi, the most common reason a station never appears. + * The table byte was previously the first character of whatever was typed. + */ + @Test + fun `the symbol table falls back to the primary table when invalid`() { + assertEquals('/', AprsBeacon.tableOf("")) + assertEquals('/', AprsBeacon.tableOf("!")) + assertEquals('/', AprsBeacon.tableOf(" ")) + assertEquals('/', AprsBeacon.tableOf("/")) + assertEquals('\\', AprsBeacon.tableOf("\\")) + // Overlays are legal: a digit or an upper-case letter selects the alternate table. + assertEquals('7', AprsBeacon.tableOf("7")) + assertEquals('S', AprsBeacon.tableOf("S")) + } + + @Test + fun `the symbol code falls back when unprintable`() { + assertEquals('>', AprsBeacon.codeOf("")) + assertEquals('>', AprsBeacon.codeOf(" ")) + assertEquals('[', AprsBeacon.codeOf("[")) + } + + /** + * Coordinates are fixed-width digits. A locale that formats decimals with a comma would + * corrupt every position, and a Turkish locale additionally lower-cases I to a dotless i. + */ + @Test + fun `a comma-decimal locale does not corrupt the coordinates`() { + val reference = line() + for (tag in listOf("de-DE", "tr-TR", "fr-FR")) { + Locale.setDefault(Locale.forLanguageTag(tag)) + assertEquals("locale $tag changed the packet", reference, line()) + } + } + + /** The whole line must be ASCII: APRS-IS is a byte protocol with no encoding negotiation. */ + @Test + fun `the line is pure ascii`() { + val text = line(comment = "café 北京") + assertTrue(text, text.all { it.code in 0x20..0x7E }) + } +}