Fix S-param mode not reloading on project deserialize - #56
Merged
Conversation
The repo-wide format job (clang-format-18) flagged pre-existing violations carried from #42; no PR could pass until these were resolved.
This was referenced Aug 10, 2026
striderZA
added a commit
that referenced
this pull request
Aug 10, 2026
… must use the local staged fixture (S1 containment neutralizes absolute out-of-project paths on load)
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.
Summary
Fixes #44.
deserialize()in the five S-param-capable engines (Amplifier, IdealFilter, Equalizer, Attenuator, Combiner) restoredsparam_mode/sparam_filepathfrom the project JSON but never reloaded the Touchstone file.update()gates the S-param path onsparam_mode && sparam_data.loaded(), so a reloaded project silently fell back to ideal/manual mode while the UI still showed S-param mode as enabled.Root cause
Confirmed by reading each engine's
deserialize(): the loader (m_sparam_data.load(...)/m_sparam.load(...)) was only ever invoked from thesetSParamFilepath()/setSParamFile()setters used by live UI interaction — never fromdeserialize(), which is also the path used for project load, component cloning, and library-part instantiation.EqualizerEngine::serialize()additionally omittedsparam_fwd_idx, which its S-param branch (equalizer_engine.cpp:51) depends on.Fix
amplifier_engine.cpp,ideal_filter_engine.cpp,equalizer_engine.cpp:deserialize()now reloadsm_sparam_datafrom the deserialized filepath and gatesm_sparam_modeon the actualloaded()result (so a stale/missing file now correctly reports mode off instead of on-but-broken).sparam_fwd_idxis preserved from JSON, not recomputed.attenuator_engine.cpp,combiner_engine.cpp: same pattern viam_sparam.load(...).equalizer_engine.cppserialize(): added missingsparam_fwd_idxfield.Test
Added
tests/test_project_file.cppcase 13 ("Round-trip: S-param mode survives save/load (issue #44)"): builds all 5 component types in S-param mode using the existingcomponent_data/amplifiers/adm-3844psm/*.s2pfixture, saves, reloads in a freshRfSimulatorApp, and assertssparamMode()/sparamLoaded()/sParamMode()are all true post-reload.Verification
testssuite: 216 test cases, 65530 assertions — pass, no regressions[project_file]filter: 12 cases, 90 assertions — pass (includes new test)[sparam]filter: 12 cases, 4057 assertions — passtest_attenuator.exe/test_combiner.exe— pass