From e2006526153180e2700889f8f819a1c94b754fe8 Mon Sep 17 00:00:00 2001 From: alvvm Date: Wed, 1 Jul 2026 12:32:47 +0200 Subject: [PATCH] fix(toolbox_transform_editor): MIMO-aware live preview The preview declared a single ephemeral output, so a multi-output (N->M) function returned M values into a 1-output node -> zero rows -> a false "function produced no output" error that disabled Create. Declare M ephemeral outputs (from the comma-separated name field, same as the real create path), plot one curve per output, and only flag "no output" when ALL declared outputs are empty. The arity error now names the declared count symmetrically (too few OR too many returns). Resolve the source ghost + all outputs in ONE catalog pass (readManyRawSamples) and reconstruct topic/field keys without doubling the slash, matching the host's single-slash input keys (pairs with PJ4 joinTopicField / resolveInputField). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01K4uZtXDT36cdFQayeuPJ5r --- .../transform_editor_plugin.cpp | 166 ++++++++++++------ 1 file changed, 115 insertions(+), 51 deletions(-) diff --git a/toolbox_transform_editor/transform_editor_plugin.cpp b/toolbox_transform_editor/transform_editor_plugin.cpp index 9b8a1a2..c0a1199 100644 --- a/toolbox_transform_editor/transform_editor_plugin.cpp +++ b/toolbox_transform_editor/transform_editor_plugin.cpp @@ -1615,52 +1615,68 @@ class TransformEditorToolbox : public PJ::ToolboxPluginBase { // preview. The ephemeral transform output lives in the engine's topic list (and // hence in catalogSnapshot) even though it is kept out of the UI catalog, so this // resolves both the source AND the transformed result by name. - std::vector> readRawSamples(const std::string& field_name) { + // Decimated (timestamp, value) samples for one already-resolved field handle. + std::vector> samplesFromHandle(PJ::sdk::FieldHandle handle) { std::vector> out; + auto read = toolboxHost().readSeries(handle); + if (!read || read->rowCount() == 0 || read->valuesAsFloat64() == nullptr) { + return out; + } + const auto ts = read->timestamps(); + const double* vals = read->valuesAsFloat64(); + const size_t n = read->rowCount(); + const size_t step = (n > 2000) ? (n / 2000) : 1; + out.reserve(n / step + 1); + for (size_t i = 0; i < n; i += step) { + out.emplace_back(ts[i], vals[i]); + } + return out; + } + + // Resolve several "topic/field" names (or a bare topic / bare field) against ONE + // catalog snapshot in a single nested pass, returning decimated samples per name + // (empty if a name did not resolve). The MIMO preview reads the source ghost + M + // outputs every tick; a per-name readRawSamples would re-acquire the snapshot and + // re-scan the whole catalog M+1 times. Field paths in the snapshot are RELATIVE to + // their topic and a ROS leaf carries a leading '/', so reconstruct "topic/field" + // WITHOUT doubling the slash (matching the host's joinTopicField) — a naive + // `tname + "/" + fname` would rebuild the old "topic//field" and fail to match. + std::vector>> readManyRawSamples(const std::vector& names) { + std::vector>> out(names.size()); auto catalog = toolboxHost().catalogSnapshot(); if (!catalog) { return out; } - // Resolve `field_name` (a full "topic/field" path, OR a bare topic name for a - // single-field derived output) to a field handle. Field paths in the snapshot - // are RELATIVE to their topic, so reconstruct the full "topic/field" name the - // same way CatalogModel does, instead of fragile substring matching. const auto fields = catalog->fields(); const auto topics = catalog->topics(); - PJ::sdk::FieldHandle handle{}; - bool found = false; + std::vector handles(names.size()); + std::vector found(names.size(), false); + std::size_t remaining = names.size(); for (const auto& t : topics) { + if (remaining == 0) { + break; + } const std::string tname(t.name.data, t.name.size); - const bool topic_is_target = (tname == field_name); const uint32_t end = t.first_field + t.field_count; for (uint32_t fi = t.first_field; fi < end && fi < fields.size(); ++fi) { const auto& f = fields[fi]; const std::string fname(f.name.data, f.name.size); - const std::string full = tname.empty() ? fname : (tname + "/" + fname); - if (topic_is_target || full == field_name || fname == field_name) { - handle = PJ::sdk::FieldHandle{f.handle}; - found = true; - break; + const std::string leaf = (!fname.empty() && fname.front() == '/') ? fname.substr(1) : fname; + const std::string full = tname.empty() ? leaf : (tname + "/" + leaf); + for (std::size_t i = 0; i < names.size(); ++i) { + if (found[i] || (tname != names[i] && full != names[i] && fname != names[i] && leaf != names[i])) { + continue; + } + handles[i] = PJ::sdk::FieldHandle{f.handle}; + found[i] = true; + --remaining; } } - if (found) { - break; - } } - if (!found) { - return out; - } - auto read = toolboxHost().readSeries(handle); - if (!read || read->rowCount() == 0 || read->valuesAsFloat64() == nullptr) { - return out; - } - const auto ts = read->timestamps(); - const double* vals = read->valuesAsFloat64(); - const size_t n = read->rowCount(); - const size_t step = (n > 2000) ? (n / 2000) : 1; - out.reserve(n / step + 1); - for (size_t i = 0; i < n; i += step) { - out.emplace_back(ts[i], vals[i]); + for (std::size_t i = 0; i < names.size(); ++i) { + if (found[i]) { + out[i] = samplesFromHandle(handles[i]); + } } return out; } @@ -1726,8 +1742,19 @@ class TransformEditorToolbox : public PJ::ToolboxPluginBase { input_topics.push_back(extra); } - const std::string preview_output = "__te_preview__"; + // Declare the SAME output topics the real Create path uses (splitOutputNames of + // the comma-separated name field) so a MIMO body returning M values matches the + // node's arity — a single-output preview node drops every row of an M-output + // function and falsely reports "no output". Each slot gets a unique ephemeral + // name; fall back to one slot while the name field is still empty (mid-typing). const std::string preview_id(kPreviewId); + std::vector output_names = splitOutputNames(dialog_.outputName()); + const std::size_t num_outputs = output_names.empty() ? 1 : output_names.size(); + std::vector preview_outputs; + preview_outputs.reserve(num_outputs); + for (std::size_t k = 0; k < num_outputs; ++k) { + preview_outputs.push_back(preview_id + "_" + std::to_string(k)); + } const std::string script = buildTransformScript(preview_id, preview_id, global, body, num_extra, dialog_.language()); @@ -1747,7 +1774,7 @@ class TransformEditorToolbox : public PJ::ToolboxPluginBase { } std::vector inputs_sv(input_topics.begin(), input_topics.end()); - std::vector outputs_sv{preview_output}; + std::vector outputs_sv(preview_outputs.begin(), preview_outputs.end()); tearDownPreview(); auto status = dp_view_.createEphemeralTransform( @@ -1761,18 +1788,41 @@ class TransformEditorToolbox : public PJ::ToolboxPluginBase { } preview_key_ = preview_id; - // Read the source (ghost) and the freshly-materialised transform output back - // from the datastore, and hand the host explicit points to plot. Both share a - // common t0 (the earliest sample) so they align on the time axis. - auto ghost_raw = readRawSamples(source); - auto result_raw = readRawSamples(preview_output); - - // The script compiled, but if the real node produced NO output while the source - // has data, the function returns nil/non-numeric (e.g. `return valu`) — flag it. - // This catches the runtime case that validateScript (compile-only) cannot, using - // the real inputs (so a valid multi-input function is NOT falsely flagged). - if (!ghost_raw.empty() && result_raw.empty()) { - dialog_.setValidationError("function produced no output — it must return a number"); + // Read the source (ghost) and each materialised output back from the datastore, + // and hand the host explicit points to plot. Resolve all of them in ONE catalog + // pass (readManyRawSamples) — a per-series read would re-scan the whole catalog + // M+1 times per tick. All share a common t0 (the earliest sample across ghost + + // outputs) so they align on the time axis. + std::vector read_names; + read_names.reserve(1 + preview_outputs.size()); + read_names.push_back(source); + read_names.insert(read_names.end(), preview_outputs.begin(), preview_outputs.end()); + auto samples = readManyRawSamples(read_names); + + auto ghost_raw = std::move(samples[0]); + std::vector>> results( + std::make_move_iterator(samples.begin() + 1), std::make_move_iterator(samples.end())); + bool any_result = false; + for (const auto& r : results) { + any_result = any_result || !r.empty(); + } + + // The script compiled, but if the source has data and NONE of the M declared + // outputs produced a row, the function never returns numbers (e.g. `return valu`) + // — flag it. Fire ONLY when every output is empty: one empty channel among several + // is legitimate (a suppressed output). validateScript (compile-only) can't catch + // this runtime case; real inputs are used, so a valid MIMO is not falsely flagged. + if (!ghost_raw.empty() && !any_result) { + // The output-name field declares N outputs; the body must return EXACTLY N + // values per point (positional). A silent MIMO no-op is almost always this + // arity mismatch — in EITHER direction (too few OR too many returns) — or a + // body that returns nothing. State the declared count symmetrically rather + // than a misleading "must return a number" (the user may have returned several). + const std::string n = std::to_string(num_outputs); + const std::string plural = num_outputs == 1 ? "" : "s"; + const std::string msg = "function produced no output — declared " + n + " output" + plural + + ", so it must return exactly " + n + " value" + plural + " per point"; + dialog_.setValidationError(msg); } int64_t t0 = 0; @@ -1781,9 +1831,11 @@ class TransformEditorToolbox : public PJ::ToolboxPluginBase { t0 = ghost_raw.front().first; have_t0 = true; } - if (!result_raw.empty() && (!have_t0 || result_raw.front().first < t0)) { - t0 = result_raw.front().first; - have_t0 = true; + for (const auto& raw : results) { + if (!raw.empty() && (!have_t0 || raw.front().first < t0)) { + t0 = raw.front().first; + have_t0 = true; + } } const auto to_points = [t0](const std::vector>& raw) { @@ -1796,12 +1848,24 @@ class TransformEditorToolbox : public PJ::ToolboxPluginBase { }; std::vector series; - // Ghost (original): faded blue + dashed, distinct from the solid orange result + // Ghost (original): faded blue + dashed, distinct from the solid result curves // — same styling as the native TransformEditorPanel. Color hex is #AARRGGBB. series.push_back({source, to_points(ghost_raw), "#5A4488ff", /*dashed=*/true}); - if (!result_raw.empty()) { - const std::string label = dialog_.outputName().empty() ? "result" : dialog_.outputName(); - series.push_back({label, to_points(result_raw), "#ff8800", /*dashed=*/false}); + // One solid curve per declared output, cycling a small palette so parallel MIMO + // outputs stay visually distinct; labelled by the user's output name. + static constexpr const char* kResultColors[] = {"#ff8800", "#0088ff", "#22aa22", "#cc2222", "#9933cc", "#00a0a0"}; + static constexpr std::size_t kNumColors = sizeof(kResultColors) / sizeof(kResultColors[0]); + for (std::size_t k = 0; k < results.size(); ++k) { + if (results[k].empty()) { + continue; + } + std::string label; + if (k < output_names.size() && !output_names[k].empty()) { + label = output_names[k]; + } else { + label = results.size() > 1 ? ("result" + std::to_string(k)) : std::string("result"); + } + series.push_back({label, to_points(results[k]), kResultColors[k % kNumColors], /*dashed=*/false}); } dialog_.setPreviewSeries(std::move(series)); }