refactor(radio): flatten LS state and only save frozen state per FM#7219
refactor(radio): flatten LS state and only save frozen state per FM#7219raphaelcoeffic wants to merge 5 commits intomodel-arenafrom
Conversation
|
This works as before with @philmoz 's example model from #7199: |
ade1852 to
8ac7525
Compare
e6131e7 to
59bab6f
Compare
Add LswPerFmTest suite (7 tests) that evidence the per-FM logical switch context (lswFm[MAX_FLIGHT_MODES]) behavior: - TimerStateIdenticalAcrossFMs: timer lastValue identical across 9 FMs - StickyStateIdenticalAcrossFMs: sticky state identical through ON/OFF - TimerStateSurvivesFMSwitch: FM switches don't affect timer counters - EvalStateOnlyWrittenToCurrentFM: only the `state` bool differs - DelayDurationDivergesAcrossFMs: timerState/timer DO diverge (delay/ duration state machine only runs for current FM in getLogicalSwitch) - TimerRealisticTiming: realistic 10ms/100ms timing, timers stay synced - StickyRealisticTiming: sticky stays synced through FM switches Findings: timer/sticky/edge lastValue (the counters) are always identical across all FMs since logicalSwitchesTimerTick updates all FMs with identical inputs. The `state` field and delay/duration `timerState` diverge because evalLogicalSwitches/getLogicalSwitch only process the current FM. Move LogicalSwitchContext/LogicalSwitchesFlightModeContext structs from switches.cpp to switches.h, add lswContext() accessor. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… context Replace lswFm[MAX_FLIGHT_MODES] (one LogicalSwitchContext per FM per LS) with a flat lswCtx[MAX_LOGICAL_SWITCHES] array. The previous commit's test suite proved that per-FM state is fully redundant: timer/sticky/edge lastValue is always identical across FMs, the `state` bit is dead in non-active FMs, and delay/duration divergence is a bug (background timer expiry) not a feature. During FM fade transitions, the mixer evaluates mixes for multiple FMs in a single cycle. To preserve the existing behavior where fading-out FMs see the LS state from the moment of transition (smooth cross-fade for LS-conditioned mixes), a uint32_t bitmap array per FM captures the frozen LS state at transition time. getSwitch() reads from the frozen bitmap for non-active FMs and from live lswCtx[] for the active FM. This replaces ~2 KB of per-FM context with 9 × 8 = 72 bytes of bitmaps + 1 byte for mixerActiveFlightMode. Changes: - Flatten lswFm[MAX_FLIGHT_MODES] → lswCtx[MAX_LOGICAL_SWITCHES] - Remove LogicalSwitchesFlightModeContext wrapper struct - Hide LogicalSwitchContext struct and lswCtx[] as implementation details in switches.cpp; expose lswGetState/lswGetLastValue/lswSetState accessors - Collapse logicalSwitchesTimerTick from FM×LS nested loop to single LS loop - Replace logicalSwitchesCopyState with lswFreezeState bitmap snapshot called from mixer on FM transition - Add mixerActiveFlightMode to distinguish active FM from temporarily assigned mixerCurrentFlightMode during fade evaluation - Replace 16-test LswPerFmTest suite with 8-test LswTest suite verifying single-context behavior (delay/duration survive FM switches correctly) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
evalLogicalSwitches was called inside evalFlightModeMixes, gated by `if (tick10ms)` — a 12-year-old workaround (opentx commit f3bc8cb) to prevent LS evaluation for non-current FMs during fade transitions. This overloaded tick10ms (a timing parameter for mix delay/slow) as a flag to control LS evaluation, obscuring the actual intent. Since LS evaluation only needs to run once per mixer cycle for the active FM, move the call to evalMixes directly, before the fade loop. This makes the intent explicit and simplifies the code: - Remove evalLogicalSwitches from evalFlightModeMixes - Call evalLogicalSwitches() once in evalMixes, gated by tick10ms - Remove the unused `isCurrentFlightmode` parameter (was always true since the tick10ms guard prevented the false path) - Expand the fade loop ternaries into an explicit if/else for clarity Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…Switch The Lua setStickySwitch API was directly encoding messages into a shared CircularBuffer<uint8_t> using a bit-packed format that limited LS indices to 6 bits (max 64 logical switches). - Add lswSetStickySwitch(idx, value) function in switches.cpp - Replace CircularBuffer class with a minimal SPSC ring buffer using a volatile struct array (GUI task writes, mixer task reads) - Remove CircularBuffer class from edgetx_helpers.h (no remaining users) - Make the buffer static in switches.cpp (no longer extern in edgetx.h) - Lua API calls lswSetStickySwitch instead of encoding directly - Remove MAX_LOGICAL_SWITCHES == 64 compile-time assertions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update the runtime cost table in model_arena.h to reflect the frozen LS bitmap (frozenLsState[]) added alongside the lswCtx[] flattening. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
59bab6f to
b7d339f
Compare
|
Some test results with E-Soar Plus template:
|
|
Just resumed testing with RM GX12
The setups transferred successfully to the tx, however there were a couple of glitches: I fixed the above, then tested a complex (F3F) setup using the mixer monitor - seems to work but really need to visualise as it's impractical to assemble the models each time. I'm going to try a quick and dirty port of my visualiser script to the TX16S. |
Could you please upload the models prior to conversion using Companion? I'd like to take potential issues with current Companion out of the equation. Also, it seems the models are using LUA scripts which are not included (ex:
Ideally, and to really exclude potential pre-existing issues with current
Alternatively, you could use 2.12.0 instead of nightly.
That would be the radio settings. I'll have a look specifically there. So far, I could not reproduce it here.
It seems to happen as well if the same models are loaded with current nightly release. So possibly not something introduced by this PR or #7199. Loading your
Is there anything that would help you testing? Would it be easier to test using the Companion simulator? |
|
@RC-SOAR Mike, the GX12 issue is different from this PR, as SA and SD have moved to switches to custom switches, you must be experiencing side effects about that. Could we have you initial model ? |
Aha - it's an old issue! I've just checked - the SF's were already disabled in the 2.10 .etx file before import into Companion 3. The reason being that the 2.10 .etx had itself been converted from an OpenTX file, and SF's were disabled during that conversion - I seem to remember this was a legacy issue which was fixed.
I can test using the actual hardware, with my visualiser script (now supports colour radios) - it would be easier than using Companion.
All noted. I will make a start tomorrow. Is it sufficient to test using a single target? (I can test on a TX16S Mk1, TX16S Mk3 and/or TX15.)
Just to clarify, the switch failure occurred after exit from the bootloader "flash firmware" screen. The problem was at a low level, switches were not responding in the Hardware Debug menu. After reboot all was normal. The switch assignments were correct for the GX12 before import (SA/SD not used). I'll see if I can reproduce the issue using a more stable test setup. |

Summary
Flatten per-FM logical switch state (
lswFm[MAX_FLIGHT_MODES]) to a single global context (lswCtx[MAX_LOGICAL_SWITCHES]), saving ~2 KB RAM and makinglogicalSwitchesTimerTick9× faster. Clarify the relationship betweenevalLogicalSwitchesand the mixer fade loop. Encapsulate the Lua sticky switch buffer behind a proper API.Background
Per-FM logical switch state was introduced in opentx/opentx#1119 (May 2014) to fix a reentrancy bug (opentx/opentx#1039) and give each flight mode independent LS evaluation during fade transitions. One month later, commit
f3bc8cb9a4(fixing opentx/opentx#1345) gatedevalLogicalSwitches()behindif (tick10ms), which is zero for non-current FMs during fades. This made per-FM LS evaluation dead code — only the active FM's context is ever evaluated, while non-active FM contexts carry stale state from the moment of transition.The per-FM state has been inert for 12 years.
Changes
Commit 1:
test(radio): prove per-FM logical switch state is redundantlastValueis always identical across FMsCommit 2:
refactor(radio): flatten per-FM logical switch state to single global contextlswFm[MAX_FLIGHT_MODES]→lswCtx[MAX_LOGICAL_SWITCHES](single global array)LogicalSwitchContextstruct as implementation detail inswitches.cpplogicalSwitchesTimerTickfrom FM×LS nested loop to single LS looplogicalSwitchesCopyState(full context copy on FM transition) withlswFreezeState(bitmap snapshot of LSstatebits intouint32_tarray)mixerActiveFlightModeto distinguish active FM from temporarily assignedmixerCurrentFlightModeduring fade evaluationgetSwitch()reads from frozen bitmap for fading-out FMs, live context for active FM — preserving existing fade cross-fade behaviorCommit 3:
refactor(radio): move evalLogicalSwitches out of evalFlightModeMixesevalLogicalSwitches()from insideevalFlightModeMixestoevalMixes, called once before the fade loopisCurrentFlightmodeparameter (was alwaystruesince thetick10msguard prevented thefalsepath for 12 years)if/elsefor clarityCommit 4:
refactor(radio): encapsulate sticky switch buffer behind lswSetStickySwitchlswSetStickySwitch(idx, value)API inswitches.cpp; Lua calls this instead of encoding messages directlyCircularBuffer<uint8_t>(bit-packed, limited to 64 LS) with a lock-free SPSC ring buffer usingstd::atomicfor correct cross-task ordering (GUI task → mixer task)CircularBufferclass fromedgetx_helpers.h(no remaining users)MAX_LOGICAL_SWITCHES == 64compile-time assertionsPreserving fade transition behavior
During FM fade transitions, the mixer evaluates mixes for multiple FMs per cycle. Fading-out FMs previously saw their (stale) LS state from
lswFm[]. To preserve this behavior, auint32_tbitmap per FM captures the frozen LS state at transition time. This replaces ~2 KB of per-FM context with 72 bytes of bitmaps + 1 byte formixerActiveFlightMode.Test plan
perFmLsStateNotEvaluatedDuringFade) verifies fade behavior🤖 Generated with Claude Code