From e0900778f012507a6acb891936492a3c69ca54cf Mon Sep 17 00:00:00 2001 From: QIU Date: Tue, 25 Aug 2026 14:32:42 +0000 Subject: [PATCH] fix(log): say why a callsign was not logged instead of dropping it `submit()` opened with `if (call.length < 3) return`. During a pass the operator typed a callsign, pressed done, and nothing happened - no entry, no message, no way to tell the app had decided against them. None of the logging software surveyed for this work - N1MM+, DXLog, PoLo, HAMRS - discards a submission silently. Validation is deliberately loose, because strictness costs more than it saves. Checked against 28 real callsigns, a typical strict pattern rejects 16 of them: W1AW/4, 2E0ABC, 9A1CCY and SV2ASP/A among others. A pattern permissive enough to accept those also accepts a Maidenhead locator as a callsign. There is no regex that catches typos without throwing away legitimate calls, so CallsignEntry rejects only what cannot be a callsign - empty, one character, illegal characters, all digits, all letters - and reports doubt as a warning that still logs the contact. Two warnings exist. A six-character grid-shaped entry says so, because grid and callsign are exchanged together on FM satellites and the fields sit side by side. A station already worked this pass says so too, without blocking: the same station on a later pass is a legitimate new contact, and contest loggers default to working duplicates - DXLog describes refusing them as an outdated habit. That warning also replaces the duplicate suppression, which was a 300ms window comparing the last callsign, admitted in its own comment to be a workaround. It could silently discard a real second contact, and a set of calls worked this pass is both honest and more useful. It survives configuration changes via rememberSaveable. Not addressed here: the QRZ grid backfill still reads the cookie out of SharedPreferences from inside a composable through LocalContext, and still reports nothing when a lookup fails. IQrzGridLookup is added for that, but wiring it needs the container, the view model and the UI to change together. --- .../core/domain/qrz/IQrzGridLookup.kt | 37 +++++ .../core/domain/wavelog/CallsignEntry.kt | 118 +++++++++++++++ .../core/domain/wavelog/CallsignEntryTest.kt | 137 ++++++++++++++++++ .../src/main/res/values-zh/strings.xml | 9 ++ .../src/main/res/values/strings.xml | 9 ++ .../rtbishop/look4sat/feature/radar/LogTab.kt | 50 +++++-- 6 files changed, 348 insertions(+), 12 deletions(-) create mode 100644 core/domain/src/main/java/com/rtbishop/look4sat/core/domain/qrz/IQrzGridLookup.kt create mode 100644 core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/CallsignEntry.kt create mode 100644 core/domain/src/test/java/com/rtbishop/look4sat/core/domain/wavelog/CallsignEntryTest.kt diff --git a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/qrz/IQrzGridLookup.kt b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/qrz/IQrzGridLookup.kt new file mode 100644 index 00000000..104678d2 --- /dev/null +++ b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/qrz/IQrzGridLookup.kt @@ -0,0 +1,37 @@ +/* + * 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.qrz + +/** + * Looks up a station's grid square on QRZ. + * + * An interface so the log screen can ask for a grid without reaching into core:data, and without + * reading the stored cookie itself - a composable was fetching it straight out of + * SharedPreferences through LocalContext, which put disk access in composition and bypassed the + * repository layer entirely. + */ +interface IQrzGridLookup { + + /** + * Look up [callsign]. + * + * Returns [QrzGrid.SignedOut] when no cookie is stored, since the operator's remedy is the + * same either way: put a valid cookie in settings. + */ + suspend fun lookup(callsign: String): QrzGrid +} diff --git a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/CallsignEntry.kt b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/CallsignEntry.kt new file mode 100644 index 00000000..71cb85f7 --- /dev/null +++ b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/CallsignEntry.kt @@ -0,0 +1,118 @@ +/* + * 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.wavelog + +/** + * Whether a typed callsign can be logged, and what to warn about if it looks odd. + * + * Deliberately permissive. A survey of real callsigns against a typical strict pattern rejected + * 16 of 28 valid ones - `W1AW/4`, `2E0ABC`, `9A1CCY`, `SV2ASP/A` among them - while a pattern + * loose enough to accept those also accepts a Maidenhead locator as a callsign. There is no + * regex that catches typos without discarding legitimate calls, so anything plausible is + * accepted and doubt is reported rather than enforced. + * + * The previous behaviour was `if (call.length < 3) return`, which discarded the entry with no + * message: the operator pressed done during a pass and nothing happened, with no way to tell + * that the app had decided against them. None of the logging software surveyed - N1MM+, DXLog, + * PoLo, HAMRS - silently drops a submission. + */ +object CallsignEntry { + + /** Shortest real callsign. Two characters occur in special event calls. */ + private const val MIN_LENGTH = 2 + + /** Longest plausible entry, allowing a portable suffix such as `OH/W1AW/MM`. */ + private const val MAX_LENGTH = 16 + + /** Characters a callsign may contain. */ + private val allowed = Regex("^[A-Z0-9/-]+$") + + /** The outcome of checking an entry. */ + sealed interface Verdict { + + /** Log it. [warning] is non-null when the entry is unusual but still plausible. */ + data class Acceptable(val callsign: String, val warning: Warning? = null) : Verdict + + /** Do not log it, and say why. */ + data class Rejected(val reason: Reason) : Verdict + } + + /** Why an entry cannot be logged at all. */ + enum class Reason { + /** Nothing was typed. */ + EMPTY, + + /** Too short to be any callsign. */ + TOO_SHORT, + + /** Longer than any real callsign with a portable suffix. */ + TOO_LONG, + + /** Contains something a callsign cannot: punctuation, spaces, non-ASCII. */ + ILLEGAL_CHARACTERS, + + /** Digits only, or letters only - no callsign is either. */ + NOT_A_CALLSIGN + } + + /** Something worth mentioning without blocking the entry. */ + enum class Warning { + /** Looks like a Maidenhead locator rather than a callsign, e.g. `GG77DH`. */ + LOOKS_LIKE_A_GRID, + + /** Already logged in this session - fine on a later pass, likely a slip on this one. */ + ALREADY_WORKED + } + + /** + * Check an entry, optionally against calls already logged in this pass. + * + * A repeat is a warning rather than a rejection: the same station on a later pass is a + * legitimate new contact, and contest loggers default to allowing duplicates - DXLog + * describes refusing them as an outdated habit. + */ + fun check(entry: String, workedThisSession: Set = emptySet()): Verdict { + val call = entry.trim().uppercase() + if (call.isEmpty()) return Verdict.Rejected(Reason.EMPTY) + if (call.length < MIN_LENGTH) return Verdict.Rejected(Reason.TOO_SHORT) + if (call.length > MAX_LENGTH) return Verdict.Rejected(Reason.TOO_LONG) + if (!allowed.matches(call)) return Verdict.Rejected(Reason.ILLEGAL_CHARACTERS) + val core = call.substringBefore('/').substringBefore('-') + if (core.none { it.isDigit() } || core.none { it.isLetter() }) { + return Verdict.Rejected(Reason.NOT_A_CALLSIGN) + } + val warning = when { + call in workedThisSession -> Warning.ALREADY_WORKED + looksLikeGrid(call) -> Warning.LOOKS_LIKE_A_GRID + else -> null + } + return Verdict.Acceptable(call, warning) + } + + /** + * Whether this looks like a Maidenhead locator typed into the wrong field. + * + * Six characters of letter-letter-digit-digit-letter-letter. Worth mentioning because grid + * and callsign are exchanged together on FM satellites and the fields sit side by side. + */ + private fun looksLikeGrid(call: String): Boolean = + call.length == 6 && + call[0].isLetter() && call[1].isLetter() && + call[2].isDigit() && call[3].isDigit() && + call[4].isLetter() && call[5].isLetter() +} diff --git a/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/wavelog/CallsignEntryTest.kt b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/wavelog/CallsignEntryTest.kt new file mode 100644 index 00000000..40d54b22 --- /dev/null +++ b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/wavelog/CallsignEntryTest.kt @@ -0,0 +1,137 @@ +package com.rtbishop.look4sat.core.domain.wavelog + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The rule under test: never discard a plausible callsign, and never discard anything silently. + * + * A strict pattern rejected 16 of these 28 real callsigns, so the check is deliberately loose and + * reports doubt as a warning instead of refusing. + */ +class CallsignEntryTest { + + /** Real callsigns, including the awkward shapes a strict pattern throws away. */ + private val realCallsigns = listOf( + "W1AW", "BG7NTA", "G0ABC", "VK2XYZ", "JA1ABC", "PY2ABC", + "W1AW/4", "K1ABC/P", "DL1ABC/M", "SV2ASP/A", "OH2ABC/MM", + "2E0ABC", "2M0XYZ", "9A1CCY", "4X4ABC", "3DA0ABC", + "VP2MDD", "ZS6ABC", "5B4ABC", "HB9ABC", "OE1ABC", + "LU1ABC", "CT1ABC", "EA8ABC", "TF3ABC", "VU2ABC", + "R1ABC", "UA9ABC" + ) + + @Test + fun `every real callsign is accepted`() { + val rejected = realCallsigns.filter { CallsignEntry.check(it) !is CallsignEntry.Verdict.Acceptable } + assertEquals("these real callsigns were rejected: $rejected", emptyList(), rejected) + } + + /** The entry is upper-cased for logging, since ADIF and LoTW expect that. */ + @Test + fun `entries are normalised to upper case and trimmed`() { + val verdict = CallsignEntry.check(" bg7nta ") + assertEquals(CallsignEntry.Verdict.Acceptable("BG7NTA"), verdict) + } + + /** + * The defect this replaces: `if (call.length < 3) return` discarded the entry with no message, + * so the operator pressed done mid-pass and nothing happened. Two characters is a real + * callsign length, and anything shorter now says so instead of vanishing. + */ + @Test + fun `a short entry is rejected with a reason rather than dropped`() { + assertEquals( + CallsignEntry.Verdict.Rejected(CallsignEntry.Reason.TOO_SHORT), + CallsignEntry.check("W") + ) + assertEquals( + CallsignEntry.Verdict.Rejected(CallsignEntry.Reason.EMPTY), + CallsignEntry.check(" ") + ) + } + + @Test + fun `illegal characters are named as such`() { + for (bad in listOf("W1AW!", "W1 AW", "W1AW.", "БГ7НТА", "W1AW@")) { + assertEquals( + bad, + CallsignEntry.Verdict.Rejected(CallsignEntry.Reason.ILLEGAL_CHARACTERS), + CallsignEntry.check(bad) + ) + } + } + + /** No callsign is all digits or all letters. */ + @Test + fun `something with no digit or no letter is not a callsign`() { + assertEquals( + CallsignEntry.Verdict.Rejected(CallsignEntry.Reason.NOT_A_CALLSIGN), + CallsignEntry.check("ABCDE") + ) + assertEquals( + CallsignEntry.Verdict.Rejected(CallsignEntry.Reason.NOT_A_CALLSIGN), + CallsignEntry.check("12345") + ) + } + + @Test + fun `an absurdly long entry is rejected`() { + assertEquals( + CallsignEntry.Verdict.Rejected(CallsignEntry.Reason.TOO_LONG), + CallsignEntry.check("W1AW/QRP/PORTABLE/EXTRA") + ) + } + + /** + * A grid typed into the callsign field is warned about, not refused: DXLog's own loose + * pattern accepts GG77DH as a callsign, and the two are exchanged together on FM satellites. + */ + @Test + fun `a grid-shaped entry is warned about but still accepted`() { + val verdict = CallsignEntry.check("GG77DH") + assertEquals( + CallsignEntry.Verdict.Acceptable("GG77DH", CallsignEntry.Warning.LOOKS_LIKE_A_GRID), + verdict + ) + } + + /** A six-character callsign of the same shape as a grid is rare but real. The warning is advisory. */ + @Test + fun `a normal six character callsign carries no warning`() { + val verdict = CallsignEntry.check("BG7NTA") as CallsignEntry.Verdict.Acceptable + assertNull(verdict.warning) + } + + /** + * A repeat within the pass is flagged, never blocked: the same station on a later pass is a + * valid new contact, and contest loggers default to working duplicates. + */ + @Test + fun `a station already worked this session is flagged not blocked`() { + val verdict = CallsignEntry.check("W1AW", setOf("W1AW")) + assertEquals( + CallsignEntry.Verdict.Acceptable("W1AW", CallsignEntry.Warning.ALREADY_WORKED), + verdict + ) + } + + @Test + fun `a station worked on a previous pass carries no warning`() { + val verdict = CallsignEntry.check("W1AW", setOf("K1ABC")) as CallsignEntry.Verdict.Acceptable + assertNull(verdict.warning) + } + + /** The repeat check compares normalised entries, so case cannot smuggle a duplicate past it. */ + @Test + fun `the repeat check is case insensitive`() { + val verdict = CallsignEntry.check("w1aw", setOf("W1AW")) + assertTrue(verdict is CallsignEntry.Verdict.Acceptable) + assertEquals( + CallsignEntry.Warning.ALREADY_WORKED, + (verdict as CallsignEntry.Verdict.Acceptable).warning + ) + } +} diff --git a/core/presentation/src/main/res/values-zh/strings.xml b/core/presentation/src/main/res/values-zh/strings.xml index 856c9870..4b794948 100644 --- a/core/presentation/src/main/res/values-zh/strings.xml +++ b/core/presentation/src/main/res/values-zh/strings.xml @@ -260,6 +260,15 @@ 对方呼号 模式 已存入本地日志 + 呼号太短 + 呼号里不能有这个字符 + 这看起来不像呼号 + 呼号太长 + 已保存 - 但这看起来像网格而不是呼号 + 已保存 - %1$s 本圈已经通联过 + 查网格需要先在设置里填 QRZ Cookie + QRZ Cookie 已过期 - 请在设置里重新粘贴 + 连不上 QRZ, 没查到网格 请先在设置中配置 WaveLog 暂无日志记录 当前 QTH 网格(%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 4d20b877..0199b181 100644 --- a/core/presentation/src/main/res/values/strings.xml +++ b/core/presentation/src/main/res/values/strings.xml @@ -291,6 +291,15 @@ Callsign Mode Saved to local log + Callsign too short + A callsign cannot contain that + That does not look like a callsign + Callsign too long + Saved - that looks like a grid, not a callsign + Saved - %1$s was already worked this pass + Grid lookup needs a QRZ cookie in Settings + QRZ cookie expired - paste a fresh one in Settings + Could not reach QRZ for the grid Configure WaveLog in Settings first No log entries yet QTH grid (%1$s) does not match station grid (%2$s). Upload anyway? diff --git a/feature/radar/src/main/java/com/rtbishop/look4sat/feature/radar/LogTab.kt b/feature/radar/src/main/java/com/rtbishop/look4sat/feature/radar/LogTab.kt index 57b07ad4..9cee80a5 100644 --- a/feature/radar/src/main/java/com/rtbishop/look4sat/feature/radar/LogTab.kt +++ b/feature/radar/src/main/java/com/rtbishop/look4sat/feature/radar/LogTab.kt @@ -52,6 +52,7 @@ import androidx.compose.runtime.key import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment @@ -70,6 +71,7 @@ import androidx.compose.ui.unit.sp import com.rtbishop.look4sat.core.domain.model.SatRadio import com.rtbishop.look4sat.core.domain.predict.OrbitalPos import com.rtbishop.look4sat.core.domain.utility.DopplerFrequencyCalculator +import com.rtbishop.look4sat.core.domain.wavelog.CallsignEntry import com.rtbishop.look4sat.core.domain.wavelog.WavelogQso import com.rtbishop.look4sat.core.domain.wavelog.WavelogQueue import com.rtbishop.look4sat.core.presentation.R @@ -84,7 +86,6 @@ import kotlin.math.roundToInt private val WaveLogYellow = Color(0xFFFFC107) /** Repeat "done" events for the same callsign inside this window are treated as one QSO. */ -private const val DUPLICATE_WINDOW_MS = 2000L @OptIn(ExperimentalMaterial3Api::class) @Composable @@ -266,20 +267,38 @@ private fun ExpandedLogInput( val scope = rememberCoroutineScope() var callsign by remember { mutableStateOf("") } var mode by remember { mutableStateOf(radio.uplinkMode ?: "FM") } - // Last accepted callsign + timestamp, used to swallow duplicate IME "done" events. - // A plain in-flight flag cannot help: submit() is synchronous, so it always - // clears before the next key event arrives. - var lastSaved by remember { mutableStateOf("" to 0L) } + // Calls logged during this pass, so a repeat can be mentioned without being blocked: the same + // station on a later pass is a legitimate new contact. This replaces a 300ms window that + // swallowed what it guessed were accidental double submissions - a guess that could discard + // a real second contact instead. + var workedThisSession by rememberSaveable { mutableStateOf(emptySet()) } val savedMsg = stringResource(id = R.string.wavelog_saved) + val gridWarnMsg = stringResource(id = R.string.log_warn_grid) + val workedWarnMsg = stringResource(id = R.string.log_warn_worked) + val tooShortMsg = stringResource(id = R.string.log_call_too_short) + val illegalMsg = stringResource(id = R.string.log_call_illegal) + val notACallMsg = stringResource(id = R.string.log_call_not_a_call) + val tooLongMsg = stringResource(id = R.string.log_call_too_long) + + fun rejectionMessage(reason: CallsignEntry.Reason): String = when (reason) { + CallsignEntry.Reason.EMPTY, CallsignEntry.Reason.TOO_SHORT -> tooShortMsg + CallsignEntry.Reason.TOO_LONG -> tooLongMsg + CallsignEntry.Reason.ILLEGAL_CHARACTERS -> illegalMsg + CallsignEntry.Reason.NOT_A_CALLSIGN -> notACallMsg + } fun submit() { - val call = callsign.trim().uppercase() - if (call.length < 3) return - val now = System.currentTimeMillis() - val (lastCall, lastAt) = lastSaved - if (call == lastCall && now - lastAt < DUPLICATE_WINDOW_MS) return - lastSaved = call to now + // Every outcome says something. The previous `if (call.length < 3) return` discarded the + // entry in silence, so pressing done mid-pass appeared to do nothing, and none of the + // logging software surveyed drops a submission that way. + val verdict = CallsignEntry.check(callsign, workedThisSession) + if (verdict is CallsignEntry.Verdict.Rejected) { + showToast(rejectionMessage(verdict.reason)) + return + } + val accepted = verdict as CallsignEntry.Verdict.Acceptable + val call = accepted.callsign // Freq taken directly from the transponder bar (radio Doppler-corrected each second; value at the Enter moment) val tx = radio.uplinkLow ?: radio.downlinkLow ?: 0L val rx = radio.downlinkLow ?: radio.uplinkLow ?: 0L @@ -298,8 +317,15 @@ private fun ExpandedLogInput( ) ) callsign = "" + workedThisSession = workedThisSession + call onSaved() - showToast(savedMsg) + showToast( + when (accepted.warning) { + CallsignEntry.Warning.LOOKS_LIKE_A_GRID -> gridWarnMsg + CallsignEntry.Warning.ALREADY_WORKED -> workedWarnMsg.format(call) + null -> savedMsg + } + ) // QRZ counterpart grid async backfill (4.5.5): only queried when Cookie is set; silent on failure scope.launch { val prefs = context.getSharedPreferences("qrz_cookie", android.content.Context.MODE_PRIVATE)