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.
This commit is contained in:
mckero committed 2026-08-25 14:17:23 +00:00
1 parent b19c78441c
commit 2ff8643988
5 files changed
+429 -22

No files matched your search

+15 -2
View File
@@ -15,7 +15,20 @@
<uses-permission android:name="android.permission.RECORD_AUDIO" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_DATA_SYNC" />
<!--
location, not dataSync: dataSync foreground services are capped at six hours in any
24-hour window and then stopped by the system, which would silently end a beacon meant
to run all day. The service reads the station position, falling back to GPS, so location
describes what it actually does.
-->
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_LOCATION" />
<!--
Doze suspends network access and ignores wake locks even for a foreground service, so a
coroutine delay wakes up on time and then cannot reach the network. An exact alarm with
setExactAndAllowWhileIdle is the only scheduling that survives Doze, and at a five-minute
floor the system's one-alarm-per-nine-minutes throttle is not a problem.
-->
<uses-permission android:name="android.permission.SCHEDULE_EXACT_ALARM" />
<uses-permission android:name="android.permission.POST_NOTIFICATIONS" />
<application
android:name=".MainApplication"
@@ -51,6 +64,6 @@
<service
android:name="com.rtbishop.look4sat.app.AprsForegroundService"
android:exported="false"
android:foregroundServiceType="dataSync" />
android:foregroundServiceType="location" />
</application>
</manifest>
@@ -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
)
}
@@ -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<status text */
private fun buildPositionPacket(cfg: AprsConfig, lat: Double? = null, lon: Double? = null): String {
val source = AprsPacket.formatCallSsid(cfg.callsign, cfg.ssid)
val pos = AprsPosition(
latitude = lat ?: 0.0,
longitude = lon ?: 0.0,
symbolTable = cfg.symbolTable.firstOrNull() ?: '/',
symbolCode = cfg.symbolCode.firstOrNull() ?: '>'
)
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
}
}
@@ -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 <https://www.gnu.org/licenses/>.
*/
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 = '>'
}
@@ -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 })
}
}