Fix GuoHe Q900 frame sync: require four consecutive 0xA5 bytes - #624
Conversation
GuoHeQ900Rig.checkHead() counted 0xA5 bytes anywhere in the read buffer and returned as soon as the 4th was seen. But every GuoHe frame begins with FOUR CONSECUTIVE 0xA5 sync bytes (GuoHeRigConstant), and a status frame's payload carries two big-endian VFO frequencies whose bytes are frequently 0xA5. When a serial read splices a prior frame's 0xA5-bearing tail onto the next frame's sync run, the counter reached 4 partway through and returned an index *into* the sync run. onReceiveData then read a 0xA5 as the length byte: (byte)0xA5 + 1 == -90 -> new byte[-90] -> NegativeArraySizeException The exception is swallowed by onReceiveData's try/catch (so it's not an app crash) but it aborts framing before clearBuffer(), leaving stale partial-frame state and silently dropping the frequency update — an intermittent rig frequency-tracking failure on the GuoHe Q900. Fix: reset the run counter on any non-0xA5 byte and return the first byte after a run of >=4 consecutive 0xA5 (the true length byte). This also correctly skips a stray leading 0xA5 that would otherwise make five in a row, and reports "no header yet" (-1) instead of overrunning when a sync run lands at the very end of a read. checkHead is now package-private static (it uses no instance state) so the framing logic is covered directly by GuoHeCheckHeadTest without standing up the rig's Timer/connector. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes GuoHe Q900 (and related Xiegu-family) frame sync detection by updating GuoHeQ900Rig.checkHead() to only accept runs of 4+ consecutive 0xA5 bytes as the sync header, preventing mis-framing when 0xA5 bytes appear in payload (e.g., status-frame frequency fields).
Changes:
- Update
checkHead(byte[])to reset the counter on non-0xA5bytes and return the index of the first non-0xA5byte after a>=4sync run (the length byte). - Make
checkHeadstatic(package-private) so it can be unit-tested directly without instantiating the rig/timer. - Add a dedicated unit test suite covering well-formed frames and common mis-framing splice scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ft8af/app/src/main/java/com/k1af/ft8af/rigs/GuoHeQ900Rig.java | Fixes header detection to require consecutive sync bytes and documents the framing rationale. |
| ft8af/app/src/test/java/com/k1af/ft8af/rigs/GuoHeCheckHeadTest.java | Adds unit tests validating correct header detection across realistic stream/splice cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Javadoc: -1 also covers "sync run present but the length byte hasn't arrived yet" (a read ending inside the run), not just "no sync". - Test comment: the scattered 0xA5s are sync bytes; the old bug was returning an index into the sync run so the caller read a sync byte as the length byte. 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 #624 +/- ##
=========================================
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:
|
Summary
GuoHeQ900Rig.checkHead()misidentifies the frame sync header, which intermittently corrupts frequency tracking on the GuoHe Q900 (and its Xiegu-family siblings that share this framing).Every GuoHe frame begins with four consecutive
0xA5sync bytes (seeGuoHeRigConstant—PTT_ON,USB_MODE,READ_FREQ, etc. all startA5 A5 A5 A5 <len> …). The oldcheckHeadcounted0xA5bytes anywhere in the read buffer and returned the moment the 4th was seen:Root cause
A status frame (
cmd 0x0b) carries two big-endian VFO frequencies atbuffer[5..12], and those bytes are frequently0xA5(common HF frequencies land on it). USB-serial reads don't respect frame boundaries, so a read routinely delivers a prior frame's0xA5-bearing tail followed by the next frame's sync run. The old counter reached 4 partway through those scattered/adjacent0xA5bytes and returned an index pointing into the sync run.onReceiveDatathen read a0xA5as the length byte:The exception is swallowed by
onReceiveData'stry/catch, so it is not an app crash — but it aborts framing beforeclearBuffer(), leaving stale partial-frame state and silently dropping the frequency update. Symptom: the app's displayed frequency intermittently fails to track the GuoHe rig's VFO.Fix
Reset the run counter on every non-
0xA5byte and return the first byte after a run of ≥4 consecutive0xA5— the true length byte:This also:
0xA5(e.g. a previous frame's CRC low byte) that would otherwise make five in a row and, under the old code, still land on a0xA5length byte;-1("no complete header yet") instead of an out-of-range index when a sync run lands at the very end of a read.checkHeadis now package-privatestatic(it referenced no instance state), so the framing logic is unit-tested directly without standing up the rig'sTimer/connector.Testing
New
GuoHeCheckHeadTest(pure logic, no Robolectric needed) — 6 cases:0xA5payload bytes are not mistaken for sync0xA5freq bytes) + next sync → finds the length byte, never a0xA5-1-1(no overrun)Four of these return the wrong index against the old implementation, so the suite fails-before / passes-after.
Risk assessment
Very low. The change is confined to a single pure
byte[] → inthelper; no protocol/DSP/native code touched, no behavior change for correctly-framed input (the well-formed case returns the identical index as before). Strictly more robust for mis-framed input. No performance impact (same single linear pass).Affected platforms
Android only — GuoHe Q900 CAT/frequency read path.