Fix FlexRadio meter-stream crash on a nameless dBm/swr meter#495
Open
patrickrb wants to merge 1 commit into
Open
Fix FlexRadio meter-stream crash on a nameless dBm/swr meter#495patrickrb wants to merge 1 commit into
patrickrb wants to merge 1 commit into
Conversation
FlexMeterList.setMeters() already guards the unknown-meter case
(infos.get(val) == null) but then unconditionally dereferences
meter.name.contains("PWR") in the dBm/swr branch.
FlexMeterInfo.nam is left null whenever the meter-definition frame
carried a .unit token but no (or an empty) .nam token — e.g. a partial
or split METER_LIST frame that supplies "N.unit=dBm" for a meter whose
.nam was never seen (the existing FlexMeterInfosTest.reparseUpdates...
case exercises exactly this "unit without name" shape). Reaching the
dBm/swr branch for such a meter threw an uncaught NullPointerException.
Meter value frames are parsed on the FlexRadio meter-stream read thread,
whose loop catches only SocketException/IOException (same context as the
FlexMeterInfos crash fixed in PR #491), so the escaping NPE crashed the
whole app on every subsequent meter update.
Root cause: unsafe coupling — the consumer assumes .nam and .unit are
always present together, but FlexMeterInfo populates them independently.
Fix: null-guard meter.name before contains("PWR"). A nameless meter can't
be identified as a PWR meter, so it simply keeps its raw dBm value, which
is the correct conservative behavior.
Adds FlexMeterListTest (pure JVM, no Robolectric) covering the nameless
dBm and swr meters (the crash), plus the named FWDPWR dBm->watt
conversion, a named non-PWR dBm meter, and the unknown-id skip guard to
lock in existing behavior.
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 #495 +/- ##
=========================================
Coverage 23.35% 23.35%
Complexity 162 162
=========================================
Files 167 167
Lines 21450 21450
Branches 3154 3154
=========================================
Hits 5009 5009
Misses 16248 16248
Partials 193 193
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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
FlexMeterList.setMeters()guards the unknown-meter case (infos.get(val) == null) but then unconditionally dereferencesmeter.name.contains("PWR")in the dBm/swr branch.FlexMeterInfo.namisnullwhenever a meter-definition frame carried a.unittoken but no (or an empty).namtoken, so a dBm/swr meter with no name threw an uncaughtNullPointerException, crashing the whole app.This is a direct follow-up to PR #491 (which hardened the sibling
FlexMeterInfosparser against a corrupt/partialMETER_LISTframe) — same subsystem, same read-thread crash class.Root cause
Unsafe coupling: the consumer (
FlexMeterList) assumes.namand.unitare always present together, butFlexMeterInfospopulates them independently from separate#-delimited tokens. A partial/splitMETER_LISTframe that supplies onlyN.unit=dBmfor a meter whose.namwas never seen (or aN.nam=with an empty value, whichsplit("=")drops) yieldsunit=dBm/swr, nam=null. The existingFlexMeterInfosTest.reparseUpdatesExistingDefinitionstest already exercises exactly this "unit without name" shape viasetMeterInfos("meter 1.unit=SWR").Why it crashes the whole app
Meter value frames are parsed on the FlexRadio meter-stream read thread, whose loop catches only
SocketException/IOException(the same context documented for the PR #491 crash). The escapingNullPointerExceptionis not caught, so the app dies — and becausesetMetersruns on every meter update, it recurs on every subsequent frame.Fix
Null-guard
meter.namebeforecontains("PWR"). A nameless meter cannot be identified as a PWR meter, so it simply retains its raw dBm value — the correct conservative behavior. One-line, localized, no protocol/behavior change for well-formed frames.Testing performed
FlexMeterListTest(pure JVM, no Robolectric). Written first; the two nameless-meter cases reproduced theNullPointerExceptionbefore the fix and pass after:dbmMeterWithoutName_doesNotCrash/swrMeterWithoutName_doesNotCrash— the crash paths.namedPwrMeter_stillConvertsDbmToPower— FWDPWR dBm→watt conversion still works (30 dBm → 1.0 W).namedNonPwrDbmMeter_keepsRawDbm— named non-PWR dBm meter keeps raw value.unknownMeterId_isSkipped— pins the existing unknown-id guard../gradlew :app:testDebugUnitTest— full suite green../gradlew :app:assembleDebug— builds/packages clean.Risk assessment
Very low. The change only adds a
!= nullshort-circuit ahead of an existing dereference; for any meter that does have a name (the normal case), behavior is identical. No native/DSP code touched, no protocol change, no performance impact (one reference comparison per dBm/swr meter update).🤖 Generated with Claude Code