From d5230b3bc67aee1c188b19a5754d219f1c7b7434 Mon Sep 17 00:00:00 2001 From: QIU Date: Fri, 14 Aug 2026 15:57:32 +0000 Subject: [PATCH] fix(qth): correct Maidenhead boundaries and longitude wrapping A full-domain round-trip probe found three related boundary bugs. 1. Exact positive limits wrapped the square/subsquare terms to zero positionToQth clamped only the A-R field index. At +90 latitude / +180 longitude the field saturated at R, but all later terms used modulo and wrapped to square 0 / subsquare a: (90, 180) -> RR00aa00 -> (80.002083, 160.004167) error: -9.998 deg latitude, -19.996 deg longitude The existing test incorrectly asserted RR00aa00 and had therefore fossilised the defect. Clamp shifted coordinates just inside the half-open upper bound so the limits land in the final cell RR99xx99. 2. isValidPosition allowed longitude through +360 Maidenhead covers -180..180, but 181..360 was accepted and produced plausible locators that decoded 20-200 degrees away: lon 181 -> decoded 161.004167 (error -19.996) lon 270 -> decoded 170.004167 (error -99.996) lon 360 -> decoded 160.004167 (error -199.996) Restrict the converter contract to -180..180. 3. Locator validation allowed S-X as field letters The first pair has 18 fields A-R, while only the later subsquare pairs use A-X. The shared [A-X]{2} regex accepted SS00aa / XX99xx and decoded them past the poles (up to lat 149.98, lon 299.96). Use A-R for the field pair. The SettingsRepo caller had a separate wrapping bug that masked part of this: it mapped longitude>180 by subtracting 180 (270 -> +90, wrong hemisphere) instead of modulo 360 (270 -> -90). Fix that at the writer too. Verification: - standalone JVM sweep: 519,841 points, old code had 1,441 large-error points with max drift 9.997917 deg lat / 19.995833 deg lon - new Kotlin regression sweep requires every 8-char round trip <=0.01 deg - QthConverterTest BUILD SUCCESSFUL - full :core:domain:test + :core:data:compileReleaseKotlin BUILD SUCCESSFUL --- .../core/data/repository/SettingsRepo.kt | 5 +- .../core/domain/utility/QthConverter.kt | 16 ++++-- .../look4sat/core/domain/QthConverterTest.kt | 53 ++++++++++++++++++- 3 files changed, 68 insertions(+), 6 deletions(-) diff --git a/core/data/src/main/java/com/rtbishop/look4sat/core/data/repository/SettingsRepo.kt b/core/data/src/main/java/com/rtbishop/look4sat/core/data/repository/SettingsRepo.kt index 9b94710d..a36cb27d 100644 --- a/core/data/src/main/java/com/rtbishop/look4sat/core/data/repository/SettingsRepo.kt +++ b/core/data/src/main/java/com/rtbishop/look4sat/core/data/repository/SettingsRepo.kt @@ -181,7 +181,10 @@ class SettingsRepo( } override fun setStationPosition(latitude: Double, longitude: Double, altitude: Double): Boolean { - val newLongitude = if (longitude > 180.0) longitude - 180 else longitude + // Wrap an out-of-range longitude into -180..180. Subtracting 180 (the + // previous behaviour) mapped 270 to +90 instead of -90, i.e. the wrong + // hemisphere, and 360 to +180 instead of 0. + val newLongitude = ((longitude + 180.0).mod(360.0)) - 180.0 val locator = positionToQth(latitude, newLongitude) ?: return false setStationPosition(latitude, newLongitude, altitude, locator) return true diff --git a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/utility/QthConverter.kt b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/utility/QthConverter.kt index 907081cf..6f77e747 100644 --- a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/utility/QthConverter.kt +++ b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/utility/QthConverter.kt @@ -74,8 +74,14 @@ fun qthToPosition(locator: String): GeoPos? { */ fun positionToQth(latitude: Double, longitude: Double, precision: Int = 8): String? { if (!isValidPosition(latitude, longitude)) return null - val newLongitude = longitude + 180 - val newLatitude = latitude + 90 + // The grid spans [0, 360) lon and [0, 180) lat once shifted. Clamping the + // field index alone (coerceIn below) is not enough: at exactly +90 lat or + // +180 lon the field saturates to R while the square/subsquare terms come + // from a modulo that has already wrapped to 0, so the encoded locator + // decoded back 10 degrees of latitude / 20 degrees of longitude away. + // Nudge the upper bound into the last cell instead. + val newLongitude = (longitude + 180).coerceIn(0.0, 360.0 - 1e-9) + val newLatitude = (latitude + 90).coerceIn(0.0, 180.0 - 1e-9) val lonFirst = (65 + (newLongitude / 20).toInt().coerceIn(0, 17)).toChar() val latFirst = (65 + (newLatitude / 10).toInt().coerceIn(0, 17)).toChar() val lonSecond = ((newLongitude % 20) / 2).toInt() @@ -94,11 +100,13 @@ fun positionToQth(latitude: Double, longitude: Double, precision: Int = 8): Stri } private fun isValidPosition(lat: Double, lon: Double): Boolean { - return (lat >= -90.0 && lat <= 90.0) && (lon >= -180.0 && lon <= 360.0) + return lat in -90.0..90.0 && lon in -180.0..180.0 } private fun isValidLocator(locator: String): Boolean { - return locator.matches("[a-xA-X]{2}\\d{2}[a-xA-X]{2}(?:\\d{2}(?:[a-xA-X]{2})?)?".toRegex()) + // Maidenhead fields are A-R (18 x 18). Subsquare letters are A-X (24). + // Accepting S-X in the first pair decodes to latitude >90 / longitude >180. + return locator.matches("[a-rA-R]{2}\\d{2}[a-xA-X]{2}(?:\\d{2}(?:[a-xA-X]{2})?)?".toRegex()) } /** diff --git a/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/QthConverterTest.kt b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/QthConverterTest.kt index bf73207a..52203486 100644 --- a/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/QthConverterTest.kt +++ b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/QthConverterTest.kt @@ -70,7 +70,9 @@ class QthConverterTest { fun `Given boundary POS stays in valid grid`() { // antipodal / edge cases must not overflow the A-R / 0-9 / a-x alphabet assert(positionToQth(-90.0, -180.0, 8) == "AA00aa00") - assert(positionToQth(90.0, 180.0, 8) == "RR00aa00") + // Exact positive bounds belong to the final cell, not a modulo-wrapped + // R-field/0-square combination that decodes 10°/20° away. + assert(positionToQth(90.0, 180.0, 8) == "RR99xx99") assert(positionToQth(0.0, 0.0, 8) == "JJ00aa00") // roundtrip stability: 8-char roundtrip is stable across a sample of positions val positions = listOf( @@ -85,6 +87,55 @@ class QthConverterTest { } } + @Test + fun `Encoded locator always decodes back within one cell`() { + // An 8-char cell is 30" lon x 15" lat, so a correct encode/decode pair + // can never differ by more than that. Field clamping used to break this + // near +90 / +180 and produced errors up to 10 deg lat / 20 deg lon. + var worstLat = 0.0 + var worstLon = 0.0 + var worst = "" + var lat = -90.0 + while (lat <= 90.0) { + var lon = -180.0 + while (lon <= 180.0) { + val qth = positionToQth(lat, lon, 8) + ?: error("valid position rejected: ($lat, $lon)") + val pos = qthToPosition(qth) ?: error("own output rejected: $qth") + val dLat = kotlin.math.abs(pos.latitude - lat) + val dLon = kotlin.math.abs(pos.longitude - lon) + if (dLat > worstLat || dLon > worstLon) { + worstLat = maxOf(worstLat, dLat) + worstLon = maxOf(worstLon, dLon) + worst = "($lat, $lon) -> $qth -> (${pos.latitude}, ${pos.longitude})" + } + lon += 0.5 + } + lat += 0.5 + } + assert(worstLat <= 0.01 && worstLon <= 0.01) { + "roundtrip drifted by (${worstLat}, ${worstLon}) deg, worst: $worst" + } + } + + @Test + fun `Given out of range longitude returns null`() { + // Maidenhead only covers -180..180; 181..360 used to be accepted and + // encoded into a plausible-looking locator 20-200 deg away. + assert(positionToQth(0.0, 181.0) == null) + assert(positionToQth(0.0, 270.0) == null) + assert(positionToQth(0.0, 360.0) == null) + } + + @Test + fun `Given locator with out of range field returns null`() { + // Fields run A-R; S-X in the first pair decoded past the poles. + assert(qthToPosition("SS00aa") == null) + assert(qthToPosition("XX99xx") == null) + assert(qthToPosition("AS00aa") == null) + assert(qthToPosition("AX99xx") == null) + } + @Test fun `Given square returns correct 3x3 neighbors`() { // Reference grid from the QTH Locator screenshot: OL42