iOS: don't upload the RR73 sign-off as a locator to PSKReporter - #615
Merged
Merged
Conversation
The FT8 end-of-QSO roger-73 report "RR73" is a 4-char Maidenhead look-alike (R,R are valid A-R field letters; 7,3 valid digits), so the decoder copies it into a message's `grid` field via `looksLikeGrid`. `PskReporter.makeSpot` gated the reported `senderLocator` on a naive `grid.count >= 4`, so every decoded RR73 was spotted to the global PSKReporter database as locator "RR73" — a phantom Arctic coordinate (83.5N, 175E) that pollutes a shared ecosystem resource and breaks WSJT-X interop (no protocol-compliant transmitter ever means grid RR73). The rest of the Kit already excludes it wherever it classifies a grid (`gridToLatLon` returns nil; `QsoEngine` maps it to the `.rr73` stage) — PSKReporter was the one consumer that missed it. This mirrors the Android fix (PskReporterSender.reportableLocator, PR #612), the iOS side of the same ecosystem-interop defect. Fix: extract a pure `reportableLocator(_:)` helper that requires a >= 4 char locator AND rejects the "RR73" sign-off (case-insensitive), and gate `makeSpot`'s locator on it. Well-formed grids are unaffected. Tests: PskReporterTests gains reportableLocator cases (accepts real grids, rejects short tokens and RR73/rr73/Rr73) and a makeSpot case asserting an RR73-grid decode is still spotted but with a nil locator. Verified red->green (reverting the helper to the old length-only check fails 4 cases). Whole-module `swift test` on Linux is blocked by an unrelated Apple-only `import Network` in WsjtxUdpService.swift; the Foundation-only PskReporter source + its tests were run in isolation via SwiftPM. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #615 +/- ##
============================================
+ Coverage 36.62% 36.68% +0.06%
Complexity 197 197
============================================
Files 216 216
Lines 26885 26912 +27
Branches 3294 3294
============================================
+ Hits 9847 9874 +27
Misses 16811 16811
Partials 227 227
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Merged
5 tasks
There was a problem hiding this comment.
Pull request overview
This PR fixes an iOS FT8Engine PSKReporter spotting bug where the FT8 end-of-QSO sign-off token “RR73” (a 4-character Maidenhead look-alike) could be uploaded as a senderLocator, polluting PSKReporter with a phantom locator.
Changes:
- Added a pure helper
PskReporter.reportableLocator(_:)to accept locators only when>= 4characters and not equal to"RR73"(case-insensitive). - Updated
PskReporter.makeSpotto usereportableLocator(_:)instead of a naive length-only check. - Added XCTest coverage for both
reportableLocator(_:)and themakeSpotRR73 behavior (spot still generated, but withsenderLocator == nil).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| ios/FT8AFKit/Sources/FT8Engine/PskReporter.swift | Centralizes PSKReporter locator policy in reportableLocator(_:) and prevents uploading RR73 as a locator. |
| ios/FT8AFKit/Tests/FT8EngineTests/PskReporterTests.swift | Adds unit tests validating the RR73 exclusion and confirming makeSpot behavior remains correct otherwise. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The iOS
PskReporter.makeSpotreported a decode'ssenderLocatorwhenever the decodedgridfield was>= 4characters. The FT8 end-of-QSO roger-73 report "RR73" is a 4-char Maidenhead look-alike (R,Rare valid A–R field letters;7,3valid digits), so the decoder copies it intogridvialooksLikeGrid— and every decoded RR73 was then spotted to the global PSKReporter database as locator "RR73", a phantom Arctic coordinate (83.5°N, 175°E).This is the iOS side of the same defect fixed on Android in PR #612 (
PskReporterSender.reportableLocator).Root cause
Per the FT8 protocol, "RR73" is packed as the roger-73 report, never as a grid — no compliant transmitter means grid RR73. The rest of the Kit already recognizes this:
gridToLatLon(Util.swift:63) returnsnilfor "RR73"/"RR" to avoid a phantom map pin.QsoEngineclassifies an incoming "RR73" as the.rr73sequencer stage (QsoEngine.swift:348-358).PskReporter.makeSpot(PskReporter.swift:65) was the one consumer that used a naive length check instead, so it leaked RR73 into the shared spot DB — polluting an ecosystem resource and breaking WSJT-X interop.Fix
Extracted a pure
reportableLocator(_:)helper that requires a>= 4char locator and rejects the "RR73" sign-off (case-insensitive), and gatedmakeSpot'ssenderLocatoron it. Well-formed grids (FN42,IO91wm, …) are unaffected — byte-identical spots.Testing performed
PskReporterTestscases:reportableLocatoraccepts real grids, rejects short tokens ("","FN","RR") and the RR73 sign-off (RR73/rr73/Rr73); and amakeSpotcase asserting an RR73-grid decode is still spotted (the call is real) but with a nil locator.PskReporter.swiftin isolation on Linux via SwiftPM: 24 tests pass. Verified red→green — reverting the helper to the old length-only check fails 4 cases.swift teston Linux is currently blocked by an unrelated Apple-onlyimport NetworkinWsjtxUdpService.swift; the Foundation-onlyPskReportersource + its tests were exercised standalone.Risk assessment
Very low. Single, focused policy change on a pure function; the only behavioral difference is that a locator equal to "RR73" is now omitted (as it already is everywhere else in the app). No protocol/encoder/DSP changes; well-formed decodes produce identical spots.
🤖 Generated with Claude Code