fix(xvtr): use status index not order for Flex band-stack key (#2342)#2511
Merged
Conversation
Flex's `display pan set band=X<n>` expects the xvtr status-object index (the number from `xvtr <n>` messages, 0-based), not the radio's 1-based `order` field. Using `order` produced a +1 shift: pressing the 144 MHz button sent `band=X1` instead of `band=X0`, tuning to 432 MHz instead. Reverts the index→order swap from PR #1887 back to the pcap-verified `X<index>` form from PR #1615. Updates the test assertions and renames the test function to reflect the correct semantics. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Thanks @jensenpat — the fix is correct and well-scoped.
I traced the data flow to confirm the change is safe:
Transverter::indexis populated from the integer key inxvtr <n>status messages (RadioModel.cpp:2963—x.index = idx;), so it's always a non-negative value once the entry exists. Dropping theorder < 0guard is therefore fine —indexdoesn't need a sentinel-missing check the way the optionalorderfield did.- The PR description's pcap reference (PR #1615 vs the #1887 regression) matches what the radio's wire protocol actually expects, and the user-visible symptom table (button N tunes band N+1) follows directly from the off-by-one between 0-based
indexand 1-basedorder. - The test rename + updated assertions (
X12/X4/X1for the new fixture) and the rewritten "missing order" case correctly reflect thatorderis no longer load-bearing for key resolution. - The MainWindow.cpp comment update is the only other touch and stays in the same maintainer block — scope is tight.
No conventions, RAII, or boundary-handling concerns. Nice clean revert with tests pinning the contract.
Collaborator
|
Claude here — thank you @jensenpat, this is a clean revert of an off-by-one regression with great forensics in the PR body. The index/order table makes the bug obvious at a glance, and dropping the now-redundant 73, Jeremy KK7GWY & Claude (AI dev partner) |
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
2 participants
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.
Fixes #2342
Root cause
XvtrPolicy::resolveBandStackKeywas returningX<order>for XVTR band buttons, but the Flex radio expectsX<index>— whereindexis the 0-based number fromxvtr <n>status messages. The radio'sorderfield is 1-based (SmartSDR labels them "Transverter 1, 2, 3"), so every button was shifted by +1:band=X1band=X2band=X3This regression was introduced in PR #1887 which swapped the pcap-verified
X<index>(from PR #1615) forX<order>without new pcap evidence.Changes
src/models/XvtrPolicy.cpp— revertxvtr.order→xvtr.index; drop the now-unnecessaryorder < 0guardsrc/gui/MainWindow.cpp— update the maintainer comment to reflectX<index>tests/xvtr_policy_test.cpp— update assertions toX<index>values; rename test; update the "missing order" case (order is now irrelevant for key resolution)Test plan
./build/xvtr_policy_test— all 21 tests pass ✅🤖 Generated with Claude Code