Fix decode-screen NPE crash when a message has a null destination call - #609
Merged
Merged
Conversation
…call
Ft8Message.checkIsCQ() dereferenced callsignTo (callsignTo.trim().split(...))
before its guard, and that guard checked the wrong variable: `if (s == null)`
after `String s = callsignTo.trim().split(" ")[0]` is dead, because
String.split()[0] is never null. The value that can actually be null is
callsignTo itself — it defaults to null and stays null for free-text/telemetry
frames and unresolved-hash decodes that still reach the published decode list.
The Compose decode screen calls checkIsCQ() unconditionally on the main thread
with no try/catch — DecodeRow (every rendered row), resolveQsoStatus, and
DecodeScreen.filterMessages — so such a message crashed the whole app on the
primary screen. This is proven reachable by the existing #254 guard in
ActiveQsoPanel (`if (msg.callsignTo != null && msg.checkIsCQ())`), a sibling
consumer of the same mainViewModel.mutableFt8MessageList; that guard was added
to ActiveQsoPanel but not to the decode-list consumers.
Root-cause fix: null-guard callsignTo inside checkIsCQ() itself, the single
choke point through which every caller funnels, so a missing destination is
treated as "not a CQ" instead of throwing. Behaviour is unchanged for every
message that has a callsignTo.
Test: Ft8MessageTest.checkIsCQ_falseWhenCallsignToNull (throws NPE before the
fix, passes after). Full :app:testDebugUnitTest and :app:assembleDebug (all
4 ABIs) verified green.
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 #609 +/- ##
=========================================
Coverage 36.62% 36.62%
Complexity 197 197
=========================================
Files 216 216
Lines 26885 26885
Branches 3294 3294
=========================================
Hits 9847 9847
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:
|
There was a problem hiding this comment.
Pull request overview
This pull request fixes a crash in the decode UI path by making Ft8Message.checkIsCQ() null-safe when callsignTo is missing (which can occur for free-text/telemetry/unresolved-hash decodes that still reach the rendered decode list).
Changes:
- Add a defensive
callsignTo == nullguard insideFt8Message.checkIsCQ()to returnfalseinstead of throwing. - Add a unit test ensuring
checkIsCQ()returnsfalse(and does not NPE) whencallsignTois left at its defaultnull.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| ft8af/app/src/main/java/com/k1af/ft8af/Ft8Message.java | Makes checkIsCQ() resilient to callsignTo == null, preventing main-thread crashes in UI callers. |
| ft8af/app/src/test/java/com/k1af/ft8af/Ft8MessageTest.java | Adds regression coverage for the null-destination case to prevent reintroducing the crash. |
💡 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.
Root cause
Ft8Message.checkIsCQ()dereferencedcallsignTobefore its guard, and the guard checked the wrong variable:callsignTodefaults tonull(public String callsignTo = null) and stays null for free-text/telemetry frames and unresolved-hash decodes that still reach the published decode list. Theif (s == null)check is dead —String.split()[0]never returns null — so the intended "missing destination → not a CQ" behaviour never actually protected the real deref.Impact (live main-thread crash on the primary screen)
The Compose decode screen calls
checkIsCQ()unconditionally on the main thread with no try/catch:DecodeRow.kt:72— for every rendered decode rowDecodeRow.kt:352— insideresolveQsoStatus(...)DecodeScreen.kt:479, 511, 515— insidefilterMessages(...)("Show only CQ", "CQ Calls", "New DXCC")A single decode with a null
callsignTotherefore crashed the whole app on the app's primary screen.This is proven reachable by the existing #254 guard already present in
ActiveQsoPanel.kt:104:ActiveQsoPanelis a sibling consumer of the samemainViewModel.mutableFt8MessageList. That guard was added there but not to the decode-list consumers, which have the identical deref.Fix
Null-guard
callsignToinsidecheckIsCQ()itself — the single choke point every caller funnels through — so a missing destination returns "not a CQ" instead of throwing. This coversDecodeRow,resolveQsoStatus,filterMessages, andMapScreenat once. Behaviour is byte-for-byte unchanged for any message that has acallsignTo.Testing performed
Ft8MessageTest.checkIsCQ_falseWhenCallsignToNull— reproduces the NPE against the unfixed code (java.lang.NullPointerException), passes after the fix.:app:testDebugUnitTest— full unit suite green.:app:assembleDebug— clean APK build across all 4 ABIs.Risk
Minimal. One-line defensive null-guard in a leaf helper; no protocol/DSP/threading behaviour changes; no change for well-formed messages.
🤖 Generated with Claude Code