From 08f91067490d9f8a4ddc00d96f1b41f5475b2a85 Mon Sep 17 00:00:00 2001 From: QIU Date: Wed, 26 Aug 2026 05:53:46 +0000 Subject: [PATCH] feat(aprs): pick a map symbol from a list instead of typing two characters The symbol table and code were free-text fields with no validation and no hint. Only the first character was ever used, and only at packet-build time, so an operator could type "satellite" into the table field, watch it persist, and beacon as "/" - the field lied about what it did. aprs.fi's troubleshooting guidance puts transmit-side symbol misconfiguration among the first things to check when a station never appears correctly. The single strongest argument for a list: \S is Satellite/Pacsat but /S is SHUTTLE. One keystroke apart, and both look right to someone typing from memory. Fourteen entries covering fixed, on-foot, field, four vehicle classes, satellite, yagi, phone, internet-only and handheld. Renderings are from aprs.org/symbols/symbolsX.txt (WB4APR, Nov 2015) rather than recalled. A symbol the operator already set that is not on the list appears first in the menu and stays selected, so opening the picker cannot silently change an existing station's appearance. The default changes from "/>" (CAR) to "/-" (House). The old default's own comment conceded it was "a reasonable stand-in for a phone", but it showed every non-driving operator as a vehicle. A house is right for most users and obviously wrong rather than misleading for the rest. This cannot disturb an existing install: saveConfig writes every key unconditionally and the enable switch calls it, so anyone who has ever turned APRS on has both symbol keys on disk and the changed fallbacks cannot reach them. All three sites move together - AprsStore's load fallback, AprsCard's blank-field fallback, and AprsBeacon.DEFAULT_SYMBOL - because leaving one behind would substitute a car whenever the stored code was unusable. The list lives in core:domain as pure data holding resource names rather than text, so the wording stays in the locale files. Tests assert that every entry survives the transmit sanitiser, that the pairs and description keys are unique, and that a pair off the list reports as absent rather than resolving to something near it. --- .../look4sat/core/data/aprs/AprsStore.kt | 11 +- .../look4sat/core/domain/aprs/AprsBeacon.kt | 11 +- .../look4sat/core/domain/aprs/AprsSymbols.kt | 81 ++++++++++++++ .../core/domain/aprs/AprsBeaconTest.kt | 6 +- .../core/domain/aprs/AprsSymbolsTest.kt | 102 +++++++++++++++++ .../src/main/res/values-zh/strings.xml | 17 +++ .../src/main/res/values/strings.xml | 17 +++ .../look4sat/feature/settings/AprsCard.kt | 105 +++++++++++++++--- 8 files changed, 330 insertions(+), 20 deletions(-) create mode 100644 core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsSymbols.kt create mode 100644 core/domain/src/test/java/com/rtbishop/look4sat/core/domain/aprs/AprsSymbolsTest.kt diff --git a/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsStore.kt b/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsStore.kt index 66800685..9d6d156b 100644 --- a/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsStore.kt +++ b/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsStore.kt @@ -24,6 +24,15 @@ object AprsStore { private const val KEY_STATUS = "status" private const val KEY_SYMBOL_TABLE = "symbol_table" private const val KEY_SYMBOL_CODE = "symbol_code" + + /** + * A house, not a car. + * + * Only fresh installs see this: saveConfig writes every key unconditionally and the enable + * switch calls it, so anyone who has ever turned APRS on has both symbol keys on disk and this + * fallback cannot reach them. + */ + private const val DEFAULT_SYMBOL_CODE = "-" private const val KEY_LAST_TIME = "last_report_time" private const val KEY_LAST_OK = "last_report_ok" private const val KEY_LAST_DETAIL = "last_report_detail" @@ -41,7 +50,7 @@ object AprsStore { intervalMin = p.getInt(KEY_INTERVAL, 5), statusText = p.getString(KEY_STATUS, "Look4Sat APRS") ?: "Look4Sat APRS", symbolTable = p.getString(KEY_SYMBOL_TABLE, "/") ?: "/", - symbolCode = p.getString(KEY_SYMBOL_CODE, ">") ?: ">" + symbolCode = p.getString(KEY_SYMBOL_CODE, DEFAULT_SYMBOL_CODE) ?: DEFAULT_SYMBOL_CODE ) } 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 index 1ab58232..33ab181e 100644 --- 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 @@ -143,5 +143,14 @@ object AprsBeacon { 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 = '>' + /** + * Substituted when the stored code is unusable. + * + * A house, not a car. The old default was '>' (CAR) with a comment conceding it was "a + * reasonable stand-in for a phone" - but a station beaconing from a handset showed up as a + * vehicle for every operator who was not driving, and aprs.fi names transmit-side symbol + * misconfiguration among the first things to check when a station looks wrong. A house is + * correct for most users and obviously wrong rather than misleading for the rest. + */ + private const val DEFAULT_SYMBOL = '-' } diff --git a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsSymbols.kt b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsSymbols.kt new file mode 100644 index 00000000..93aa5fac --- /dev/null +++ b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsSymbols.kt @@ -0,0 +1,81 @@ +/* + * 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 + +/** + * One APRS symbol an operator might plausibly want. + * + * [table] and [code] are the two characters that go into the packet. [descriptionKey] names a + * string resource rather than holding text, because core:domain has no access to resources and + * hardcoding English here would put wording outside the locale files. + */ +data class AprsSymbol(val table: Char, val code: Char, val descriptionKey: String) + +/** + * A short list of symbols worth offering, instead of two free-text fields. + * + * The fields accepted anything and used only the first character, so typing "satellite" into the + * table field persisted the whole word and beaconed as `/` - the field lied about what it did. + * aprs.fi's own troubleshooting guidance puts transmit-side symbol misconfiguration among the first + * things to check when a station does not appear as expected. + * + * The strongest single argument for a list: `\S` is Satellite/Pacsat but `/S` is SHUTTLE. One + * keystroke apart, and both look correct to someone typing from memory. + * + * Renderings are from aprs.org/symbols/symbolsX.txt (WB4APR, 25 Nov 2015). The list is deliberately + * short - it covers fixed, portable, vehicle and satellite postures, not all 400-odd symbols. + */ +object AprsSymbols { + + /** Fixed home station. Correct for most users, and wrong in an obvious way for the rest. */ + val HOUSE = AprsSymbol('/', '-', "aprs_symbol_house") + + val curated = listOf( + HOUSE, + AprsSymbol('\\', '-', "aprs_symbol_house_alt"), + AprsSymbol('/', '[', "aprs_symbol_person"), + AprsSymbol('/', 'y', "aprs_symbol_yagi"), + // Alternate table. /S is SHUTTLE, which is not what anyone means here. + AprsSymbol('\\', 'S', "aprs_symbol_satellite"), + AprsSymbol('/', ';', "aprs_symbol_portable"), + AprsSymbol('/', '$', "aprs_symbol_phone"), + AprsSymbol('/', 'I', "aprs_symbol_tcpip"), + AprsSymbol('\\', 'K', "aprs_symbol_ht"), + AprsSymbol('/', '>', "aprs_symbol_car"), + AprsSymbol('/', 'k', "aprs_symbol_truck"), + AprsSymbol('/', 'v', "aprs_symbol_van"), + AprsSymbol('/', 'R', "aprs_symbol_rv"), + AprsSymbol('/', 'b', "aprs_symbol_bike") + ) + + /** + * Find the curated entry matching a stored pair, or null when it is not on the list. + * + * Null matters: an operator may have set a symbol this list does not offer, and the picker must + * show it as-is rather than silently substituting the nearest entry. + */ + fun find(table: Char, code: Char): AprsSymbol? = + curated.firstOrNull { it.table == table && it.code == code } + + /** Same, from whatever strings the settings screen holds. Blank means the shipped default. */ + fun find(table: String, code: String): AprsSymbol? { + val t = table.firstOrNull() ?: HOUSE.table + val c = code.firstOrNull() ?: HOUSE.code + return find(t, c) + } +} 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 index 08a7cf9e..df823d8a 100644 --- 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 @@ -145,8 +145,10 @@ class AprsBeaconTest { @Test fun `the symbol code falls back when unprintable`() { - assertEquals('>', AprsBeacon.codeOf("")) - assertEquals('>', AprsBeacon.codeOf(" ")) + // A house, not a car. The old fallback was '>' (CAR), which showed a phone-based beacon as + // a vehicle for every operator who was not driving. + assertEquals('-', AprsBeacon.codeOf("")) + assertEquals('-', AprsBeacon.codeOf(" ")) assertEquals('[', AprsBeacon.codeOf("[")) } diff --git a/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/aprs/AprsSymbolsTest.kt b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/aprs/AprsSymbolsTest.kt new file mode 100644 index 00000000..720380c8 --- /dev/null +++ b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/aprs/AprsSymbolsTest.kt @@ -0,0 +1,102 @@ +package com.rtbishop.look4sat.core.domain.aprs + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The list exists because two free-text fields accepted anything and used only the first character, + * so an operator could type a word, watch it persist, and beacon as something else entirely. + */ +class AprsSymbolsTest { + + /** + * The pair that justifies the whole feature. `\S` is Satellite/Pacsat; `/S` is SHUTTLE. One + * keystroke apart, and both look right to someone typing from memory. + */ + @Test + fun `the satellite symbol uses the alternate table`() { + val satellite = AprsSymbols.curated.single { it.descriptionKey == "aprs_symbol_satellite" } + assertEquals('\\', satellite.table) + assertEquals('S', satellite.code) + } + + /** Every entry must survive the sanitiser that runs at packet-build time. */ + @Test + fun `every curated symbol passes the transmit sanitiser`() { + for (symbol in AprsSymbols.curated) { + assertEquals( + symbol.descriptionKey + " table must survive tableOf", + symbol.table, + AprsBeacon.tableOf(symbol.table.toString()) + ) + assertEquals( + symbol.descriptionKey + " code must survive codeOf", + symbol.code, + AprsBeacon.codeOf(symbol.code.toString()) + ) + } + } + + /** Two entries rendering the same pair would be a UI ambiguity. */ + @Test + fun `no two curated symbols are the same pair`() { + val pairs = AprsSymbols.curated.map { it.table to it.code } + assertEquals("pairs must be unique", pairs.size, pairs.toSet().size) + } + + /** Each needs its own description, or the list reads as duplicates. */ + @Test + fun `every curated symbol has a distinct description key`() { + val keys = AprsSymbols.curated.map { it.descriptionKey } + assertEquals("description keys must be unique", keys.size, keys.toSet().size) + assertTrue("keys must be resource names", keys.all { it.startsWith("aprs_symbol_") }) + } + + @Test + fun `a stored pair on the list is found`() { + assertEquals(AprsSymbols.HOUSE, AprsSymbols.find('/', '-')) + assertNotNull(AprsSymbols.find('\\', 'S')) + } + + /** + * A pair the list does not offer must report as absent rather than resolving to something near + * it. The picker relies on this to show an existing setting untouched. + */ + @Test + fun `a stored pair off the list is not silently substituted`() { + assertNull(AprsSymbols.find('/', 'S')) + assertNull(AprsSymbols.find('/', '!')) + assertNull(AprsSymbols.find('\\', 'y')) + } + + /** The settings screen holds strings, and only the first character counts. */ + @Test + fun `the string overload reads the first character`() { + assertEquals(AprsSymbols.HOUSE, AprsSymbols.find("/", "-")) + assertEquals(AprsSymbols.HOUSE, AprsSymbols.find("/junk", "-junk")) + } + + /** Blank fields mean the shipped default, which is what the save path substitutes. */ + @Test + fun `blank strings resolve to the default symbol`() { + assertEquals(AprsSymbols.HOUSE, AprsSymbols.find("", "")) + } + + /** The default has to be on the list, or the picker opens showing nothing selected. */ + @Test + fun `the default symbol is on the curated list`() { + assertTrue(AprsSymbols.HOUSE in AprsSymbols.curated) + } + + /** Short enough to scan in a bottom sheet during setup. */ + @Test + fun `the list stays short`() { + assertTrue( + "a curated list of ${AprsSymbols.curated.size} defeats the point", + AprsSymbols.curated.size in 8..20 + ) + } +} diff --git a/core/presentation/src/main/res/values-zh/strings.xml b/core/presentation/src/main/res/values-zh/strings.xml index b75408a7..39acf7e4 100644 --- a/core/presentation/src/main/res/values-zh/strings.xml +++ b/core/presentation/src/main/res/values-zh/strings.xml @@ -28,6 +28,23 @@ 周期(分钟) 状态文本 符号表 + 房屋 - 固定台站 + 房屋, 备用符号表 + 步行操作员 + 固定站八木天线 + 卫星台站 + 便携 / 野外运行 + 手机 + 仅互联网台站 + 手持电台 + 轿车 + 卡车 + 厢式车 + 房车 + 自行车 + 地图符号 + 自定义 (%1$s%2$s) + 你的台站在 APRS 地图上的显示方式 符号码 保存 取消 diff --git a/core/presentation/src/main/res/values/strings.xml b/core/presentation/src/main/res/values/strings.xml index a44bc0f4..4c606b05 100644 --- a/core/presentation/src/main/res/values/strings.xml +++ b/core/presentation/src/main/res/values/strings.xml @@ -29,6 +29,23 @@ Interval (min) Status text Symbol table + House - fixed station + House, alternate table + Person on foot + Yagi at a fixed site + Satellite station + Portable / field operation + Phone + Internet-only station + Handheld radio + Car + Truck + Van + Motorhome + Bicycle + Map symbol + Custom (%1$s%2$s) + How your station appears on the APRS map Symbol code Save Cancel diff --git a/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/AprsCard.kt b/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/AprsCard.kt index 841c666d..10536613 100644 --- a/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/AprsCard.kt +++ b/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/AprsCard.kt @@ -11,6 +11,12 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.material3.AlertDialog +import androidx.compose.material3.DropdownMenuItem +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.ExposedDropdownMenuBox +import androidx.compose.material3.ExposedDropdownMenuDefaults +import androidx.compose.material3.MenuAnchorType +import com.rtbishop.look4sat.core.domain.aprs.AprsSymbols import androidx.compose.material3.ElevatedCard import androidx.compose.material3.IconButton import androidx.compose.material3.LocalTextStyle @@ -179,6 +185,7 @@ fun AprsCard() { } /** APRS settings dialog (plenty of room, full config) */ +@OptIn(ExperimentalMaterial3Api::class) @Composable private fun AprsSettingsDialog( config: AprsConfig, @@ -194,6 +201,7 @@ private fun AprsSettingsDialog( var status by remember { mutableStateOf(config.statusText) } var symbolTable by remember { mutableStateOf(config.symbolTable) } var symbolCode by remember { mutableStateOf(config.symbolCode) } + var symbolMenuExpanded by remember { mutableStateOf(false) } val textStyle = LocalTextStyle.current.copy(fontSize = 14.sp) val context = LocalContext.current @@ -274,24 +282,62 @@ private fun AprsSettingsDialog( textStyle = textStyle ) } - Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { + // A list rather than two free-text fields. The fields accepted any string and used + // only the first character, so typing "satellite" persisted the word and beaconed + // as `/`. The pair that makes this worth doing: \S is Satellite but /S is SHUTTLE. + val selected = AprsSymbols.find(symbolTable, symbolCode) + val customLabel = stringResource( + id = R.string.prefs_aprs_symbol_custom, + symbolTable.take(1), + symbolCode.take(1) + ) + val selectedLabel = selected?.let { symbolLabel(it.descriptionKey) } ?: customLabel + ExposedDropdownMenuBox( + expanded = symbolMenuExpanded, + onExpandedChange = { symbolMenuExpanded = it } + ) { OutlinedTextField( - value = symbolTable, - onValueChange = { symbolTable = it }, - label = { Text(stringResource(id = R.string.prefs_aprs_symbol_table)) }, - singleLine = true, - modifier = Modifier.weight(1f), - textStyle = textStyle - ) - OutlinedTextField( - value = symbolCode, - onValueChange = { symbolCode = it }, - label = { Text(stringResource(id = R.string.prefs_aprs_symbol_code)) }, - singleLine = true, - modifier = Modifier.weight(1f), - textStyle = textStyle + value = selectedLabel, + onValueChange = {}, + readOnly = true, + label = { Text(stringResource(id = R.string.prefs_aprs_symbol)) }, + trailingIcon = { + ExposedDropdownMenuDefaults.TrailingIcon(expanded = symbolMenuExpanded) + }, + textStyle = textStyle, + modifier = Modifier + .menuAnchor(MenuAnchorType.PrimaryNotEditable) + .fillMaxWidth() ) + ExposedDropdownMenu( + expanded = symbolMenuExpanded, + onDismissRequest = { symbolMenuExpanded = false } + ) { + // An existing setting that is not on the list appears first and stays + // selectable, so opening the menu cannot silently change it. + if (selected == null) { + DropdownMenuItem( + text = { Text(text = customLabel, maxLines = 1) }, + onClick = { symbolMenuExpanded = false } + ) + } + AprsSymbols.curated.forEach { symbol -> + DropdownMenuItem( + text = { Text(text = symbolLabel(symbol.descriptionKey), maxLines = 1) }, + onClick = { + symbolTable = symbol.table.toString() + symbolCode = symbol.code.toString() + symbolMenuExpanded = false + } + ) + } + } } + Text( + text = stringResource(id = R.string.prefs_aprs_symbol_help), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) Text( text = stringResource(id = R.string.prefs_aprs_passcode_hint), fontSize = 12.sp, @@ -316,7 +362,7 @@ private fun AprsSettingsDialog( intervalMin = interval.toIntOrNull() ?: 5, statusText = status, symbolTable = symbolTable.ifBlank { "/" }, - symbolCode = symbolCode.ifBlank { ">" } + symbolCode = symbolCode.ifBlank { "-" } ) ) }) { Text(stringResource(id = R.string.prefs_aprs_save)) } @@ -339,3 +385,30 @@ private fun AprsSwitchRow(labelResId: Int, checked: Boolean, onCheckedChange: (( Switch(checked = checked, onCheckedChange = onCheckedChange) } } + +/** + * Resolve a symbol's description key to its localised text. + * + * AprsSymbols names a resource rather than holding text, because core:domain cannot reach resources + * and hardcoding English there would put wording outside the locale files. The mapping has to live + * on this side, and an unknown key falls back to the key itself rather than crashing - a missing + * translation should not take the settings screen down. + */ +@Composable +private fun symbolLabel(descriptionKey: String): String = when (descriptionKey) { + "aprs_symbol_house" -> stringResource(id = R.string.aprs_symbol_house) + "aprs_symbol_house_alt" -> stringResource(id = R.string.aprs_symbol_house_alt) + "aprs_symbol_person" -> stringResource(id = R.string.aprs_symbol_person) + "aprs_symbol_yagi" -> stringResource(id = R.string.aprs_symbol_yagi) + "aprs_symbol_satellite" -> stringResource(id = R.string.aprs_symbol_satellite) + "aprs_symbol_portable" -> stringResource(id = R.string.aprs_symbol_portable) + "aprs_symbol_phone" -> stringResource(id = R.string.aprs_symbol_phone) + "aprs_symbol_tcpip" -> stringResource(id = R.string.aprs_symbol_tcpip) + "aprs_symbol_ht" -> stringResource(id = R.string.aprs_symbol_ht) + "aprs_symbol_car" -> stringResource(id = R.string.aprs_symbol_car) + "aprs_symbol_truck" -> stringResource(id = R.string.aprs_symbol_truck) + "aprs_symbol_van" -> stringResource(id = R.string.aprs_symbol_van) + "aprs_symbol_rv" -> stringResource(id = R.string.aprs_symbol_rv) + "aprs_symbol_bike" -> stringResource(id = R.string.aprs_symbol_bike) + else -> descriptionKey +}