fix(flex): Fix active-slice ALC and TX waveform meter routing - #5344
fix(flex): Fix active-slice ALC and TX waveform meter routing#5344jensenpat wants to merge 1 commit into
Conversation
ten9876
left a comment
There was a problem hiding this comment.
Issue fit
#5340's diagnosis is correct and — unusually — was verified all the way down: the claim that FlexLib attaches meters to slices only for Source == "SLC" was independently confirmed against Meter.cs:41 and Radio.cs:6168-6175 by two analysis passes, and the 8400M num=0/9 capture demonstrates the arithmetic the old resolver trusted is unsound. The direction — key TX waveform meters by slice context, demote the explicit-source map to fallback — is right. The implementation rests on an ordering assumption the client itself does not preserve, and that is where the blockers live.
Scope
Four files, all explained by the issue. Clean checklist; no CHANGELOG; commit signed; tests are socket-free model injection in the already-registered meter_model_test. Preflight: no sockets, no fake peers.
Blockers
1. The slice-context scheme trusts an ordering the client's own decode destroys, the context is never invalidated, and the double-registration defeats both fallbacks (inline ×2). Four facets, one root:
- Wire order is not preserved.
FlexBackend::decodeMeterStatusgroups a multi-meter status into aQMap<int, …>and iterates in meter-index order. Flex reuses meter IDs after removal, so a slice created after another was torn down can carry a TX block with a lower reused ID than its own SLC block — processed whilem_manifestSliceContextstill points at the previous slice, writingbySlice[wrongSlice]. The explicitbyTxSourceentry holds the right answer, but the new priority order consults the poisoned entry first. The old explicit-first code was immune whenever the source was explicit. - The context has no invalidation. It is set on any SLC def and cleared only in
clear(), so a mid-session re-announce of a single TX meter (unit change, profile reload) with no preceding SLC block lands under whatever slice defined meters last — overwriting another slice's correct entry. - A poisoned entry permanently disables the #4609 fallback. The single-implicit-meter rescue requires
bySlice.size() == 1; one stale entry keyed to a dead slice (whichremoveMeter's by-value purge cannot remove while the meter under it lives) makes it2forever — the exact single-modulator dead-gauge class #4609 fixed, reintroduced by bookkeeping. - Dual registration defeats the explicit-single-meter case too. A manifest with one explicit-source TX ALC block (SLC context present) fills both maps; move TX to the other slice and bySlice misses, the base-arithmetic key misses, and the fallback is gated on
byTxSource.isEmpty()— false — so the gauge dies where the pre-PR scalar followed transmit anywhere. No test covers this shape (the new test covers only the implicitnum=0single-meter case).
Fix directions that keep the PR's (correct) model: iterate the decoded meter group in wire/insertion order and reset m_manifestSliceContext at each message boundary, so context genuinely means "this block's slice"; register byTxSource or bySlice, not both (context-derived defs are slice-known; explicit-context-less defs are source-known); and make the last-resort fallback key on "exactly one distinct meter index across both maps" rather than map emptiness.
2. The clear paths emit 0.0f, which on the dBFS gauge is full-scale top, not "no reading" (inline). kAlcGaugeFloorDbfs is −20; setActiveTxSlice and active-meter removal emit swAlcChanged(0.0f), and the consumer forwards it verbatim whenever the interlock reports transmitting — so a TX-slice reassignment or meter removal mid-key pegs the Phone/CW ALC gauge full red for the transient, and TciServer caches 0.0 as a genuine max-ALC reading for TCI clients. Emit the floor (or an explicit invalid marker the consumer maps to the floor).
Nits (non-blocking)
- The behavioral narrowing to
m_activeTxSlice >= 0has no in-tree victim — verified: Icom setstxSlice=truebefore publishing meters, HL2 sets it in its slice delta, and the seam path recomputes on every delta — but the PR's own test edits (setActiveTxSlice(0)added to two previously-passing tests) document the semantic change, and any backend publishing ALC without ever marking a TX slice goes silently dead where it used to work. Allowing the unique-entry fallback whenactiveTxSlice < 0costs nothing (one transmitter answers itself) and closes the window. - The registration ladder is copy-pasted three times, the resolver ladder three times (this PR itself had to edit all copies in lockstep to flip the priority — the drift risk demonstrating itself), the erase-by-value loop now has six hand-rolled copies where
QMap::removeIfis the in-repo idiom, and the SC_* resolvers' deliberate lack of the single-meter fallback is undocumented (the day a single-modulator backend publishes SC_*, #4609 recurs one meter-name over). - The ALC branch is now source-narrowed (
isTxWaveformMeter && name=="ALC") where the old branch was source-agnostic; no in-tree victim, but an ALC def under any other source now falls through the whole chain with no warning while its values are stored — aqCWarningon an unrouted ALC def would match the project's recent loud-drop commits. - The learnings doc's resolution table omits the third rung (single-implicit fallback) and no longer explains that the byTxSource key still comes from the base arithmetic the same doc declares unsound; the code comment stating the 8400M shape omits the firmware version (AGENTS.md wants it at the comment, not only in the doc);
convertAlcToGaugeDbfsno longer reads member state and can be file-static; the commit cites Principle XI where the load-bearing principle is I.
What was verified vs read
- Verified: the FlexLib attachment rule (two passes,
Radio.csline-cited); the context set/clear lifecycle (set on any SLC, cleared only inclear()); the decode-order fact underlying blocker 1a; the fallback gates (activeTxSlice >= 0,byTxSource.isEmpty(),size()==1); the −20 floor vs the 0.0 emits and the interlock-gated consumer; the in-tree no-victim status of the narrowing (Icom/HL2 marking traced to their publish sites); and the test edits documenting the tightening. - Refuted along the way: per-packet resolution cost (pre-resolved, clean), stale-map aliasing on index reuse (
removeMeterpurges both maps correctly), emit-spam onsetActiveTxSlice(early-returns), and unit-at-update-time hazards (guarded, equivalent). - Not run: no hardware; the 8400M capture is the author's, and the blocker scenarios are constructed from the client's own decode semantics rather than firmware behavior — they need no radio to demonstrate.
| m_swAlcUnit = def.unit; | ||
| else if (isTxWaveformMeter(def) && def.name == "ALC") { | ||
| if (m_manifestSliceContext >= 0) { | ||
| m_swAlcIdxBySlice[m_manifestSliceContext] = def.index; |
There was a problem hiding this comment.
Blocker 1 (root) — this write trusts an ordering the client's own decode does not preserve, from a context that is never invalidated. FlexBackend::decodeMeterStatus iterates its decoded group in meter-index order, not wire order, and Flex reuses meter IDs after removal — so a reused-lower-ID TX block can be processed under the previous slice's context. The context itself is set on any SLC def and cleared only in clear(), so a mid-session single-meter re-announce (unit change, profile reload, the band-recall slice churn this radio family is known for) lands under whatever slice defined meters last, clobbering another slice's correct entry — and the new bySlice-first priority means the still-correct explicit byTxSource entry is never consulted. A poisoned entry keyed to a dead slice also makes bySlice.size()==2 forever, permanently disabling the #4609 single-modulator fallback.
Keeping the model but making it sound: iterate the decode group in wire/insertion order and reset the context at message boundaries (context then genuinely means "this block's slice"); register byTxSource or bySlice per def, not both; and key the last-resort fallback on "one distinct meter index across both maps" rather than map emptiness — which also fixes the explicit-single-meter manifest where the current byTxSource.isEmpty() gate leaves the gauge dead on a TX move (untested shape; the new test covers only the implicit case).
| m_swAlc = 0.0f; | ||
| logCompressionSummary("active-slice-change", true); | ||
| emit micMetersChanged(m_micLevel, m_compLevel, m_micPeak, m_compPeak); | ||
| emit swAlcChanged(m_swAlc); |
There was a problem hiding this comment.
Blocker 2 — 0.0f is the dBFS gauge's full-scale top, not "no reading". kAlcGaugeFloorDbfs is −20, and the consumer forwards this signal verbatim whenever the interlock reports transmitting — so a TX-slice reassignment (or active-meter removal, same emit at the removeMeter path) mid-key pegs the Phone/CW ALC gauge full red for the transient, and TciServer caches 0.0 as a genuine max-ALC reading for its clients. Emit the floor for the dBFS shape (or an invalid marker the consumers map to the floor); the Percent shape's clear value is genuinely 0.
Closes #5340.
Problem
On a multi-slice FLEX radio, AetherSDR could select the wrong software-ALC meter after TX ownership moved away from Slice A. The prior scalar ALC index was last-definition-wins, while the attempted per-slice resolver assumed that
TX-meterSourceIndexvalues formed a slice-relative sequence.That assumption is not a FlexLib API contract. FlexLib only interprets
Meter.SourceIndexas a slice ID whenMeter.Source == Meter.SOURCE_SLICE(SLC) and only those meters are attached toSlice.Meters. TX waveform meters such asALC,COMPPEAK,SC_MIC,SC_FILT_1, andSC_FILT_2remain radio-level meters.Live FLEX-8400M firmware 4.2.18 evidence demonstrated the failure directly:
SLC num=0, thenTX- num=0, ALC meter 22.SLC num=1, thenTX- num=9, ALC meter 40.numcannot recover slice IDs from the mixed0/9shape.Fix
SLCmanifest context.COMPPEAK, and the TX filter Level taps by the active TX slice.Validation
-j22.meter_model_testmeter_applet_capability_testmeter_applet_voltage_state_testhealth_applet_testphone_cw_level_meter_state_testtools/check_engine_boundary.py --strict: passed with only known-baseline warnings.git diff --check: passed.origin/mainmerge tree: clean.Live radio proof
Tested locally through the automation bridge against a FLEX-8400M running firmware 4.2.18.41174, at 14.074 MHz DIGU into the explicitly authorized ANT2 dummy load. Tune Power was 5%; the physical ceiling was 10 W. The tuner remained bypassed, and the visible TX-antenna set was asserted as exactly
{ANT2}immediately before each key.During the Slice B burst, Slice A's meter remained stale at -150 dBFS while meter 40 updated and drove the public
swAlcvalue. This proves the active-slice selection rather than merely proving that some ALC meter moved.Both bursts completed without a safety abort. Final state was independently verified as one Slice A, forward power 0, Tune Power restored to 10%, and MOX, Tune, model transmitting, and radio transmitting all false.
The radio-generated two-tone path intentionally bypasses microphone/DAX audio, so this validates ALC and active TX waveform routing. It does not claim dynamic speech-compression behavior.
Generated with OpenAI Codex (Daybreak Blue)