Fix CAT meter parser crash on non-numeric rig replies#489
Merged
Conversation
The numeric CAT meter parsers on Yaesu3Command (getALCOrSWR38, getSWROrALC39, get590ALCOrSWR) and ElecraftCommand (getSWRMeter) call Integer.parseInt on a fixed substring of the rig's reply, validating only the token length and never that the substring is numeric. get590ALCOrSWR additionally had no length guard at all. These parsers run per-CAT-poll inside onReceiveData. On the Bluetooth SPP path (BluetoothSerialService -> BluetoothRigConnector.onSerialRead -> BaseRig.onReceiveData) that runs on the main thread with no surrounding try/catch, so a garbled or non-numeric meter reply threw NumberFormatException (or StringIndexOutOfBoundsException for the unguarded get590ALCOrSWR) and hard-crashed the app. Treat malformed meter data as 'no reading' (0) instead of throwing, matching the existing getFrequency() convention in the same files. 0 is the safe sentinel: it never falsely trips the SWR-halt / ALC protection, which key off high values. Add CatMeterParserTest covering numeric, non-numeric, and short data for all four parsers. 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 #489 +/- ##
=========================================
Coverage 23.34% 23.35%
- Complexity 149 162 +13
=========================================
Files 165 167 +2
Lines 21401 21450 +49
Branches 3149 3154 +5
=========================================
+ Hits 4997 5009 +12
- Misses 16215 16248 +33
- Partials 189 193 +4
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 hardens CAT meter parsing against malformed rig replies to prevent main-thread crashes (notably on the Bluetooth SPP path), while preserving behavior for valid numeric meter data.
Changes:
- Add numeric parsing guards (length checks +
NumberFormatExceptionhandling) for Yaesu/Kenwood meter parsers inYaesu3Command. - Add length check +
NumberFormatExceptionhandling for Elecraft SWR meter parsing inElecraftCommand. - Introduce a new unit test suite (
CatMeterParserTest) covering valid, non-numeric, and short meter replies across the affected parsers.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| ft8af/app/src/main/java/com/k1af/ft8af/rigs/Yaesu3Command.java | Prevents crashes by guarding substring bounds and safely parsing meter tokens via a shared helper that returns 0 on malformed input. |
| ft8af/app/src/main/java/com/k1af/ft8af/rigs/ElecraftCommand.java | Adds length check and safe parsing to avoid NumberFormatException crashes on malformed SWR replies. |
| ft8af/app/src/test/java/com/k1af/ft8af/rigs/CatMeterParserTest.java | Adds focused unit coverage for all affected meter parsers across valid and malformed inputs. |
💡 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
The numeric CAT meter parsers validate the reply's token length but never that the substring is actually numeric before calling
Integer.parseInt:Yaesu3Command.getALCOrSWR38—parseInt(data.substring(1,4))(FTDX/FT-950 family)Yaesu3Command.getSWROrALC39—parseInt(data.substring(1,4))(FT-891 family)Yaesu3Command.get590ALCOrSWR—parseInt(data.substring(1,5))(Kenwood TS-590/2000/570) — also had no length guard at allElecraftCommand.getSWRMeter—parseInt(data.substring(0,3))These parsers run per-CAT-poll inside
onReceiveData. On the Bluetooth SPP path the reply is dispatched to the parser on the main thread with no surrounding try/catch:So a garbled or non-numeric meter reply from the rig threw
NumberFormatException(orStringIndexOutOfBoundsExceptionfor the unguardedget590ALCOrSWR) and hard-crashed the app. (The USB path is caught bySerialInputOutputManager.run(), which only tears down the CAT thread — hence this bites Bluetooth-connected rigs.)Fix
Treat malformed meter data as "no reading" (return
0) instead of throwing, matching the existinggetFrequency()convention already used in the same two files. Added a length guard toget590ALCOrSWRto match its sibling parsers.0is the safe sentinel: it never falsely trips the SWR-halt / ALC auto-volume protection (MeterProtectionController), which key off high values.Testing
CatMeterParserTestcovers numeric, non-numeric, and short data for all four parsers (12 cases). Verified the 6 malformed-data cases fail withNumberFormatException/StringIndexOutOfBoundsExceptionagainst the old code, and all 12 pass after the fix.:app:testDebugUnitTestsuite passes.:app:assembleDebug(including native build) succeeds.Risk
Very low. Behavior is unchanged for valid meter replies (same parsed integer); only malformed/garbled input changes from crash to 0. No protocol, DSP, or threading behavior touched. No device was attached in this environment, so no on-device verification was performed.
🤖 Generated with Claude Code