From 89d7671ada103cae15f41e106908a0abf819ff5e Mon Sep 17 00:00:00 2001 From: RF Simulator Bot Date: Mon, 10 Aug 2026 08:23:28 +0200 Subject: [PATCH 1/2] refactor: ComponentEngineBase, unified S-param API, enforced registry/drawer consistency M6: add ComponentEngineBase (common/component_engine_base.h) owning the id/graph-node/SignalNode/dirty-flag/cached-input boilerplate every engine repeated; migrate amplifier, equalizer, mixer, splitter, adc, signal_generator, attenuator, combiner, coax. (pfb_channelizer + ideal_filter migrated in the DSP-accuracy branch where their fixes landed.) M5: attenuator/combiner S-param API renamed to the canonical setSParamFilepath/sparamFilepath/sparamLoaded/sparamData (was setSParamFile/sParamFile/m_sparam); serialization key unified to sparam_filepath with a sparam_path fallback on deserialize so old project files still load. M7: registerDrawers' 11-branch if/else chain replaced with a drawer map built once; a registry row with no drawer now LOG_ERRORs instead of silently rendering an empty panel, and InspectorPanel::hasDrawer() powers a new consistency test (every registry type has a drawer, maps to a real NodeKind, and supports_sparam_file matches the 5 engines that actually implement S-param). Fix the supports_sparam_file drift (equalizer/filter/attenuator/combiner were false). Also carries the coax preset-index clamp (moved with the coax file to keep it whole). --- adc/include/adc_engine.h | 19 +-- adc/src/adc_engine.cpp | 16 +-- amplifier/include/amplifier_engine.h | 19 +-- amplifier/src/amplifier_engine.cpp | 15 +-- app/AGENTS.md | 2 +- app/include/inspector_panel.h | 7 + app/src/component_type_registry.cpp | 4 + app/src/inspector_panel.cpp | 127 +++++++++++------- attenuator/include/attenuator_engine.h | 32 ++--- attenuator/src/attenuator_engine.cpp | 45 +++---- coax/include/coax_cable_engine.h | 19 +-- coax/src/coax_cable_engine.cpp | 33 ++--- combiner/include/combiner_engine.h | 27 ++-- combiner/src/combiner_engine.cpp | 37 +++-- common/component_engine_base.h | 72 ++++++++++ equalizer/include/equalizer_engine.h | 18 +-- equalizer/src/equalizer_engine.cpp | 22 +-- mixer/include/mixer_engine.h | 20 +-- mixer/src/mixer_engine.cpp | 20 +-- .../include/signal_generator_engine.h | 14 +- .../src/signal_generator_engine.cpp | 7 +- splitter/include/splitter_engine.h | 20 +-- splitter/src/splitter_engine.cpp | 18 +-- tests/test_attenuator.cpp | 8 +- tests/test_coax_cable_engine.cpp | 22 +++ tests/test_combiner.cpp | 6 +- tests/test_component_dispatch.cpp | 54 ++++++++ tests/test_project_file.cpp | 12 +- tests/test_signal_domain.cpp | 20 ++- 29 files changed, 359 insertions(+), 376 deletions(-) create mode 100644 common/component_engine_base.h diff --git a/adc/include/adc_engine.h b/adc/include/adc_engine.h index 084d545..2fa9ad6 100644 --- a/adc/include/adc_engine.h +++ b/adc/include/adc_engine.h @@ -2,20 +2,16 @@ #include -#include "component_interface.h" +#include "component_engine_base.h" #include "node_graph_engine.h" #include "signal_node.h" -class AdcEngine : public IComponentEngine { +class AdcEngine : public ComponentEngineBase { public: AdcEngine(int id, NodeGraphEngine &graph); - int id() const override { return m_id; } - int graphNodeId() const override { return m_graph_node_id; } std::string_view type_name() const override { return "adc"; } std::string hoverSummary() const override; - int inputPinId() const override; - int outputPinId() const override; void update(double dt) override; nlohmann::json serialize() const override; @@ -37,18 +33,7 @@ class AdcEngine : public IComponentEngine { } } - SignalNode &node() override { return m_node; } - const SignalNode &node() const override { return m_node; } - private: - int m_id; - int m_graph_node_id = -1; - NodeGraphEngine *m_graph; - SignalNode m_node; - double m_fs_Hz = 1e9; double m_nsd_dBm_per_Hz = -155.0; - bool m_dirty = true; - const Spectrum *m_cached_input_ptr = nullptr; - uint64_t m_cached_input_generation = 0; }; diff --git a/adc/src/adc_engine.cpp b/adc/src/adc_engine.cpp index 53fa2f7..9b64212 100644 --- a/adc/src/adc_engine.cpp +++ b/adc/src/adc_engine.cpp @@ -19,26 +19,14 @@ static double alias_frequency(double f_RF, double Fs) { // ---- Engine methods ---- -AdcEngine::AdcEngine(int id, NodeGraphEngine &graph) : m_id(id), m_graph(&graph) { - m_graph_node_id = m_graph->addNode("ADC " + std::to_string(id), &m_node, 1, 1); - m_node.inputs.resize(1); - m_node.outputs.resize(1); +AdcEngine::AdcEngine(int id, NodeGraphEngine &graph) : ComponentEngineBase(id, graph, "ADC", 1, 1) { LOG_INFO("ADC [adc%d] added.", id); } -int AdcEngine::inputPinId() const { return m_graph ? m_graph->inputPinId(m_graph_node_id) : -1; } - -int AdcEngine::outputPinId() const { return m_graph ? m_graph->outputPinId(m_graph_node_id) : -1; } - void AdcEngine::update(double /*dt*/) { const Spectrum *input = m_node.inputs.empty() ? nullptr : m_node.inputs[0]; - if (!m_dirty && input == m_cached_input_ptr && - (!input || input->generation == m_cached_input_generation)) + if (!beginUpdate(input)) return; - m_dirty = false; - m_cached_input_ptr = input; - if (input) - m_cached_input_generation = input->generation; auto &out = m_node.outputs[0]; diff --git a/amplifier/include/amplifier_engine.h b/amplifier/include/amplifier_engine.h index 761cc4f..509490c 100644 --- a/amplifier/include/amplifier_engine.h +++ b/amplifier/include/amplifier_engine.h @@ -1,21 +1,17 @@ #pragma once -#include "component_interface.h" +#include "component_engine_base.h" #include "node_graph_engine.h" #include "nonlinear_model.h" #include "s_parameter_data.h" #include "signal_node.h" #include -class AmplifierEngine : public IComponentEngine { +class AmplifierEngine : public ComponentEngineBase { public: AmplifierEngine(int id, NodeGraphEngine &graph); - int id() const override { return m_id; } - int graphNodeId() const override { return m_graph_node_id; } std::string_view type_name() const override { return "amplifier"; } std::string hoverSummary() const override; - int inputPinId() const override; - int outputPinId() const override; void setGain_dB(double g) { if (g != m_gain_dB) { @@ -34,9 +30,6 @@ class AmplifierEngine : public IComponentEngine { nlohmann::json serialize() const override; void deserialize(const nlohmann::json &) override; - SignalNode &node() override { return m_node; } - const SignalNode &node() const override { return m_node; } - double gain_dB() const { return m_gain_dB; } double nf_dB() const { return m_nf_dB; } bool enableNonlinear() const { return m_nonlinear.enabled(); } @@ -78,16 +71,8 @@ class AmplifierEngine : public IComponentEngine { const SParameterData &sparamData() const { return m_sparam_data; } private: - int m_id; - int m_graph_node_id = -1; - NodeGraphEngine *m_graph = nullptr; - - SignalNode m_node; double m_gain_dB = 0.0; double m_nf_dB = 0.0; - bool m_dirty = true; - const Spectrum *m_cached_input_ptr = nullptr; - uint64_t m_cached_input_generation = 0; NonlinearModel m_nonlinear; // S-parameter state diff --git a/amplifier/src/amplifier_engine.cpp b/amplifier/src/amplifier_engine.cpp index 5f7a980..440e04e 100644 --- a/amplifier/src/amplifier_engine.cpp +++ b/amplifier/src/amplifier_engine.cpp @@ -3,11 +3,8 @@ #include #include -AmplifierEngine::AmplifierEngine(int id, NodeGraphEngine &graph) : m_id(id), m_graph(&graph) { - m_graph_node_id = graph.addNode("Amplifier " + std::to_string(id), &m_node, 1, 1); - m_node.inputs.resize(1); - m_node.outputs.resize(1); -} +AmplifierEngine::AmplifierEngine(int id, NodeGraphEngine &graph) + : ComponentEngineBase(id, graph, "Amplifier", 1, 1) {} void AmplifierEngine::setSParamFilepath(const std::string &path) { m_sparam_filepath = path; @@ -17,14 +14,6 @@ void AmplifierEngine::setSParamFilepath(const std::string &path) { m_dirty = true; } -int AmplifierEngine::inputPinId() const { - return m_graph ? m_graph->inputPinId(m_graph_node_id) : -1; -} - -int AmplifierEngine::outputPinId() const { - return m_graph ? m_graph->outputPinId(m_graph_node_id) : -1; -} - void AmplifierEngine::update(double dt) { (void)dt; const Spectrum *in_ptr = m_node.inputs.empty() ? nullptr : m_node.inputs[0]; diff --git a/app/AGENTS.md b/app/AGENTS.md index e1be2b0..8e60c48 100644 --- a/app/AGENTS.md +++ b/app/AGENTS.md @@ -34,7 +34,7 @@ Application orchestrator layer containing `RfSimulatorApp`, `ComponentRegistry`, - Destructor saves window state via `SessionState` ## Work Guidance -- Add a new component = one `ComponentTypeRegistry` row (`type`, `project_type`, `menu_label`, `label_prefix`, `kind`, `create`, `draw_inspector`) + a `NodeKind`/symbol entry in `node_graph` — the menu, add, duplicate, save/load, inspector, and form paths all dispatch through the registry, so no per-file edits in `RfSimulatorApp` are needed +- Add a new component = one `ComponentTypeRegistry` row (`type`, `project_type`, `menu_label`, `label_prefix`, `kind`, `create`, `draw_inspector`) + a `NodeKind`/symbol entry in `node_graph` + a drawer-map entry in `InspectorPanel::drawerMap()` (app/src/inspector_panel.cpp) — the menu, add, duplicate, save/load, and form paths all dispatch through the registry, so no per-file edits in `RfSimulatorApp` are needed. Missing drawers are logged at startup and caught by the registry/drawer consistency test in `test_component_dispatch.cpp`. ## Verification - Round-trip tests in `tests/test_project_file.cpp` diff --git a/app/include/inspector_panel.h b/app/include/inspector_panel.h index 7dc442f..2bd0b1f 100644 --- a/app/include/inspector_panel.h +++ b/app/include/inspector_panel.h @@ -65,6 +65,13 @@ class InspectorPanel { // callbacks to this panel's property drawers. void registerDrawers(ComponentTypeRegistry ®istry); + // True if a property drawer is registered for the given canonical type + // key (e.g. "amplifier"). Exposed so tests can assert that every + // ComponentTypeRegistry row has inspector coverage; a registry type with + // no drawer logs an error from registerDrawers() instead of silently + // rendering an empty properties panel. + static bool hasDrawer(std::string_view type); + void drawAmplifierProperties(AmplifierEngine &engine, int index); void drawCoaxCableProperties(CoaxCableEngine &engine, int index); void drawEqualizerProperties(EqualizerEngine &engine, int index); diff --git a/app/src/component_type_registry.cpp b/app/src/component_type_registry.cpp index 5da77c5..7922f84 100644 --- a/app/src/component_type_registry.cpp +++ b/app/src/component_type_registry.cpp @@ -72,6 +72,7 @@ ComponentTypeRegistry::ComponentTypeRegistry() { att.label_prefix = "Attenuator"; att.kind = NodeKind::Attenuator; att.authorable = true; + att.supports_sparam_file = true; att.fields = { {"attenuation_dB", "Attenuation", "dB", FieldKind::Number, true, 0.0, 100.0, {}, {}, ""}, }; @@ -101,6 +102,7 @@ ComponentTypeRegistry::ComponentTypeRegistry() { flt.label_prefix = "IdealFilter"; flt.kind = NodeKind::IdealFilter; flt.authorable = true; + flt.supports_sparam_file = true; flt.fields = { {"filter_type", "Filter Type", @@ -155,6 +157,7 @@ ComponentTypeRegistry::ComponentTypeRegistry() { eq.label_prefix = "Equalizer"; eq.kind = NodeKind::Equalizer; eq.authorable = true; + eq.supports_sparam_file = true; eq.fields = { {"ref_gain_dB", "Reference Gain", "dB", FieldKind::Number, false, -50.0, 50.0, {}, {}, ""}, {"ref_freq_Hz", @@ -191,6 +194,7 @@ ComponentTypeRegistry::ComponentTypeRegistry() { comb.label_prefix = "Combiner"; comb.kind = NodeKind::Combiner; comb.authorable = true; + comb.supports_sparam_file = true; comb.fields = { {"manual_mode", "Manual Mode", "", FieldKind::Bool, false, 0, 0, {}, false, ""}, }; diff --git a/app/src/inspector_panel.cpp b/app/src/inspector_panel.cpp index 892b6c9..c154626 100644 --- a/app/src/inspector_panel.cpp +++ b/app/src/inspector_panel.cpp @@ -16,6 +16,7 @@ #include "splitter_engine.h" #include "utils.h" #include +#include #include #include "component_registry.h" @@ -23,56 +24,82 @@ InspectorPanel::InspectorPanel(NodeGraphEngine &graph, ComponentRegistry &components) : m_graph(graph), m_components(&components) {} +namespace { + +using DrawerFn = std::function; + +// Canonical type key -> property drawer. Built once; registerDrawers() copies +// each entry onto the matching ComponentTypeRegistry row, and hasDrawer() +// lets tests assert every registered type has inspector coverage. +const std::map &drawerMap() { + static const std::map drawers = { + {"generator", + [](InspectorPanel &p, IComponentEngine &e) { + p.drawGeneratorProperties(static_cast(e), e.id()); + }}, + {"amplifier", + [](InspectorPanel &p, IComponentEngine &e) { + p.drawAmplifierProperties(static_cast(e), e.id()); + }}, + {"splitter", + [](InspectorPanel &p, IComponentEngine &e) { + p.drawSplitterProperties(static_cast(e), e.id()); + }}, + {"mixer", + [](InspectorPanel &p, IComponentEngine &e) { + p.drawMixerProperties(static_cast(e), e.id()); + }}, + {"adc", + [](InspectorPanel &p, IComponentEngine &e) { + p.drawAdcProperties(static_cast(e), e.id()); + }}, + {"pfb", + [](InspectorPanel &p, IComponentEngine &e) { + p.drawPFBProperties(static_cast(e)); + }}, + {"filter", + [](InspectorPanel &p, IComponentEngine &e) { + p.drawIdealFilterProperties(static_cast(e), e.id()); + }}, + {"coax", + [](InspectorPanel &p, IComponentEngine &e) { + p.drawCoaxCableProperties(static_cast(e), e.id()); + }}, + {"equalizer", + [](InspectorPanel &p, IComponentEngine &e) { + p.drawEqualizerProperties(static_cast(e), e.id()); + }}, + {"attenuator", + [](InspectorPanel &p, IComponentEngine &e) { + p.drawAttenuatorProperties(static_cast(e), e.id()); + }}, + {"combiner", + [](InspectorPanel &p, IComponentEngine &e) { + p.drawCombinerProperties(static_cast(e), e.id()); + }}, + }; + return drawers; +} + +} // namespace + void InspectorPanel::registerDrawers(ComponentTypeRegistry ®istry) { + const auto &drawers = drawerMap(); for (auto *d : registry.all()) { - if (d->type == "generator") { - d->draw_inspector = [](InspectorPanel &p, IComponentEngine &e) { - p.drawGeneratorProperties(static_cast(e), e.id()); - }; - } else if (d->type == "amplifier") { - d->draw_inspector = [](InspectorPanel &p, IComponentEngine &e) { - p.drawAmplifierProperties(static_cast(e), e.id()); - }; - } else if (d->type == "splitter") { - d->draw_inspector = [](InspectorPanel &p, IComponentEngine &e) { - p.drawSplitterProperties(static_cast(e), e.id()); - }; - } else if (d->type == "mixer") { - d->draw_inspector = [](InspectorPanel &p, IComponentEngine &e) { - p.drawMixerProperties(static_cast(e), e.id()); - }; - } else if (d->type == "adc") { - d->draw_inspector = [](InspectorPanel &p, IComponentEngine &e) { - p.drawAdcProperties(static_cast(e), e.id()); - }; - } else if (d->type == "pfb") { - d->draw_inspector = [](InspectorPanel &p, IComponentEngine &e) { - p.drawPFBProperties(static_cast(e)); - }; - } else if (d->type == "filter") { - d->draw_inspector = [](InspectorPanel &p, IComponentEngine &e) { - p.drawIdealFilterProperties(static_cast(e), e.id()); - }; - } else if (d->type == "coax") { - d->draw_inspector = [](InspectorPanel &p, IComponentEngine &e) { - p.drawCoaxCableProperties(static_cast(e), e.id()); - }; - } else if (d->type == "equalizer") { - d->draw_inspector = [](InspectorPanel &p, IComponentEngine &e) { - p.drawEqualizerProperties(static_cast(e), e.id()); - }; - } else if (d->type == "attenuator") { - d->draw_inspector = [](InspectorPanel &p, IComponentEngine &e) { - p.drawAttenuatorProperties(static_cast(e), e.id()); - }; - } else if (d->type == "combiner") { - d->draw_inspector = [](InspectorPanel &p, IComponentEngine &e) { - p.drawCombinerProperties(static_cast(e), e.id()); - }; + auto it = drawers.find(d->type); + if (it != drawers.end()) { + d->draw_inspector = it->second; + } else { + // A registry row with no drawer used to end up with an empty + // properties panel silently; fail loudly so adding a component + // type also registers a drawer here. + LOG_ERROR("No inspector drawer registered for component type '%s'", d->type.c_str()); } } } +bool InspectorPanel::hasDrawer(std::string_view type) { return drawerMap().count(type) > 0; } + InspectorPanel::Hit InspectorPanel::findSelected() const { int n = ImNodes::NumSelectedNodes(); if (n != 1) @@ -577,18 +604,18 @@ void InspectorPanel::drawAttenuatorProperties(AttenuatorEngine &engine, int inde engine.setAttenuation(static_cast(atten_f)); } - bool sparam_mode = engine.sParamMode(); + bool sparam_mode = engine.sparamMode(); if (ImGui::Checkbox("S-param mode", &sparam_mode)) { engine.setSParamMode(sparam_mode); } if (sparam_mode) { - std::string path = engine.sParamFile(); + std::string path = engine.sparamFilepath(); char path_buf[512]; strncpy(path_buf, path.c_str(), sizeof(path_buf) - 1); path_buf[sizeof(path_buf) - 1] = '\0'; if (ImGui::InputText("S-param file", path_buf, sizeof(path_buf))) { - engine.setSParamFile(path_buf); + engine.setSParamFilepath(path_buf); } } @@ -603,18 +630,18 @@ void InspectorPanel::drawCombinerProperties(CombinerEngine &engine, int index) { ImGui::TextDisabled("Combiner: 2 inputs → 1 output"); - bool sparam_mode = engine.sParamMode(); + bool sparam_mode = engine.sparamMode(); if (ImGui::Checkbox("S-parameter mode", &sparam_mode)) { engine.setSParamMode(sparam_mode); } if (sparam_mode) { - std::string path = engine.sParamFile(); + std::string path = engine.sparamFilepath(); char path_buf[512]; strncpy(path_buf, path.c_str(), sizeof(path_buf) - 1); path_buf[sizeof(path_buf) - 1] = '\0'; if (ImGui::InputText("S-param file", path_buf, sizeof(path_buf))) { - engine.setSParamFile(path_buf); + engine.setSParamFilepath(path_buf); } } diff --git a/attenuator/include/attenuator_engine.h b/attenuator/include/attenuator_engine.h index 677fb8f..5d39000 100644 --- a/attenuator/include/attenuator_engine.h +++ b/attenuator/include/attenuator_engine.h @@ -1,6 +1,6 @@ #pragma once -#include "component_interface.h" +#include "component_engine_base.h" #include "node_graph_engine.h" #include "s_parameter_data.h" #include "signal_node.h" @@ -8,47 +8,33 @@ #include #include -class AttenuatorEngine : public IComponentEngine { +class AttenuatorEngine : public ComponentEngineBase { public: AttenuatorEngine(int id, NodeGraphEngine &graph); - int id() const override { return m_id; } - int graphNodeId() const override { return m_graph_node_id; } std::string_view type_name() const override { return "attenuator"; } std::string hoverSummary() const override; - int inputPinId() const override; int outputPinId() const override { return outputPinId(0); } int outputPinId(int index) const override; void update(double dt) override; nlohmann::json serialize() const override; void deserialize(const nlohmann::json &) override; - SignalNode &node() override { return m_node; } - const SignalNode &node() const override { return m_node; } - void setAttenuation(double dB); double attenuation() const { return m_atten_dB; } void setSParamMode(bool enabled); - bool sParamMode() const { return m_sparam_mode; } + bool sparamMode() const { return m_sparam_mode; } - void setSParamFile(const std::string &path); - const std::string &sParamFile() const { return m_sparam_path; } + void setSParamFilepath(const std::string &path); + bool sparamLoaded() const { return m_sparam_data.loaded(); } + const std::string &sparamFilepath() const { return m_sparam_filepath; } + const SParameterData &sparamData() const { return m_sparam_data; } private: - int m_id; - int m_graph_node_id = -1; - NodeGraphEngine *m_graph = nullptr; - - SignalNode m_node; - double m_atten_dB = 0.0; bool m_sparam_mode = false; - std::string m_sparam_path; - SParameterData m_sparam; - - bool m_dirty = true; - const Spectrum *m_cached_input_ptr = nullptr; - uint64_t m_cached_input_generation = 0; + std::string m_sparam_filepath; + SParameterData m_sparam_data; }; diff --git a/attenuator/src/attenuator_engine.cpp b/attenuator/src/attenuator_engine.cpp index 86f9877..59f5866 100644 --- a/attenuator/src/attenuator_engine.cpp +++ b/attenuator/src/attenuator_engine.cpp @@ -5,15 +5,8 @@ #include #include -AttenuatorEngine::AttenuatorEngine(int id, NodeGraphEngine &graph) : m_id(id), m_graph(&graph) { - m_graph_node_id = graph.addNode("Attenuator " + std::to_string(id), &m_node, 1, 1); - m_node.inputs.resize(1); - m_node.outputs.resize(1); -} - -int AttenuatorEngine::inputPinId() const { - return m_graph ? m_graph->inputPinId(m_graph_node_id) : -1; -} +AttenuatorEngine::AttenuatorEngine(int id, NodeGraphEngine &graph) + : ComponentEngineBase(id, graph, "Attenuator", 1, 1) {} int AttenuatorEngine::outputPinId(int index) const { if (!m_graph || m_graph_node_id < 0 || index != 0) @@ -38,9 +31,9 @@ void AttenuatorEngine::setSParamMode(bool enabled) { m_dirty = true; } -void AttenuatorEngine::setSParamFile(const std::string &path) { - m_sparam_path = path; - m_sparam_mode = m_sparam.load(path); +void AttenuatorEngine::setSParamFilepath(const std::string &path) { + m_sparam_filepath = path; + m_sparam_mode = m_sparam_data.load(path); m_dirty = true; } @@ -49,7 +42,7 @@ void AttenuatorEngine::update(double dt) { const Spectrum *in_ptr = m_node.inputs.empty() ? nullptr : m_node.inputs[0]; // --- S-parameter mode --- - if (m_sparam_mode && m_sparam.loaded()) { + if (m_sparam_mode && m_sparam_data.loaded()) { if (!m_dirty && in_ptr == m_cached_input_ptr && (!in_ptr || in_ptr->generation == m_cached_input_generation)) return; @@ -66,12 +59,12 @@ void AttenuatorEngine::update(double dt) { buildDefaultFrequencyGrid(out.frequencies); const size_t N = out.frequencies.size(); - int idx = 1 * m_sparam.numPorts() + 0; // S21 index + int idx = 1 * m_sparam_data.numPorts() + 0; // S21 index out.tones = in_ptr ? in_ptr->tones : std::vector{}; out.is_complex_baseband = in_ptr ? in_ptr->is_complex_baseband : false; for (auto &t : out.tones) { - auto S = m_sparam.interpolate(t.freq_Hz, idx); + auto S = m_sparam_data.interpolate(t.freq_Hz, idx); t.power_dBm += 20.0 * std::log10(std::abs(S)); t.phase_deg += std::arg(S) * 180.0 / std::numbers::pi; } @@ -98,7 +91,7 @@ void AttenuatorEngine::update(double dt) { out.noise_total_W.resize(N); for (size_t i = 0; i < N; ++i) { - auto S = m_sparam.interpolate(out.frequencies[i], idx); + auto S = m_sparam_data.interpolate(out.frequencies[i], idx); double mag_sq = std::norm(S); // |S21|^2 double noise_in = (in_ptr && i < in_ptr->noise_total_W.size() ? in_ptr->noise_total_W[i] : 0.0); @@ -114,13 +107,8 @@ void AttenuatorEngine::update(double dt) { } // --- Manual mode --- - if (!m_dirty && in_ptr == m_cached_input_ptr && - (!in_ptr || in_ptr->generation == m_cached_input_generation)) + if (!beginUpdate(in_ptr)) return; - m_dirty = false; - m_cached_input_ptr = in_ptr; - if (in_ptr) - m_cached_input_generation = in_ptr->generation; auto &out = m_node.outputs[0]; @@ -176,17 +164,18 @@ void AttenuatorEngine::update(double dt) { } nlohmann::json AttenuatorEngine::serialize() const { - return { - {"atten_dB", m_atten_dB}, {"sparam_mode", m_sparam_mode}, {"sparam_path", m_sparam_path}}; + return {{"atten_dB", m_atten_dB}, + {"sparam_mode", m_sparam_mode}, + {"sparam_filepath", m_sparam_filepath}}; } void AttenuatorEngine::deserialize(const nlohmann::json &j) { m_atten_dB = j.contains("atten_dB") ? j["atten_dB"].get() : j.value("attenuation_dB", 0.0); - m_sparam_path = j.value("sparam_path", ""); - if (!m_sparam_path.empty()) - m_sparam.load(m_sparam_path); - m_sparam_mode = j.value("sparam_mode", false) && m_sparam.loaded(); + m_sparam_filepath = j.value("sparam_filepath", j.value("sparam_path", "")); + if (!m_sparam_filepath.empty()) + m_sparam_data.load(m_sparam_filepath); + m_sparam_mode = j.value("sparam_mode", false) && m_sparam_data.loaded(); m_dirty = true; } diff --git a/coax/include/coax_cable_engine.h b/coax/include/coax_cable_engine.h index 7339e17..a34c78f 100644 --- a/coax/include/coax_cable_engine.h +++ b/coax/include/coax_cable_engine.h @@ -1,28 +1,21 @@ #pragma once #include "coax_presets.h" -#include "component_interface.h" +#include "component_engine_base.h" #include "node_graph_engine.h" #include "signal_node.h" #include -class CoaxCableEngine : public IComponentEngine { +class CoaxCableEngine : public ComponentEngineBase { public: CoaxCableEngine(int id, NodeGraphEngine &graph); - int id() const override { return m_id; } - int graphNodeId() const override { return m_graph_node_id; } std::string_view type_name() const override { return "coax"; } - int inputPinId() const override; - int outputPinId() const override; void update(double dt) override; nlohmann::json serialize() const override; void deserialize(const nlohmann::json &) override; - SignalNode &node() override { return m_node; } - const SignalNode &node() const override { return m_node; } - std::string hoverSummary() const override; void setPresetIndex(int idx); @@ -35,16 +28,8 @@ class CoaxCableEngine : public IComponentEngine { const CableSpec &preset() const { return kCoaxCablePresets[m_preset_index]; } private: - int m_id; - int m_graph_node_id = -1; - NodeGraphEngine *m_graph = nullptr; - - SignalNode m_node; // 1 input, 1 output int m_preset_index = 4; // default to MT 340 double m_length_m = 1.0; double m_connectors_loss_dB = 0.0; - bool m_dirty = true; - const Spectrum *m_cached_input_ptr = nullptr; - uint64_t m_cached_input_generation = 0; bool m_warned_above_max = false; // rate-limit flag for over-max_freq warning }; diff --git a/coax/src/coax_cable_engine.cpp b/coax/src/coax_cable_engine.cpp index d467a63..52dc61c 100644 --- a/coax/src/coax_cable_engine.cpp +++ b/coax/src/coax_cable_engine.cpp @@ -4,19 +4,8 @@ #include #include -CoaxCableEngine::CoaxCableEngine(int id, NodeGraphEngine &graph) : m_id(id), m_graph(&graph) { - m_graph_node_id = graph.addNode("Coax Cable " + std::to_string(id), &m_node, 1, 1); - m_node.inputs.resize(1); - m_node.outputs.resize(1); -} - -int CoaxCableEngine::inputPinId() const { - return m_graph ? m_graph->inputPinId(m_graph_node_id) : -1; -} - -int CoaxCableEngine::outputPinId() const { - return m_graph ? m_graph->outputPinId(m_graph_node_id) : -1; -} +CoaxCableEngine::CoaxCableEngine(int id, NodeGraphEngine &graph) + : ComponentEngineBase(id, graph, "Coax Cable", 1, 1) {} void CoaxCableEngine::setPresetIndex(int idx) { if (idx < 0 || static_cast(idx) >= kCoaxCablePresets.size()) @@ -47,14 +36,8 @@ void CoaxCableEngine::setConnectorsLossDB(double db) { void CoaxCableEngine::update(double dt) { (void)dt; const Spectrum *in_ptr = m_node.inputs.empty() ? nullptr : m_node.inputs[0]; - if (!m_dirty && in_ptr == m_cached_input_ptr && - (!in_ptr || in_ptr->generation == m_cached_input_generation)) { + if (!beginUpdate(in_ptr)) return; - } - m_dirty = false; - m_cached_input_ptr = in_ptr; - if (in_ptr) - m_cached_input_generation = in_ptr->generation; auto &out = m_node.outputs[0]; @@ -130,9 +113,13 @@ nlohmann::json CoaxCableEngine::serialize() const { } void CoaxCableEngine::deserialize(const nlohmann::json &j) { - m_preset_index = j.value("preset_index", 4); - m_length_m = j.value("length_m", 1.0); - m_connectors_loss_dB = j.value("connectors_loss_dB", 0.0); + // Clamp to the same ranges the setters enforce: a corrupted/hand-edited + // .rfsim with an out-of-range preset index must not reach preset()'s + // kCoaxCablePresets[] indexing (OOB/UB). + m_preset_index = + std::clamp(j.value("preset_index", 4), 0, static_cast(kCoaxCablePresets.size()) - 1); + m_length_m = std::clamp(j.value("length_m", 1.0), 0.0, 1000.0); + m_connectors_loss_dB = std::clamp(j.value("connectors_loss_dB", 0.0), 0.0, 30.0); m_dirty = true; } diff --git a/combiner/include/combiner_engine.h b/combiner/include/combiner_engine.h index 57af50f..c65a951 100644 --- a/combiner/include/combiner_engine.h +++ b/combiner/include/combiner_engine.h @@ -1,18 +1,16 @@ #pragma once -#include "component_interface.h" +#include "component_engine_base.h" #include "node_graph_engine.h" #include "s_parameter_data.h" #include "signal_node.h" #include "spectrum.h" #include -class CombinerEngine : public IComponentEngine { +class CombinerEngine : public ComponentEngineBase { public: CombinerEngine(int id, NodeGraphEngine &graph); - int id() const override { return m_id; } - int graphNodeId() const override { return m_graph_node_id; } std::string_view type_name() const override { return "combiner"; } std::string hoverSummary() const override; @@ -26,30 +24,23 @@ class CombinerEngine : public IComponentEngine { void update(double dt) override; nlohmann::json serialize() const override; void deserialize(const nlohmann::json &) override; - SignalNode &node() override { return m_node; } - const SignalNode &node() const override { return m_node; } void setManualMode(bool enabled); bool manualMode() const { return m_manual_mode; } void setSParamMode(bool enabled); - bool sParamMode() const { return m_sparam_mode; } - void setSParamFile(const std::string &path); - const std::string &sParamFile() const { return m_sparam_path; } + bool sparamMode() const { return m_sparam_mode; } + void setSParamFilepath(const std::string &path); + bool sparamLoaded() const { return m_sparam_data.loaded(); } + const std::string &sparamFilepath() const { return m_sparam_filepath; } + const SParameterData &sparamData() const { return m_sparam_data; } private: - int m_id; - int m_graph_node_id = -1; - NodeGraphEngine *m_graph = nullptr; - - SignalNode m_node; - bool m_manual_mode = true; bool m_sparam_mode = false; - std::string m_sparam_path; - SParameterData m_sparam; + std::string m_sparam_filepath; + SParameterData m_sparam_data; - bool m_dirty = true; const Spectrum *m_cached_input0_ptr = nullptr; const Spectrum *m_cached_input1_ptr = nullptr; uint64_t m_cached_input0_generation = 0; diff --git a/combiner/src/combiner_engine.cpp b/combiner/src/combiner_engine.cpp index 1103621..50cb038 100644 --- a/combiner/src/combiner_engine.cpp +++ b/combiner/src/combiner_engine.cpp @@ -5,11 +5,8 @@ #include #include -CombinerEngine::CombinerEngine(int id, NodeGraphEngine &graph) : m_id(id), m_graph(&graph) { - m_graph_node_id = graph.addNode("Combiner " + std::to_string(id), &m_node, 2, 1); - m_node.inputs.resize(2); - m_node.outputs.resize(1); -} +CombinerEngine::CombinerEngine(int id, NodeGraphEngine &graph) + : ComponentEngineBase(id, graph, "Combiner", 2, 1) {} int CombinerEngine::inputPinId(int port) const { if (!m_graph || m_graph_node_id < 0 || port < 0 || port >= 2) @@ -47,9 +44,9 @@ void CombinerEngine::setSParamMode(bool enabled) { m_dirty = true; } -void CombinerEngine::setSParamFile(const std::string &path) { - m_sparam_path = path; - m_sparam_mode = m_sparam.load(path); +void CombinerEngine::setSParamFilepath(const std::string &path) { + m_sparam_filepath = path; + m_sparam_mode = m_sparam_data.load(path); m_dirty = true; } @@ -58,15 +55,15 @@ std::string CombinerEngine::hoverSummary() const { return "Combiner: 2→1, -3 d nlohmann::json CombinerEngine::serialize() const { return {{"manual_mode", m_manual_mode}, {"sparam_mode", m_sparam_mode}, - {"sparam_path", m_sparam_path}}; + {"sparam_filepath", m_sparam_filepath}}; } void CombinerEngine::deserialize(const nlohmann::json &j) { m_manual_mode = j.value("manual_mode", true); - m_sparam_path = j.value("sparam_path", ""); - if (!m_sparam_path.empty()) - m_sparam.load(m_sparam_path); - m_sparam_mode = j.value("sparam_mode", false) && m_sparam.loaded(); + m_sparam_filepath = j.value("sparam_filepath", j.value("sparam_path", "")); + if (!m_sparam_filepath.empty()) + m_sparam_data.load(m_sparam_filepath); + m_sparam_mode = j.value("sparam_mode", false) && m_sparam_data.loaded(); m_dirty = true; } @@ -76,7 +73,7 @@ void CombinerEngine::update(double dt) { const Spectrum *in1 = m_node.inputs.size() > 1 ? m_node.inputs[1] : nullptr; // --- S-parameter mode --- - if (m_sparam_mode && m_sparam.loaded()) { + if (m_sparam_mode && m_sparam_data.loaded()) { if (!m_dirty && in0 == m_cached_input0_ptr && in1 == m_cached_input1_ptr && (!in0 || in0->generation == m_cached_input0_generation) && (!in1 || in1->generation == m_cached_input1_generation)) @@ -114,15 +111,15 @@ void CombinerEngine::update(double dt) { // 3-port device: port 0 = input 0, port 1 = input 1, port 2 = output // S21: input 0 -> output, S31: input 1 -> output - int idx_S21 = 2 * m_sparam.numPorts() + 0; - int idx_S31 = 2 * m_sparam.numPorts() + 1; + int idx_S21 = 2 * m_sparam_data.numPorts() + 0; + int idx_S31 = 2 * m_sparam_data.numPorts() + 1; std::vector combined_tones; // Apply S21 to input 0 tones if (in0) { for (const auto &t : in0->tones) { - auto S = m_sparam.interpolate(t.freq_Hz, idx_S21); + auto S = m_sparam_data.interpolate(t.freq_Hz, idx_S21); Spectrum::Tone t_out = t; t_out.power_dBm += 20.0 * std::log10(std::abs(S)); t_out.phase_deg += std::arg(S) * 180.0 / std::numbers::pi; @@ -133,7 +130,7 @@ void CombinerEngine::update(double dt) { // Apply S31 to input 1 tones if (in1) { for (const auto &t : in1->tones) { - auto S = m_sparam.interpolate(t.freq_Hz, idx_S31); + auto S = m_sparam_data.interpolate(t.freq_Hz, idx_S31); Spectrum::Tone t_out = t; t_out.power_dBm += 20.0 * std::log10(std::abs(S)); t_out.phase_deg += std::arg(S) * 180.0 / std::numbers::pi; @@ -154,8 +151,8 @@ void CombinerEngine::update(double dt) { out.noise_total_W.resize(N); for (size_t i = 0; i < N; ++i) { - auto S21 = m_sparam.interpolate(out.frequencies[i], idx_S21); - auto S31 = m_sparam.interpolate(out.frequencies[i], idx_S31); + auto S21 = m_sparam_data.interpolate(out.frequencies[i], idx_S21); + auto S31 = m_sparam_data.interpolate(out.frequencies[i], idx_S31); double mag_sq_S21 = std::norm(S21); double mag_sq_S31 = std::norm(S31); diff --git a/common/component_engine_base.h b/common/component_engine_base.h new file mode 100644 index 0000000..e4430dd --- /dev/null +++ b/common/component_engine_base.h @@ -0,0 +1,72 @@ +#pragma once + +#include "component_interface.h" +#include "node_graph_engine.h" +#include "signal_node.h" +#include +#include +#include + +// ComponentEngineBase — shared boilerplate for the graph-attached DSP engines. +// +// Owns the members and accessors that every engine repeats verbatim: +// - the component id and graph node id (id(), graphNodeId()) +// - the owning NodeGraphEngine pointer plus the graph node created by the +// constructor via NodeGraphEngine::addNode (constructor shape: label + +// id, input/output pin counts) +// - the engine's SignalNode and the node() accessors +// - the dirty flag that forces recomputation on the next update() +// - the cached single-input (pointer, generation) pair with beginUpdate(), +// the standard single-input dirty-check prologue +// +// Subclasses still implement the IComponentEngine pure virtuals +// (type_name(), hoverSummary(), update(), serialize(), deserialize()) and may +// override inputPinId()/outputPinId() for multi-pin layouts. +class ComponentEngineBase : public IComponentEngine { + public: + ComponentEngineBase(int id, NodeGraphEngine &graph, std::string_view label, int num_inputs, + int num_outputs) + : m_id(id), m_graph(&graph) { + m_graph_node_id = graph.addNode(std::string(label) + " " + std::to_string(id), &m_node, + num_inputs, num_outputs); + m_node.inputs.resize(num_inputs); + m_node.outputs.resize(num_outputs); + } + + int id() const override { return m_id; } + int graphNodeId() const override { return m_graph_node_id; } + + int inputPinId() const override { return m_graph ? m_graph->inputPinId(m_graph_node_id) : -1; } + int outputPinId() const override { + return m_graph ? m_graph->outputPinId(m_graph_node_id) : -1; + } + + SignalNode &node() override { return m_node; } + const SignalNode &node() const override { return m_node; } + + protected: + // Standard single-input dirty-check prologue. Returns true when update() + // must recompute (dirty flag set, or the input pointer/generation changed + // since the last update); on that path it clears the dirty flag and + // records the current input for the next comparison. Returns false when + // the input is unchanged and the engine is clean — callers return without + // recomputing. + bool beginUpdate(const Spectrum *in_ptr) { + if (!m_dirty && in_ptr == m_cached_input_ptr && + (!in_ptr || in_ptr->generation == m_cached_input_generation)) + return false; + m_dirty = false; + m_cached_input_ptr = in_ptr; + if (in_ptr) + m_cached_input_generation = in_ptr->generation; + return true; + } + + int m_id; + int m_graph_node_id = -1; + NodeGraphEngine *m_graph = nullptr; + bool m_dirty = true; + const Spectrum *m_cached_input_ptr = nullptr; + uint64_t m_cached_input_generation = 0; + SignalNode m_node; +}; diff --git a/equalizer/include/equalizer_engine.h b/equalizer/include/equalizer_engine.h index 5c509c4..b1ed9e5 100644 --- a/equalizer/include/equalizer_engine.h +++ b/equalizer/include/equalizer_engine.h @@ -1,24 +1,18 @@ #pragma once -#include "component_interface.h" +#include "component_engine_base.h" #include "node_graph_engine.h" #include "s_parameter_data.h" #include "signal_node.h" #include #include -class EqualizerEngine : public IComponentEngine { +class EqualizerEngine : public ComponentEngineBase { public: EqualizerEngine(int id, NodeGraphEngine &graph); - int id() const override { return m_id; } - int graphNodeId() const override { return m_graph_node_id; } std::string_view type_name() const override { return "equalizer"; } std::string hoverSummary() const override; - int inputPinId() const override; - int outputPinId() const override; - SignalNode &node() override { return m_node; } - const SignalNode &node() const override { return m_node; } void update(double dt) override; nlohmann::json serialize() const override; void deserialize(const nlohmann::json &) override; @@ -52,14 +46,6 @@ class EqualizerEngine : public IComponentEngine { const SParameterData &sparamData() const { return m_sparam_data; } private: - int m_id; - int m_graph_node_id = -1; - NodeGraphEngine *m_graph = nullptr; - SignalNode m_node; - bool m_dirty = true; - const Spectrum *m_cached_input_ptr = nullptr; - uint64_t m_cached_input_generation = 0; - // Ideal mode double m_ref_gain_dB = 0.0; double m_ref_freq_Hz = 1e9; diff --git a/equalizer/src/equalizer_engine.cpp b/equalizer/src/equalizer_engine.cpp index 1ccd203..e9ac1a7 100644 --- a/equalizer/src/equalizer_engine.cpp +++ b/equalizer/src/equalizer_engine.cpp @@ -4,19 +4,8 @@ #include #include -EqualizerEngine::EqualizerEngine(int id, NodeGraphEngine &graph) : m_id(id), m_graph(&graph) { - m_graph_node_id = graph.addNode("Equalizer " + std::to_string(id), &m_node, 1, 1); - m_node.inputs.resize(1); - m_node.outputs.resize(1); -} - -int EqualizerEngine::inputPinId() const { - return m_graph ? m_graph->inputPinId(m_graph_node_id) : -1; -} - -int EqualizerEngine::outputPinId() const { - return m_graph ? m_graph->outputPinId(m_graph_node_id) : -1; -} +EqualizerEngine::EqualizerEngine(int id, NodeGraphEngine &graph) + : ComponentEngineBase(id, graph, "Equalizer", 1, 1) {} void EqualizerEngine::setSParamFilepath(const std::string &path) { m_sparam_filepath = path; @@ -86,13 +75,8 @@ void EqualizerEngine::update(double dt) { } // --- Ideal mode --- - if (!m_dirty && in_ptr == m_cached_input_ptr && - (!in_ptr || in_ptr->generation == m_cached_input_generation)) + if (!beginUpdate(in_ptr)) return; - m_dirty = false; - m_cached_input_ptr = in_ptr; - if (in_ptr) - m_cached_input_generation = in_ptr->generation; auto &out = m_node.outputs[0]; diff --git a/mixer/include/mixer_engine.h b/mixer/include/mixer_engine.h index 7ee0f12..b638582 100644 --- a/mixer/include/mixer_engine.h +++ b/mixer/include/mixer_engine.h @@ -2,19 +2,14 @@ #include -#include "component_interface.h" -#include "node_graph_engine.h" +#include "component_engine_base.h" #include "signal_node.h" -class MixerEngine : public IComponentEngine { +class MixerEngine : public ComponentEngineBase { public: MixerEngine(int id, NodeGraphEngine &graph); - int id() const override { return m_id; } - int graphNodeId() const override { return m_graph_node_id; } std::string_view type_name() const override { return "mixer"; } std::string hoverSummary() const override; - int inputPinId() const override; - int outputPinId() const override; void setLoFreq_Hz(double f) { if (f != m_lo_freq_Hz) { @@ -43,19 +38,8 @@ class MixerEngine : public IComponentEngine { nlohmann::json serialize() const override; void deserialize(const nlohmann::json &) override; - SignalNode &node() override { return m_node; } - const SignalNode &node() const override { return m_node; } - private: - int m_id; - int m_graph_node_id = -1; - NodeGraphEngine *m_graph = nullptr; - - SignalNode m_node; double m_lo_freq_Hz = 1e9; double m_conv_gain_dB = -6.0; double m_nf_dB = 0.0; - bool m_dirty = true; - const Spectrum *m_cached_input_ptr = nullptr; - uint64_t m_cached_input_generation = 0; }; diff --git a/mixer/src/mixer_engine.cpp b/mixer/src/mixer_engine.cpp index 7f04359..b59ddea 100644 --- a/mixer/src/mixer_engine.cpp +++ b/mixer/src/mixer_engine.cpp @@ -2,28 +2,14 @@ #include #include -MixerEngine::MixerEngine(int id, NodeGraphEngine &graph) : m_id(id), m_graph(&graph) { - m_graph_node_id = graph.addNode("Mixer " + std::to_string(id), &m_node, 1, 1); - m_node.inputs.resize(1); - m_node.outputs.resize(1); -} - -int MixerEngine::inputPinId() const { return m_graph ? m_graph->inputPinId(m_graph_node_id) : -1; } - -int MixerEngine::outputPinId() const { - return m_graph ? m_graph->outputPinId(m_graph_node_id) : -1; -} +MixerEngine::MixerEngine(int id, NodeGraphEngine &graph) + : ComponentEngineBase(id, graph, "Mixer", 1, 1) {} void MixerEngine::update(double dt) { (void)dt; const Spectrum *in_ptr = m_node.inputs.empty() ? nullptr : m_node.inputs[0]; - if (!m_dirty && in_ptr == m_cached_input_ptr && - (!in_ptr || in_ptr->generation == m_cached_input_generation)) + if (!beginUpdate(in_ptr)) return; - m_dirty = false; - m_cached_input_ptr = in_ptr; - if (in_ptr) - m_cached_input_generation = in_ptr->generation; auto &out = m_node.outputs[0]; diff --git a/signal_generator/include/signal_generator_engine.h b/signal_generator/include/signal_generator_engine.h index 995bf04..9d4a90a 100644 --- a/signal_generator/include/signal_generator_engine.h +++ b/signal_generator/include/signal_generator_engine.h @@ -1,17 +1,14 @@ #pragma once -#include "component_interface.h" +#include "component_engine_base.h" #include "node_graph_engine.h" #include "signal_node.h" -class SignalGeneratorEngine : public IComponentEngine { +class SignalGeneratorEngine : public ComponentEngineBase { public: SignalGeneratorEngine(int id, NodeGraphEngine &graph); - int id() const override { return m_id; } - int graphNodeId() const override { return m_graph_node_id; } std::string_view type_name() const override { return "generator"; } std::string hoverSummary() const override; - int outputPinId() const override; void addTone(double freq_Hz, double power_dBm, double phase_deg = 0.0) { m_tones.push_back({freq_Hz, power_dBm, phase_deg}); @@ -37,19 +34,12 @@ class SignalGeneratorEngine : public IComponentEngine { double fs_Hz() const { return m_fs_Hz; } void setFs_Hz(double fs) { m_fs_Hz = fs; } - SignalNode &node() override { return m_node; } - const SignalNode &node() const override { return m_node; } void update(double dt) override; nlohmann::json serialize() const override; void deserialize(const nlohmann::json &) override; private: - int m_id; - int m_graph_node_id = -1; - NodeGraphEngine *m_graph = nullptr; std::vector m_tones; - SignalNode m_node; - bool m_dirty = true; double m_fs_Hz = 0.0; void rebuildFrequencyGrid(); diff --git a/signal_generator/src/signal_generator_engine.cpp b/signal_generator/src/signal_generator_engine.cpp index bc4eea9..d4e9a75 100644 --- a/signal_generator/src/signal_generator_engine.cpp +++ b/signal_generator/src/signal_generator_engine.cpp @@ -2,15 +2,10 @@ #include SignalGeneratorEngine::SignalGeneratorEngine(int id, NodeGraphEngine &graph) - : m_id(id), m_graph(&graph) { - m_graph_node_id = graph.addNode("Generator " + std::to_string(id), &m_node, 0, 1); + : ComponentEngineBase(id, graph, "Generator", 0, 1) { rebuildFrequencyGrid(); } -int SignalGeneratorEngine::outputPinId() const { - return m_graph ? m_graph->outputPinId(m_graph_node_id) : -1; -} - void SignalGeneratorEngine::rebuildFrequencyGrid() { const double start_Hz = MIN_FREQ; const double stop_Hz = MAX_FREQ; diff --git a/splitter/include/splitter_engine.h b/splitter/include/splitter_engine.h index 82c62a4..55060c6 100644 --- a/splitter/include/splitter_engine.h +++ b/splitter/include/splitter_engine.h @@ -2,18 +2,14 @@ #include -#include "component_interface.h" -#include "node_graph_engine.h" +#include "component_engine_base.h" #include "signal_node.h" -class SplitterEngine : public IComponentEngine { +class SplitterEngine : public ComponentEngineBase { public: SplitterEngine(int id, NodeGraphEngine &graph); - int id() const override { return m_id; } - int graphNodeId() const override { return m_graph_node_id; } std::string_view type_name() const override { return "splitter"; } std::string hoverSummary() const override; - int inputPinId() const override; int outputPinId(int index) const override; int outputPinId() const override { return outputPinId(0); } @@ -21,18 +17,6 @@ class SplitterEngine : public IComponentEngine { nlohmann::json serialize() const override; void deserialize(const nlohmann::json &) override; - SignalNode &node() override { return m_node; } - const SignalNode &node() const override { return m_node; } - private: - int m_id; - int m_graph_node_id = -1; - NodeGraphEngine *m_graph = nullptr; - - SignalNode m_node; - bool m_dirty = true; - const Spectrum *m_cached_input_ptr = nullptr; - uint64_t m_cached_input_generation = 0; - static constexpr double SPLIT_LOSS_DB = 3.010299956639812; }; diff --git a/splitter/src/splitter_engine.cpp b/splitter/src/splitter_engine.cpp index 8199e3d..6fbc90b 100644 --- a/splitter/src/splitter_engine.cpp +++ b/splitter/src/splitter_engine.cpp @@ -1,15 +1,8 @@ #include "splitter_engine.h" #include -SplitterEngine::SplitterEngine(int id, NodeGraphEngine &graph) : m_id(id), m_graph(&graph) { - m_graph_node_id = graph.addNode("Splitter " + std::to_string(id), &m_node, 1, 2); - m_node.inputs.resize(1); - m_node.outputs.resize(2); -} - -int SplitterEngine::inputPinId() const { - return m_graph ? m_graph->inputPinId(m_graph_node_id) : -1; -} +SplitterEngine::SplitterEngine(int id, NodeGraphEngine &graph) + : ComponentEngineBase(id, graph, "Splitter", 1, 2) {} int SplitterEngine::outputPinId(int index) const { if (!m_graph || m_graph_node_id < 0) @@ -27,13 +20,8 @@ int SplitterEngine::outputPinId(int index) const { void SplitterEngine::update(double dt) { (void)dt; const Spectrum *in_ptr = m_node.inputs.empty() ? nullptr : m_node.inputs[0]; - if (!m_dirty && in_ptr == m_cached_input_ptr && - (!in_ptr || in_ptr->generation == m_cached_input_generation)) + if (!beginUpdate(in_ptr)) return; - m_dirty = false; - m_cached_input_ptr = in_ptr; - if (in_ptr) - m_cached_input_generation = in_ptr->generation; for (size_t out_idx = 0; out_idx < m_node.outputs.size(); ++out_idx) { auto &out = m_node.outputs[out_idx]; diff --git a/tests/test_attenuator.cpp b/tests/test_attenuator.cpp index c273ca6..a7211d7 100644 --- a/tests/test_attenuator.cpp +++ b/tests/test_attenuator.cpp @@ -111,10 +111,10 @@ TEST_CASE("Attenuator: S-param mode frequency-dependent gain", "[attenuator][spa NodeGraphEngine graph; AttenuatorEngine atten(1, graph); - atten.setSParamFile( + atten.setSParamFilepath( std::string(PROJECT_SOURCE_DIR) + "/component_data/fixed_attenuators/atn01-0040psm/ATN01-0040PSM_SM_25C_De.s2p"); - REQUIRE(atten.sParamMode()); + REQUIRE(atten.sparamMode()); Spectrum input = buildTestSpectrum(); atten.node().inputs[0] = &input; @@ -131,7 +131,7 @@ TEST_CASE("Attenuator: S-param mode frequency-dependent gain", "[attenuator][spa TEST_CASE("Attenuator: S-param noise model", "[attenuator][sparam]") { NodeGraphEngine graph; AttenuatorEngine atten(1, graph); - atten.setSParamFile( + atten.setSParamFilepath( std::string(PROJECT_SOURCE_DIR) + "/component_data/fixed_attenuators/atn01-0040psm/ATN01-0040PSM_SM_25C_De.s2p"); @@ -151,7 +151,7 @@ TEST_CASE("Attenuator: S-param noise model", "[attenuator][sparam]") { TEST_CASE("Attenuator: S-param phase shift", "[attenuator][sparam]") { NodeGraphEngine graph; AttenuatorEngine atten(1, graph); - atten.setSParamFile( + atten.setSParamFilepath( std::string(PROJECT_SOURCE_DIR) + "/component_data/fixed_attenuators/atn01-0040psm/ATN01-0040PSM_SM_25C_De.s2p"); diff --git a/tests/test_coax_cable_engine.cpp b/tests/test_coax_cable_engine.cpp index fcf264d..b677f90 100644 --- a/tests/test_coax_cable_engine.cpp +++ b/tests/test_coax_cable_engine.cpp @@ -139,6 +139,28 @@ TEST_CASE("Coax cable length 0 leaves only connector loss", "[coax][edge]") { REQUIRE(with_zero_len == Approx(-1.0).margin(0.001)); } +TEST_CASE("Coax cable deserialize clamps out-of-range preset index", "[coax][deserialize]") { + NodeGraphEngine graph; + CoaxCableEngine cable(0, graph); + + // Corrupted/hand-edited .rfsim with an out-of-range preset index must be + // clamped to a valid preset instead of indexing out of bounds. + cable.deserialize({{"preset_index", 99}, {"length_m", 1.0}, {"connectors_loss_dB", 0.0}}); + REQUIRE(cable.presetIndex() >= 0); + REQUIRE(static_cast(cable.presetIndex()) < kCoaxCablePresets.size()); + REQUIRE(cable.preset().name == kCoaxCablePresets[cable.presetIndex()].name); + + cable.deserialize({{"preset_index", -1}, {"length_m", 1.0}, {"connectors_loss_dB", 0.0}}); + REQUIRE(cable.presetIndex() >= 0); + REQUIRE(static_cast(cable.presetIndex()) < kCoaxCablePresets.size()); + REQUIRE(cable.preset().name == kCoaxCablePresets[cable.presetIndex()].name); + + // Clamped length / connector-loss values must match the setter ranges. + cable.deserialize({{"preset_index", 4}, {"length_m", 5000.0}, {"connectors_loss_dB", 99.0}}); + REQUIRE(cable.lengthM() == Approx(1000.0)); + REQUIRE(cable.connectorsLossDB() == Approx(30.0)); +} + TEST_CASE("Coax cable clamps negative length to 0", "[coax][edge]") { NodeGraphEngine graph; CoaxCableEngine cable(0, graph); diff --git a/tests/test_combiner.cpp b/tests/test_combiner.cpp index 15fa9fe..f989593 100644 --- a/tests/test_combiner.cpp +++ b/tests/test_combiner.cpp @@ -100,9 +100,9 @@ TEST_CASE("Combiner: S-param mode", "[combiner][sparam]") { NodeGraphEngine graph; CombinerEngine combiner(1, graph); - combiner.setSParamFile(std::string(PROJECT_SOURCE_DIR) + - "/component_data/splitters/mpd-0226ch/MPD-0226CH_CH_25C_F.s3p"); - REQUIRE(combiner.sParamMode()); + combiner.setSParamFilepath(std::string(PROJECT_SOURCE_DIR) + + "/component_data/splitters/mpd-0226ch/MPD-0226CH_CH_25C_F.s3p"); + REQUIRE(combiner.sparamMode()); Spectrum input0 = buildTestSpectrum(1e9, -10.0, 0.0); Spectrum input1 = buildTestSpectrum(2e9, -20.0, 0.0); diff --git a/tests/test_component_dispatch.cpp b/tests/test_component_dispatch.cpp index dd8ed06..553c213 100644 --- a/tests/test_component_dispatch.cpp +++ b/tests/test_component_dispatch.cpp @@ -11,10 +11,13 @@ #include "imgui.h" #include "imnodes.h" #include "implot.h" +#include "inspector_panel.h" #include "pfb_channelizer_engine.h" #include #include #include +#include +#include struct ImGuiFixture { ImGuiFixture() { @@ -116,3 +119,54 @@ TEST_CASE_METHOD(ImGuiFixture, REQUIRE(engine->type_name() == d->type); } } + +// Data-driven consistency across the three places a component type is +// declared: ComponentTypeRegistry rows, InspectorPanel drawers, and the +// NodeKind enum. A new type that forgets any one of them fails here loudly +// instead of silently rendering an empty properties panel or an unknown node +// kind. The expected tables are compiled against the enum's current values, +// so dropping a NodeKind or forgetting to register a drawer breaks the build +// or the test. +TEST_CASE("Registry rows, inspector drawers, and NodeKinds stay consistent", "[dispatch]") { + // Canonical type key -> NodeKind, mirroring ComponentTypeRegistry rows. + const std::map expected_kind = { + {"generator", NodeKind::Generator}, + {"amplifier", NodeKind::Amplifier}, + {"splitter", NodeKind::Splitter}, + {"mixer", NodeKind::Mixer}, + {"adc", NodeKind::Adc}, + {"pfb", NodeKind::PFB}, + {"filter", NodeKind::IdealFilter}, + {"coax", NodeKind::CoaxCable}, + {"equalizer", NodeKind::Equalizer}, + {"attenuator", NodeKind::Attenuator}, + {"combiner", NodeKind::Combiner}, + }; + // supports_sparam_file must be true exactly for the engines that + // implement the Touchstone S-param API (verified against + // AmplifierEngine, IdealFilterEngine, EqualizerEngine, AttenuatorEngine, + // CombinerEngine). + const std::map expected_sparam = { + {"amplifier", true}, {"attenuator", true}, {"combiner", true}, {"equalizer", true}, + {"filter", true}, {"adc", false}, {"coax", false}, {"generator", false}, + {"mixer", false}, {"pfb", false}, {"splitter", false}, + }; + + for (const auto *d : ComponentTypeRegistry::instance().all()) { + CAPTURE(d->type); + // (a) Every registered type must have an inspector drawer. + CHECK(InspectorPanel::hasDrawer(d->type)); + + // (b) Every type must map to a real NodeKind matching the canonical + // table; a table miss means the enum doesn't cover the type yet. + auto kind_it = expected_kind.find(d->type); + REQUIRE(kind_it != expected_kind.end()); + CHECK(kind_it->second != NodeKind::Unknown); + CHECK(d->kind == kind_it->second); + + // (c) The S-param capability flag must match engine reality. + auto sparam_it = expected_sparam.find(d->type); + REQUIRE(sparam_it != expected_sparam.end()); + CHECK(d->supports_sparam_file == sparam_it->second); + } +} diff --git a/tests/test_project_file.cpp b/tests/test_project_file.cpp index 0cae55f..590e9fc 100644 --- a/tests/test_project_file.cpp +++ b/tests/test_project_file.cpp @@ -472,12 +472,12 @@ TEST_CASE_METHOD(ImGuiFixture, "Round-trip: S-param mode survives save/load (iss REQUIRE(eq.sparamLoaded()); auto &atten = app.testComponents().add(10004, app.testGraphEngine()); - atten.setSParamFile(s2p); - REQUIRE(atten.sParamMode()); + atten.setSParamFilepath(s2p); + REQUIRE(atten.sparamMode()); auto &comb = app.testComponents().add(10005, app.testGraphEngine()); - comb.setSParamFile(s2p); - REQUIRE(comb.sParamMode()); + comb.setSParamFilepath(s2p); + REQUIRE(comb.sparamMode()); REQUIRE(app.componentCount() == 5); app.saveProject(path); @@ -504,11 +504,11 @@ TEST_CASE_METHOD(ImGuiFixture, "Round-trip: S-param mode survives save/load (iss auto attens = app.testComponents().byType(); REQUIRE(attens.size() == 1); - CHECK(attens[0]->sParamMode() == true); + CHECK(attens[0]->sparamMode() == true); auto combs = app.testComponents().byType(); REQUIRE(combs.size() == 1); - CHECK(combs[0]->sParamMode() == true); + CHECK(combs[0]->sparamMode() == true); } std::remove(path.c_str()); } \ No newline at end of file diff --git a/tests/test_signal_domain.cpp b/tests/test_signal_domain.cpp index bad2786..56370d9 100644 --- a/tests/test_signal_domain.cpp +++ b/tests/test_signal_domain.cpp @@ -154,7 +154,7 @@ TEST_CASE("Attenuator: propagates is_complex_baseband (manual mode)", "[domain][ TEST_CASE("Attenuator: propagates is_complex_baseband (S-param mode)", "[domain][attenuator]") { NodeGraphEngine graph; AttenuatorEngine atten(0, graph); - atten.setSParamFile(attenuatorS2pPath()); + atten.setSParamFilepath(attenuatorS2pPath()); Spectrum in; in.frequencies = {1e9, 2e9}; @@ -505,6 +505,24 @@ TEST_CASE("Amplifier: propagates fs_Hz (S-param mode)", "[domain][amplifier]") { REQUIRE(amp.node().outputs[0].fs_Hz == Approx(500e6)); } +TEST_CASE("Attenuator: propagates fs_Hz (S-param mode)", "[domain][attenuator]") { + NodeGraphEngine graph; + AttenuatorEngine atten(0, graph); + atten.setSParamFilepath(attenuatorS2pPath()); + REQUIRE(atten.sparamMode()); + + Spectrum in; + in.frequencies = {1e9, 2e9}; + in.tones = {{1e9, -10.0, 0.0}}; + in.noise_total_W.assign(2, 1e-21); + in.fs_Hz = 500e6; + + atten.node().inputs[0] = ∈ + atten.update(0.0); + + REQUIRE(atten.node().outputs[0].fs_Hz == Approx(500e6)); +} + // Real multi-engine post-ADC chains: fs_Hz must reach the PFB and channels must be populated // without any manual setFs_Hz() (issue #43 regression tests). From 5a9cfece9bb13d4cc0226bf31c2dcd49cb4b3f45 Mon Sep 17 00:00:00 2001 From: Jaco du Preez Date: Mon, 10 Aug 2026 09:22:26 +0200 Subject: [PATCH 2/2] Fix #56 test resolution after merging master: atten/comb S-param path must use the local staged fixture (S1 containment neutralizes absolute out-of-project paths on load) --- tests/test_project_file.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_project_file.cpp b/tests/test_project_file.cpp index b395e33..4616aa7 100644 --- a/tests/test_project_file.cpp +++ b/tests/test_project_file.cpp @@ -547,11 +547,11 @@ TEST_CASE_METHOD(ImGuiFixture, "Round-trip: S-param mode survives save/load (iss REQUIRE(eq.sparamLoaded()); auto &atten = app.testComponents().add(10004, app.testGraphEngine()); - atten.setSParamFilepath(s2p); + atten.setSParamFilepath(local_s2p); REQUIRE(atten.sparamMode()); auto &comb = app.testComponents().add(10005, app.testGraphEngine()); - comb.setSParamFilepath(s2p); + comb.setSParamFilepath(local_s2p); REQUIRE(comb.sparamMode()); REQUIRE(app.componentCount() == 5);