From b7b24c1fa130feb59f12bc5ab47c556e8715f1c1 Mon Sep 17 00:00:00 2001 From: alvvm Date: Tue, 16 Jun 2026 13:53:09 +0200 Subject: [PATCH 01/11] Add PlotMarkers canonical builtin object + codec (markers Phase 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New SDK builtin type for time-series plot markers (findings), the foundation of the anomaly-detection pipeline feature. - PlotMarkers (kPlotMarkers = 18): homogeneous, id-less marker records (Region / Event / ValueBand / Label) with shared semantic fields (status, severity, category, label, description, color, metadata) + a PlotMarkers list container. No id/source/scope by design — identity is owned by the host marker store; location carries provenance/reach. - Hand-written PJ.PlotMarkers wire codec following the image_annotations_codec idiom; PlotMarkers.proto contract. - Registered in C++ + C-ABI builtin enums, name()/parse()/typeOf(), and the ABI layout sentinel (static_assert id == 18). - Round-trip codec tests; full pj_base suite green (32/32) under ASAN. - Design (use-cases, architecture) + wire-format docs. ColorRGBA reused via image_annotations.hpp per the existing house convention (mesh3d / scene_entities do the same). abi/baseline.abi refresh + SDK MINOR version bump deferred to the release-cut step (additions-only). Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/plot_markers_architecture.md | 195 ++++++++++ docs/plot_markers_format.md | 77 ++++ docs/plot_markers_use_cases.md | 160 ++++++++ pj_base/CMakeLists.txt | 2 + .../pj_base/builtin/builtin_object.hpp | 10 + .../include/pj_base/builtin/plot_markers.hpp | 106 ++++++ .../pj_base/builtin/plot_markers_codec.hpp | 26 ++ pj_base/include/pj_base/builtin_object_abi.h | 1 + pj_base/proto/pj/PlotMarkers.proto | 66 ++++ pj_base/src/builtin/plot_markers_codec.cpp | 358 ++++++++++++++++++ pj_base/tests/abi_layout_sentinels_test.cpp | 1 + pj_base/tests/builtin_object_test.cpp | 3 + pj_base/tests/plot_markers_codec_test.cpp | 150 ++++++++ 13 files changed, 1155 insertions(+) create mode 100644 docs/plot_markers_architecture.md create mode 100644 docs/plot_markers_format.md create mode 100644 docs/plot_markers_use_cases.md create mode 100644 pj_base/include/pj_base/builtin/plot_markers.hpp create mode 100644 pj_base/include/pj_base/builtin/plot_markers_codec.hpp create mode 100644 pj_base/proto/pj/PlotMarkers.proto create mode 100644 pj_base/src/builtin/plot_markers_codec.cpp create mode 100644 pj_base/tests/plot_markers_codec_test.cpp diff --git a/docs/plot_markers_architecture.md b/docs/plot_markers_architecture.md new file mode 100644 index 0000000..a096d1b --- /dev/null +++ b/docs/plot_markers_architecture.md @@ -0,0 +1,195 @@ +# Plot Markers — Architecture + +> **Status: design draft.** The *what/why* and examples live in +> [plot_markers_use_cases.md](plot_markers_use_cases.md). This document is the +> *how*: the type, the store, the API surface, and how PJ4 renders markers. Names +> and signatures are illustrative until frozen. +> +> Plot Markers reuse the **concept** of [`ImageAnnotations`](image_annotations_format.md) +> (a canonical SDK builtin object with a wire codec) but not its structure. + +## 1. Layering & data flow + +Three layers, with a clean boundary at the SDK C-ABI: + +``` + Producer (toolbox plugin / future agent / ingestion parser) + │ add(dataset, father, marker) → id + │ delete(dataset, father, id) + │ query(dataset, [series], [filters]) → markers + ▼ ── SDK C-ABI (marker host-view service) ── + Host marker store ← owns identity (the marker id) and the mutable set + ▼ + PJ4 consumers: pj_plotting Qwt overlay · markers panel · JSON export +``` + +- **SDK (`plotjuggler_sdk`)** owns the **type + codec** and the **marker host-view + service contract** (the C-ABI a producer speaks). This is the only code a plugin or + agent compiles against — which is exactly why the typed contract must live here. +- **Host (`PJ4`)** owns the **marker store** (the identity owner) behind that + contract, plus rendering, the panel, and export. +- Producers and the agent **never** see the store — only the API. The store is an + implementation detail and can change without touching a single producer. + +## 2. The `PlotMarker` / `PlotMarkers` type + +A marker is a **homogeneous, id-less record**; a topic holds a **list** of them. This +is the structural departure from `ImageAnnotations` (which groups heterogeneous +primitives `points[]`/`circles[]`/`texts[]`): markers are one uniform record type +distinguished by a `kind`. + +```cpp +namespace PJ { +namespace sdk { + +enum class MarkerKind : uint8_t { + kRegion, ///< time span [t_start, t_end] — shaded vertical band + kEvent, ///< single time t_start (+ optional value) — tick / point + kValueBand, ///< value span [value_low, value_high] — horizontal band (series-only) + kLabel, ///< text callout anchored at t_start +}; + +enum class MarkerStatus : uint8_t { kNone, kPass, kFail }; +enum class MarkerSeverity : uint8_t { kInfo, kWarning, kError, kCritical }; + +/// Producer-specific key/value extension hatch — keeps the schema stable as +/// producers attach extra fields (threshold, peak, from/to, …) without a schema bump. +struct MarkerProperty { + std::string key; + std::string value; + bool operator==(const MarkerProperty&) const = default; +}; + +/// One marker. Carries NO id (the store owns identity), NO source (no builtin +/// records its creator), NO scope (the topic it lives under says that). +struct PlotMarker { + MarkerKind kind = MarkerKind::kRegion; + + // --- anchor (interpret by kind; irrelevant fields ignored) --- + Timestamp t_start = 0; ///< Region start · Event/Label time · (ValueBand: ignored) + Timestamp t_end = 0; ///< Region end · (others: ignored) + double value_low = 0.0; ///< ValueBand low · Event point value · (others: ignored) + double value_high = 0.0; ///< ValueBand high · (others: ignored) + bool has_value = false; ///< Event: value_low is a meaningful point value. + + // --- semantics / presentation (shared by every kind) --- + MarkerStatus status = MarkerStatus::kNone; + MarkerSeverity severity = MarkerSeverity::kInfo; + std::string category; + std::string label; + std::string description; + ColorRGBA color = {0, 0, 0, 0}; ///< a=0 → derive from severity. + std::vector metadata; + + bool operator==(const PlotMarker&) const = default; +}; + +/// The canonical object a marker query/render reads: the set of markers for one +/// topic (one series, or the dataset-global topic). +struct PlotMarkers { + std::vector markers; + bool operator==(const PlotMarkers&) const = default; + [[nodiscard]] bool empty() const noexcept { return markers.empty(); } +}; + +} // namespace sdk +} // namespace PJ +``` + +Design notes: +- **Flat anchor + `kind`** (not a `oneof`, not per-kind vectors). Simplest for the + hand-written wire codec; the cost is that an invalid combination (a `Region` with + `value_high` set) is *representable* — the codec/renderer just ignore irrelevant + fields. +- **`has_value`** because a bare `double` cannot express "absent" for the optional + `Event` value. +- **`ColorRGBA`** currently lives in `image_annotations.hpp`; reuse here requires + promoting it to a shared vocabulary header so `PlotMarkers` does not include + image-annotation code. +- New canonical type → append `kPlotMarkers` to `BuiltinObjectType` / + `PJ_builtin_object_type_t` (append-only; **MINOR** SDK bump; refresh + `abi/baseline.abi`) + a `plot_markers_codec` mirroring the `image_annotations_codec` + *pattern*. + +## 3. Why a dedicated marker store (not ObjectStore, not DataEngine) + +The feature's core operation is **delete one specific marker**. That single +requirement decides the storage: + +- **`DataEngine` (columnar) — no.** It stores scalar samples as numeric columns. A + marker is a heterogeneous record with a *span* `[t1,t2]`, an enum severity, strings. + Forcing it into columns and inventing a "sample timestamp" for a span is a deep + impedance mismatch. +- **`ObjectStore` — no (for the real design).** It is a timestamped blob log whose + only removal verbs are **front-eviction** (`evictBefore`, `removeTopic`, `clear`): + it *structurally cannot delete entry #42*. To get per-marker delete on top of it you + must store the whole set as **one snapshot blob** and rewrite it on every edit — and + smuggle an `id` *into* the canonical type that no other builtin has. +- **Dedicated marker store — yes.** One marker per entry, real per-id add/delete, and + identity owned by the store (a handle like `ObjectStore`'s native `SequentialUID`). + This keeps `PlotMarker` clean and consistent with all 16 other builtins, and the + `id` lives *outside* the payload, surfaced by the API. + +The `id` you asked about earlier is therefore a **store handle**, not a type field. + +> **Fallback.** If shipping speed ever outweighs cleanliness, back the same API with +> an `ObjectStore` snapshot (one blob per topic, `id` carried inside the blob, +> rewrite-on-edit). The producer-facing API is identical either way — this is purely +> a host-internal choice. + +## 4. The marker host-view service (SDK C-ABI) + +Store-agnostic, symmetric (create + query), addressed by data identity: + +```text +add (dataset, father_name, PlotMarker) -> MarkerId +delete(dataset, father_name, MarkerId) -> Status +query (dataset, father_name, [time_range], [min_severity], …) -> {MarkerId, PlotMarker}[] +``` + +- `father_name` is the series topic (e.g. `cmd_vel/x`) or the dataset-level global + topic. There is no panel/GUI addressing. +- **Per-series query is direct** — read the markers under that topic; no scan. +- Cross-series aggregation ("all `warning+` markers in the dataset") enumerates the + dataset's marker topics (a known limitation, acceptable at marker scale). +- This is the contract the **AI agent** drives and that **plugins** call; both are + data-domain operations, which is why they belong on the SDK ABI (and "create a + plot / edit layout" deliberately does **not** — that is app-control, not data). + +## 5. Producers + +All produce the **same** `PlotMarker` type; they differ only in where the markers +come from: +- **Toolbox plugin (today).** Analysis over already-loaded series → `add`. The first + production user of the marker write path (analogous to the existing object-write + path that no toolbox uses yet). +- **AI agent (future).** Drives the same `add`/`delete`/`query` API to manage markers + programmatically. +- **Ingestion parser (future).** Markers that already exist in a recording (an event + channel, annotation messages) decode into `PlotMarker`s — the same role an image + parser plays for `ImageAnnotations`. + +## 6. Rendering (PJ4 host) + +- A **`PlotMarkersItem : QwtPlotItem`** overlay in `pj_plotting/widget/`, modeled on + `CurveTracker` (the existing custom Qwt item). Its `draw(painter, xMap, yMap, rect)` + maps marker times→pixels via `canvasMap()` and paints: `Region`/`ValueBand` as + translucent fills, `Event` as ticks/points, `Label` as text. +- **Render rule:** a series marker draws on every plot showing that series; a global + marker draws on every plot of its dataset whose visible X-window overlaps it. +- **Repaint** on data-change (the marker store notifying the host) and on time/zoom + changes, exactly as `CurveTracker` repaints on `PlaybackEngine::currentTimeChanged`. +- **Markers panel** in `pj_app` (modeled on `CurveListPanel`): filterable list, + click-row → `PlaybackEngine::setCurrentTime(marker_time)`. +- This is the **`pj_plotting` Qwt path, not the scene2D image path** — markers + annotate time-series plots, not image frames. + +## 7. Open / deferred + +- **Headless CLI.** PJ4 is GUI-only today (`pj_app/src/main.cpp` always opens a + window). The data model is headless-ready (Qt-free `pj_runtime`/store), but a true + no-GUI runner is a separate effort; v1 exposes JSON export as a host action. +- **Cross-series aggregate query** enumerates topics (per-series query is direct). +- **Concurrent writers** to the same topic (agent + plugin) need host-side locking on + that topic. +- **`ObjectStore`-snapshot fallback** (§3) remains available behind the same API. diff --git a/docs/plot_markers_format.md b/docs/plot_markers_format.md new file mode 100644 index 0000000..ddaed1f --- /dev/null +++ b/docs/plot_markers_format.md @@ -0,0 +1,77 @@ +# Plot Markers Format + +PlotJuggler uses a canonical `PJ.PlotMarkers` wire format when plot-marker +findings need to be stored, transported, or replayed as bytes. A `PlotMarkers` +value is the set of markers for one topic (one series, or a dataset-global +topic); the codec serializes it to the protobuf-wire payload described by the +schema. + +A marker is a *time-centric* finding on a plot, the analog of the *image-centric* +[`ImageAnnotations`](image_annotations_format.md). It is **not** structured like +`ImageAnnotations`: a marker is a homogeneous record distinguished by `kind`, and +`PlotMarkers` is a flat list of them. For the broader builtin type catalog, see +[builtin_type.md](builtin_type.md). + +## Contract + +The schema identifier for this format is: + +```text +PJ.PlotMarkers +``` + +The public C++ helpers live in: + +```cpp +#include +``` + +`serializePlotMarkers()` writes this payload. `deserializePlotMarkers()` reads it +back into `PJ::sdk::PlotMarkers`. + +The field-level contract is `pj_base/proto/pj/PlotMarkers.proto` and its imported +`pj_base/proto/pj/*.proto` files (`Color.proto`, `KeyValuePair.proto`). As with +the other builtins, the C++ codec uses PlotJuggler's private wire primitives +rather than generated Protobuf code; the `.proto` files are the source of truth +for field numbers and wire types. + +## What the marker does NOT carry (by design) + +- **No `id`.** Identity (the delete handle) is owned by the host marker store and + surfaced by the marker API — not serialized into the value. This keeps + `PlotMarker` consistent with every other builtin, none of which carry an id. +- **No `source`.** No builtin records its creator; provenance is the dataset/topic + the marker lives under. Producer-specific extras go in `metadata`. +- **No `scope`.** A marker's reach is decided by *which topic* it is addressed to + (a series topic vs. a dataset-global topic), not by a field. + +## SDK Mapping + +| Schema field (`PlotMarker`) | SDK behavior | +|-----------------------------|--------------| +| `kind` | `PlotMarker::kind`. Unknown values decode to `kRegion`. | +| `t_start` / `t_end` | `int64` ns; `PlotMarker::t_start` / `t_end`. | +| `value_low` / `value_high` | `double`; ValueBand bounds / optional Event point value. | +| `has_value` | `PlotMarker::has_value` (the Event point value is meaningful). | +| `status` / `severity` | enums; unknown values decode to `kNone` / `kInfo`. | +| `category` / `label` / `description` | strings. | +| `color` | `PJ.Color` message; alpha 0 means "derive from severity". | +| `metadata` | `repeated PJ.KeyValuePair` → `std::vector`. | + +## Codec Rules + +Colors are stored as normalized `double` channels in `[0, 1]` (a `PJ.Color` +message); the SDK stores RGBA `uint8_t`. Decode clamps to `[0, 1]` and rounds to +the nearest byte, so a round trip may differ by one channel value due to +floating-point rounding. + +Enum fields are written as their raw numeric value and always emitted. The reader +maps unknown `kind` to `kRegion`, unknown `status` to `kNone`, and unknown +`severity` to `kInfo`, so forward-compatible payloads still decode. + +A `PlotMarkers` value with no markers serializes to an empty byte buffer. Decoding +a null or empty buffer is treated as invalid input by the current reader. + +The reader decodes the mapped fields and skips unknown fields (including unknown +nested fields), so compatible schema additions are tolerated. Malformed protobuf +data, invalid length-delimited fields, or truncated nested messages fail decoding. diff --git a/docs/plot_markers_use_cases.md b/docs/plot_markers_use_cases.md new file mode 100644 index 0000000..5863543 --- /dev/null +++ b/docs/plot_markers_use_cases.md @@ -0,0 +1,160 @@ +# Plot Markers — Use Cases & Examples + +> **Status: design draft.** This document defines *what* the Plot Markers feature is +> for and *how it is used*, through concrete examples. The architecture (types, +> store, API surface, rendering) lives in +> [plot_markers_architecture.md](plot_markers_architecture.md). Field and API names +> here are illustrative until the type is frozen. +> +> Plot Markers borrow the *concept* of [`ImageAnnotations`](image_annotations_format.md) +> — a canonical SDK builtin object with a wire codec — but **not its structure**. +> An image annotation overlays a video frame; a plot marker annotates a *time-series +> plot*, and its shape is its own (see the architecture doc). + +## 1. Motivation + +Plugins — and a future **AI agent** — need a way to put **graphical markers** on +plots (a shaded time region, a point at an event, a value band) and to **ask which +markers exist** on a given series. Today there is no such API: annotations exist +only for *images*, tied to an image frame, not to plot time. + +The decisive framing: **a marker is a structured *finding*, not just a drawing.** It +carries semantic content — a pass/fail status, a severity, a category — alongside its +anchor in time. That content is what the JSON report and the query API operate on, +whether or not the marker is ever drawn. + +A marker has three consumers of one data model: +- the **plot renderer** (a Qwt overlay on the time-series plot), +- a **markers panel** (list, filter, jump-to-time), +- a **JSON report** (pass/fail + anomalies + timestamps + severity). + +Producers create *and delete* markers, and any producer (or the host) can *query* +them — the API is **symmetric and bidirectional**. Today the producer is an analysis +**toolbox** plugin; a future **AI agent** and an **ingestion parser** (markers that +already exist in a recording) are natural additional producers sharing the exact same +type. + +## 2. Vocabulary + +A **marker** is a record with a `kind`, an anchor, and shared semantic fields. There +are four kinds: + +| Kind | Anchor | Example | Visual form | +|------|--------|---------|-------------| +| `Region` | time span `[t_start, t_end]` | "velocity exceeds 1 rad/s here" | translucent vertical band | +| `Event` | single time `t` (+ optional value) | "`OK → ERROR` transition" | tick / point at `(t, value)` | +| `ValueBand` | value span `[y_low, y_high]` | "valid operating range" | translucent horizontal band | +| `Label` | a time `t` + text | free annotation | text callout | + +Every kind carries the same **semantic fields**: + +| Field | Meaning | +|-------|---------| +| `kind` | One of the four above. | +| `status` | `none` \| `pass` \| `fail` — the finding verdict. | +| `severity` | `info` \| `warning` \| `error` \| `critical` — drives default color. | +| `category` | Free string for the anomaly / annotation type (e.g. `"overspeed"`). | +| `label` | Short human-readable title (tooltip, panel, the `Label` kind's text). | +| `description` | Optional longer text. | +| `color` | Optional RGBA override; default derives from `severity`. | +| `metadata` | Key/value bag for producer-specific extras (e.g. `peak=1.83`, or the threshold that produced the finding). | +| anchor | The timestamps and/or value range appropriate to `kind`. | + +Three things a marker **does not** carry, and why (see the architecture doc for the +full reasoning): +- **no `id`** in the authored marker — identity is owned by the *store* and handed + back on `add` (consistent with every other builtin, none of which carry an id); +- **no `source`** — no builtin records its creator; provenance is the location, and + optional provenance goes in `metadata`; +- **no `scope`** — *where* a marker is addressed says it (see §3). + +> `ValueBand` is the one exception to §3: a y-range is in a specific series' units, so +> it is always **series-bound** and never global. + +## 3. Addressing model + +A marker lives under `(dataset, father-name)` — **exactly like a timeseries**. You +don't tag a marker with "where it belongs"; you *put it* where it belongs: + +- **Series marker** → addressed to that series' topic (e.g. `cmd_vel/x`). It renders + on every plot showing `cmd_vel/x`. +- **Global marker** → addressed to a dataset-level "global" topic. It renders on + every plot of that dataset whose visible time window overlaps the marker. + +So "is this marker global or scoped?" is answered by *which father-name you addressed +it to*, not by a field in the payload. + +## 4. Use cases + +- **UC-1 — Region from a threshold.** *"Highlight where velocity exceeds 1 rad/s."* + A toolbox plugin scans `joint_2/vel`, coalesces each above-threshold run into one + `Region` (`severity=warning`, `category="overspeed"`, `metadata.peak=1.83`), and + `add`s it to the `joint_2/vel` topic. The plot shows translucent shaded spans. + +- **UC-2 — Event markers from state transitions.** *"Mark every `OK → ERROR` + transition."* The plugin `add`s an `Event` at each transition time + (`status=fail`, `severity=error`) to the `/status` series. The plot shows ticks. + +- **UC-3 — ValueBand operating range.** A plugin `add`s a `ValueBand` for the nominal + range of `motor/temp` (series-bound); samples leaving the band become obvious. + +- **UC-4 — Agent creates and deletes markers.** An agent, driving the SDK API, calls + `add(dataset="Waymo", father="cmd_vel/x", marker=Region{1.0s..2.0s, severity=error, + label="discontinuity"})` and gets back a store-assigned id. Later it removes that + one marker with `delete(dataset="Waymo", father="cmd_vel/x", id)`. Adding more + markers to the same topic and deleting specific ones covers all editing needs — no + in-place "modify." + +- **UC-5 — Query by series (bidirectional read).** The agent (or a report tool) asks + *"give me the markers on `cmd_vel/x` in the Waymo dataset"* → + `query(dataset="Waymo", series="cmd_vel/x")` returns that topic's markers directly + (no scanning). Optional filters narrow by time range and/or `severity ≥ warning`. + +- **UC-6 — JSON report.** A run exports a report — `overall_status`, plus every + finding with its timestamps and severity. The data model supports this with no GUI; + a true headless CLI entry point is a separate, deferred effort (PJ4 is GUI-only + today), so v1 exposes export as a host action. + +- **UC-7 — GUI inspection.** A user sees markers colored by severity, hovers for a + tooltip with the semantic fields, and uses a **markers panel** to filter (by + severity / kind / series), jump to a marker's time, and toggle visibility. + +## 5. Illustrative JSON (a query result / report) + +The wire form is the codec-serialized marker list; this JSON is the *report view* of +the same data. Note there is no `source`/`scope` field, and `id` is the store handle +returned alongside each marker, not part of the authored marker. + +```json +{ + "report": { "overall_status": "fail", "dataset": "Waymo" }, + "markers": [ + { + "id": 1, + "kind": "region", + "series": "cmd_vel/x", + "t_start": 1.00, + "t_end": 2.00, + "status": "fail", + "severity": "error", + "category": "discontinuity", + "label": "cmd_vel/x discontinuity", + "metadata": { "jump": 3.4 } + }, + { + "id": 2, + "kind": "event", + "series": "/status", + "t": 19.05, + "status": "fail", + "severity": "error", + "category": "state_transition", + "label": "OK -> ERROR", + "metadata": { "from": "OK", "to": "ERROR" } + } + ] +} +``` + +`series` here reflects the *topic the marker was addressed to*; `"__global__"` (or +similar) marks a dataset-global marker. diff --git a/pj_base/CMakeLists.txt b/pj_base/CMakeLists.txt index d733fbe..addc8f9 100644 --- a/pj_base/CMakeLists.txt +++ b/pj_base/CMakeLists.txt @@ -14,6 +14,7 @@ add_library(pj_base STATIC src/builtin/mesh3d_codec.cpp src/builtin/occupancy_grid_codec.cpp src/builtin/occupancy_grid_update_codec.cpp + src/builtin/plot_markers_codec.cpp src/builtin/point_cloud_codec.cpp src/builtin/poses_in_frame_codec.cpp src/builtin/scene_entities_codec.cpp @@ -104,6 +105,7 @@ if(PJ_BUILD_TESTS) tests/asset_video_codec_test.cpp tests/time_spine_test.cpp tests/poses_in_frame_codec_test.cpp + tests/plot_markers_codec_test.cpp ) foreach(test_src ${PJ_BASE_TESTS}) diff --git a/pj_base/include/pj_base/builtin/builtin_object.hpp b/pj_base/include/pj_base/builtin/builtin_object.hpp index b08822b..746bd15 100644 --- a/pj_base/include/pj_base/builtin/builtin_object.hpp +++ b/pj_base/include/pj_base/builtin/builtin_object.hpp @@ -37,6 +37,7 @@ #include "pj_base/builtin/mesh3d.hpp" #include "pj_base/builtin/occupancy_grid.hpp" #include "pj_base/builtin/occupancy_grid_update.hpp" +#include "pj_base/builtin/plot_markers.hpp" #include "pj_base/builtin/point_cloud.hpp" #include "pj_base/builtin/poses_in_frame.hpp" #include "pj_base/builtin/robot_description.hpp" @@ -64,6 +65,7 @@ enum class BuiltinObjectType : uint16_t { kOccupancyGridUpdate = 15, ///< sdk::OccupancyGridUpdate — incremental sub-rectangle patch for an OccupancyGrid. kLog = 16, ///< sdk::Log — textual log message (level + text + name). kPosesInFrame = 17, ///< sdk::PosesInFrame — array of poses in one reference frame. + kPlotMarkers = 18, ///< sdk::PlotMarkers — findings on a time-series plot (regions, events, bands, labels). }; /// A-priori classification of a schema. Currently carries only the type; @@ -110,6 +112,8 @@ struct SchemaClassification { return "kLog"; case BuiltinObjectType::kPosesInFrame: return "kPosesInFrame"; + case BuiltinObjectType::kPlotMarkers: + return "kPlotMarkers"; } return "kNone"; } @@ -168,6 +172,9 @@ struct SchemaClassification { if (s == "kPosesInFrame") { return BuiltinObjectType::kPosesInFrame; } + if (s == "kPlotMarkers") { + return BuiltinObjectType::kPlotMarkers; + } return std::nullopt; } @@ -229,6 +236,9 @@ using BuiltinObject = std::any; if (t == typeid(PosesInFrame)) { return BuiltinObjectType::kPosesInFrame; } + if (t == typeid(PlotMarkers)) { + return BuiltinObjectType::kPlotMarkers; + } return BuiltinObjectType::kNone; } diff --git a/pj_base/include/pj_base/builtin/plot_markers.hpp b/pj_base/include/pj_base/builtin/plot_markers.hpp new file mode 100644 index 0000000..1b6981e --- /dev/null +++ b/pj_base/include/pj_base/builtin/plot_markers.hpp @@ -0,0 +1,106 @@ +/** + * @file plot_markers.hpp + * @brief Markers (findings) anchored to a time-series plot: shaded time + * regions, point events, value bands, and text labels. + * + * PlotMarkers is to a time-series plot what ImageAnnotations is to a video + * frame — a canonical SDK builtin object with a wire codec — but its shape is + * its own. Where ImageAnnotations groups heterogeneous drawing primitives + * (points/circles/texts), a marker is a *homogeneous record* distinguished by a + * `kind`, and a topic holds a flat list of them. + * + * A marker deliberately carries NO id (identity is owned by the host marker + * store and surfaced by the marker API, like every other builtin which carries + * none), NO source (no builtin records its creator — provenance is the + * dataset/topic the marker lives under, with optional extras in `metadata`), + * and NO scope (a marker's reach is decided by which topic it is addressed to: + * a series topic vs. a dataset-global topic). + * + * Like ImageAnnotations, marker data is small and owned outright via + * std::vector; eager ingestion is the natural default. + */ +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include +#include +#include + +#include "pj_base/builtin/image_annotations.hpp" // for ColorRGBA +#include "pj_base/types.hpp" + +namespace PJ { +namespace sdk { + +/// What a marker marks; selects which anchor fields are meaningful. +enum class MarkerKind : uint8_t { + kRegion, ///< time span [t_start, t_end] — shaded vertical band. + kEvent, ///< single time t_start (+ optional value) — tick / point. + kValueBand, ///< value span [value_low, value_high] — horizontal band (series-only). + kLabel, ///< text callout anchored at t_start. +}; + +/// The finding verdict carried by a marker. +enum class MarkerStatus : uint8_t { + kNone, ///< Not a pass/fail finding (a plain annotation). + kPass, + kFail, +}; + +/// Severity; drives the default color when `color` is unset (alpha 0). +enum class MarkerSeverity : uint8_t { + kInfo, + kWarning, + kError, + kCritical, +}; + +/// Free-form producer key/value metadata. The extension hatch that keeps the +/// schema stable as producers attach extra fields (threshold, peak, from/to, +/// ...) without a schema change. Serializes as a canonical PJ.KeyValuePair. +struct MarkerProperty { + std::string key; + std::string value; + bool operator==(const MarkerProperty&) const = default; +}; + +/// One marker: a homogeneous, identity-less record. Identity (the delete +/// handle) is owned by the host marker store, not this value. +struct PlotMarker { + MarkerKind kind = MarkerKind::kRegion; + + // --- anchor (interpret by kind; irrelevant fields ignored) --- + Timestamp t_start = 0; ///< Region start · Event/Label time · (ValueBand: ignored). + Timestamp t_end = 0; ///< Region end · (others: ignored). + double value_low = 0.0; ///< ValueBand low · Event point value · (others: ignored). + double value_high = 0.0; ///< ValueBand high · (others: ignored). + bool has_value = false; ///< Event: `value_low` is a meaningful point value. + + // --- semantics / presentation (shared by every kind) --- + MarkerStatus status = MarkerStatus::kNone; + MarkerSeverity severity = MarkerSeverity::kInfo; + std::string category; ///< e.g. "overspeed", "state_transition". + std::string label; ///< Short title (tooltip / panel / Label content). + std::string description; ///< Optional longer text. + ColorRGBA color = {0, 0, 0, 0}; ///< a=0 → derive color from `severity`. + std::vector metadata; + + bool operator==(const PlotMarker&) const = default; +}; + +/// The canonical object a marker topic holds and the codec (de)serializes: +/// the set of markers for one topic (one series, or the dataset-global topic). +struct PlotMarkers { + std::vector markers; + bool operator==(const PlotMarkers&) const = default; + + /// True if there are no markers. + [[nodiscard]] bool empty() const noexcept { + return markers.empty(); + } +}; + +} // namespace sdk +} // namespace PJ diff --git a/pj_base/include/pj_base/builtin/plot_markers_codec.hpp b/pj_base/include/pj_base/builtin/plot_markers_codec.hpp new file mode 100644 index 0000000..22d586c --- /dev/null +++ b/pj_base/include/pj_base/builtin/plot_markers_codec.hpp @@ -0,0 +1,26 @@ +#pragma once +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include +#include + +#include "pj_base/builtin/plot_markers.hpp" +#include "pj_base/expected.hpp" + +namespace PJ { + +/// Wire-format identifier for canonical plot markers. +inline constexpr std::string_view kSchemaPlotMarkers = "PJ.PlotMarkers"; + +/// Serializes a sdk::PlotMarkers to canonical PJ.PlotMarkers wire bytes. +[[nodiscard]] std::vector serializePlotMarkers(const sdk::PlotMarkers& markers); + +/// Decodes canonical PJ.PlotMarkers wire bytes into sdk::PlotMarkers. +/// +/// Returns an error for null, empty, truncated, or malformed payloads. +[[nodiscard]] Expected deserializePlotMarkers(const uint8_t* data, size_t size); + +} // namespace PJ diff --git a/pj_base/include/pj_base/builtin_object_abi.h b/pj_base/include/pj_base/builtin_object_abi.h index b9296ae..b29051c 100644 --- a/pj_base/include/pj_base/builtin_object_abi.h +++ b/pj_base/include/pj_base/builtin_object_abi.h @@ -57,6 +57,7 @@ typedef enum PJ_builtin_object_type_t { PJ_BUILTIN_OBJECT_TYPE_OCCUPANCY_GRID_UPDATE = 15, PJ_BUILTIN_OBJECT_TYPE_LOG = 16, PJ_BUILTIN_OBJECT_TYPE_POSES_IN_FRAME = 17, + PJ_BUILTIN_OBJECT_TYPE_PLOT_MARKERS = 18, /* Reserve future types; appended at the tail. Numeric values are stable * across releases — never renumber. Each new value here must match the * matching kFoo entry in BuiltinObjectType (builtin_object.hpp). */ diff --git a/pj_base/proto/pj/PlotMarkers.proto b/pj_base/proto/pj/PlotMarkers.proto new file mode 100644 index 0000000..b5cc3a2 --- /dev/null +++ b/pj_base/proto/pj/PlotMarkers.proto @@ -0,0 +1,66 @@ +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +// PlotJuggler canonical plot markers protobuf schema. +// +// Markers (findings) anchored to a time-series plot. Unlike ImageAnnotations, +// a marker is a homogeneous record distinguished by `kind`, and the container +// holds a flat list of them. A marker carries no id (identity is owned by the +// host marker store), no source, and no scope (its reach is decided by which +// topic it is addressed to). The C++ codec is hand-written against this layout; +// these field numbers and wire types are the source of truth. + +syntax = "proto3"; + +import "pj/Color.proto"; +import "pj/KeyValuePair.proto"; + +package PJ; + +// What a marker marks; selects which anchor fields are meaningful. +enum MarkerKind { + MARKER_KIND_REGION = 0; // time span [t_start, t_end] + MARKER_KIND_EVENT = 1; // single time t_start (+ optional value) + MARKER_KIND_VALUE_BAND = 2; // value span [value_low, value_high] (series-only) + MARKER_KIND_LABEL = 3; // text callout anchored at t_start +} + +// The finding verdict carried by a marker. +enum MarkerStatus { + MARKER_STATUS_NONE = 0; + MARKER_STATUS_PASS = 1; + MARKER_STATUS_FAIL = 2; +} + +// Severity; drives the default color when `color` is unset. +enum MarkerSeverity { + MARKER_SEVERITY_INFO = 0; + MARKER_SEVERITY_WARNING = 1; + MARKER_SEVERITY_ERROR = 2; + MARKER_SEVERITY_CRITICAL = 3; +} + +// One marker — a homogeneous, identity-less record. +message PlotMarker { + // Anchor (interpret by kind; irrelevant fields are ignored). + MarkerKind kind = 1; + int64 t_start = 2; // ns; Region start / Event / Label time + int64 t_end = 3; // ns; Region end + double value_low = 4; // ValueBand low / Event point value + double value_high = 5; // ValueBand high + bool has_value = 6; // Event: value_low is a meaningful point value + + // Semantics / presentation (shared by every kind). + MarkerStatus status = 7; + MarkerSeverity severity = 8; + string category = 9; + string label = 10; + string description = 11; + PJ.Color color = 12; // alpha 0 → derive from severity + repeated PJ.KeyValuePair metadata = 13; +} + +// The set of markers for one topic (one series, or the dataset-global topic). +message PlotMarkers { + repeated PJ.PlotMarker markers = 1; +} diff --git a/pj_base/src/builtin/plot_markers_codec.cpp b/pj_base/src/builtin/plot_markers_codec.cpp new file mode 100644 index 0000000..e361044 --- /dev/null +++ b/pj_base/src/builtin/plot_markers_codec.cpp @@ -0,0 +1,358 @@ +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#include "pj_base/builtin/plot_markers_codec.hpp" + +#include +#include +#include +#include +#include + +#include "protobuf_wire.hpp" + +namespace PJ { +namespace { + +using builtin_wire::Reader; +using builtin_wire::Tag; +using builtin_wire::WireType; +using builtin_wire::Writer; +using sdk::ColorRGBA; +using sdk::MarkerKind; +using sdk::MarkerProperty; +using sdk::MarkerSeverity; +using sdk::MarkerStatus; +using sdk::PlotMarker; +using sdk::PlotMarkers; + +// ---- enum mapping (write raw value; read with default fallback) ---- + +MarkerKind mapKind(uint64_t v) { + switch (v) { + case 0: + return MarkerKind::kRegion; + case 1: + return MarkerKind::kEvent; + case 2: + return MarkerKind::kValueBand; + case 3: + return MarkerKind::kLabel; + default: + return MarkerKind::kRegion; + } +} + +MarkerStatus mapStatus(uint64_t v) { + switch (v) { + case 1: + return MarkerStatus::kPass; + case 2: + return MarkerStatus::kFail; + case 0: + default: + return MarkerStatus::kNone; + } +} + +MarkerSeverity mapSeverity(uint64_t v) { + switch (v) { + case 1: + return MarkerSeverity::kWarning; + case 2: + return MarkerSeverity::kError; + case 3: + return MarkerSeverity::kCritical; + case 0: + default: + return MarkerSeverity::kInfo; + } +} + +uint8_t normalizedToByte(double value) { + value = std::clamp(value, 0.0, 1.0); + return static_cast(value * 255.0 + 0.5); +} + +// ---- writers ---- + +void writeColor(Writer& writer, const ColorRGBA& color) { + writer.doubleField(1, static_cast(color.r) / 255.0); + writer.doubleField(2, static_cast(color.g) / 255.0); + writer.doubleField(3, static_cast(color.b) / 255.0); + writer.doubleField(4, static_cast(color.a) / 255.0); +} + +void writeProperty(Writer& writer, const MarkerProperty& property) { + writer.string(1, property.key); + writer.string(2, property.value); +} + +void writeMarker(Writer& writer, const PlotMarker& marker) { + writer.varint(1, static_cast(marker.kind)); + writer.varint(2, static_cast(marker.t_start)); + writer.varint(3, static_cast(marker.t_end)); + writer.doubleField(4, marker.value_low); + writer.doubleField(5, marker.value_high); + writer.varint(6, marker.has_value ? 1u : 0u); + writer.varint(7, static_cast(marker.status)); + writer.varint(8, static_cast(marker.severity)); + writer.string(9, marker.category); + writer.string(10, marker.label); + writer.string(11, marker.description); + writer.message(12, [&](Writer& nested) { writeColor(nested, marker.color); }); + for (const auto& property : marker.metadata) { + writer.message(13, [&](Writer& nested) { writeProperty(nested, property); }); + } +} + +// ---- readers ---- + +bool decodeColor(Reader& reader, ColorRGBA& out) { + double r = 0.0; + double g = 0.0; + double b = 0.0; + double a = 0.0; + + while (!reader.eof()) { + Tag tag; + if (!reader.readTag(tag)) { + return false; + } + if (tag.type == WireType::kFixed64 && tag.field >= 1 && tag.field <= 4) { + double value = 0.0; + if (!reader.readDouble(value)) { + return false; + } + switch (tag.field) { + case 1: + r = value; + break; + case 2: + g = value; + break; + case 3: + b = value; + break; + case 4: + a = value; + break; + default: + break; + } + } else if (!reader.skip(tag.type)) { + return false; + } + } + + out = {normalizedToByte(r), normalizedToByte(g), normalizedToByte(b), normalizedToByte(a)}; + return true; +} + +bool decodeProperty(Reader& reader, MarkerProperty& out) { + while (!reader.eof()) { + Tag tag; + if (!reader.readTag(tag)) { + return false; + } + if (tag.field == 1 && tag.type == WireType::kLengthDelimited) { + if (!reader.readString(out.key)) { + return false; + } + } else if (tag.field == 2 && tag.type == WireType::kLengthDelimited) { + if (!reader.readString(out.value)) { + return false; + } + } else if (!reader.skip(tag.type)) { + return false; + } + } + return true; +} + +bool decodeMarker(Reader& reader, PlotMarker& out) { + while (!reader.eof()) { + Tag tag; + if (!reader.readTag(tag)) { + return false; + } + + switch (tag.field) { + case 1: + if (tag.type == WireType::kVarint) { + uint64_t v = 0; + if (!reader.readVarint(v)) { + return false; + } + out.kind = mapKind(v); + continue; + } + break; + case 2: + if (tag.type == WireType::kVarint) { + uint64_t v = 0; + if (!reader.readVarint(v)) { + return false; + } + out.t_start = static_cast(v); + continue; + } + break; + case 3: + if (tag.type == WireType::kVarint) { + uint64_t v = 0; + if (!reader.readVarint(v)) { + return false; + } + out.t_end = static_cast(v); + continue; + } + break; + case 4: + if (tag.type == WireType::kFixed64) { + if (!reader.readDouble(out.value_low)) { + return false; + } + continue; + } + break; + case 5: + if (tag.type == WireType::kFixed64) { + if (!reader.readDouble(out.value_high)) { + return false; + } + continue; + } + break; + case 6: + if (tag.type == WireType::kVarint) { + uint64_t v = 0; + if (!reader.readVarint(v)) { + return false; + } + out.has_value = (v != 0); + continue; + } + break; + case 7: + if (tag.type == WireType::kVarint) { + uint64_t v = 0; + if (!reader.readVarint(v)) { + return false; + } + out.status = mapStatus(v); + continue; + } + break; + case 8: + if (tag.type == WireType::kVarint) { + uint64_t v = 0; + if (!reader.readVarint(v)) { + return false; + } + out.severity = mapSeverity(v); + continue; + } + break; + case 9: + if (tag.type == WireType::kLengthDelimited) { + if (!reader.readString(out.category)) { + return false; + } + continue; + } + break; + case 10: + if (tag.type == WireType::kLengthDelimited) { + if (!reader.readString(out.label)) { + return false; + } + continue; + } + break; + case 11: + if (tag.type == WireType::kLengthDelimited) { + if (!reader.readString(out.description)) { + return false; + } + continue; + } + break; + case 12: + if (tag.type == WireType::kLengthDelimited) { + Reader nested; + if (!reader.readMessage(nested) || !decodeColor(nested, out.color)) { + return false; + } + continue; + } + break; + case 13: { + if (tag.type != WireType::kLengthDelimited) { + break; + } + Reader nested; + MarkerProperty property; + if (!reader.readMessage(nested) || !decodeProperty(nested, property)) { + return false; + } + out.metadata.push_back(std::move(property)); + continue; + } + default: + break; + } + + if (!reader.skip(tag.type)) { + return false; + } + } + return true; +} + +} // namespace + +std::vector serializePlotMarkers(const sdk::PlotMarkers& markers) { + std::vector out; + Writer writer(out); + + for (const auto& marker : markers.markers) { + writer.message(1, [&](Writer& nested) { writeMarker(nested, marker); }); + } + + return out; +} + +Expected deserializePlotMarkers(const uint8_t* data, size_t size) { + if (data == nullptr || size == 0) { + return unexpected(std::string("PlotMarkers wire: empty buffer")); + } + + Reader reader(data, size); + sdk::PlotMarkers markers; + + while (!reader.eof()) { + Tag tag; + if (!reader.readTag(tag)) { + return unexpected(std::string("PlotMarkers wire: bad tag")); + } + + if (tag.field == 1 && tag.type == WireType::kLengthDelimited) { + Reader nested; + if (!reader.readMessage(nested)) { + return unexpected(std::string("PlotMarkers wire: bad nested message length")); + } + PlotMarker marker; + if (!decodeMarker(nested, marker)) { + return unexpected(std::string("PlotMarkers wire: PlotMarker decode failed")); + } + markers.markers.push_back(std::move(marker)); + } else if (!reader.skip(tag.type)) { + return unexpected(std::string("PlotMarkers wire: skip failed")); + } + } + + return markers; +} + +} // namespace PJ diff --git a/pj_base/tests/abi_layout_sentinels_test.cpp b/pj_base/tests/abi_layout_sentinels_test.cpp index c44353a..c2d62f3 100644 --- a/pj_base/tests/abi_layout_sentinels_test.cpp +++ b/pj_base/tests/abi_layout_sentinels_test.cpp @@ -122,6 +122,7 @@ static_assert(PJ_BUILTIN_OBJECT_TYPE_CAMERA_INFO == 14, "CameraInfo type id pinn static_assert(PJ_BUILTIN_OBJECT_TYPE_OCCUPANCY_GRID_UPDATE == 15, "OccupancyGridUpdate type id pinned"); static_assert(PJ_BUILTIN_OBJECT_TYPE_LOG == 16, "Log type id pinned"); static_assert(PJ_BUILTIN_OBJECT_TYPE_POSES_IN_FRAME == 17, "PosesInFrame type id pinned"); +static_assert(PJ_BUILTIN_OBJECT_TYPE_PLOT_MARKERS == 18, "PlotMarkers type id pinned"); static_assert(sizeof(PJ_schema_classification_t) == 4, "PJ_schema_classification_t layout pinned"); static_assert(offsetof(PJ_schema_classification_t, object_type) == 0, "object_type at offset 0"); static_assert(offsetof(PJ_schema_classification_t, reserved) == 2, "reserved at offset 2"); diff --git a/pj_base/tests/builtin_object_test.cpp b/pj_base/tests/builtin_object_test.cpp index 916dc61..789d28e 100644 --- a/pj_base/tests/builtin_object_test.cpp +++ b/pj_base/tests/builtin_object_test.cpp @@ -20,6 +20,7 @@ using PJ::sdk::name; using PJ::sdk::OccupancyGrid; using PJ::sdk::OccupancyGridUpdate; using PJ::sdk::parseBuiltinObjectType; +using PJ::sdk::PlotMarkers; using PJ::sdk::PointCloud; using PJ::sdk::PosesInFrame; using PJ::sdk::RobotDescription; @@ -45,6 +46,7 @@ TEST(BuiltinObjectTest, TypeOfRecognizesKnownBuiltinTypes) { EXPECT_EQ(typeOf(BuiltinObject{OccupancyGridUpdate{}}), BuiltinObjectType::kOccupancyGridUpdate); EXPECT_EQ(typeOf(BuiltinObject{Log{}}), BuiltinObjectType::kLog); EXPECT_EQ(typeOf(BuiltinObject{PosesInFrame{}}), BuiltinObjectType::kPosesInFrame); + EXPECT_EQ(typeOf(BuiltinObject{PlotMarkers{}}), BuiltinObjectType::kPlotMarkers); } TEST(BuiltinObjectTest, NameAndParseRoundTripForEveryEnumEntry) { @@ -66,6 +68,7 @@ TEST(BuiltinObjectTest, NameAndParseRoundTripForEveryEnumEntry) { BuiltinObjectType::kOccupancyGridUpdate, BuiltinObjectType::kLog, BuiltinObjectType::kPosesInFrame, + BuiltinObjectType::kPlotMarkers, }) { const auto parsed = parseBuiltinObjectType(name(t)); ASSERT_TRUE(parsed.has_value()) << "parseBuiltinObjectType failed for " << name(t); diff --git a/pj_base/tests/plot_markers_codec_test.cpp b/pj_base/tests/plot_markers_codec_test.cpp new file mode 100644 index 0000000..dde898a --- /dev/null +++ b/pj_base/tests/plot_markers_codec_test.cpp @@ -0,0 +1,150 @@ +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#include "pj_base/builtin/plot_markers_codec.hpp" + +#include + +#include +#include + +namespace PJ { +namespace { + +using sdk::ColorRGBA; +using sdk::MarkerKind; +using sdk::MarkerProperty; +using sdk::MarkerSeverity; +using sdk::MarkerStatus; +using sdk::PlotMarker; +using sdk::PlotMarkers; + +// Decode the bytes produced by serializePlotMarkers back into a sdk::PlotMarkers. +sdk::PlotMarkers roundTrip(const sdk::PlotMarkers& input) { + auto bytes = serializePlotMarkers(input); + auto result = deserializePlotMarkers(bytes.data(), bytes.size()); + EXPECT_TRUE(result.has_value()); + return result.has_value() ? *result : sdk::PlotMarkers{}; +} + +// Compare two ColorRGBA values allowing 1-LSB drift per channel from the +// uint8 -> double-in-[0,1] -> uint8 quantization round-trip. +::testing::AssertionResult ColorEq(const ColorRGBA& a, const ColorRGBA& b) { + auto near = [](uint8_t x, uint8_t y) { return x > y ? (x - y) <= 1 : (y - x) <= 1; }; + if (near(a.r, b.r) && near(a.g, b.g) && near(a.b, b.b) && near(a.a, b.a)) { + return ::testing::AssertionSuccess(); + } + return ::testing::AssertionFailure() << "Color mismatch"; +} + +// ----------------------------------------------------------------------------- +// Empty handling +// ----------------------------------------------------------------------------- + +TEST(PlotMarkersCodecTest, EmptySetProducesEmptyBytes) { + PlotMarkers markers; + EXPECT_TRUE(serializePlotMarkers(markers).empty()); +} + +TEST(PlotMarkersCodecTest, NullOrEmptyBufferIsError) { + EXPECT_FALSE(deserializePlotMarkers(nullptr, 0).has_value()); + const uint8_t byte = 0; + EXPECT_FALSE(deserializePlotMarkers(&byte, 0).has_value()); +} + +// ----------------------------------------------------------------------------- +// Round-trip per kind (default color a=0 round-trips exactly) +// ----------------------------------------------------------------------------- + +TEST(PlotMarkersCodecTest, RegionRoundTrip) { + PlotMarker m; + m.kind = MarkerKind::kRegion; + m.t_start = 1'000'000'000; // 1s in ns + m.t_end = 2'000'000'000; + m.status = MarkerStatus::kFail; + m.severity = MarkerSeverity::kError; + m.category = "overspeed"; + m.label = "joint_2 velocity > 1 rad/s"; + m.description = "sustained above threshold"; + m.metadata = {{"peak", "1.83"}}; + + PlotMarkers in; + in.markers = {m}; + EXPECT_TRUE(roundTrip(in) == in); +} + +TEST(PlotMarkersCodecTest, EventRoundTrip) { + PlotMarker m; + m.kind = MarkerKind::kEvent; + m.t_start = 1'905'100'000; + m.value_low = 42.5; + m.has_value = true; + m.status = MarkerStatus::kFail; + m.severity = MarkerSeverity::kCritical; + m.category = "state_transition"; + m.label = "OK -> ERROR"; + m.metadata = {{"from", "OK"}, {"to", "ERROR"}}; + + PlotMarkers in; + in.markers = {m}; + EXPECT_TRUE(roundTrip(in) == in); +} + +TEST(PlotMarkersCodecTest, ValueBandRoundTrip) { + PlotMarker m; + m.kind = MarkerKind::kValueBand; + m.value_low = -0.5; + m.value_high = 0.5; + m.severity = MarkerSeverity::kInfo; + m.label = "operating range"; + + PlotMarkers in; + in.markers = {m}; + EXPECT_TRUE(roundTrip(in) == in); +} + +TEST(PlotMarkersCodecTest, LabelRoundTrip) { + PlotMarker m; + m.kind = MarkerKind::kLabel; + m.t_start = 500'000'000; + m.label = "sensor recalibrated"; + + PlotMarkers in; + in.markers = {m}; + EXPECT_TRUE(roundTrip(in) == in); +} + +TEST(PlotMarkersCodecTest, MultipleMarkersPreserveOrder) { + PlotMarker a; + a.kind = MarkerKind::kRegion; + a.t_start = 10; + a.t_end = 20; + a.label = "a"; + PlotMarker b; + b.kind = MarkerKind::kEvent; + b.t_start = 30; + b.label = "b"; + + PlotMarkers in; + in.markers = {a, b}; + auto out = roundTrip(in); + ASSERT_EQ(out.markers.size(), 2u); + EXPECT_TRUE(out == in); +} + +TEST(PlotMarkersCodecTest, ColorRoundTripWithinOneLsb) { + PlotMarker m; + m.kind = MarkerKind::kRegion; + m.t_start = 0; + m.t_end = 100; + m.color = {255, 128, 0, 200}; + + PlotMarkers in; + in.markers = {m}; + auto out = roundTrip(in); + ASSERT_EQ(out.markers.size(), 1u); + EXPECT_TRUE(ColorEq(m.color, out.markers[0].color)); +} + +} // namespace +} // namespace PJ From e01363c75647d0a12f7a347248b152fcadfcbccc Mon Sep 17 00:00:00 2001 From: alvvm Date: Tue, 16 Jun 2026 14:25:44 +0200 Subject: [PATCH 02/11] Add pj.marker_store.v1 C-ABI service (create/delete/query markers) The plugin-facing contract for plot markers, mirroring the object read/write host pattern. - C vtable PJ_marker_store_host_vtable_t + fat pointer + an owning PJ_marker_query_handle_t (plugin_data_api.h): add / remove / query / query_bytes / release_query. Markers cross as serialized PJ.PlotMarkers bytes; ids cross as uint64. - C++ MarkerStoreHostView (plugin_data_api.hpp) wrapping the codec so callers work in typed sdk::PlotMarker; add(single|batch) -> ids, remove(id), query -> [{id, marker}]. + MarkerEntry. - MarkerStoreService trait (service_traits.hpp), "pj.marker_store.v1". ABI is additive (new service); abi/baseline.abi refresh deferred to the release-cut step. Co-Authored-By: Claude Opus 4.8 (1M context) --- pj_base/include/pj_base/plugin_data_api.h | 61 ++++++++++ .../include/pj_base/sdk/plugin_data_api.hpp | 111 ++++++++++++++++++ .../include/pj_base/sdk/service_traits.hpp | 12 ++ 3 files changed, 184 insertions(+) diff --git a/pj_base/include/pj_base/plugin_data_api.h b/pj_base/include/pj_base/plugin_data_api.h index d48648d..e3bfe88 100644 --- a/pj_base/include/pj_base/plugin_data_api.h +++ b/pj_base/include/pj_base/plugin_data_api.h @@ -729,6 +729,67 @@ typedef struct { const PJ_object_read_host_vtable_t* vtable; } PJ_object_read_host_t; +/* ========================================================================== + * Marker-store host ("pj.marker_store.v1", protocol v4) + * + * Exposed to Toolbox plugins (and, later, the scripting layer) to create, + * delete, and query plot markers. Unlike the object store — a timestamped blob + * log that can only front-evict — markers are a mutable, id-addressed set per + * (dataset, topic): add() mints a stable id, remove() deletes by it. Markers + * cross the ABI as serialized PJ.PlotMarkers bytes (the canonical codec); ids + * cross as uint64. A marker's reach is decided by which topic it is addressed + * to (a series topic vs. a reserved dataset-global topic name). + * ========================================================================== */ + +/* Opaque OWNING handle for a query result (serialized markers + parallel ids). + * Valid until release_query(handle); never dereferenced by the plugin. */ +struct PJ_marker_query_handle_s; +typedef struct PJ_marker_query_handle_s* PJ_marker_query_handle_t; + +/* ABI-APPENDABLE: new slots may be added at the tail; struct_size gates read. */ +typedef struct PJ_marker_store_host_vtable_t { + uint32_t abi_version; + uint32_t struct_size; + + /* [main-thread] Add markers (serialized PJ.PlotMarkers) under (dataset, + * topic). The store mints one stable id per marker, in payload order, and + * writes them to out_ids (capacity out_ids_capacity); *out_count is always + * set to the TOTAL number of markers in the payload (so the caller can detect + * truncation). Returns false (out_error set) on a malformed payload. */ + bool (*add)( + void* ctx, uint32_t dataset_id, PJ_string_view_t topic, const uint8_t* markers_data, uint64_t markers_size, + uint64_t* out_ids, uint64_t out_ids_capacity, uint64_t* out_count, PJ_error_t* out_error) PJ_NOEXCEPT; + + /* [main-thread] Remove a marker by id from (dataset, topic). Returns false + * (out_error set) if the topic or id does not exist. */ + bool (*remove)( + void* ctx, uint32_t dataset_id, PJ_string_view_t topic, uint64_t marker_id, PJ_error_t* out_error) PJ_NOEXCEPT; + + /* [main-thread] Query all markers under (dataset, topic). On success + * allocates an owning handle carrying the serialized PJ.PlotMarkers bytes and + * the parallel id array; the caller releases it via release_query. An empty + * topic yields a valid handle whose marker/id count is zero. */ + bool (*query)( + void* ctx, uint32_t dataset_id, PJ_string_view_t topic, PJ_marker_query_handle_t* out_handle, + PJ_error_t* out_error) PJ_NOEXCEPT; + + /* [thread-safe] Expose the query result behind an owning handle: serialized + * PJ.PlotMarkers bytes plus the parallel id array (out_count ids, matching + * the markers in the payload, in order). Valid until release_query(handle). */ + void (*query_bytes)( + PJ_marker_query_handle_t handle, const uint8_t** out_data, uint64_t* out_size, const uint64_t** out_ids, + uint64_t* out_count) PJ_NOEXCEPT; + + /* [thread-safe] Release a query handle. Idempotent on NULL. */ + void (*release_query)(PJ_marker_query_handle_t handle) PJ_NOEXCEPT; +} PJ_marker_store_host_vtable_t; + +/* ABI-FROZEN: fat pointer layout permanent. */ +typedef struct { + void* ctx; + const PJ_marker_store_host_vtable_t* vtable; +} PJ_marker_store_host_t; + /** * Colormap registry service ("pj.colormap.v1", protocol_version 1). * diff --git a/pj_base/include/pj_base/sdk/plugin_data_api.hpp b/pj_base/include/pj_base/sdk/plugin_data_api.hpp index 9ac8844..ba206bc 100644 --- a/pj_base/include/pj_base/sdk/plugin_data_api.hpp +++ b/pj_base/include/pj_base/sdk/plugin_data_api.hpp @@ -16,6 +16,7 @@ #include #include "pj_base/builtin/builtin_object.hpp" +#include "pj_base/builtin/plot_markers_codec.hpp" #include "pj_base/expected.hpp" #include "pj_base/number_parse.hpp" #include "pj_base/plugin_data_api.h" @@ -683,6 +684,116 @@ class ToolboxObjectReadHostView { PJ_object_read_host_t host_{}; }; +/// A marker plus its store-assigned id, as returned by MarkerStoreHostView::query. +struct MarkerEntry { + uint64_t id = 0; + PlotMarker marker; +}; + +// --------------------------------------------------------------------------- +// Marker store host view ("pj.marker_store.v1") +// +// Create / delete / query plot markers on a (dataset, topic). Unlike the object +// store, the marker set is mutable and id-addressed: add() mints a stable id, +// remove() deletes by it. Markers cross the ABI as serialized PJ.PlotMarkers +// bytes; this view wraps the codec so callers work in typed sdk::PlotMarker. +// --------------------------------------------------------------------------- +class MarkerStoreHostView { + public: + MarkerStoreHostView() = default; + explicit MarkerStoreHostView(PJ_marker_store_host_t host) : host_(host) {} + + [[nodiscard]] bool valid() const { + return host_.ctx != nullptr && host_.vtable != nullptr; + } + + /// Add a batch of markers to (dataset, topic); returns their new ids in order. + [[nodiscard]] Expected> add( + DatasetId dataset, std::string_view topic, const PlotMarkers& markers) const { + if (!valid() || host_.vtable->add == nullptr) { + return unexpected("marker store host is not bound"); + } + const std::vector bytes = PJ::serializePlotMarkers(markers); + std::vector ids(markers.markers.size()); + uint64_t count = 0; + PJ_error_t err{}; + if (!host_.vtable->add( + host_.ctx, dataset, toAbiString(topic), bytes.data(), bytes.size(), ids.data(), ids.size(), &count, + &err)) { + return unexpected(errorToString(err)); + } + ids.resize(count < ids.size() ? count : ids.size()); + return ids; + } + + /// Add a single marker; returns its new id. + [[nodiscard]] Expected add(DatasetId dataset, std::string_view topic, const PlotMarker& marker) const { + PlotMarkers one; + one.markers.push_back(marker); + auto ids = add(dataset, topic, one); + if (!ids) { + return unexpected(ids.error()); + } + if (ids->empty()) { + return unexpected("marker store add returned no id"); + } + return ids->front(); + } + + /// Remove a marker by id from (dataset, topic). + [[nodiscard]] Status remove(DatasetId dataset, std::string_view topic, uint64_t id) const { + if (!valid() || host_.vtable->remove == nullptr) { + return unexpected("marker store host is not bound"); + } + PJ_error_t err{}; + if (!host_.vtable->remove(host_.ctx, dataset, toAbiString(topic), id, &err)) { + return unexpected(errorToString(err)); + } + return okStatus(); + } + + /// Query all markers under (dataset, topic), each paired with its id. + [[nodiscard]] Expected> query(DatasetId dataset, std::string_view topic) const { + if (!valid() || host_.vtable->query == nullptr) { + return unexpected("marker store host is not bound"); + } + PJ_marker_query_handle_t handle = nullptr; + PJ_error_t err{}; + if (!host_.vtable->query(host_.ctx, dataset, toAbiString(topic), &handle, &err)) { + return unexpected(errorToString(err)); + } + const uint8_t* data = nullptr; + uint64_t size = 0; + const uint64_t* ids = nullptr; + uint64_t count = 0; + host_.vtable->query_bytes(handle, &data, &size, &ids, &count); + + std::vector out; + Expected decoded = + (size == 0) ? Expected(PlotMarkers{}) : PJ::deserializePlotMarkers(data, size); + if (decoded.has_value()) { + const auto& markers = decoded->markers; + const uint64_t n = count < markers.size() ? count : markers.size(); + out.reserve(n); + for (uint64_t i = 0; i < n; ++i) { + out.push_back(MarkerEntry{ids[i], markers[i]}); + } + } + host_.vtable->release_query(handle); + if (!decoded.has_value()) { + return unexpected(decoded.error()); + } + return out; + } + + [[nodiscard]] const PJ_marker_store_host_t& raw() const noexcept { + return host_; + } + + private: + PJ_marker_store_host_t host_{}; +}; + // --------------------------------------------------------------------------- // Object write host view (protocol v4) // diff --git a/pj_base/include/pj_base/sdk/service_traits.hpp b/pj_base/include/pj_base/sdk/service_traits.hpp index fffe99c..0d45212 100644 --- a/pj_base/include/pj_base/sdk/service_traits.hpp +++ b/pj_base/include/pj_base/sdk/service_traits.hpp @@ -134,6 +134,18 @@ struct ToolboxObjectReadHostService { static_assert(detail::isValidServiceName(kName), "kName must match the pj naming rule"); }; +/// Marker store host — create / delete / query plot markers. Resolved by +/// analysis/scripting toolboxes (and, later, the headless runner's script host) +/// that emit findings as markers. The marker set is mutable and id-addressed. +struct MarkerStoreService { + static constexpr const char* kName = "pj.marker_store.v1"; + static constexpr uint32_t kMinVersion = 1; + using Raw = PJ_marker_store_host_t; + using Vtable = PJ_marker_store_host_vtable_t; + using View = MarkerStoreHostView; + static_assert(detail::isValidServiceName(kName), "kName must match the pj naming rule"); +}; + struct ToolboxHostService { // "pj.toolbox_write.v1" for symmetry with "pj.source_write.v1" and // "pj.parser_write.v1" — this service IS the toolbox write surface From 4cb7eef03c1ba7ec03521c5b1831a48dec7ca55a Mon Sep 17 00:00:00 2001 From: alvvm Date: Wed, 17 Jun 2026 13:56:53 +0200 Subject: [PATCH 03/11] markers: drop pj.marker_store.v1 service; add object-on-dataset toolbox slot Markers become a builtin object published to the ObjectStore via the generic object-write surface; the producer republishes the whole PlotMarkers set (last-writer-publish). The dedicated pj.marker_store.v1 service (vtable, MarkerStoreHostView, service trait) is removed. Add a tail-appended, idempotent toolbox-host slot register_object_topic_on_dataset(DatasetId, ...) so a toolbox can annotate an existing dataset (the object-write path was previously source-scoped). Add marker object-topic naming helpers (markerObjectTopicName / kGlobalMarkerTopic) to plot_markers.hpp. Update the ABI layout sentinel (88->96) and the two mock vtables. The PlotMarkers type + codec are unchanged. Docs (plot_markers_*) updated to the ObjectStore + republish model. MINOR bump + abi/baseline.abi refresh deferred to release. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/plot_markers_architecture.md | 175 ++++++++++-------- docs/plot_markers_format.md | 8 +- docs/plot_markers_use_cases.md | 23 +-- .../include/pj_base/builtin/plot_markers.hpp | 22 +++ pj_base/include/pj_base/plugin_data_api.h | 74 ++------ .../include/pj_base/sdk/plugin_data_api.hpp | 135 +++----------- .../include/pj_base/sdk/service_traits.hpp | 12 -- pj_base/tests/abi_layout_sentinels_test.cpp | 5 +- .../pj_plugins/testing/toolbox_test_store.hpp | 1 + pj_plugins/tests/toolbox_plugin_test.cpp | 4 +- 10 files changed, 178 insertions(+), 281 deletions(-) diff --git a/docs/plot_markers_architecture.md b/docs/plot_markers_architecture.md index a096d1b..18cc905 100644 --- a/docs/plot_markers_architecture.md +++ b/docs/plot_markers_architecture.md @@ -1,35 +1,45 @@ # Plot Markers — Architecture -> **Status: design draft.** The *what/why* and examples live in -> [plot_markers_use_cases.md](plot_markers_use_cases.md). This document is the -> *how*: the type, the store, the API surface, and how PJ4 renders markers. Names -> and signatures are illustrative until frozen. +> **Status: REVISED (was a dedicated-store draft).** Markers are now a **builtin +> object** stored in the **ObjectStore** — there is NO dedicated marker store and NO +> `pj.marker_store.v1` service. The *what/why* and examples live in +> [plot_markers_use_cases.md](plot_markers_use_cases.md) (parts may still describe the +> old model — being updated). The full reasoning for the pivot is in the session plan +> `atomic-foraging-kettle.md`. > > Plot Markers reuse the **concept** of [`ImageAnnotations`](image_annotations_format.md) -> (a canonical SDK builtin object with a wire codec) but not its structure. +> (a canonical SDK builtin object with a wire codec) but not its structure — and, like +> every builtin object, they ride the generic ObjectStore pipeline. ## 1. Layering & data flow -Three layers, with a clean boundary at the SDK C-ABI: +A marker set is a **builtin object** (`PlotMarkers` = a list of `PlotMarker`), one per +`(dataset, marker-topic)`, stored in the host **ObjectStore**. The **producer owns its +set and republishes the whole blob** on any change (last-writer-publish); the store is +never mutated marker-by-marker, so no per-id delete / id-in-payload / RMW race is needed. ``` - Producer (toolbox plugin / future agent / ingestion parser) - │ add(dataset, father, marker) → id - │ delete(dataset, father, id) - │ query(dataset, [series], [filters]) → markers - ▼ ── SDK C-ABI (marker host-view service) ── - Host marker store ← owns identity (the marker id) and the mutable set + Producer (toolbox plugin / future Lua / in-process host) + │ build PlotMarkers set → serialize (PlotMarkers codec) + │ register object topic "__markers__/" on the dataset + │ push the whole serialized set (republish) + ▼ ── generic object-write surface (no marker-specific service) ── + Host ObjectStore ← holds the latest serialized PlotMarkers blob per topic ▼ - PJ4 consumers: pj_plotting Qwt overlay · markers panel · JSON export + PJ4 consumers: pj_plotting Qwt overlay (latestAt + deserialize) · JSON export ``` -- **SDK (`plotjuggler_sdk`)** owns the **type + codec** and the **marker host-view - service contract** (the C-ABI a producer speaks). This is the only code a plugin or - agent compiles against — which is exactly why the typed contract must live here. -- **Host (`PJ4`)** owns the **marker store** (the identity owner) behind that - contract, plus rendering, the panel, and export. -- Producers and the agent **never** see the store — only the API. The store is an - implementation detail and can change without touching a single producer. +- **SDK (`plotjuggler_sdk`)** owns only the **type + codec** (`PlotMarkers`) plus the + marker object-topic naming convention (`markerObjectTopicName`, `kGlobalMarkerTopic` + in `pj_base/builtin/plot_markers.hpp`). Producers write via the **generic + object-write surface** (`registerObjectTopic*` + `pushOwnedObject`) — the same one + images/point clouds use. Annotating an *existing* dataset uses + `registerObjectTopicOnDataset(DatasetId, …)` (idempotent: re-resolves the topic). +- **Host (`PJ4`)** owns the **ObjectStore** (which already holds all object media), + rendering, and export. No marker-specific store or service. +- Producers never address individual markers — they publish a set. Identity, if ever + needed (acks / cross-run correlation), would be layered on top without changing the + SDK type or the object pipeline. ## 2. The `PlotMarker` / `PlotMarkers` type @@ -111,76 +121,76 @@ Design notes: `abi/baseline.abi`) + a `plot_markers_codec` mirroring the `image_annotations_codec` *pattern*. -## 3. Why a dedicated marker store (not ObjectStore, not DataEngine) - -The feature's core operation is **delete one specific marker**. That single -requirement decides the storage: - -- **`DataEngine` (columnar) — no.** It stores scalar samples as numeric columns. A - marker is a heterogeneous record with a *span* `[t1,t2]`, an enum severity, strings. - Forcing it into columns and inventing a "sample timestamp" for a span is a deep - impedance mismatch. -- **`ObjectStore` — no (for the real design).** It is a timestamped blob log whose - only removal verbs are **front-eviction** (`evictBefore`, `removeTopic`, `clear`): - it *structurally cannot delete entry #42*. To get per-marker delete on top of it you - must store the whole set as **one snapshot blob** and rewrite it on every edit — and - smuggle an `id` *into* the canonical type that no other builtin has. -- **Dedicated marker store — yes.** One marker per entry, real per-id add/delete, and - identity owned by the store (a handle like `ObjectStore`'s native `SequentialUID`). - This keeps `PlotMarker` clean and consistent with all 16 other builtins, and the - `id` lives *outside* the payload, surfaced by the API. - -The `id` you asked about earlier is therefore a **store handle**, not a type field. - -> **Fallback.** If shipping speed ever outweighs cleanliness, back the same API with -> an `ObjectStore` snapshot (one blob per topic, `id` carried inside the blob, -> rewrite-on-edit). The producer-facing API is identical either way — this is purely -> a host-internal choice. - -## 4. The marker host-view service (SDK C-ABI) - -Store-agnostic, symmetric (create + query), addressed by data identity: +## 3. Why ObjectStore + republish (not a dedicated store, not DataEngine) + +Markers were originally planned in a dedicated mutable store keyed on "delete one +specific marker". With the in-process `pj_scripting` Lua engine becoming the primary +producer, the model flipped to **producer-owns-the-set + republish**, which removes the +need for store-side per-marker mutation — and that makes the existing **ObjectStore** +the right home (markers become just another builtin object). + +- **`DataEngine` (columnar) — no.** Strictly numeric columns (`PrimitiveType`), + append-only immutable chunks. A marker is a structured record over a span; it fits + neither the type nor the mutability model. +- **`ObjectStore` — yes.** A marker *set* for a topic is one serialized `PlotMarkers` + blob = one object entry. The producer republishes the whole blob on every change, so + ObjectStore's append + latest-at semantics suffice — no per-entry delete needed. + Editing one marker = the producer mutates its in-memory set and re-pushes; there is + no read-modify-write against the store and no concurrency race for a single owner. + Set a keep-latest retention budget so superseded snapshots don't accumulate; the + overlay reads `latestAt(MAX)` so markers show regardless of the playback cursor. +- **No dedicated store, no id in the payload.** `PlotMarker` stays id-less like every + other builtin; identity (if ever needed for acks / cross-run correlation) is layered + on top, not baked into the type. + +## 4. The object-write surface (SDK C-ABI) + +Markers reuse the **generic object pipeline** — there is no marker-specific service. +A producer builds its set, serializes it, registers the object topic, and pushes: ```text -add (dataset, father_name, PlotMarker) -> MarkerId -delete(dataset, father_name, MarkerId) -> Status -query (dataset, father_name, [time_range], [min_severity], …) -> {MarkerId, PlotMarker}[] +register object topic markerObjectTopicName(topic) on the dataset +push the whole serialized PlotMarkers set (republish, last-writer wins) ``` -- `father_name` is the series topic (e.g. `cmd_vel/x`) or the dataset-level global - topic. There is no panel/GUI addressing. -- **Per-series query is direct** — read the markers under that topic; no scan. -- Cross-series aggregation ("all `warning+` markers in the dataset") enumerates the - dataset's marker topics (a known limitation, acceptable at marker scale). -- This is the contract the **AI agent** drives and that **plugins** call; both are - data-domain operations, which is why they belong on the SDK ABI (and "create a - plot / edit layout" deliberately does **not** — that is app-control, not data). +- The marker topic is a series field path (e.g. `cmd_vel/x`) or `kGlobalMarkerTopic`; + the object topic name is `markerObjectTopicName(topic)` = `"__markers__/" + topic` + (a reserved namespace so it never collides with media object topics). +- **Producers creating their own dataset** use `registerObjectTopic(source, …)`. + **Producers annotating an existing dataset** (a compiled toolbox over loaded data) + use `registerObjectTopicOnDataset(DatasetId, …)` — a tail-appended, **idempotent** + toolbox-host slot that re-resolves the topic on each republish. In-process producers + (host / future Lua) can equally call `ObjectStore` directly. +- **Per-series read is direct** (one object topic per series). Cross-series aggregation + ("all `warning+` markers in the dataset") enumerates the dataset's marker topics + (a known limitation, acceptable at marker scale). ## 5. Producers -All produce the **same** `PlotMarker` type; they differ only in where the markers -come from: -- **Toolbox plugin (today).** Analysis over already-loaded series → `add`. The first - production user of the marker write path (analogous to the existing object-write - path that no toolbox uses yet). -- **AI agent (future).** Drives the same `add`/`delete`/`query` API to manage markers - programmatically. -- **Ingestion parser (future).** Markers that already exist in a recording (an event - channel, annotation messages) decode into `PlotMarker`s — the same role an image - parser plays for `ImageAnnotations`. +All produce the **same** `PlotMarkers` set and publish it the same way; they differ +only in where the markers come from: +- **In-process host (today).** The Help → Add Demo Markers action builds a set and + publishes it via `ObjectStore` directly — the simplest exercise of the path. +- **Toolbox plugin (today).** Analysis over already-loaded series → republish its set + via `registerObjectTopicOnDataset` + `pushOwnedObject`. +- **Lua scripting / AI agent (future, `pj_scripting`).** The primary producer: an + in-process script regenerates and republishes its set; the agent most naturally emits + Lua. No new ABI — it rides the same path. +- **Ingestion parser (future).** Markers already present in a recording decode into + `PlotMarkers` — the same role an image parser plays for `ImageAnnotations`. ## 6. Rendering (PJ4 host) - A **`PlotMarkersItem : QwtPlotItem`** overlay in `pj_plotting/widget/`, modeled on - `CurveTracker` (the existing custom Qwt item). Its `draw(painter, xMap, yMap, rect)` - maps marker times→pixels via `canvasMap()` and paints: `Region`/`ValueBand` as - translucent fills, `Event` as ticks/points, `Label` as text. + `CurveTracker`. Its `draw(painter, xMap, yMap, rect)` reads the marker set for each + target `(dataset, topic)` from `ObjectStore::latestAt` + `deserializePlotMarkers`, + maps marker times→pixels, and paints: `Region`/`ValueBand` as translucent fills, + `Event` as ticks/points, `Label` as text. - **Render rule:** a series marker draws on every plot showing that series; a global - marker draws on every plot of its dataset whose visible X-window overlaps it. -- **Repaint** on data-change (the marker store notifying the host) and on time/zoom - changes, exactly as `CurveTracker` repaints on `PlaybackEngine::currentTimeChanged`. -- **Markers panel** in `pj_app` (modeled on `CurveListPanel`): filterable list, - click-row → `PlaybackEngine::setCurrentTime(marker_time)`. + marker draws on every plot of its dataset. +- **Repaint** on `SessionManager::markersChanged` (a store-agnostic signal a producer + fires after republishing) and on time/zoom changes. +- A **markers panel** in `pj_app` (filterable list, click-row → seek) is optional/future. - This is the **`pj_plotting` Qwt path, not the scene2D image path** — markers annotate time-series plots, not image frames. @@ -189,7 +199,8 @@ come from: - **Headless CLI.** PJ4 is GUI-only today (`pj_app/src/main.cpp` always opens a window). The data model is headless-ready (Qt-free `pj_runtime`/store), but a true no-GUI runner is a separate effort; v1 exposes JSON export as a host action. -- **Cross-series aggregate query** enumerates topics (per-series query is direct). -- **Concurrent writers** to the same topic (agent + plugin) need host-side locking on - that topic. -- **`ObjectStore`-snapshot fallback** (§3) remains available behind the same API. +- **Cross-series aggregate query** enumerates topics (per-series read is direct). +- **Concurrent writers** to the same topic: the republish model assumes a single owner + per marker topic; two live writers to one topic would clobber (last-writer wins). +- **Stable identity** (acks / correlating the same finding across producer re-runs) is + not provided by anonymous republished sets — it would be layered on if needed. diff --git a/docs/plot_markers_format.md b/docs/plot_markers_format.md index ddaed1f..1e9643e 100644 --- a/docs/plot_markers_format.md +++ b/docs/plot_markers_format.md @@ -37,9 +37,11 @@ for field numbers and wire types. ## What the marker does NOT carry (by design) -- **No `id`.** Identity (the delete handle) is owned by the host marker store and - surfaced by the marker API — not serialized into the value. This keeps - `PlotMarker` consistent with every other builtin, none of which carry an id. +- **No `id`.** A producer owns its set and republishes it wholesale (last-writer + -publish), so no per-marker id is carried; identity (if ever needed for acks / + cross-run correlation) is a host concern layered on top — not serialized into the + value. This keeps `PlotMarker` consistent with every other builtin, none of which + carry an id. - **No `source`.** No builtin records its creator; provenance is the dataset/topic the marker lives under. Producer-specific extras go in `metadata`. - **No `scope`.** A marker's reach is decided by *which topic* it is addressed to diff --git a/docs/plot_markers_use_cases.md b/docs/plot_markers_use_cases.md index 5863543..5606893 100644 --- a/docs/plot_markers_use_cases.md +++ b/docs/plot_markers_use_cases.md @@ -98,17 +98,17 @@ it to*, not by a field in the payload. - **UC-3 — ValueBand operating range.** A plugin `add`s a `ValueBand` for the nominal range of `motor/temp` (series-bound); samples leaving the band become obvious. -- **UC-4 — Agent creates and deletes markers.** An agent, driving the SDK API, calls - `add(dataset="Waymo", father="cmd_vel/x", marker=Region{1.0s..2.0s, severity=error, - label="discontinuity"})` and gets back a store-assigned id. Later it removes that - one marker with `delete(dataset="Waymo", father="cmd_vel/x", id)`. Adding more - markers to the same topic and deleting specific ones covers all editing needs — no +- **UC-4 — Agent republishes a marker set.** An agent (or script) builds the set for a + topic — e.g. a `Region{1.0s..2.0s, severity=error, label="discontinuity"}` on + `cmd_vel/x` of the Waymo dataset — and publishes the whole `PlotMarkers` set to that + topic. To add, change, or remove a marker it edits its in-memory set and republishes + the whole set (last-writer-publish) — there is no per-marker store mutation or in-place "modify." -- **UC-5 — Query by series (bidirectional read).** The agent (or a report tool) asks - *"give me the markers on `cmd_vel/x` in the Waymo dataset"* → - `query(dataset="Waymo", series="cmd_vel/x")` returns that topic's markers directly - (no scanning). Optional filters narrow by time range and/or `severity ≥ warning`. +- **UC-5 — Read by series.** A report tool asks *"give me the markers on `cmd_vel/x` in + the Waymo dataset"* → it reads that series' marker object topic (`latestAt`) and + deserializes the `PlotMarkers` set directly (no scanning). Filtering by time range / + `severity ≥ warning` is done on the deserialized set. - **UC-6 — JSON report.** A run exports a report — `overall_status`, plus every finding with its timestamps and severity. The data model supports this with no GUI; @@ -122,8 +122,9 @@ it to*, not by a field in the payload. ## 5. Illustrative JSON (a query result / report) The wire form is the codec-serialized marker list; this JSON is the *report view* of -the same data. Note there is no `source`/`scope` field, and `id` is the store handle -returned alongside each marker, not part of the authored marker. +the same data. Note there is no `source`/`scope` field, and no per-marker `id` — a +producer owns and republishes its whole set, so identity (if ever needed for acks / +cross-run correlation) is a host concern layered on top, not part of the marker. ```json { diff --git a/pj_base/include/pj_base/builtin/plot_markers.hpp b/pj_base/include/pj_base/builtin/plot_markers.hpp index 1b6981e..00eb232 100644 --- a/pj_base/include/pj_base/builtin/plot_markers.hpp +++ b/pj_base/include/pj_base/builtin/plot_markers.hpp @@ -26,6 +26,7 @@ #include #include +#include #include #include "pj_base/builtin/image_annotations.hpp" // for ColorRGBA @@ -102,5 +103,26 @@ struct PlotMarkers { } }; +/// Reserved marker-topic name for dataset-global markers (drawn on every plot of +/// the dataset, regardless of which series it shows). +inline constexpr std::string_view kGlobalMarkerTopic = "__global__"; + +/// Marker sets live in the host ObjectStore as serialized PlotMarkers objects. +/// They sit under object topics in a reserved namespace so they never collide +/// with media object topics (images, point clouds). Producers (plugins, scripts, +/// host) and the plot overlay MUST agree on this mapping: the object topic for a +/// marker topic `T` is `kMarkerObjectTopicPrefix + T`. A producer owns its set +/// and republishes the whole PlotMarkers blob on every change (last-writer wins); +/// the store is never mutated marker-by-marker. +inline constexpr std::string_view kMarkerObjectTopicPrefix = "__markers__/"; + +/// Object-topic name carrying the marker set addressed to `marker_topic` (a +/// series field path, or kGlobalMarkerTopic for dataset-global markers). +[[nodiscard]] inline std::string markerObjectTopicName(std::string_view marker_topic) { + std::string name(kMarkerObjectTopicPrefix); + name.append(marker_topic); + return name; +} + } // namespace sdk } // namespace PJ diff --git a/pj_base/include/pj_base/plugin_data_api.h b/pj_base/include/pj_base/plugin_data_api.h index e3bfe88..98f90fd 100644 --- a/pj_base/include/pj_base/plugin_data_api.h +++ b/pj_base/include/pj_base/plugin_data_api.h @@ -522,6 +522,19 @@ typedef struct PJ_toolbox_host_vtable_t { bool (*push_owned_object)( void* ctx, PJ_object_topic_handle_t topic, int64_t timestamp_ns, const uint8_t* data, uint64_t size, PJ_error_t* out_error) PJ_NOEXCEPT; + + /* [main-thread] Register an object topic on an EXISTING dataset, addressed by + * its DatasetId, rather than under a data source the toolbox created. This is + * the ANNOTATION path: a toolbox attaches objects (e.g. plot markers) to data + * loaded by another source. Idempotent — if a topic with this name already + * exists on the dataset, its existing handle is returned (so a producer that + * republishes its whole set just re-resolves the handle each time). + * `metadata_json` is opaque (as in register_object_topic). Returns false (with + * out_error populated) if the dataset does not exist. + * ABI-APPENDED slot: gate via struct_size before calling. */ + bool (*register_object_topic_on_dataset)( + void* ctx, uint32_t dataset_id, PJ_string_view_t topic_name, PJ_string_view_t metadata_json, + PJ_object_topic_handle_t* out_handle, PJ_error_t* out_error) PJ_NOEXCEPT; } PJ_toolbox_host_vtable_t; typedef struct { @@ -729,67 +742,6 @@ typedef struct { const PJ_object_read_host_vtable_t* vtable; } PJ_object_read_host_t; -/* ========================================================================== - * Marker-store host ("pj.marker_store.v1", protocol v4) - * - * Exposed to Toolbox plugins (and, later, the scripting layer) to create, - * delete, and query plot markers. Unlike the object store — a timestamped blob - * log that can only front-evict — markers are a mutable, id-addressed set per - * (dataset, topic): add() mints a stable id, remove() deletes by it. Markers - * cross the ABI as serialized PJ.PlotMarkers bytes (the canonical codec); ids - * cross as uint64. A marker's reach is decided by which topic it is addressed - * to (a series topic vs. a reserved dataset-global topic name). - * ========================================================================== */ - -/* Opaque OWNING handle for a query result (serialized markers + parallel ids). - * Valid until release_query(handle); never dereferenced by the plugin. */ -struct PJ_marker_query_handle_s; -typedef struct PJ_marker_query_handle_s* PJ_marker_query_handle_t; - -/* ABI-APPENDABLE: new slots may be added at the tail; struct_size gates read. */ -typedef struct PJ_marker_store_host_vtable_t { - uint32_t abi_version; - uint32_t struct_size; - - /* [main-thread] Add markers (serialized PJ.PlotMarkers) under (dataset, - * topic). The store mints one stable id per marker, in payload order, and - * writes them to out_ids (capacity out_ids_capacity); *out_count is always - * set to the TOTAL number of markers in the payload (so the caller can detect - * truncation). Returns false (out_error set) on a malformed payload. */ - bool (*add)( - void* ctx, uint32_t dataset_id, PJ_string_view_t topic, const uint8_t* markers_data, uint64_t markers_size, - uint64_t* out_ids, uint64_t out_ids_capacity, uint64_t* out_count, PJ_error_t* out_error) PJ_NOEXCEPT; - - /* [main-thread] Remove a marker by id from (dataset, topic). Returns false - * (out_error set) if the topic or id does not exist. */ - bool (*remove)( - void* ctx, uint32_t dataset_id, PJ_string_view_t topic, uint64_t marker_id, PJ_error_t* out_error) PJ_NOEXCEPT; - - /* [main-thread] Query all markers under (dataset, topic). On success - * allocates an owning handle carrying the serialized PJ.PlotMarkers bytes and - * the parallel id array; the caller releases it via release_query. An empty - * topic yields a valid handle whose marker/id count is zero. */ - bool (*query)( - void* ctx, uint32_t dataset_id, PJ_string_view_t topic, PJ_marker_query_handle_t* out_handle, - PJ_error_t* out_error) PJ_NOEXCEPT; - - /* [thread-safe] Expose the query result behind an owning handle: serialized - * PJ.PlotMarkers bytes plus the parallel id array (out_count ids, matching - * the markers in the payload, in order). Valid until release_query(handle). */ - void (*query_bytes)( - PJ_marker_query_handle_t handle, const uint8_t** out_data, uint64_t* out_size, const uint64_t** out_ids, - uint64_t* out_count) PJ_NOEXCEPT; - - /* [thread-safe] Release a query handle. Idempotent on NULL. */ - void (*release_query)(PJ_marker_query_handle_t handle) PJ_NOEXCEPT; -} PJ_marker_store_host_vtable_t; - -/* ABI-FROZEN: fat pointer layout permanent. */ -typedef struct { - void* ctx; - const PJ_marker_store_host_vtable_t* vtable; -} PJ_marker_store_host_t; - /** * Colormap registry service ("pj.colormap.v1", protocol_version 1). * diff --git a/pj_base/include/pj_base/sdk/plugin_data_api.hpp b/pj_base/include/pj_base/sdk/plugin_data_api.hpp index ba206bc..831f943 100644 --- a/pj_base/include/pj_base/sdk/plugin_data_api.hpp +++ b/pj_base/include/pj_base/sdk/plugin_data_api.hpp @@ -684,116 +684,6 @@ class ToolboxObjectReadHostView { PJ_object_read_host_t host_{}; }; -/// A marker plus its store-assigned id, as returned by MarkerStoreHostView::query. -struct MarkerEntry { - uint64_t id = 0; - PlotMarker marker; -}; - -// --------------------------------------------------------------------------- -// Marker store host view ("pj.marker_store.v1") -// -// Create / delete / query plot markers on a (dataset, topic). Unlike the object -// store, the marker set is mutable and id-addressed: add() mints a stable id, -// remove() deletes by it. Markers cross the ABI as serialized PJ.PlotMarkers -// bytes; this view wraps the codec so callers work in typed sdk::PlotMarker. -// --------------------------------------------------------------------------- -class MarkerStoreHostView { - public: - MarkerStoreHostView() = default; - explicit MarkerStoreHostView(PJ_marker_store_host_t host) : host_(host) {} - - [[nodiscard]] bool valid() const { - return host_.ctx != nullptr && host_.vtable != nullptr; - } - - /// Add a batch of markers to (dataset, topic); returns their new ids in order. - [[nodiscard]] Expected> add( - DatasetId dataset, std::string_view topic, const PlotMarkers& markers) const { - if (!valid() || host_.vtable->add == nullptr) { - return unexpected("marker store host is not bound"); - } - const std::vector bytes = PJ::serializePlotMarkers(markers); - std::vector ids(markers.markers.size()); - uint64_t count = 0; - PJ_error_t err{}; - if (!host_.vtable->add( - host_.ctx, dataset, toAbiString(topic), bytes.data(), bytes.size(), ids.data(), ids.size(), &count, - &err)) { - return unexpected(errorToString(err)); - } - ids.resize(count < ids.size() ? count : ids.size()); - return ids; - } - - /// Add a single marker; returns its new id. - [[nodiscard]] Expected add(DatasetId dataset, std::string_view topic, const PlotMarker& marker) const { - PlotMarkers one; - one.markers.push_back(marker); - auto ids = add(dataset, topic, one); - if (!ids) { - return unexpected(ids.error()); - } - if (ids->empty()) { - return unexpected("marker store add returned no id"); - } - return ids->front(); - } - - /// Remove a marker by id from (dataset, topic). - [[nodiscard]] Status remove(DatasetId dataset, std::string_view topic, uint64_t id) const { - if (!valid() || host_.vtable->remove == nullptr) { - return unexpected("marker store host is not bound"); - } - PJ_error_t err{}; - if (!host_.vtable->remove(host_.ctx, dataset, toAbiString(topic), id, &err)) { - return unexpected(errorToString(err)); - } - return okStatus(); - } - - /// Query all markers under (dataset, topic), each paired with its id. - [[nodiscard]] Expected> query(DatasetId dataset, std::string_view topic) const { - if (!valid() || host_.vtable->query == nullptr) { - return unexpected("marker store host is not bound"); - } - PJ_marker_query_handle_t handle = nullptr; - PJ_error_t err{}; - if (!host_.vtable->query(host_.ctx, dataset, toAbiString(topic), &handle, &err)) { - return unexpected(errorToString(err)); - } - const uint8_t* data = nullptr; - uint64_t size = 0; - const uint64_t* ids = nullptr; - uint64_t count = 0; - host_.vtable->query_bytes(handle, &data, &size, &ids, &count); - - std::vector out; - Expected decoded = - (size == 0) ? Expected(PlotMarkers{}) : PJ::deserializePlotMarkers(data, size); - if (decoded.has_value()) { - const auto& markers = decoded->markers; - const uint64_t n = count < markers.size() ? count : markers.size(); - out.reserve(n); - for (uint64_t i = 0; i < n; ++i) { - out.push_back(MarkerEntry{ids[i], markers[i]}); - } - } - host_.vtable->release_query(handle); - if (!decoded.has_value()) { - return unexpected(decoded.error()); - } - return out; - } - - [[nodiscard]] const PJ_marker_store_host_t& raw() const noexcept { - return host_; - } - - private: - PJ_marker_store_host_t host_{}; -}; - // --------------------------------------------------------------------------- // Object write host view (protocol v4) // @@ -1332,6 +1222,31 @@ class ToolboxHostView { return okStatus(); } + /// Register an object topic on an EXISTING dataset (by DatasetId) — the + /// annotation path used to attach objects (e.g. plot markers) to data that + /// another source loaded. Idempotent: returns the existing topic's handle if + /// one with this name already exists on the dataset, so a producer that + /// republishes its whole set just re-resolves the handle each time. Returns + /// `unexpected` on older hosts that don't expose the slot. + [[nodiscard]] Expected registerObjectTopicOnDataset( + DatasetId dataset, std::string_view name, std::string_view metadata_json) const { + if (!valid()) { + return unexpected("toolbox host is not bound"); + } + if (!hasTailSlot( + offsetof(PJ_toolbox_host_vtable_t, register_object_topic_on_dataset), + host_.vtable->register_object_topic_on_dataset)) { + return unexpected("toolbox host does not support object topics on existing datasets (older host)"); + } + ObjectTopicHandle handle{}; + PJ_error_t err{}; + if (!host_.vtable->register_object_topic_on_dataset( + host_.ctx, static_cast(dataset), toAbiString(name), toAbiString(metadata_json), &handle, &err)) { + return unexpected(errorToString(err)); + } + return handle; + } + [[nodiscard]] const PJ_toolbox_host_t& raw() const noexcept { return host_; } diff --git a/pj_base/include/pj_base/sdk/service_traits.hpp b/pj_base/include/pj_base/sdk/service_traits.hpp index 0d45212..fffe99c 100644 --- a/pj_base/include/pj_base/sdk/service_traits.hpp +++ b/pj_base/include/pj_base/sdk/service_traits.hpp @@ -134,18 +134,6 @@ struct ToolboxObjectReadHostService { static_assert(detail::isValidServiceName(kName), "kName must match the pj naming rule"); }; -/// Marker store host — create / delete / query plot markers. Resolved by -/// analysis/scripting toolboxes (and, later, the headless runner's script host) -/// that emit findings as markers. The marker set is mutable and id-addressed. -struct MarkerStoreService { - static constexpr const char* kName = "pj.marker_store.v1"; - static constexpr uint32_t kMinVersion = 1; - using Raw = PJ_marker_store_host_t; - using Vtable = PJ_marker_store_host_vtable_t; - using View = MarkerStoreHostView; - static_assert(detail::isValidServiceName(kName), "kName must match the pj naming rule"); -}; - struct ToolboxHostService { // "pj.toolbox_write.v1" for symmetry with "pj.source_write.v1" and // "pj.parser_write.v1" — this service IS the toolbox write surface diff --git a/pj_base/tests/abi_layout_sentinels_test.cpp b/pj_base/tests/abi_layout_sentinels_test.cpp index c2d62f3..249fcf6 100644 --- a/pj_base/tests/abi_layout_sentinels_test.cpp +++ b/pj_base/tests/abi_layout_sentinels_test.cpp @@ -179,7 +179,10 @@ static_assert(offsetof(PJ_toolbox_host_vtable_t, append_arrow_stream) == 48, "to static_assert(offsetof(PJ_toolbox_host_vtable_t, read_series_arrow) == 64, "toolbox host read slot pinned"); static_assert(offsetof(PJ_toolbox_host_vtable_t, register_object_topic) == 72, "toolbox host object-topic slot pinned"); static_assert(offsetof(PJ_toolbox_host_vtable_t, push_owned_object) == 80, "toolbox host object-push tail slot pinned"); -static_assert(sizeof(PJ_toolbox_host_vtable_t) == 88, "Toolbox host size (update deliberately on append)"); +static_assert( + offsetof(PJ_toolbox_host_vtable_t, register_object_topic_on_dataset) == 88, + "toolbox host object-topic-on-dataset tail slot pinned"); +static_assert(sizeof(PJ_toolbox_host_vtable_t) == 96, "Toolbox host size (update deliberately on append)"); // --- ABI version symbol ------------------------------------------------------ static_assert(PJ_ABI_VERSION == 5, "v5 ABI version"); diff --git a/pj_plugins/include/pj_plugins/testing/toolbox_test_store.hpp b/pj_plugins/include/pj_plugins/testing/toolbox_test_store.hpp index 67d59a1..1edf4ee 100644 --- a/pj_plugins/include/pj_plugins/testing/toolbox_test_store.hpp +++ b/pj_plugins/include/pj_plugins/testing/toolbox_test_store.hpp @@ -147,6 +147,7 @@ class ToolboxTestStore { // slots into clear "older host" errors. .register_object_topic = nullptr, .push_owned_object = nullptr, + .register_object_topic_on_dataset = nullptr, }; return PJ_toolbox_host_t{.ctx = this, .vtable = &vtable}; } diff --git a/pj_plugins/tests/toolbox_plugin_test.cpp b/pj_plugins/tests/toolbox_plugin_test.cpp index d3b7142..15ed676 100644 --- a/pj_plugins/tests/toolbox_plugin_test.cpp +++ b/pj_plugins/tests/toolbox_plugin_test.cpp @@ -84,9 +84,11 @@ PJ_toolbox_host_t makeToolboxHost(ToolboxState* state) { .read_series_arrow = tbReadSeriesArrow, // Tail slots — left null because this mock host doesn't exercise the // object-topic surface. ToolboxHostView::registerObjectTopic / - // pushOwnedObject return `unexpected("older host")` for null slots. + // pushOwnedObject / registerObjectTopicOnDataset return + // `unexpected("older host")` for null slots. .register_object_topic = nullptr, .push_owned_object = nullptr, + .register_object_topic_on_dataset = nullptr, }; return PJ_toolbox_host_t{.ctx = state, .vtable = &vtable}; } From 0137e686088033c684ccbcafb598b972b28336fc Mon Sep 17 00:00:00 2001 From: alvvm Date: Thu, 18 Jun 2026 11:24:38 +0200 Subject: [PATCH 04/11] markers: MarkerTimeline dialog widget + dataset-scoped read & object-retention toolbox ABI slots Dialog protocol: add the MarkerTimeline custom widget (TimelineMark setters/event/parser/callback) for an editable multi-marker strip, with a single shared TimelineMark<->JSON codec used by all four sites. Toolbox host ABI (tail-appended, struct_size-gated): lookup_topic_on_dataset on the object-read vtable (dataset-scoped topic resolution; the name-only lookup is ambiguous across datasets) and set_object_topic_retention on the write vtable (keep-latest-N so a republishing producer's snapshots don't accumulate). Toolbox-host vtable 96->104; sentinels + mock vtables updated. Add sdk::markerSeriesKey (single-slash join tolerating a leading-'/' field) so producer and overlay build the per-series key identically. Doc refresh. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/dialog-sdk-reference.md | 13 ++++ docs/plot_markers_architecture.md | 10 ++- .../include/pj_base/builtin/plot_markers.hpp | 18 +++++ pj_base/include/pj_base/plugin_data_api.h | 17 +++++ .../include/pj_base/sdk/plugin_data_api.hpp | 35 ++++++++++ pj_base/tests/abi_layout_sentinels_test.cpp | 5 +- .../pj_plugins/host/widget_data_view.hpp | 34 ++++++++++ .../pj_plugins/host/widget_event_builder.hpp | 9 +++ .../pj_plugins/sdk/dialog_plugin_typed.hpp | 9 +++ .../include/pj_plugins/sdk/widget_data.hpp | 67 +++++++++++++++++++ .../include/pj_plugins/sdk/widget_event.hpp | 10 +++ pj_plugins/tests/toolbox_plugin_test.cpp | 1 + 12 files changed, 221 insertions(+), 7 deletions(-) diff --git a/docs/dialog-sdk-reference.md b/docs/dialog-sdk-reference.md index 5f475e9..09d5532 100644 --- a/docs/dialog-sdk-reference.md +++ b/docs/dialog-sdk-reference.md @@ -106,6 +106,18 @@ For the full tutorial, see [dialog-plugin-guide.md](../pj_plugins/docs/dialog-pl | `setOkEnabled(bool)` | Enable/disable OK button (targets `"buttonBox"`) | | `setOkEnabled(name, bool)` | Enable/disable OK button (custom name) | +### MarkerTimeline (custom widget, class name `MarkerTimeline`) + +Editable multi-marker strip: any number of resizable Region spans + single-point Event marks, each draggable. + +| Method | Description | +|--------|-------------| +| `setMarkerTimelineBounds(name, min, max)` | Integer step domain marks live in (set before the marks) | +| `setMarkerTimelineMarks(name, marks)` | Replace the whole `std::vector` set (last-writer-wins; empty clears) | +| `setMarkerTimelineTimeSpan(name, min_ns, max_ns)` | Map the step domain onto `[min_ns, max_ns]` for hover labels (`0,0` → raw steps) | + +`TimelineMark{int id; bool region; int start; int end;}` — `region=false` is a point Event (`end` ignored). + ### Generic (any widget) | Method | Description | @@ -144,6 +156,7 @@ Override these in your `DialogPluginTyped` subclass. Return `true` when state ch | `onCodeChangedWithCursor(name, code, cursor)` | QPlainTextEdit code editor | Edited code + caret offset (`cursor < 0` when no opt-in / not reported); defaults to `onCodeChanged` | | `onItemsDropped(name, items)` | Any widget with `setDropTarget` | Dropped item labels | | `onChartViewChanged(name, x_min, x_max, y_min, y_max)` | QFrame chart container | Visible chart range | +| `onMarkerTimelineChanged(name, marks)` | MarkerTimeline | Full `std::vector` set after a drag/resize/delete | | `onTabChanged(name, index)` | QTabWidget | New tab index | --- diff --git a/docs/plot_markers_architecture.md b/docs/plot_markers_architecture.md index 18cc905..e8f492e 100644 --- a/docs/plot_markers_architecture.md +++ b/docs/plot_markers_architecture.md @@ -1,11 +1,9 @@ # Plot Markers — Architecture -> **Status: REVISED (was a dedicated-store draft).** Markers are now a **builtin -> object** stored in the **ObjectStore** — there is NO dedicated marker store and NO -> `pj.marker_store.v1` service. The *what/why* and examples live in -> [plot_markers_use_cases.md](plot_markers_use_cases.md) (parts may still describe the -> old model — being updated). The full reasoning for the pivot is in the session plan -> `atomic-foraging-kettle.md`. +> Markers are a **builtin object** stored in the **ObjectStore** — there is no +> dedicated marker store and no marker-store service; producers republish a whole +> `PlotMarkers` set under an object topic (last-writer-wins). The *what/why* and +> examples live in [plot_markers_use_cases.md](plot_markers_use_cases.md). > > Plot Markers reuse the **concept** of [`ImageAnnotations`](image_annotations_format.md) > (a canonical SDK builtin object with a wire codec) but not its structure — and, like diff --git a/pj_base/include/pj_base/builtin/plot_markers.hpp b/pj_base/include/pj_base/builtin/plot_markers.hpp index 00eb232..ad29151 100644 --- a/pj_base/include/pj_base/builtin/plot_markers.hpp +++ b/pj_base/include/pj_base/builtin/plot_markers.hpp @@ -124,5 +124,23 @@ inline constexpr std::string_view kMarkerObjectTopicPrefix = "__markers__/"; return name; } +/// Per-series marker topic key from a curve's (topic_name, field_name). The +/// producer (e.g. the markers toolbox, via the host catalog_key_resolver) and +/// the plot overlay MUST build this key identically, so they share this helper. +/// Joins with a single '/', tolerating a field path that already carries a +/// leading '/': "/sensor/pressure" + "/data" → "/sensor/pressure/data" (not +/// the doubled "/sensor/pressure//data"). +[[nodiscard]] inline std::string markerSeriesKey(std::string_view topic_name, std::string_view field_name) { + std::string key(topic_name); + if (field_name.empty()) { + return key; // no field → no separator (avoid a dangling trailing slash) + } + if (field_name.front() != '/') { + key.push_back('/'); + } + key.append(field_name); + return key; +} + } // namespace sdk } // namespace PJ diff --git a/pj_base/include/pj_base/plugin_data_api.h b/pj_base/include/pj_base/plugin_data_api.h index 98f90fd..32d738c 100644 --- a/pj_base/include/pj_base/plugin_data_api.h +++ b/pj_base/include/pj_base/plugin_data_api.h @@ -535,6 +535,15 @@ typedef struct PJ_toolbox_host_vtable_t { bool (*register_object_topic_on_dataset)( void* ctx, uint32_t dataset_id, PJ_string_view_t topic_name, PJ_string_view_t metadata_json, PJ_object_topic_handle_t* out_handle, PJ_error_t* out_error) PJ_NOEXCEPT; + + /* [main-thread] Bound an object topic's retention to keep at most `max_entries` + * of the most recent snapshots (0 = unlimited). Generic capability: a producer + * that republishes a whole set under one topic at a sentinel timestamp (e.g. + * plot markers) sets max_entries=1 so superseded snapshots are evicted instead + * of accumulating. Returns false (out_error populated) on a bad handle. + * ABI-APPENDED slot: gate via struct_size before calling. */ + bool (*set_object_topic_retention)( + void* ctx, PJ_object_topic_handle_t topic, uint64_t max_entries, PJ_error_t* out_error) PJ_NOEXCEPT; } PJ_toolbox_host_vtable_t; typedef struct { @@ -734,6 +743,14 @@ typedef struct PJ_object_read_host_vtable_t { /* [main-thread] Time range [min, max] for a topic. Returns false if the * topic is unknown or empty. */ bool (*time_range)(void* ctx, PJ_object_topic_handle_t topic, int64_t* out_min_ts, int64_t* out_max_ts) PJ_NOEXCEPT; + + /* [main-thread] Look up an object topic on a SPECIFIC dataset by name. + * Object-topic identity is (dataset_id, name): the same name can exist on + * several loaded datasets, so the name-only `lookup_topic` above is ambiguous + * across datasets. This dataset-scoped form returns the topic owned by + * `dataset_id`, or {id=0} on miss. ABI-APPENDED slot: gate via struct_size. */ + PJ_object_topic_handle_t (*lookup_topic_on_dataset)( + void* ctx, uint32_t dataset_id, PJ_string_view_t topic_name) PJ_NOEXCEPT; } PJ_object_read_host_vtable_t; /* ABI-FROZEN: fat pointer layout permanent. */ diff --git a/pj_base/include/pj_base/sdk/plugin_data_api.hpp b/pj_base/include/pj_base/sdk/plugin_data_api.hpp index 831f943..2e63c7c 100644 --- a/pj_base/include/pj_base/sdk/plugin_data_api.hpp +++ b/pj_base/include/pj_base/sdk/plugin_data_api.hpp @@ -676,6 +676,21 @@ class ToolboxObjectReadHostView { return {lo, hi}; } + /// Dataset-scoped topic lookup — disambiguates a topic name that exists on + /// several loaded datasets (the name-only lookupTopic cannot). nullopt on miss + /// or when the host predates this tail slot. + [[nodiscard]] std::optional lookupTopicOnDataset(DatasetId dataset, std::string_view name) const { + if (!valid() || !PJ_HAS_TAIL_SLOT(PJ_object_read_host_vtable_t, host_.vtable, lookup_topic_on_dataset)) { + return std::nullopt; + } + ObjectTopicHandle h = + host_.vtable->lookup_topic_on_dataset(host_.ctx, static_cast(dataset), toAbiString(name)); + if (h.id == 0) { + return std::nullopt; + } + return h; + } + [[nodiscard]] const PJ_object_read_host_t& raw() const noexcept { return host_; } @@ -1247,6 +1262,26 @@ class ToolboxHostView { return handle; } + /// Bound a topic's retention to the last `max_entries` snapshots (0 = unlimited). + /// A producer republishing a whole set under one topic at a sentinel timestamp + /// passes 1 so superseded snapshots are evicted instead of accumulating. Returns + /// an error on a host that predates this slot. + [[nodiscard]] Status setObjectTopicRetention(ObjectTopicHandle topic, uint64_t max_entries) const { + if (!valid()) { + return unexpected("toolbox host is not bound"); + } + if (!hasTailSlot( + offsetof(PJ_toolbox_host_vtable_t, set_object_topic_retention), + host_.vtable->set_object_topic_retention)) { + return unexpected("toolbox host does not support object retention (older host)"); + } + PJ_error_t err{}; + if (!host_.vtable->set_object_topic_retention(host_.ctx, topic, max_entries, &err)) { + return unexpected(errorToString(err)); + } + return {}; + } + [[nodiscard]] const PJ_toolbox_host_t& raw() const noexcept { return host_; } diff --git a/pj_base/tests/abi_layout_sentinels_test.cpp b/pj_base/tests/abi_layout_sentinels_test.cpp index 249fcf6..2ec5829 100644 --- a/pj_base/tests/abi_layout_sentinels_test.cpp +++ b/pj_base/tests/abi_layout_sentinels_test.cpp @@ -182,7 +182,10 @@ static_assert(offsetof(PJ_toolbox_host_vtable_t, push_owned_object) == 80, "tool static_assert( offsetof(PJ_toolbox_host_vtable_t, register_object_topic_on_dataset) == 88, "toolbox host object-topic-on-dataset tail slot pinned"); -static_assert(sizeof(PJ_toolbox_host_vtable_t) == 96, "Toolbox host size (update deliberately on append)"); +static_assert( + offsetof(PJ_toolbox_host_vtable_t, set_object_topic_retention) == 96, + "toolbox host object-retention tail slot pinned"); +static_assert(sizeof(PJ_toolbox_host_vtable_t) == 104, "Toolbox host size (update deliberately on append)"); // --- ABI version symbol ------------------------------------------------------ static_assert(PJ_ABI_VERSION == 5, "v5 ABI version"); diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp index 4037ecc..fff87a2 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp @@ -13,6 +13,7 @@ #include #include "pj_base/number_parse.hpp" +#include "pj_plugins/sdk/widget_data.hpp" // TimelineMark namespace PJ { @@ -314,6 +315,39 @@ class WidgetDataView { return getString(name, "date_range_latest"); } + // --- MarkerTimeline --- + [[nodiscard]] std::optional markerTimelineMin(std::string_view name) const { + return getInt(name, "marker_timeline_min"); + } + [[nodiscard]] std::optional markerTimelineMax(std::string_view name) const { + return getInt(name, "marker_timeline_max"); + } + [[nodiscard]] std::optional> markerTimelineMarks(std::string_view name) const { + const nlohmann::json* w = widget(name); + if (!w) { + return std::nullopt; + } + auto it = w->find("marker_timeline_marks"); + if (it == w->end() || !it->is_array()) { + return std::nullopt; + } + return timelineMarksFromJson(*it); + } + /// [min_ns, max_ns] time window for hover labels, parsed from string-encoded ns. + [[nodiscard]] std::optional> markerTimelineTimeSpan( + std::string_view name) const { + auto mn = getString(name, "marker_timeline_time_min_ns"); + auto mx = getString(name, "marker_timeline_time_max_ns"); + if (!mn.has_value() || !mx.has_value()) { + return std::nullopt; + } + try { + return std::make_pair(static_cast(std::stoll(*mn)), static_cast(std::stoll(*mx))); + } catch (...) { + return std::nullopt; + } + } + // --- Field validity indicator (generic) --- [[nodiscard]] std::optional fieldValid(std::string_view name) const { return getBool(name, "valid"); diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_event_builder.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_event_builder.hpp index 683741b..4161837 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_event_builder.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_event_builder.hpp @@ -3,6 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 #include +#include // TimelineMark #include #include #include @@ -142,6 +143,14 @@ struct WidgetEventBuilder { return j.dump(); } + /// MarkerTimeline: the mark set changed (user drag / resize / delete). Carries + /// the whole set so the plugin replaces its draft wholesale. + [[nodiscard]] static std::string markerTimelineChanged(const std::vector& marks) { + nlohmann::json j; + j["marker_timeline_marks"] = timelineMarksToJson(marks); + return j.dump(); + } + /// ChartPreviewWidget: visible range changed via zoom or pan. [[nodiscard]] static std::string chartViewChanged(double x_min, double x_max, double y_min, double y_max) { nlohmann::json j; diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_typed.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_typed.hpp index 640f2f0..dee5b38 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_typed.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_typed.hpp @@ -103,6 +103,12 @@ class DialogPluginTyped : public DialogPluginBase { return false; } + /// MarkerTimeline: a mark was moved, resized, or deleted. `marks` is the full set. + virtual bool onMarkerTimelineChanged( + std::string_view /*widget_name*/, const std::vector& /*marks*/) { + return false; + } + /// DateRangePicker: the date/time range filter changed. from_iso/to_iso are /// ISO-8601 datetime strings (empty = unbounded on that side). virtual bool onDateRangeChanged( @@ -121,6 +127,9 @@ class DialogPluginTyped : public DialogPluginBase { if (auto v = event.rangeChanged()) { return onRangeChanged(widget_name, v->lower, v->upper); } + if (auto v = event.markerTimelineChanged()) { + return onMarkerTimelineChanged(widget_name, *v); + } if (auto v = event.dateRangeChanged()) { return onDateRangeChanged(widget_name, v->from_iso, v->to_iso); } diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp index 4db30cb..760e8d7 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp @@ -26,6 +26,45 @@ struct ChartSeries { std::string color; // optional hex "#rrggbb" }; +/// One mark on a MarkerTimeline. `region` true → a resizable [start,end] span; +/// false → a single-point event at `start` (end ignored). Positions are in the +/// timeline's integer step domain (see setMarkerTimelineBounds). `id` is a stable +/// per-mark handle the host echoes back on edits. +struct TimelineMark { + int id = 0; + bool region = true; + int start = 0; + int end = 0; +}; + +/// The single source of truth for the TimelineMark ⇆ JSON wire shape, shared by +/// the data setter, the event builder, the event parser, and the data-view (so a +/// field change cannot drift between the four). +[[nodiscard]] inline nlohmann::json timelineMarksToJson(const std::vector& marks) { + nlohmann::json arr = nlohmann::json::array(); + for (const auto& m : marks) { + arr.push_back({{"id", m.id}, {"region", m.region}, {"start", m.start}, {"end", m.end}}); + } + return arr; +} + +[[nodiscard]] inline std::vector timelineMarksFromJson(const nlohmann::json& arr) { + std::vector marks; + if (!arr.is_array()) { + return marks; + } + marks.reserve(arr.size()); + for (const auto& jm : arr) { + TimelineMark m; + m.id = jm.value("id", 0); + m.region = jm.value("region", true); + m.start = jm.value("start", 0); + m.end = jm.value("end", 0); + marks.push_back(m); + } + return marks; +} + /// Builder for the JSON string returned by get_widget_data(). /// Each method targets an existing widget in the .ui file by its objectName. class WidgetData { @@ -341,6 +380,34 @@ class WidgetData { return *this; } + // --- MarkerTimeline (editable multi-marker strip) --- + + /// Set the integer [min, max] step domain of a MarkerTimeline (mark positions + /// live in these units). Set before the marks, like the RangeSlider bounds. + WidgetData& setMarkerTimelineBounds(std::string_view name, int min, int max) { + auto& e = entry(name); + e["marker_timeline_min"] = min; + e["marker_timeline_max"] = max; + return *this; + } + + /// Replace the whole mark set shown on a MarkerTimeline (last-writer-wins, + /// mirroring the producer's republish model). An empty vector clears it. + WidgetData& setMarkerTimelineMarks(std::string_view name, const std::vector& marks) { + entry(name)["marker_timeline_marks"] = timelineMarksToJson(marks); + return *this; + } + + /// Map the MarkerTimeline's [min,max] step domain onto the absolute time window + /// [min_ns, max_ns] for hover/tooltip labels. Nanoseconds are carried as strings + /// to avoid double precision loss. Pass min_ns == max_ns == 0 to show raw steps. + WidgetData& setMarkerTimelineTimeSpan(std::string_view name, std::int64_t min_ns, std::int64_t max_ns) { + auto& e = entry(name); + e["marker_timeline_time_min_ns"] = std::to_string(min_ns); + e["marker_timeline_time_max_ns"] = std::to_string(max_ns); + return *this; + } + // --- Field validity indicator (generic) --- /// Mark a field's value valid/invalid for a small inline indicator (e.g. a diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_event.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_event.hpp index d0db1cc..4af4f42 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_event.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_event.hpp @@ -4,6 +4,7 @@ #include #include +#include // TimelineMark #include #include #include @@ -163,6 +164,15 @@ class WidgetEvent { return RangeValues{lo->get(), hi->get()}; } + /// MarkerTimeline: the full mark set after a user edit (drag / resize / delete). + std::optional> markerTimelineChanged() const { + auto it = data_.find("marker_timeline_marks"); + if (it == data_.end() || !it->is_array()) { + return std::nullopt; + } + return timelineMarksFromJson(*it); + } + /// ChartPreviewWidget: visible range changed via zoom or pan. struct ChartViewState { double x_min; diff --git a/pj_plugins/tests/toolbox_plugin_test.cpp b/pj_plugins/tests/toolbox_plugin_test.cpp index 15ed676..67cee61 100644 --- a/pj_plugins/tests/toolbox_plugin_test.cpp +++ b/pj_plugins/tests/toolbox_plugin_test.cpp @@ -89,6 +89,7 @@ PJ_toolbox_host_t makeToolboxHost(ToolboxState* state) { .register_object_topic = nullptr, .push_owned_object = nullptr, .register_object_topic_on_dataset = nullptr, + .set_object_topic_retention = nullptr, }; return PJ_toolbox_host_t{.ctx = state, .vtable = &vtable}; } From 4a276effec914210647463f753d715d5e80dcf48 Mon Sep 17 00:00:00 2001 From: alvvm Date: Fri, 19 Jun 2026 10:24:37 +0200 Subject: [PATCH 05/11] feat(dialog): save-as file picker + chart-marker overlay; codec cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Additive dialog-protocol surface (ABI-safe, no version bump): - setSaveFilePicker / isSaveFilePicker — a native "Save As" picker (getSaveFileName) so a not-yet-existing file can be created; reported via the existing onFileSelected. - ChartMarker + setChartMarkers / chartMarkers — overlay markers (events/regions/ value-bands) on a chart preview, alongside setChartSeries. - Fix a stale comment that referenced QLineSeries (the preview is Qwt, not Qt Charts). Cleanup: decodeMarker now drives the shared builtin_wire::parseFields loop like the sibling codecs, replacing the hand-rolled continue/break/skip control flow. Behavior and wire format are unchanged (codec round-trip tests pass). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01RcygzPWUrHf9jHGGwzEzZo --- docs/dialog-sdk-reference.md | 6 +- pj_base/src/builtin/plot_markers_codec.cpp | 175 +++++++----------- .../pj_plugins/host/widget_data_view.hpp | 54 ++++++ .../include/pj_plugins/sdk/widget_data.hpp | 94 +++++++++- 4 files changed, 208 insertions(+), 121 deletions(-) diff --git a/docs/dialog-sdk-reference.md b/docs/dialog-sdk-reference.md index 09d5532..6f05f7c 100644 --- a/docs/dialog-sdk-reference.md +++ b/docs/dialog-sdk-reference.md @@ -56,7 +56,8 @@ For the full tutorial, see [dialog-plugin-guide.md](../pj_plugins/docs/dialog-pl | `setButtonIcon(name, svg_data)` | Set an inline SVG icon (custom/one-off) | | `setButtonIconNamed(name, icon_id)` | Set a button icon by id, resolved from the host's themed icon set (consistent tinting; unknown id → no icon) | | `setShortcut(name, key_sequence)` | Assign keyboard shortcut (e.g. `"Ctrl+A"`) | -| `setFilePicker(name, text, filter, title)` | Turn into file picker | +| `setFilePicker(name, text, filter, title)` | Turn into an **open** file picker (existing file) | +| `setSaveFilePicker(name, text, filter, title)` | Turn into a **save-as** file picker (navigate + type a new filename); reports via `onFileSelected` | | `setFolderPicker(name, text, title)` | Turn into folder picker | ### QListWidget @@ -80,7 +81,8 @@ For the full tutorial, see [dialog-plugin-guide.md](../pj_plugins/docs/dialog-pl | Method | Description | |--------|-------------| | `setChartSeries(name, vector)` | Create/update chart series inside a QFrame | -| `clearChart(name)` | Remove chart series | +| `setChartMarkers(name, vector)` | Overlay markers (events/regions/value-bands) on top of the series | +| `clearChart(name)` | Remove chart series and markers | | `setChartZoomEnabled(name, bool)` | Enable chart zoom/pan events | ### QPlainTextEdit diff --git a/pj_base/src/builtin/plot_markers_codec.cpp b/pj_base/src/builtin/plot_markers_codec.cpp index e361044..df763d7 100644 --- a/pj_base/src/builtin/plot_markers_codec.cpp +++ b/pj_base/src/builtin/plot_markers_codec.cpp @@ -14,6 +14,7 @@ namespace PJ { namespace { +using builtin_wire::parseFields; using builtin_wire::Reader; using builtin_wire::Tag; using builtin_wire::WireType; @@ -171,143 +172,91 @@ bool decodeProperty(Reader& reader, MarkerProperty& out) { } bool decodeMarker(Reader& reader, PlotMarker& out) { - while (!reader.eof()) { - Tag tag; - if (!reader.readTag(tag)) { - return false; - } - + // Handler returns true when it consumed the field, false to let parseFields skip + // it (unknown field, or a known field with the wrong wire type). + return parseFields(reader, [&](Tag tag, Reader& r) { switch (tag.field) { - case 1: - if (tag.type == WireType::kVarint) { - uint64_t v = 0; - if (!reader.readVarint(v)) { - return false; - } - out.kind = mapKind(v); - continue; + case 1: { + uint64_t v = 0; + if (tag.type != WireType::kVarint || !r.readVarint(v)) { + return false; } - break; - case 2: - if (tag.type == WireType::kVarint) { - uint64_t v = 0; - if (!reader.readVarint(v)) { - return false; - } - out.t_start = static_cast(v); - continue; + out.kind = mapKind(v); + return true; + } + case 2: { + uint64_t v = 0; + if (tag.type != WireType::kVarint || !r.readVarint(v)) { + return false; } - break; - case 3: - if (tag.type == WireType::kVarint) { - uint64_t v = 0; - if (!reader.readVarint(v)) { - return false; - } - out.t_end = static_cast(v); - continue; + out.t_start = static_cast(v); + return true; + } + case 3: { + uint64_t v = 0; + if (tag.type != WireType::kVarint || !r.readVarint(v)) { + return false; } - break; + out.t_end = static_cast(v); + return true; + } case 4: - if (tag.type == WireType::kFixed64) { - if (!reader.readDouble(out.value_low)) { - return false; - } - continue; - } - break; + return tag.type == WireType::kFixed64 && r.readDouble(out.value_low); case 5: - if (tag.type == WireType::kFixed64) { - if (!reader.readDouble(out.value_high)) { - return false; - } - continue; - } - break; - case 6: - if (tag.type == WireType::kVarint) { - uint64_t v = 0; - if (!reader.readVarint(v)) { - return false; - } - out.has_value = (v != 0); - continue; + return tag.type == WireType::kFixed64 && r.readDouble(out.value_high); + case 6: { + uint64_t v = 0; + if (tag.type != WireType::kVarint || !r.readVarint(v)) { + return false; } - break; - case 7: - if (tag.type == WireType::kVarint) { - uint64_t v = 0; - if (!reader.readVarint(v)) { - return false; - } - out.status = mapStatus(v); - continue; + out.has_value = (v != 0); + return true; + } + case 7: { + uint64_t v = 0; + if (tag.type != WireType::kVarint || !r.readVarint(v)) { + return false; } - break; - case 8: - if (tag.type == WireType::kVarint) { - uint64_t v = 0; - if (!reader.readVarint(v)) { - return false; - } - out.severity = mapSeverity(v); - continue; + out.status = mapStatus(v); + return true; + } + case 8: { + uint64_t v = 0; + if (tag.type != WireType::kVarint || !r.readVarint(v)) { + return false; } - break; + out.severity = mapSeverity(v); + return true; + } case 9: - if (tag.type == WireType::kLengthDelimited) { - if (!reader.readString(out.category)) { - return false; - } - continue; - } - break; + return tag.type == WireType::kLengthDelimited && r.readString(out.category); case 10: - if (tag.type == WireType::kLengthDelimited) { - if (!reader.readString(out.label)) { - return false; - } - continue; - } - break; + return tag.type == WireType::kLengthDelimited && r.readString(out.label); case 11: - if (tag.type == WireType::kLengthDelimited) { - if (!reader.readString(out.description)) { - return false; - } - continue; - } - break; - case 12: - if (tag.type == WireType::kLengthDelimited) { - Reader nested; - if (!reader.readMessage(nested) || !decodeColor(nested, out.color)) { - return false; - } - continue; + return tag.type == WireType::kLengthDelimited && r.readString(out.description); + case 12: { + if (tag.type != WireType::kLengthDelimited) { + return false; } - break; + Reader nested; + return r.readMessage(nested) && decodeColor(nested, out.color); + } case 13: { if (tag.type != WireType::kLengthDelimited) { - break; + return false; } Reader nested; MarkerProperty property; - if (!reader.readMessage(nested) || !decodeProperty(nested, property)) { + if (!r.readMessage(nested) || !decodeProperty(nested, property)) { return false; } out.metadata.push_back(std::move(property)); - continue; + return true; } default: - break; - } - - if (!reader.skip(tag.type)) { - return false; + return false; } - } - return true; + }); } } // namespace diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp index fff87a2..a3d1d70 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp @@ -196,6 +196,49 @@ class WidgetDataView { return result; } + /// A marker overlaid on the chart (see PJ::ChartMarker / setChartMarkers). + /// Interpret by `kind`: "event" (point at x0,y0 if has_value, else vline at x0), + /// "region" (x-span [x0,x1]), "value_band" (y-band [y0,y1]; line when y0==y1), "label". + struct ChartMarkerView { + std::string kind; + double x0 = 0.0; + double x1 = 0.0; + double y0 = 0.0; + double y1 = 0.0; + bool has_value = false; + std::string color; // optional hex "#rrggbb"; empty means use a default palette + std::string label; + }; + + [[nodiscard]] std::optional> chartMarkers(std::string_view name) const { + const nlohmann::json* w = widget(name); + if (!w) { + return std::nullopt; + } + auto it = w->find("chart_markers"); + if (it == w->end() || !it->is_array()) { + return std::nullopt; + } + std::vector result; + result.reserve(it->size()); + for (const auto& m : *it) { + if (!m.is_object()) { + continue; + } + ChartMarkerView mv; + mv.kind = m.value("kind", std::string()); + mv.x0 = m.value("x0", 0.0); + mv.x1 = m.value("x1", 0.0); + mv.y0 = m.value("y0", 0.0); + mv.y1 = m.value("y1", 0.0); + mv.has_value = m.value("has_value", false); + mv.color = m.value("color", std::string()); + mv.label = m.value("label", std::string()); + result.push_back(std::move(mv)); + } + return result; + } + /// Returns whether interactive zoom is enabled on this chart widget. [[nodiscard]] std::optional chartZoomEnabled(std::string_view name) const { return getBool(name, "chart_zoom_enabled"); @@ -258,6 +301,17 @@ class WidgetDataView { return it != w->end() && it->is_string() && it->get() == "file_picker"; } + /// A "save-as" file picker (navigate + type a new filename), as opposed to the + /// open-existing-file picker above. Shares the same filter/title accessors. + [[nodiscard]] bool isSaveFilePicker(std::string_view name) const { + const nlohmann::json* w = widget(name); + if (!w) { + return false; + } + auto it = w->find("action"); + return it != w->end() && it->is_string() && it->get() == "save_file_picker"; + } + [[nodiscard]] std::optional filePickerFilter(std::string_view name) const { return getString(name, "filter"); } diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp index 760e8d7..708c467 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp @@ -18,14 +18,72 @@ struct ChartPoint { }; /// A named series of XY points for chart display (used by setChartSeries). -/// If `color` is non-empty (e.g. "#ff7f0e"), it overrides the chart theme color -/// for this series; otherwise the Qt Charts theme picks one. +/// If `color` is non-empty (e.g. "#ff7f0e"), it overrides the built-in palette +/// color for this series; otherwise the host's chart palette picks one. struct ChartSeries { std::string label; std::vector points; std::string color; // optional hex "#rrggbb" }; +/// One marker overlaid on a chart preview (used by setChartMarkers). Interpret by +/// `kind`, mirroring the PlotMarkers vocabulary: +/// "event" → a point at (x0, y0) when `has_value`, else a vertical line at x0; +/// "region" → a shaded x-span [x0, x1]; +/// "value_band" → a shaded y-band [y0, y1] (a horizontal line when y0 == y1); +/// "label" → a labelled mark at x0. +/// X/Y are in the chart's own units (e.g. seconds-from-start), so the producer must +/// rebase to match the series points it pushes alongside. +struct ChartMarker { + std::string kind; // "event" | "region" | "value_band" | "label" + double x0 = 0.0; + double x1 = 0.0; + double y0 = 0.0; + double y1 = 0.0; + bool has_value = false; // event: y0 is a real point value (draw a point, not a bare vline) + std::string color; // optional hex "#rrggbb"; empty → host derives from a default palette + std::string label; +}; + +/// The single source of truth for the ChartMarker ⇆ JSON wire shape, shared by the +/// data setter and the data-view (so a field change cannot drift between them). +[[nodiscard]] inline nlohmann::json chartMarkersToJson(const std::vector& marks) { + nlohmann::json arr = nlohmann::json::array(); + for (const auto& m : marks) { + nlohmann::json jm = {{"kind", m.kind}, {"x0", m.x0}, {"x1", m.x1}, + {"y0", m.y0}, {"y1", m.y1}, {"has_value", m.has_value}}; + if (!m.color.empty()) { + jm["color"] = m.color; + } + if (!m.label.empty()) { + jm["label"] = m.label; + } + arr.push_back(std::move(jm)); + } + return arr; +} + +[[nodiscard]] inline std::vector chartMarkersFromJson(const nlohmann::json& arr) { + std::vector marks; + if (!arr.is_array()) { + return marks; + } + marks.reserve(arr.size()); + for (const auto& jm : arr) { + ChartMarker m; + m.kind = jm.value("kind", std::string()); + m.x0 = jm.value("x0", 0.0); + m.x1 = jm.value("x1", 0.0); + m.y0 = jm.value("y0", 0.0); + m.y1 = jm.value("y1", 0.0); + m.has_value = jm.value("has_value", false); + m.color = jm.value("color", std::string()); + m.label = jm.value("label", std::string()); + marks.push_back(std::move(m)); + } + return marks; +} + /// One mark on a MarkerTimeline. `region` true → a resizable [start,end] span; /// false → a single-point event at `start` (end ignored). Positions are in the /// timeline's integer step domain (see setMarkerTimelineBounds). `id` is a stable @@ -206,8 +264,8 @@ class WidgetData { // --- Chart (QFrame used as chart container) --- - /// Set chart series data on a QFrame widget. The host will create or update - /// a chart view inside the frame, displaying one QLineSeries per entry. + /// Set chart series data on a QFrame widget. The host creates or updates a Qwt + /// plot inside the frame, drawing one line curve per entry. WidgetData& setChartSeries(std::string_view name, const std::vector& series) { auto& e = entry(name); nlohmann::json arr = nlohmann::json::array(); @@ -226,9 +284,18 @@ class WidgetData { return *this; } - /// Remove all series from the chart inside the named QFrame. + /// Overlay markers on the chart inside the named QFrame (drawn on top of the + /// series). Pass an empty vector to clear just the markers. See ChartMarker. + WidgetData& setChartMarkers(std::string_view name, const std::vector& markers) { + entry(name)["chart_markers"] = chartMarkersToJson(markers); + return *this; + } + + /// Remove all series AND markers from the chart inside the named QFrame. WidgetData& clearChart(std::string_view name) { - entry(name)["chart_series"] = nlohmann::json::array(); + auto& e = entry(name); + e["chart_series"] = nlohmann::json::array(); + e["chart_markers"] = nlohmann::json::array(); return *this; } @@ -324,6 +391,21 @@ class WidgetData { return *this; } + /// Turn a QPushButton into a native "Save As" file picker. Unlike setFilePicker + /// (which opens an existing file), this lets the user navigate to a directory AND + /// type a new filename, so a not-yet-existing rule/config file can be created. The + /// chosen path is reported through the same onFileSelected(name, path) event — the + /// plugin distinguishes save from open by the widget name. + WidgetData& setSaveFilePicker( + std::string_view name, std::string_view button_text, std::string_view filter, std::string_view title) { + auto& e = entry(name); + e["button_text"] = button_text; + e["action"] = "save_file_picker"; + e["filter"] = filter; + e["title"] = title; + return *this; + } + WidgetData& setFolderPicker(std::string_view name, std::string_view button_text, std::string_view title) { auto& e = entry(name); e["button_text"] = button_text; From 9c60467e0cf25bdd624162273f9a77bbc83cc67d Mon Sep 17 00:00:00 2001 From: alvvm Date: Mon, 22 Jun 2026 09:20:53 +0200 Subject: [PATCH 06/11] chore(sdk): bump to 0.12.0 (PlotMarkers builtin on top of merged 0.11.0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding the PlotMarkers canonical builtin (slot 19) is a MINOR addition over main's 0.11.0, so the merged SDK is a distinct 0.12.0 — not the upstream 0.11.0 (which lacks PlotMarkers). Keeps version identity clean for downstream pins. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BeoYQHmF6ixvYv9KXXgyVh --- CMakeLists.txt | 2 +- conanfile.py | 4 ++-- recipe.yaml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0cf071c..b45781b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -118,7 +118,7 @@ endif() if(PJ_INSTALL_SDK) include(CMakePackageConfigHelpers) - set(PJ_PACKAGE_VERSION "0.11.0") + set(PJ_PACKAGE_VERSION "0.12.0") set(PJ_PACKAGE_CMAKE_DIR ${CMAKE_INSTALL_LIBDIR}/cmake/plotjuggler_sdk) install(EXPORT plotjuggler_sdkTargets diff --git a/conanfile.py b/conanfile.py index dce74df..32034c5 100644 --- a/conanfile.py +++ b/conanfile.py @@ -6,7 +6,7 @@ plugin_sdk — umbrella for plugin authors (base + dialog SDK + parser SDK) plugin_host — umbrella for host loaders (data_source/parser/toolbox/dialog) -A consuming Conan recipe declares e.g. `plotjuggler_sdk/0.11.0` and then: +A consuming Conan recipe declares e.g. `plotjuggler_sdk/0.12.0` and then: find_package(plotjuggler_sdk REQUIRED COMPONENTS plugin_sdk) target_link_libraries(my_plugin PRIVATE plotjuggler_sdk::plugin_sdk) @@ -30,7 +30,7 @@ class PlotjugglerSdkConan(ConanFile): name = "plotjuggler_sdk" - version = "0.11.0" + version = "0.12.0" # Apache-2.0 covers the whole SDK (pj_base + pj_plugins). See LICENSE. license = "Apache-2.0" url = "https://github.com/PlotJuggler/plotjuggler_sdk" diff --git a/recipe.yaml b/recipe.yaml index 2439291..28c93f1 100644 --- a/recipe.yaml +++ b/recipe.yaml @@ -1,7 +1,7 @@ schema_version: 1 context: - version: "0.11.0" + version: "0.12.0" package: name: plotjuggler_sdk From b469d75f33a02ce53d58232639985be54d39c3e7 Mon Sep 17 00:00:00 2001 From: alvvm Date: Mon, 22 Jun 2026 13:42:29 +0200 Subject: [PATCH 07/11] feat(pj_base): add pj.markers.v1 host service (whole-series markers + ephemeral preview) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The whole-series analog of pj.data_processors.v1: a plugin submits a marker generator by data (script + input series + an output marker-topic + params) and the HOST runs it over the whole series and publishes the resulting PlotMarkers to the ObjectStore. Nothing executable crosses the boundary. - PJ_markers_host_vtable_t (plugin_data_api.h): create/remove/list/config + tail-appended set_marker_preview / clear_marker_preview for a live, ephemeral (non-persisted) preview generator read back through the object surface. - MarkersHostView (sdk/plugin_data_api.hpp) + MarkersHostService trait. - params_json {"scope":"all"} makes a global generator publish across every dataset; absent = the active dataset only. - markers_api_test.cpp: fake-host ABI test (forwarding, count-then-fill, borrowed-string lifetime, binary-safe payload, preview slots). Single output_marker_topic (not an outputs[] array): a generator writes exactly one marker topic (global or per-series). NOTE: MINOR version bump (conanfile.py / PJ_PACKAGE_VERSION / recipe.yaml) and the abidiff baseline refresh are pending pre-merge — this commit adds API on a feature branch (WIP) and does not bump the release version yet. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01QsY1SE2yBsX2qXvkgEnX8U --- pj_base/CMakeLists.txt | 1 + pj_base/include/pj_base/plugin_data_api.h | 89 ++++++ .../include/pj_base/sdk/plugin_data_api.hpp | 134 +++++++++ .../include/pj_base/sdk/service_traits.hpp | 13 + pj_base/tests/markers_api_test.cpp | 261 ++++++++++++++++++ 5 files changed, 498 insertions(+) create mode 100644 pj_base/tests/markers_api_test.cpp diff --git a/pj_base/CMakeLists.txt b/pj_base/CMakeLists.txt index c77c5fe..675e5ba 100644 --- a/pj_base/CMakeLists.txt +++ b/pj_base/CMakeLists.txt @@ -74,6 +74,7 @@ if(PJ_BUILD_TESTS) tests/number_parse_test.cpp tests/plugin_data_api_test.cpp tests/data_processors_api_test.cpp + tests/markers_api_test.cpp tests/settings_store_host_test.cpp tests/data_source_protocol_test.cpp # TODO: data_source_plugin_base_test.cpp and diff --git a/pj_base/include/pj_base/plugin_data_api.h b/pj_base/include/pj_base/plugin_data_api.h index 6e43ea2..d662c46 100644 --- a/pj_base/include/pj_base/plugin_data_api.h +++ b/pj_base/include/pj_base/plugin_data_api.h @@ -863,6 +863,95 @@ typedef struct { const PJ_data_processors_host_vtable_t* vtable; } PJ_data_processors_host_t; +/** + * Markers host service ("pj.markers.v1", protocol_version 1). + * + * The WHOLE-SERIES analog of pj.data_processors.v1. A plugin submits a marker + * generator by DATA — nothing executable crosses the boundary — and the HOST runs + * it and owns its lifecycle. Differences from data_processors: + * - The host runs the script over the WHOLE input series (random access, look + * back AND forward), not sample-by-sample, and the result is a discrete + * PlotMarkers set (events / regions / value bands), NOT a derived time series. + * - There is exactly ONE output, addressed by `output_marker_topic` (a single + * marker-topic key): kGlobalMarkerTopic ("__global__") for dataset-global + * markers, or a per-series key (see markerSeriesKey). The host publishes the + * serialized PlotMarkers to the ObjectStore under markerObjectTopicName(that + * key). There is no `outputs` array: a generator never writes more than one + * marker topic. + * + * - id : plugin-namespaced stable key (upsert key); the host scopes + * list/remove/config to the calling plugin (per-plugin isolation, + * identical to data_processors). + * - inputs : series/field names the script reads via series("name"). + * - script : the marker rule source. First-line directive + * ("-- pj-script: ") selects the backend (Luau today). + * BINARY-SAFE PJ_string_view_t {data,size} — never NUL-terminated. + * - params_json : optional host-interpreted JSON. Today one key is recognized: + * {"scope":"all"} makes a global generator (output_marker_topic = + * kGlobalMarkerTopic) publish across EVERY dataset; absent/other = + * the active dataset only. Other keys are reserved/ignored. + * Binary-safe PJ_string_view_t {data,size}. + * + * The host owns execution + the runtime, so a generator survives plugin unload and + * re-runs on data change (the markers recompute live). Adding a future backend + * (WASM/Python) is purely additive — the script slot is opaque bytes. + * + * ABI-APPENDABLE: new slots may be added at the tail; struct_size gates read. + */ +typedef struct PJ_markers_host_vtable_t { + uint32_t protocol_version; // = 1 + uint32_t struct_size; // = sizeof(PJ_markers_host_vtable_t) + + /* [main-thread] Create or replace (upsert by id) a marker generator. The host + * runs `script` over `inputs` and publishes its PlotMarkers to the object topic + * for `output_marker_topic`. Transactional: on failure no partial state is left + * AND any previously published set for this id is preserved. All string arguments + * are borrowed for the duration of the call. */ + bool (*create_marker_generator)( + void* ctx, PJ_string_view_t id, const PJ_string_view_t* inputs, uint64_t input_count, + PJ_string_view_t output_marker_topic, PJ_string_view_t script, PJ_string_view_t params_json, + PJ_error_t* out_error) PJ_NOEXCEPT; + + /* [main-thread] Remove a generator by id. An unknown id is an error. */ + bool (*remove_marker_generator)(void* ctx, PJ_string_view_t id, PJ_error_t* out_error) PJ_NOEXCEPT; + + /* [main-thread] Enumerate the ids of THIS plugin's live generators. + * Count-then-fill: pass capacity 0 to read *out_count, then call again with a + * buffer of that size. On success the first min(capacity, *out_count) entries of + * out_ids are filled and point into host storage valid only until the next call + * on this vtable. */ + bool (*list_marker_generator_ids)( + void* ctx, PJ_string_view_t* out_ids, uint64_t capacity, uint64_t* out_count, PJ_error_t* out_error) PJ_NOEXCEPT; + + /* [main-thread] Read a generator's full recipe as JSON + * {"inputs":[...],"output":"...","params":{...}} for re-edit (e.g. after a session + * reload). *out_recipe_json is borrowed, valid only until the next call on this + * vtable. An unknown id is an error. */ + bool (*marker_generator_config)( + void* ctx, PJ_string_view_t id, PJ_string_view_t* out_recipe_json, PJ_error_t* out_error) PJ_NOEXCEPT; + + /* [main-thread] Set (replace) THE live preview generator for this plugin. The host + * runs `script` over `inputs` EPHEMERALLY — it publishes the resulting PlotMarkers + * to a host-chosen per-plugin preview object topic but does NOT persist it (the + * generator never appears in list/config and is dropped on session save). The + * preview object topic name is returned in *out_preview_object_topic (borrowed, + * valid until the next call on this vtable) so the caller can read the markers back + * through the object read surface to render a live preview. Calling again replaces + * the previous preview. Tail-appended slot (gated by struct_size). */ + bool (*set_marker_preview)( + void* ctx, const PJ_string_view_t* inputs, uint64_t input_count, PJ_string_view_t script, + PJ_string_view_t params_json, PJ_string_view_t* out_preview_object_topic, PJ_error_t* out_error) PJ_NOEXCEPT; + + /* [main-thread] Tear down this plugin's preview generator (and its preview object + * topic). A no-op if no preview is set. Tail-appended slot. */ + bool (*clear_marker_preview)(void* ctx, PJ_error_t* out_error) PJ_NOEXCEPT; +} PJ_markers_host_vtable_t; + +typedef struct { + void* ctx; + const PJ_markers_host_vtable_t* vtable; +} PJ_markers_host_t; + /** * Settings store service ("pj.settings.v1", protocol_version 1). * diff --git a/pj_base/include/pj_base/sdk/plugin_data_api.hpp b/pj_base/include/pj_base/sdk/plugin_data_api.hpp index 839890e..b1a32cf 100644 --- a/pj_base/include/pj_base/sdk/plugin_data_api.hpp +++ b/pj_base/include/pj_base/sdk/plugin_data_api.hpp @@ -1450,6 +1450,140 @@ class DataProcessorsHostView { PJ_data_processors_host_t host_{}; }; +// --------------------------------------------------------------------------- +// MarkersHostView — typed C++ view over PJ_markers_host_t +// --------------------------------------------------------------------------- + +/// C++ wrapper around PJ_markers_host_t for plugins that submit WHOLE-SERIES +/// marker generators to the host (see the C ABI doc-comment on +/// PJ_markers_host_vtable_t). Empty-constructible; `valid()` tells whether the host +/// exposed the service. Strings returned by `list()`/`recipeOf()` are copied into +/// owned values, so they stay valid past the next vtable call. Unlike +/// DataProcessorsHostView there is a single `output_marker_topic`, not an output +/// list: a generator writes exactly one marker topic (global or per-series). +class MarkersHostView { + public: + MarkersHostView() = default; + explicit MarkersHostView(PJ_markers_host_t host) : host_(host) {} + + [[nodiscard]] bool valid() const noexcept { + return host_.vtable != nullptr && host_.ctx != nullptr; + } + + /// Create or replace (upsert by id) a marker generator. `output_marker_topic` is + /// the single target marker-topic key (kGlobalMarkerTopic or markerSeriesKey). + /// `params_json` is forwarded verbatim to the script. + [[nodiscard]] Status createMarkerGenerator( + std::string_view id, Span inputs, std::string_view output_marker_topic, + std::string_view script, std::string_view params_json) const { + if (!valid() || host_.vtable->create_marker_generator == nullptr) { + return unexpected("markers host is not bound"); + } + if (output_marker_topic.empty()) { + return unexpected("marker generator requires an output marker topic"); + } + std::vector in_abi; + in_abi.reserve(inputs.size()); + for (const auto& name : inputs) { + in_abi.push_back(toAbiString(name)); + } + PJ_error_t err{}; + if (!host_.vtable->create_marker_generator( + host_.ctx, toAbiString(id), in_abi.data(), in_abi.size(), toAbiString(output_marker_topic), + toAbiString(script), toAbiString(params_json), &err)) { + return unexpected(errorToString(err)); + } + return okStatus(); + } + + /// Remove a previously created generator by id. + [[nodiscard]] Status remove(std::string_view id) const { + if (!valid() || host_.vtable->remove_marker_generator == nullptr) { + return unexpected("markers host is not bound"); + } + PJ_error_t err{}; + if (!host_.vtable->remove_marker_generator(host_.ctx, toAbiString(id), &err)) { + return unexpected(errorToString(err)); + } + return okStatus(); + } + + /// Enumerate the ids of this plugin's live generators (owned copies). + [[nodiscard]] Expected> list() const { + if (!valid() || host_.vtable->list_marker_generator_ids == nullptr) { + return unexpected("markers host is not bound"); + } + PJ_error_t err{}; + uint64_t count = 0; + if (!host_.vtable->list_marker_generator_ids(host_.ctx, nullptr, 0, &count, &err)) { + return unexpected(errorToString(err)); + } + std::vector borrowed(count); + uint64_t filled = 0; + if (count != 0 && + !host_.vtable->list_marker_generator_ids(host_.ctx, borrowed.data(), borrowed.size(), &filled, &err)) { + return unexpected(errorToString(err)); + } + std::vector ids; + ids.reserve(filled); + for (uint64_t i = 0; i < filled; ++i) { + ids.emplace_back(toStringView(borrowed[i])); + } + return ids; + } + + /// Read a generator's full recipe JSON (owned copy) for re-edit. + [[nodiscard]] Expected recipeOf(std::string_view id) const { + if (!valid() || host_.vtable->marker_generator_config == nullptr) { + return unexpected("markers host is not bound"); + } + PJ_error_t err{}; + PJ_string_view_t recipe{}; + if (!host_.vtable->marker_generator_config(host_.ctx, toAbiString(id), &recipe, &err)) { + return unexpected(errorToString(err)); + } + return std::string(toStringView(recipe)); + } + + /// Set (replace) the live, EPHEMERAL preview generator: the host runs `script` over + /// `inputs` and publishes the markers to a preview object topic without persisting + /// it. Returns the preview object topic name (owned copy) so the caller can read the + /// markers back through the object read surface. `params_json` is forwarded verbatim. + [[nodiscard]] Expected setPreview( + Span inputs, std::string_view script, std::string_view params_json) const { + if (!valid() || host_.vtable->set_marker_preview == nullptr) { + return unexpected("markers host does not support preview"); + } + std::vector in_abi; + in_abi.reserve(inputs.size()); + for (const auto& name : inputs) { + in_abi.push_back(toAbiString(name)); + } + PJ_error_t err{}; + PJ_string_view_t topic{}; + if (!host_.vtable->set_marker_preview( + host_.ctx, in_abi.data(), in_abi.size(), toAbiString(script), toAbiString(params_json), &topic, &err)) { + return unexpected(errorToString(err)); + } + return std::string(toStringView(topic)); + } + + /// Tear down the live preview generator (and its preview object topic). No-op if none. + [[nodiscard]] Status clearPreview() const { + if (!valid() || host_.vtable->clear_marker_preview == nullptr) { + return unexpected("markers host does not support preview"); + } + PJ_error_t err{}; + if (!host_.vtable->clear_marker_preview(host_.ctx, &err)) { + return unexpected(errorToString(err)); + } + return okStatus(); + } + + private: + PJ_markers_host_t host_{}; +}; + // --------------------------------------------------------------------------- // SettingsView — typed C++ view over PJ_settings_store_t // --------------------------------------------------------------------------- diff --git a/pj_base/include/pj_base/sdk/service_traits.hpp b/pj_base/include/pj_base/sdk/service_traits.hpp index 4cc4eff..e0d83a2 100644 --- a/pj_base/include/pj_base/sdk/service_traits.hpp +++ b/pj_base/include/pj_base/sdk/service_traits.hpp @@ -171,6 +171,19 @@ struct DataProcessorsHostService { static_assert(detail::isValidServiceName(kName), "kName must match the pj naming rule"); }; +/// Optional service for plugins that submit WHOLE-SERIES marker generators to the +/// host by data (script + input names + a single output marker-topic + params). +/// The host runs the script and publishes the resulting PlotMarkers. The marker +/// analog of DataProcessorsHostService; see PJ_markers_host_vtable_t. +struct MarkersHostService { + static constexpr const char* kName = "pj.markers.v1"; + static constexpr uint32_t kMinVersion = 1; + using Raw = PJ_markers_host_t; + using Vtable = PJ_markers_host_vtable_t; + using View = MarkersHostView; + static_assert(detail::isValidServiceName(kName), "kName must match the pj naming rule"); +}; + /// Optional QSettings-like key/value persistence exposed to any plugin family. /// Host-backed (QSettings in the GUI app, JSON in a headless host); keys are /// namespaced per plugin by the host. diff --git a/pj_base/tests/markers_api_test.cpp b/pj_base/tests/markers_api_test.cpp new file mode 100644 index 0000000..8021b8d --- /dev/null +++ b/pj_base/tests/markers_api_test.cpp @@ -0,0 +1,261 @@ +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#include + +#include +#include +#include +#include + +#include "pj_base/plugin_data_api.h" +#include "pj_base/sdk/plugin_data_api.hpp" +#include "pj_base/span.hpp" + +namespace PJ { +namespace { + +// Fake host: records the last create/remove call and serves list/config from +// host-owned storage (so the borrowed-string lifetime contract can be tested). +struct FakeMarkersHost { + bool create_called = false; + bool create_should_fail = false; + std::string last_id; + std::vector last_inputs; + std::string last_output; + std::string last_script; + std::string last_params; + + std::string removed_id; + + std::vector stored_ids; // host storage the listed views point into + std::string recipe_storage; // host storage the recipe view points into + + bool preview_set = false; + bool preview_cleared = false; + std::string last_preview_script; + std::string preview_topic_storage = "__markers__/__preview__/anomaly"; // host-owned, borrowed back +}; + +bool mkCreate( + void* ctx, PJ_string_view_t id, const PJ_string_view_t* inputs, uint64_t input_count, + PJ_string_view_t output_marker_topic, PJ_string_view_t script, PJ_string_view_t params_json, + PJ_error_t* out_error) noexcept { + auto* self = static_cast(ctx); + if (self->create_should_fail) { + if (out_error != nullptr) { + sdk::fillError(out_error, 1, "markers", "create boom"); + } + return false; + } + self->create_called = true; + self->last_id = std::string(sdk::toStringView(id)); + self->last_inputs.clear(); + for (uint64_t i = 0; i < input_count; ++i) { + self->last_inputs.emplace_back(sdk::toStringView(inputs[i])); + } + self->last_output = std::string(sdk::toStringView(output_marker_topic)); + self->last_script = std::string(sdk::toStringView(script)); + self->last_params = std::string(sdk::toStringView(params_json)); + return true; +} + +bool mkRemove(void* ctx, PJ_string_view_t id, PJ_error_t* /*out_error*/) noexcept { + static_cast(ctx)->removed_id = std::string(sdk::toStringView(id)); + return true; +} + +bool mkList( + void* ctx, PJ_string_view_t* out_ids, uint64_t capacity, uint64_t* out_count, PJ_error_t* /*out_error*/) noexcept { + auto* self = static_cast(ctx); + *out_count = self->stored_ids.size(); + const uint64_t n = std::min(capacity, self->stored_ids.size()); + for (uint64_t i = 0; i < n; ++i) { + out_ids[i] = sdk::toAbiString(self->stored_ids[i]); + } + return true; +} + +bool mkConfig( + void* ctx, PJ_string_view_t /*id*/, PJ_string_view_t* out_recipe_json, PJ_error_t* /*out_error*/) noexcept { + out_recipe_json[0] = sdk::toAbiString(static_cast(ctx)->recipe_storage); + return true; +} + +bool mkSetPreview( + void* ctx, const PJ_string_view_t* /*inputs*/, uint64_t /*input_count*/, PJ_string_view_t script, + PJ_string_view_t /*params_json*/, PJ_string_view_t* out_preview_object_topic, PJ_error_t* /*out_error*/) noexcept { + auto* self = static_cast(ctx); + self->preview_set = true; + self->last_preview_script = std::string(sdk::toStringView(script)); + out_preview_object_topic[0] = sdk::toAbiString(self->preview_topic_storage); + return true; +} + +bool mkClearPreview(void* ctx, PJ_error_t* /*out_error*/) noexcept { + static_cast(ctx)->preview_cleared = true; + return true; +} + +PJ_markers_host_vtable_t makeVtable() { + return PJ_markers_host_vtable_t{ + .protocol_version = 1, + .struct_size = sizeof(PJ_markers_host_vtable_t), + .create_marker_generator = mkCreate, + .remove_marker_generator = mkRemove, + .list_marker_generator_ids = mkList, + .marker_generator_config = mkConfig, + .set_marker_preview = mkSetPreview, + .clear_marker_preview = mkClearPreview, + }; +} + +TEST(MarkersApiTest, CreateForwardsAllArgsIntact) { + FakeMarkersHost host; + const auto vtable = makeVtable(); + sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); + + const std::string_view inputs[] = {"imu/accel/x", "imu/accel/y"}; + auto status = view.createMarkerGenerator( + "vib_detect", PJ::Span(inputs), "__global__", "-- pj-script: lua\nreturn {}", + R"({"threshold":3.0})"); + + ASSERT_TRUE(status) << status.error(); + EXPECT_TRUE(host.create_called); + EXPECT_EQ(host.last_id, "vib_detect"); + ASSERT_EQ(host.last_inputs.size(), 2u); + EXPECT_EQ(host.last_inputs[0], "imu/accel/x"); + EXPECT_EQ(host.last_inputs[1], "imu/accel/y"); + EXPECT_EQ(host.last_output, "__global__"); + EXPECT_EQ(host.last_script, "-- pj-script: lua\nreturn {}"); + EXPECT_EQ(host.last_params, R"({"threshold":3.0})"); +} + +TEST(MarkersApiTest, CreateFailureSurfacesError) { + FakeMarkersHost host; + host.create_should_fail = true; + const auto vtable = makeVtable(); + sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); + + auto status = view.createMarkerGenerator("x", PJ::Span{}, "topic", "s", "{}"); + + EXPECT_FALSE(status); + EXPECT_NE(status.error().find("create boom"), std::string::npos); + EXPECT_FALSE(host.create_called); +} + +TEST(MarkersApiTest, RemoveForwardsId) { + FakeMarkersHost host; + const auto vtable = makeVtable(); + sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); + + ASSERT_TRUE(view.remove("vib_detect")); + EXPECT_EQ(host.removed_id, "vib_detect"); +} + +TEST(MarkersApiTest, ListCountThenFillReturnsOwnedCopies) { + FakeMarkersHost host; + host.stored_ids = {"alpha", "beta", "gamma"}; + const auto vtable = makeVtable(); + sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); + + auto ids = view.list(); + ASSERT_TRUE(ids) << ids.error(); + ASSERT_EQ(ids->size(), 3u); + EXPECT_EQ((*ids)[0], "alpha"); + EXPECT_EQ((*ids)[2], "gamma"); + + host.stored_ids[0] = "clobbered"; + EXPECT_EQ((*ids)[0], "alpha"); // owned copy +} + +TEST(MarkersApiTest, RecipeOfCopiesBorrowedJson) { + FakeMarkersHost host; + host.recipe_storage = R"({"inputs":["a"],"output":"__global__","params":{}})"; + const auto vtable = makeVtable(); + sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); + + auto recipe = view.recipeOf("id"); + ASSERT_TRUE(recipe) << recipe.error(); + const std::string expected = host.recipe_storage; + + host.recipe_storage = "CLOBBERED"; + EXPECT_EQ(*recipe, expected); // owned copy +} + +TEST(MarkersApiTest, UnboundViewReportsNotBound) { + sdk::MarkersHostView view; // default-constructed = not bound + EXPECT_FALSE(view.valid()); + + auto status = view.createMarkerGenerator("x", PJ::Span{}, "topic", "s", "{}"); + + EXPECT_FALSE(status); + EXPECT_NE(status.error().find("not bound"), std::string::npos); +} + +// The view fails fast on an empty output marker topic: a generator must address +// exactly one target (global or per-series). The guard lives in the view so a +// misuse never reaches the vtable (the host enforces it authoritatively too). +TEST(MarkersApiTest, CreateRejectsEmptyOutputTopic) { + FakeMarkersHost host; + const auto vtable = makeVtable(); + sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); + + auto status = + view.createMarkerGenerator("x", PJ::Span{}, "", "-- pj-script: lua\nreturn {}", "{}"); + + EXPECT_FALSE(status); + EXPECT_NE(status.error().find("output"), std::string::npos); + EXPECT_FALSE(host.create_called); // rejected before the ABI call +} + +// Regression guard: `script` / `params_json` are PJ_string_view_t {data,size} -- +// binary-safe, NOT NUL-terminated. A payload carrying embedded NULs and the WASM +// "\0asm" magic round-trips byte-for-byte (keeps the WASM/Python backend door open). +TEST(MarkersApiTest, BinarySafePayloadRoundTrips) { + FakeMarkersHost host; + const auto vtable = makeVtable(); + sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); + + const char wasm_bytes[] = {'\0', 'a', 's', 'm', '\x01', '\0', '\0', '\0', 'X', 'Y'}; + const std::string blob(wasm_bytes, sizeof(wasm_bytes)); // 10 bytes, 3 embedded NULs + ASSERT_EQ(blob.size(), 10u); + + auto status = view.createMarkerGenerator( + "fft", PJ::Span{}, "__global__", std::string_view(blob.data(), blob.size()), "{}"); + + ASSERT_TRUE(status) << status.error(); + EXPECT_EQ(host.last_script.size(), blob.size()); // not truncated at the first NUL + EXPECT_EQ(host.last_script, blob); +} + +// The preview slot forwards the script and returns the (borrowed) preview object +// topic name as an owned copy for read-back. +TEST(MarkersApiTest, SetPreviewForwardsScriptAndReturnsTopic) { + FakeMarkersHost host; + const auto vtable = makeVtable(); + sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); + + const std::string_view inputs[] = {"in"}; + auto topic = view.setPreview(PJ::Span(inputs), "createEvent(0.0)", "{}"); + ASSERT_TRUE(topic) << topic.error(); + EXPECT_TRUE(host.preview_set); + EXPECT_EQ(host.last_preview_script, "createEvent(0.0)"); + EXPECT_EQ(*topic, host.preview_topic_storage); + + const std::string expected = host.preview_topic_storage; + host.preview_topic_storage = "CLOBBERED"; // owned copy survives + EXPECT_EQ(*topic, expected); +} + +TEST(MarkersApiTest, ClearPreviewForwards) { + FakeMarkersHost host; + const auto vtable = makeVtable(); + sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); + + ASSERT_TRUE(view.clearPreview()); + EXPECT_TRUE(host.preview_cleared); +} + +} // namespace +} // namespace PJ From 9853aa9ffd364dc35691e53c7b4261dd79d41271 Mon Sep 17 00:00:00 2001 From: alvvm Date: Fri, 26 Jun 2026 13:27:07 +0200 Subject: [PATCH 08/11] feat(sdk)!: replace pj.markers.v1 with unified pj.generators.v1 service Unify the whole-series host-driven service under one SDK contract, PJ_generators_host_vtable_t / GeneratorsHostService ("pj.generators.v1"), with a string `kind` discriminator: - kind="markers" (objects -> ObjectStore) is implemented; shared `language` param, compile-only validate_script, ephemeral-preview flag, and out_topics return apply across kinds. - kind="transform" (per-sample timeseries -> DerivedEngine) is RESERVED, not implemented; its end-state home is decided when the transform-editor work merges. Removes PJ_markers_host_vtable_t / MarkersHostService (the old pj.markers.v1) and markers_api_test; adds generators_api_test. A speculative kind="field" that briefly existed on this branch was dropped (zero consumers, violated the object/timeseries engine split); it changed no ABI struct since `kind` is a runtime string. Versioning: API removal, normally MAJOR. Ships as 0.13.0 because no public tag ever carried pj.markers.v1 (no released plugin breaks). The first public release carrying pj.generators.v1 MUST be tagged 1.0.0 -- recorded in CHANGELOG.md + a comment by `version` in conanfile.py. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_013N1nzqco4Y9AQhnQgXo5tQ --- CHANGELOG.md | 28 ++ CMakeLists.txt | 2 +- conanfile.py | 10 +- pj_base/CMakeLists.txt | 2 +- .../include/pj_base/builtin/plot_markers.hpp | 16 + pj_base/include/pj_base/plugin_data_api.h | 139 ++++---- .../include/pj_base/sdk/plugin_data_api.hpp | 163 ++++++---- .../include/pj_base/sdk/service_traits.hpp | 20 +- pj_base/tests/generators_api_test.cpp | 306 ++++++++++++++++++ pj_base/tests/markers_api_test.cpp | 261 --------------- recipe.yaml | 2 +- 11 files changed, 537 insertions(+), 412 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 pj_base/tests/generators_api_test.cpp delete mode 100644 pj_base/tests/markers_api_test.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..88b7eb4 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,28 @@ +# Changelog + +All notable changes to `plotjuggler_sdk` are recorded here. Versioning policy is in +[`CLAUDE.md`](./CLAUDE.md) → "Release Versioning". + +## [Unreleased] — on branch `feature/plot-markers`, not yet publicly tagged + +### Host service: `pj.markers.v1` → `pj.generators.v1` (UNRELEASED BREAK) + +The whole-series host-driven service was unified under a single contract with a string +`kind` discriminator: + +- **Removed** `PJ_markers_host_vtable_t` / `MarkersHostService` (the old `pj.markers.v1`). +- **Added** `PJ_generators_host_vtable_t` / `GeneratorsHostService` (`pj.generators.v1`) + with `kind="markers"` implemented (objects → ObjectStore), a `language` param, + compile-only `validate_script`, an ephemeral-preview flag, and `out_topics` return. +- `kind="transform"` (per-sample timeseries → DerivedEngine) is **reserved**, not + implemented; its end-state home (a generators `kind` vs the separate + `pj.data_processors.v1` service) is decided when the transform-editor work merges. +- A speculative `kind="field"` (object engine emitting a DataEngine series) that + briefly existed on this branch was **removed** before release — zero consumers, and + it violated the object/timeseries engine split. Its removal changed no ABI struct + (`kind` is a runtime string, not a vtable enum), so it carried no version bump. + +**Versioning note.** This is an API removal — normally MAJOR. It ships as `0.13.0` +only because no PUBLIC tag ever contained `pj.markers.v1` (it never left this branch), +so no released plugin is broken. **The first public release that carries +`pj.generators.v1` must be tagged `1.0.0`** per the pre-1.0 break rule in `CLAUDE.md`. diff --git a/CMakeLists.txt b/CMakeLists.txt index b45781b..5862048 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -118,7 +118,7 @@ endif() if(PJ_INSTALL_SDK) include(CMakePackageConfigHelpers) - set(PJ_PACKAGE_VERSION "0.12.0") + set(PJ_PACKAGE_VERSION "0.13.0") set(PJ_PACKAGE_CMAKE_DIR ${CMAKE_INSTALL_LIBDIR}/cmake/plotjuggler_sdk) install(EXPORT plotjuggler_sdkTargets diff --git a/conanfile.py b/conanfile.py index 32034c5..7a66f4c 100644 --- a/conanfile.py +++ b/conanfile.py @@ -6,7 +6,7 @@ plugin_sdk — umbrella for plugin authors (base + dialog SDK + parser SDK) plugin_host — umbrella for host loaders (data_source/parser/toolbox/dialog) -A consuming Conan recipe declares e.g. `plotjuggler_sdk/0.12.0` and then: +A consuming Conan recipe declares e.g. `plotjuggler_sdk/0.13.0` and then: find_package(plotjuggler_sdk REQUIRED COMPONENTS plugin_sdk) target_link_libraries(my_plugin PRIVATE plotjuggler_sdk::plugin_sdk) @@ -30,7 +30,13 @@ class PlotjugglerSdkConan(ConanFile): name = "plotjuggler_sdk" - version = "0.12.0" + # UNRELEASED BREAK: 0.13.0 replaces the host service `pj.markers.v1` with + # `pj.generators.v1` (removed `PJ_markers_host_vtable_t` / `MarkersHostService`). + # This is an API removal — normally MAJOR — but no PUBLIC tag ever shipped + # `pj.markers.v1` (it lived only on this branch), so no released plugin breaks and + # 0.13.0 stays a valid pre-1.0 step. The FIRST public release that carries + # `pj.generators.v1` MUST be tagged 1.0.0. See CHANGELOG.md. + version = "0.13.0" # Apache-2.0 covers the whole SDK (pj_base + pj_plugins). See LICENSE. license = "Apache-2.0" url = "https://github.com/PlotJuggler/plotjuggler_sdk" diff --git a/pj_base/CMakeLists.txt b/pj_base/CMakeLists.txt index 675e5ba..f3e85ef 100644 --- a/pj_base/CMakeLists.txt +++ b/pj_base/CMakeLists.txt @@ -74,7 +74,7 @@ if(PJ_BUILD_TESTS) tests/number_parse_test.cpp tests/plugin_data_api_test.cpp tests/data_processors_api_test.cpp - tests/markers_api_test.cpp + tests/generators_api_test.cpp tests/settings_store_host_test.cpp tests/data_source_protocol_test.cpp # TODO: data_source_plugin_base_test.cpp and diff --git a/pj_base/include/pj_base/builtin/plot_markers.hpp b/pj_base/include/pj_base/builtin/plot_markers.hpp index ad29151..75fcc3f 100644 --- a/pj_base/include/pj_base/builtin/plot_markers.hpp +++ b/pj_base/include/pj_base/builtin/plot_markers.hpp @@ -124,6 +124,22 @@ inline constexpr std::string_view kMarkerObjectTopicPrefix = "__markers__/"; return name; } +/// Reserved marker-topic infix for an EPHEMERAL preview set (a generator created +/// with PJ_GENERATOR_FLAG_EPHEMERAL). The host addresses a preview to the marker +/// topic `kPreviewMarkerTopic + ` so its object topic sorts under the +/// marker namespace and renders like any set, yet is recognizable as throwaway and +/// excluded from session save. The host and the plot overlay MUST agree on this. +inline constexpr std::string_view kPreviewMarkerTopic = "__preview__/"; + +/// True if `marker_topic` (or its object-topic form) names an ephemeral preview set. +/// Accepts either the bare marker topic or the markerObjectTopicName() form. +[[nodiscard]] inline bool isPreviewMarkerTopic(std::string_view topic) { + if (topic.starts_with(kMarkerObjectTopicPrefix)) { + topic.remove_prefix(kMarkerObjectTopicPrefix.size()); + } + return topic.starts_with(kPreviewMarkerTopic); +} + /// Per-series marker topic key from a curve's (topic_name, field_name). The /// producer (e.g. the markers toolbox, via the host catalog_key_resolver) and /// the plot overlay MUST build this key identically, so they share this helper. diff --git a/pj_base/include/pj_base/plugin_data_api.h b/pj_base/include/pj_base/plugin_data_api.h index d662c46..e005e6a 100644 --- a/pj_base/include/pj_base/plugin_data_api.h +++ b/pj_base/include/pj_base/plugin_data_api.h @@ -864,93 +864,96 @@ typedef struct { } PJ_data_processors_host_t; /** - * Markers host service ("pj.markers.v1", protocol_version 1). - * - * The WHOLE-SERIES analog of pj.data_processors.v1. A plugin submits a marker - * generator by DATA — nothing executable crosses the boundary — and the HOST runs - * it and owns its lifecycle. Differences from data_processors: - * - The host runs the script over the WHOLE input series (random access, look - * back AND forward), not sample-by-sample, and the result is a discrete - * PlotMarkers set (events / regions / value bands), NOT a derived time series. - * - There is exactly ONE output, addressed by `output_marker_topic` (a single - * marker-topic key): kGlobalMarkerTopic ("__global__") for dataset-global - * markers, or a per-series key (see markerSeriesKey). The host publishes the - * serialized PlotMarkers to the ObjectStore under markerObjectTopicName(that - * key). There is no `outputs` array: a generator never writes more than one - * marker topic. - * + * Generators host service ("pj.generators.v1", protocol_version 1). + * + * One host-driven service for plugins that submit WHOLE-SERIES *generators* by + * DATA — nothing executable crosses the boundary; the HOST compiles, runs, and + * owns the lifecycle (a generator survives plugin unload and re-runs on data + * change). A single polymorphic surface covers every output shape via a `kind` + * discriminator: + * - kind="markers": the script emits a discrete PlotMarkers set (events / + * regions / value bands). Exactly one output topic; the host publishes the + * serialized PlotMarkers to the ObjectStore under markerObjectTopicName(key). + * ("transform" — per-sample numeric series materialized into the DataEngine via + * the DerivedEngine — is reserved for the MIMO backend, added later. Timeseries + * outputs come from transform generators, never from object/marker generators.) + * + * Arguments shared by every kind: * - id : plugin-namespaced stable key (upsert key); the host scopes - * list/remove/config to the calling plugin (per-plugin isolation, - * identical to data_processors). + * list/remove/config to the calling plugin (per-plugin isolation). + * - kind : output discriminator, see above ("markers"; "transform" reserved). + * - language : script backend, "luau" today; the host rejects anything else. * - inputs : series/field names the script reads via series("name"). - * - script : the marker rule source. First-line directive - * ("-- pj-script: ") selects the backend (Luau today). - * BINARY-SAFE PJ_string_view_t {data,size} — never NUL-terminated. - * - params_json : optional host-interpreted JSON. Today one key is recognized: - * {"scope":"all"} makes a global generator (output_marker_topic = - * kGlobalMarkerTopic) publish across EVERY dataset; absent/other = - * the active dataset only. Other keys are reserved/ignored. - * Binary-safe PJ_string_view_t {data,size}. - * - * The host owns execution + the runtime, so a generator survives plugin unload and - * re-runs on data change (the markers recompute live). Adding a future backend - * (WASM/Python) is purely additive — the script slot is opaque bytes. + * - outputs : target topic key(s). markers: a single marker-topic key + * (kGlobalMarkerTopic or a per-series key, see markerSeriesKey). + * MAY be empty for an ephemeral preview, in which case the host + * names them and returns the resolved names in out_topics. + * - script : the rule source, opaque/binary-safe PJ_string_view_t {data,size}. + * - params_json : optional host-interpreted JSON. {"scope":"all"} makes a global + * generator publish across EVERY dataset; absent = active dataset. + * - flags : bitset; PJ_GENERATOR_FLAG_EPHEMERAL marks a preview (never + * persisted, dropped on remove). Reserved bits must be 0. * * ABI-APPENDABLE: new slots may be added at the tail; struct_size gates read. */ -typedef struct PJ_markers_host_vtable_t { - uint32_t protocol_version; // = 1 - uint32_t struct_size; // = sizeof(PJ_markers_host_vtable_t) - - /* [main-thread] Create or replace (upsert by id) a marker generator. The host - * runs `script` over `inputs` and publishes its PlotMarkers to the object topic - * for `output_marker_topic`. Transactional: on failure no partial state is left - * AND any previously published set for this id is preserved. All string arguments - * are borrowed for the duration of the call. */ - bool (*create_marker_generator)( - void* ctx, PJ_string_view_t id, const PJ_string_view_t* inputs, uint64_t input_count, - PJ_string_view_t output_marker_topic, PJ_string_view_t script, PJ_string_view_t params_json, - PJ_error_t* out_error) PJ_NOEXCEPT; - /* [main-thread] Remove a generator by id. An unknown id is an error. */ - bool (*remove_marker_generator)(void* ctx, PJ_string_view_t id, PJ_error_t* out_error) PJ_NOEXCEPT; +/* create_generator flags. */ +#define PJ_GENERATOR_FLAG_EPHEMERAL (1u << 0) /* preview: never persisted, dropped on remove_generator */ + +typedef struct PJ_generators_host_vtable_t { + uint32_t protocol_version; // = 1 + uint32_t struct_size; // = sizeof(PJ_generators_host_vtable_t) + + /* [main-thread] Create or replace (upsert by id) a generator of `kind`. The host + * compiles `script` with `language`, runs it over `inputs`, and routes the output + * by kind (markers -> ObjectStore; transform [reserved] -> DerivedEngine). `outputs` + * may be empty for an ephemeral preview, in which case the host auto-names the sink(s). + * The resolved physical topic name(s) are written to out_topics using the + * count-then-fill convention: pass out_topics=NULL/capacity=0 to read *out_count, + * or a buffer to receive min(capacity,*out_count) entries (borrowed, valid only + * until the next call on this vtable); pass out_count=NULL to ignore them. + * Transactional: on failure no partial state is left AND any previously published + * output for this id is preserved. All string arguments are borrowed for the call. */ + bool (*create_generator)( + void* ctx, PJ_string_view_t id, PJ_string_view_t kind, PJ_string_view_t language, + const PJ_string_view_t* inputs, uint64_t input_count, const PJ_string_view_t* outputs, uint64_t output_count, + PJ_string_view_t script, PJ_string_view_t params_json, uint32_t flags, PJ_string_view_t* out_topics, + uint64_t out_topics_capacity, uint64_t* out_topics_count, PJ_error_t* out_error) PJ_NOEXCEPT; + + /* [main-thread] Remove a generator by id (persistent or ephemeral). Unknown id is an error. */ + bool (*remove_generator)(void* ctx, PJ_string_view_t id, PJ_error_t* out_error) PJ_NOEXCEPT; /* [main-thread] Enumerate the ids of THIS plugin's live generators. * Count-then-fill: pass capacity 0 to read *out_count, then call again with a * buffer of that size. On success the first min(capacity, *out_count) entries of * out_ids are filled and point into host storage valid only until the next call - * on this vtable. */ - bool (*list_marker_generator_ids)( + * on this vtable. Ephemeral previews are excluded. */ + bool (*list_generator_ids)( void* ctx, PJ_string_view_t* out_ids, uint64_t capacity, uint64_t* out_count, PJ_error_t* out_error) PJ_NOEXCEPT; /* [main-thread] Read a generator's full recipe as JSON - * {"inputs":[...],"output":"...","params":{...}} for re-edit (e.g. after a session - * reload). *out_recipe_json is borrowed, valid only until the next call on this - * vtable. An unknown id is an error. */ - bool (*marker_generator_config)( + * {"kind":"...","language":"...","inputs":[...],"outputs":[...],"params":{...}} for + * re-edit (e.g. after a session reload). *out_recipe_json is borrowed, valid only + * until the next call on this vtable. An unknown id is an error. */ + bool (*generator_config)( void* ctx, PJ_string_view_t id, PJ_string_view_t* out_recipe_json, PJ_error_t* out_error) PJ_NOEXCEPT; - /* [main-thread] Set (replace) THE live preview generator for this plugin. The host - * runs `script` over `inputs` EPHEMERALLY — it publishes the resulting PlotMarkers - * to a host-chosen per-plugin preview object topic but does NOT persist it (the - * generator never appears in list/config and is dropped on session save). The - * preview object topic name is returned in *out_preview_object_topic (borrowed, - * valid until the next call on this vtable) so the caller can read the markers back - * through the object read surface to render a live preview. Calling again replaces - * the previous preview. Tail-appended slot (gated by struct_size). */ - bool (*set_marker_preview)( - void* ctx, const PJ_string_view_t* inputs, uint64_t input_count, PJ_string_view_t script, - PJ_string_view_t params_json, PJ_string_view_t* out_preview_object_topic, PJ_error_t* out_error) PJ_NOEXCEPT; - - /* [main-thread] Tear down this plugin's preview generator (and its preview object - * topic). A no-op if no preview is set. Tail-appended slot. */ - bool (*clear_marker_preview)(void* ctx, PJ_error_t* out_error) PJ_NOEXCEPT; -} PJ_markers_host_vtable_t; + /* [main-thread] Validate a script WITHOUT installing anything: the host compiles + * it with `language` for the given `kind` to catch syntax and module-load errors. + * Returns true if it compiles; on false, *out_error carries the reason (suitable + * for a UI semaphore / error box). Compilation-only: it does NOT run the script + * over data, so a runtime error or empty output is not detected here (use an + * ephemeral create for that). No node, topic or catalog entry is created. An + * unknown language or kind is an error. */ + bool (*validate_script)( + void* ctx, PJ_string_view_t kind, PJ_string_view_t language, PJ_string_view_t script, + PJ_string_view_t params_json, PJ_error_t* out_error) PJ_NOEXCEPT; +} PJ_generators_host_vtable_t; typedef struct { void* ctx; - const PJ_markers_host_vtable_t* vtable; -} PJ_markers_host_t; + const PJ_generators_host_vtable_t* vtable; +} PJ_generators_host_t; /** * Settings store service ("pj.settings.v1", protocol_version 1). diff --git a/pj_base/include/pj_base/sdk/plugin_data_api.hpp b/pj_base/include/pj_base/sdk/plugin_data_api.hpp index b1a32cf..bee5086 100644 --- a/pj_base/include/pj_base/sdk/plugin_data_api.hpp +++ b/pj_base/include/pj_base/sdk/plugin_data_api.hpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -1451,77 +1452,118 @@ class DataProcessorsHostView { }; // --------------------------------------------------------------------------- -// MarkersHostView — typed C++ view over PJ_markers_host_t +// GeneratorsHostView — typed C++ view over PJ_generators_host_t // --------------------------------------------------------------------------- -/// C++ wrapper around PJ_markers_host_t for plugins that submit WHOLE-SERIES -/// marker generators to the host (see the C ABI doc-comment on -/// PJ_markers_host_vtable_t). Empty-constructible; `valid()` tells whether the host -/// exposed the service. Strings returned by `list()`/`recipeOf()` are copied into -/// owned values, so they stay valid past the next vtable call. Unlike -/// DataProcessorsHostView there is a single `output_marker_topic`, not an output -/// list: a generator writes exactly one marker topic (global or per-series). -class MarkersHostView { +/// C++ wrapper around PJ_generators_host_t for plugins that submit WHOLE-SERIES +/// generators to the host (see the C ABI doc-comment on PJ_generators_host_vtable_t). +/// Empty-constructible; `valid()` tells whether the host exposed the service. Strings +/// returned by `list()`/`configOf()`/`createGenerator()` are copied into owned values, +/// so they stay valid past the next vtable call. One polymorphic surface serves every +/// `kind` (today "markers"; "transform" reserved); preview is +/// `createGenerator(..., flags=EPHEMERAL)` and teardown is `remove(id)` — there are no +/// dedicated preview verbs. +class GeneratorsHostView { public: - MarkersHostView() = default; - explicit MarkersHostView(PJ_markers_host_t host) : host_(host) {} + GeneratorsHostView() = default; + explicit GeneratorsHostView(PJ_generators_host_t host) : host_(host) {} [[nodiscard]] bool valid() const noexcept { return host_.vtable != nullptr && host_.ctx != nullptr; } - /// Create or replace (upsert by id) a marker generator. `output_marker_topic` is - /// the single target marker-topic key (kGlobalMarkerTopic or markerSeriesKey). - /// `params_json` is forwarded verbatim to the script. - [[nodiscard]] Status createMarkerGenerator( - std::string_view id, Span inputs, std::string_view output_marker_topic, - std::string_view script, std::string_view params_json) const { - if (!valid() || host_.vtable->create_marker_generator == nullptr) { - return unexpected("markers host is not bound"); - } - if (output_marker_topic.empty()) { - return unexpected("marker generator requires an output marker topic"); + /// Create or replace (upsert by id) a generator of `kind` (today "markers"; the + /// "transform" backend is reserved). `outputs` may be empty for an ephemeral preview + /// (flags & PJ_GENERATOR_FLAG_EPHEMERAL), + /// in which case the host names the sink(s). Returns the resolved physical output + /// topic name(s) (owned copies) so the caller can read results back through the kind's + /// read surface. `params_json` is forwarded verbatim to the script. + [[nodiscard]] Expected> createGenerator( + std::string_view id, std::string_view kind, std::string_view language, Span inputs, + Span outputs, std::string_view script, std::string_view params_json, + uint32_t flags = 0) const { + if (!valid() || host_.vtable->create_generator == nullptr) { + return unexpected("generators host is not bound"); } std::vector in_abi; in_abi.reserve(inputs.size()); for (const auto& name : inputs) { in_abi.push_back(toAbiString(name)); } + std::vector out_abi; + out_abi.reserve(outputs.size()); + for (const auto& name : outputs) { + out_abi.push_back(toAbiString(name)); + } PJ_error_t err{}; - if (!host_.vtable->create_marker_generator( - host_.ctx, toAbiString(id), in_abi.data(), in_abi.size(), toAbiString(output_marker_topic), - toAbiString(script), toAbiString(params_json), &err)) { + // The resolved sink name(s) are filled on the SAME upsert call. Pre-size to the + // caller's output count (the host resolves exactly that many for non-empty + // outputs); for an auto-named ephemeral preview reserve headroom and grow once if + // the host resolved more than fit — a re-upsert with identical args is idempotent. + uint64_t capacity = out_abi.empty() ? 8 : out_abi.size(); + std::vector resolved(capacity); + uint64_t count = 0; + if (!host_.vtable->create_generator( + host_.ctx, toAbiString(id), toAbiString(kind), toAbiString(language), in_abi.data(), in_abi.size(), + out_abi.data(), out_abi.size(), toAbiString(script), toAbiString(params_json), flags, resolved.data(), + resolved.size(), &count, &err)) { return unexpected(errorToString(err)); } - return okStatus(); + if (count > capacity) { + resolved.assign(count, PJ_string_view_t{}); + if (!host_.vtable->create_generator( + host_.ctx, toAbiString(id), toAbiString(kind), toAbiString(language), in_abi.data(), in_abi.size(), + out_abi.data(), out_abi.size(), toAbiString(script), toAbiString(params_json), flags, resolved.data(), + resolved.size(), &count, &err)) { + return unexpected(errorToString(err)); + } + } + std::vector topics; + topics.reserve(count); + for (uint64_t i = 0; i < count && i < resolved.size(); ++i) { + topics.emplace_back(toStringView(resolved[i])); + } + return topics; + } + + /// Convenience: create a kind="markers" generator writing the single + /// `output_marker_topic` key (kGlobalMarkerTopic or markerSeriesKey). Pass + /// flags=PJ_GENERATOR_FLAG_EPHEMERAL for a live preview (output may be left empty + /// to let the host name the preview topic). Returns the resolved object topic(s). + [[nodiscard]] Expected> createMarkerGenerator( + std::string_view id, Span inputs, std::string_view output_marker_topic, + std::string_view script, std::string_view params_json, uint32_t flags = 0) const { + std::array outs{output_marker_topic}; + Span out_span = + output_marker_topic.empty() ? Span{} : Span(outs.data(), 1); + return createGenerator(id, "markers", "luau", inputs, out_span, script, params_json, flags); } - /// Remove a previously created generator by id. + /// Remove a previously created generator by id (persistent or ephemeral preview). [[nodiscard]] Status remove(std::string_view id) const { - if (!valid() || host_.vtable->remove_marker_generator == nullptr) { - return unexpected("markers host is not bound"); + if (!valid() || host_.vtable->remove_generator == nullptr) { + return unexpected("generators host is not bound"); } PJ_error_t err{}; - if (!host_.vtable->remove_marker_generator(host_.ctx, toAbiString(id), &err)) { + if (!host_.vtable->remove_generator(host_.ctx, toAbiString(id), &err)) { return unexpected(errorToString(err)); } return okStatus(); } - /// Enumerate the ids of this plugin's live generators (owned copies). + /// Enumerate the ids of this plugin's live (non-ephemeral) generators (owned copies). [[nodiscard]] Expected> list() const { - if (!valid() || host_.vtable->list_marker_generator_ids == nullptr) { - return unexpected("markers host is not bound"); + if (!valid() || host_.vtable->list_generator_ids == nullptr) { + return unexpected("generators host is not bound"); } PJ_error_t err{}; uint64_t count = 0; - if (!host_.vtable->list_marker_generator_ids(host_.ctx, nullptr, 0, &count, &err)) { + if (!host_.vtable->list_generator_ids(host_.ctx, nullptr, 0, &count, &err)) { return unexpected(errorToString(err)); } std::vector borrowed(count); uint64_t filled = 0; - if (count != 0 && - !host_.vtable->list_marker_generator_ids(host_.ctx, borrowed.data(), borrowed.size(), &filled, &err)) { + if (count != 0 && !host_.vtable->list_generator_ids(host_.ctx, borrowed.data(), borrowed.size(), &filled, &err)) { return unexpected(errorToString(err)); } std::vector ids; @@ -1533,55 +1575,38 @@ class MarkersHostView { } /// Read a generator's full recipe JSON (owned copy) for re-edit. - [[nodiscard]] Expected recipeOf(std::string_view id) const { - if (!valid() || host_.vtable->marker_generator_config == nullptr) { - return unexpected("markers host is not bound"); + [[nodiscard]] Expected configOf(std::string_view id) const { + if (!valid() || host_.vtable->generator_config == nullptr) { + return unexpected("generators host is not bound"); } PJ_error_t err{}; PJ_string_view_t recipe{}; - if (!host_.vtable->marker_generator_config(host_.ctx, toAbiString(id), &recipe, &err)) { + if (!host_.vtable->generator_config(host_.ctx, toAbiString(id), &recipe, &err)) { return unexpected(errorToString(err)); } return std::string(toStringView(recipe)); } - /// Set (replace) the live, EPHEMERAL preview generator: the host runs `script` over - /// `inputs` and publishes the markers to a preview object topic without persisting - /// it. Returns the preview object topic name (owned copy) so the caller can read the - /// markers back through the object read surface. `params_json` is forwarded verbatim. - [[nodiscard]] Expected setPreview( - Span inputs, std::string_view script, std::string_view params_json) const { - if (!valid() || host_.vtable->set_marker_preview == nullptr) { - return unexpected("markers host does not support preview"); - } - std::vector in_abi; - in_abi.reserve(inputs.size()); - for (const auto& name : inputs) { - in_abi.push_back(toAbiString(name)); - } - PJ_error_t err{}; - PJ_string_view_t topic{}; - if (!host_.vtable->set_marker_preview( - host_.ctx, in_abi.data(), in_abi.size(), toAbiString(script), toAbiString(params_json), &topic, &err)) { - return unexpected(errorToString(err)); - } - return std::string(toStringView(topic)); - } - - /// Tear down the live preview generator (and its preview object topic). No-op if none. - [[nodiscard]] Status clearPreview() const { - if (!valid() || host_.vtable->clear_marker_preview == nullptr) { - return unexpected("markers host does not support preview"); + /// Validate a script WITHOUT installing anything: compile + module-load only (no + /// inputs, no run, no side effects). Cheap enough to drive a live red/green editor + /// semaphore. Runtime/empty-output errors are NOT caught here — use an ephemeral + /// createGenerator for that. `params_json` is forwarded verbatim. + [[nodiscard]] Status validateScript( + std::string_view kind, std::string_view language, std::string_view script, + std::string_view params_json = "{}") const { + if (!valid() || host_.vtable->validate_script == nullptr) { + return unexpected("generators host does not support validation"); } PJ_error_t err{}; - if (!host_.vtable->clear_marker_preview(host_.ctx, &err)) { + if (!host_.vtable->validate_script( + host_.ctx, toAbiString(kind), toAbiString(language), toAbiString(script), toAbiString(params_json), &err)) { return unexpected(errorToString(err)); } return okStatus(); } private: - PJ_markers_host_t host_{}; + PJ_generators_host_t host_{}; }; // --------------------------------------------------------------------------- diff --git a/pj_base/include/pj_base/sdk/service_traits.hpp b/pj_base/include/pj_base/sdk/service_traits.hpp index e0d83a2..7a64cd8 100644 --- a/pj_base/include/pj_base/sdk/service_traits.hpp +++ b/pj_base/include/pj_base/sdk/service_traits.hpp @@ -171,16 +171,18 @@ struct DataProcessorsHostService { static_assert(detail::isValidServiceName(kName), "kName must match the pj naming rule"); }; -/// Optional service for plugins that submit WHOLE-SERIES marker generators to the -/// host by data (script + input names + a single output marker-topic + params). -/// The host runs the script and publishes the resulting PlotMarkers. The marker -/// analog of DataProcessorsHostService; see PJ_markers_host_vtable_t. -struct MarkersHostService { - static constexpr const char* kName = "pj.markers.v1"; +/// Optional service for plugins that submit WHOLE-SERIES generators to the host by +/// data (kind + language + script + input names + output topic(s) + params). The +/// host compiles and runs the script and routes the output by `kind`: "markers" +/// publishes a PlotMarkers set to the ObjectStore ("transform" — DerivedEngine +/// timeseries — is reserved). The whole-series analog of DataProcessorsHostService; +/// see PJ_generators_host_vtable_t. +struct GeneratorsHostService { + static constexpr const char* kName = "pj.generators.v1"; static constexpr uint32_t kMinVersion = 1; - using Raw = PJ_markers_host_t; - using Vtable = PJ_markers_host_vtable_t; - using View = MarkersHostView; + using Raw = PJ_generators_host_t; + using Vtable = PJ_generators_host_vtable_t; + using View = GeneratorsHostView; static_assert(detail::isValidServiceName(kName), "kName must match the pj naming rule"); }; diff --git a/pj_base/tests/generators_api_test.cpp b/pj_base/tests/generators_api_test.cpp new file mode 100644 index 0000000..9a1c3d0 --- /dev/null +++ b/pj_base/tests/generators_api_test.cpp @@ -0,0 +1,306 @@ +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#include + +#include +#include +#include +#include + +#include "pj_base/plugin_data_api.h" +#include "pj_base/sdk/plugin_data_api.hpp" +#include "pj_base/span.hpp" + +namespace PJ { +namespace { + +// Fake host: records the last create/remove/validate call and serves list/config +// from host-owned storage (so the borrowed-string lifetime contract can be tested). +// create_generator resolves output sink names — echoing provided outputs, or an +// auto-named topic when outputs is empty (ephemeral preview). +struct FakeGeneratorsHost { + bool create_called = false; + bool create_should_fail = false; + std::string last_id; + std::string last_kind; + std::string last_language; + std::vector last_inputs; + std::vector last_outputs; + std::string last_script; + std::string last_params; + uint32_t last_flags = 0; + + std::string removed_id; + + std::vector stored_ids; // host storage the listed views point into + std::string recipe_storage; // host storage the recipe view points into + + std::vector resolved_storage; // host storage out_topics point into + std::string auto_topic = "__markers__/__preview__/anomaly"; // host-named ephemeral sink + + bool validate_called = false; + bool validate_should_fail = false; + std::string last_validate_kind; + std::string last_validate_script; +}; + +bool genCreate( + void* ctx, PJ_string_view_t id, PJ_string_view_t kind, PJ_string_view_t language, const PJ_string_view_t* inputs, + uint64_t input_count, const PJ_string_view_t* outputs, uint64_t output_count, PJ_string_view_t script, + PJ_string_view_t params_json, uint32_t flags, PJ_string_view_t* out_topics, uint64_t out_topics_capacity, + uint64_t* out_topics_count, PJ_error_t* out_error) noexcept { + auto* self = static_cast(ctx); + if (self->create_should_fail) { + if (out_error != nullptr) { + sdk::fillError(out_error, 1, "generators", "create boom"); + } + return false; + } + self->create_called = true; + self->last_id = std::string(sdk::toStringView(id)); + self->last_kind = std::string(sdk::toStringView(kind)); + self->last_language = std::string(sdk::toStringView(language)); + self->last_inputs.clear(); + for (uint64_t i = 0; i < input_count; ++i) { + self->last_inputs.emplace_back(sdk::toStringView(inputs[i])); + } + self->last_outputs.clear(); + for (uint64_t i = 0; i < output_count; ++i) { + self->last_outputs.emplace_back(sdk::toStringView(outputs[i])); + } + self->last_script = std::string(sdk::toStringView(script)); + self->last_params = std::string(sdk::toStringView(params_json)); + self->last_flags = flags; + + // Resolve sink names: echo provided outputs, else host-named (ephemeral preview). + self->resolved_storage.clear(); + if (output_count > 0) { + for (uint64_t i = 0; i < output_count; ++i) { + self->resolved_storage.emplace_back(sdk::toStringView(outputs[i])); + } + } else { + self->resolved_storage.push_back(self->auto_topic); + } + if (out_topics_count != nullptr) { + *out_topics_count = self->resolved_storage.size(); + } + if (out_topics != nullptr) { + const uint64_t n = std::min(out_topics_capacity, self->resolved_storage.size()); + for (uint64_t i = 0; i < n; ++i) { + out_topics[i] = sdk::toAbiString(self->resolved_storage[i]); + } + } + return true; +} + +bool genRemove(void* ctx, PJ_string_view_t id, PJ_error_t* /*out_error*/) noexcept { + static_cast(ctx)->removed_id = std::string(sdk::toStringView(id)); + return true; +} + +bool genList( + void* ctx, PJ_string_view_t* out_ids, uint64_t capacity, uint64_t* out_count, PJ_error_t* /*out_error*/) noexcept { + auto* self = static_cast(ctx); + *out_count = self->stored_ids.size(); + const uint64_t n = std::min(capacity, self->stored_ids.size()); + for (uint64_t i = 0; i < n; ++i) { + out_ids[i] = sdk::toAbiString(self->stored_ids[i]); + } + return true; +} + +bool genConfig( + void* ctx, PJ_string_view_t /*id*/, PJ_string_view_t* out_recipe_json, PJ_error_t* /*out_error*/) noexcept { + out_recipe_json[0] = sdk::toAbiString(static_cast(ctx)->recipe_storage); + return true; +} + +bool genValidate( + void* ctx, PJ_string_view_t kind, PJ_string_view_t /*language*/, PJ_string_view_t script, + PJ_string_view_t /*params_json*/, PJ_error_t* out_error) noexcept { + auto* self = static_cast(ctx); + self->validate_called = true; + self->last_validate_kind = std::string(sdk::toStringView(kind)); + self->last_validate_script = std::string(sdk::toStringView(script)); + if (self->validate_should_fail) { + if (out_error != nullptr) { + sdk::fillError(out_error, 1, "generators", "syntax boom"); + } + return false; + } + return true; +} + +PJ_generators_host_vtable_t makeVtable() { + return PJ_generators_host_vtable_t{ + .protocol_version = 1, + .struct_size = sizeof(PJ_generators_host_vtable_t), + .create_generator = genCreate, + .remove_generator = genRemove, + .list_generator_ids = genList, + .generator_config = genConfig, + .validate_script = genValidate, + }; +} + +TEST(GeneratorsApiTest, CreateForwardsAllArgsAndReturnsResolvedTopics) { + FakeGeneratorsHost host; + const auto vtable = makeVtable(); + sdk::GeneratorsHostView view(PJ_generators_host_t{.ctx = &host, .vtable = &vtable}); + + const std::string_view inputs[] = {"imu/accel/x", "imu/accel/y"}; + const std::string_view outputs[] = {"/anomaly/region"}; + auto topics = view.createGenerator( + "vib_detect", "markers", "luau", PJ::Span(inputs), + PJ::Span(outputs), "createPointMarker(0, 1)", R"({"threshold":3.0})"); + + ASSERT_TRUE(topics) << topics.error(); + EXPECT_TRUE(host.create_called); + EXPECT_EQ(host.last_id, "vib_detect"); + EXPECT_EQ(host.last_kind, "markers"); + EXPECT_EQ(host.last_language, "luau"); + ASSERT_EQ(host.last_inputs.size(), 2u); + EXPECT_EQ(host.last_inputs[0], "imu/accel/x"); + ASSERT_EQ(host.last_outputs.size(), 1u); + EXPECT_EQ(host.last_outputs[0], "/anomaly/region"); + EXPECT_EQ(host.last_params, R"({"threshold":3.0})"); + EXPECT_EQ(host.last_flags, 0u); + ASSERT_EQ(topics->size(), 1u); + EXPECT_EQ((*topics)[0], "/anomaly/region"); +} + +// Ephemeral preview: no outputs supplied + EPHEMERAL flag → host auto-names the sink +// and returns it. The returned name is an owned copy (survives host mutation). +TEST(GeneratorsApiTest, EphemeralPreviewAutoNamesAndReturnsTopic) { + FakeGeneratorsHost host; + const auto vtable = makeVtable(); + sdk::GeneratorsHostView view(PJ_generators_host_t{.ctx = &host, .vtable = &vtable}); + + const std::string_view inputs[] = {"in"}; + auto topics = view.createMarkerGenerator( + "anomaly/__preview__", PJ::Span(inputs), /*output=*/"", "createPointMarker(0.0)", "{}", + PJ_GENERATOR_FLAG_EPHEMERAL); + + ASSERT_TRUE(topics) << topics.error(); + EXPECT_EQ(host.last_kind, "markers"); + EXPECT_EQ(host.last_flags, PJ_GENERATOR_FLAG_EPHEMERAL); + EXPECT_TRUE(host.last_outputs.empty()); + ASSERT_EQ(topics->size(), 1u); + const std::string expected = host.auto_topic; + host.auto_topic = "CLOBBERED"; // owned copy survives + EXPECT_EQ((*topics)[0], expected); +} + +TEST(GeneratorsApiTest, CreateFailureSurfacesError) { + FakeGeneratorsHost host; + host.create_should_fail = true; + const auto vtable = makeVtable(); + sdk::GeneratorsHostView view(PJ_generators_host_t{.ctx = &host, .vtable = &vtable}); + + auto topics = view.createMarkerGenerator( + "x", PJ::Span{}, "__global__", "s", "{}"); + + EXPECT_FALSE(topics); + EXPECT_NE(topics.error().find("create boom"), std::string::npos); + EXPECT_FALSE(host.create_called); +} + +TEST(GeneratorsApiTest, RemoveForwardsId) { + FakeGeneratorsHost host; + const auto vtable = makeVtable(); + sdk::GeneratorsHostView view(PJ_generators_host_t{.ctx = &host, .vtable = &vtable}); + + ASSERT_TRUE(view.remove("vib_detect")); + EXPECT_EQ(host.removed_id, "vib_detect"); +} + +TEST(GeneratorsApiTest, ListCountThenFillReturnsOwnedCopies) { + FakeGeneratorsHost host; + host.stored_ids = {"alpha", "beta", "gamma"}; + const auto vtable = makeVtable(); + sdk::GeneratorsHostView view(PJ_generators_host_t{.ctx = &host, .vtable = &vtable}); + + auto ids = view.list(); + ASSERT_TRUE(ids) << ids.error(); + ASSERT_EQ(ids->size(), 3u); + EXPECT_EQ((*ids)[0], "alpha"); + EXPECT_EQ((*ids)[2], "gamma"); + + host.stored_ids[0] = "clobbered"; + EXPECT_EQ((*ids)[0], "alpha"); // owned copy +} + +TEST(GeneratorsApiTest, ConfigOfCopiesBorrowedJson) { + FakeGeneratorsHost host; + host.recipe_storage = R"({"kind":"markers","inputs":["a"],"outputs":["__global__"],"params":{}})"; + const auto vtable = makeVtable(); + sdk::GeneratorsHostView view(PJ_generators_host_t{.ctx = &host, .vtable = &vtable}); + + auto recipe = view.configOf("id"); + ASSERT_TRUE(recipe) << recipe.error(); + const std::string expected = host.recipe_storage; + + host.recipe_storage = "CLOBBERED"; + EXPECT_EQ(*recipe, expected); // owned copy +} + +TEST(GeneratorsApiTest, UnboundViewReportsNotBound) { + sdk::GeneratorsHostView view; // default-constructed = not bound + EXPECT_FALSE(view.valid()); + + auto topics = view.createMarkerGenerator( + "x", PJ::Span{}, "__global__", "s", "{}"); + + EXPECT_FALSE(topics); + EXPECT_NE(topics.error().find("not bound"), std::string::npos); +} + +// Regression guard: `script` / `params_json` are PJ_string_view_t {data,size} -- +// binary-safe, NOT NUL-terminated. A payload carrying embedded NULs and the WASM +// "\0asm" magic round-trips byte-for-byte (keeps the WASM/Python backend door open). +TEST(GeneratorsApiTest, BinarySafePayloadRoundTrips) { + FakeGeneratorsHost host; + const auto vtable = makeVtable(); + sdk::GeneratorsHostView view(PJ_generators_host_t{.ctx = &host, .vtable = &vtable}); + + const char wasm_bytes[] = {'\0', 'a', 's', 'm', '\x01', '\0', '\0', '\0', 'X', 'Y'}; + const std::string blob(wasm_bytes, sizeof(wasm_bytes)); // 10 bytes, 3 embedded NULs + ASSERT_EQ(blob.size(), 10u); + + const std::string_view outputs[] = {"__global__"}; + auto topics = view.createGenerator( + "fft", "markers", "luau", PJ::Span{}, PJ::Span(outputs), + std::string_view(blob.data(), blob.size()), "{}"); + + ASSERT_TRUE(topics) << topics.error(); + EXPECT_EQ(host.last_script.size(), blob.size()); // not truncated at the first NUL + EXPECT_EQ(host.last_script, blob); +} + +// validate_script forwards kind + script and reports success; a compile failure +// surfaces the host's error message (drives the editor red/green semaphore). +TEST(GeneratorsApiTest, ValidateForwardsAndSucceeds) { + FakeGeneratorsHost host; + const auto vtable = makeVtable(); + sdk::GeneratorsHostView view(PJ_generators_host_t{.ctx = &host, .vtable = &vtable}); + + ASSERT_TRUE(view.validateScript("markers", "luau", "createPointMarker(0.0)")); + EXPECT_TRUE(host.validate_called); + EXPECT_EQ(host.last_validate_kind, "markers"); + EXPECT_EQ(host.last_validate_script, "createPointMarker(0.0)"); +} + +TEST(GeneratorsApiTest, ValidateFailureSurfacesError) { + FakeGeneratorsHost host; + host.validate_should_fail = true; + const auto vtable = makeVtable(); + sdk::GeneratorsHostView view(PJ_generators_host_t{.ctx = &host, .vtable = &vtable}); + + auto status = view.validateScript("markers", "luau", "this is not lua"); + EXPECT_FALSE(status); + EXPECT_NE(status.error().find("syntax boom"), std::string::npos); +} + +} // namespace +} // namespace PJ diff --git a/pj_base/tests/markers_api_test.cpp b/pj_base/tests/markers_api_test.cpp deleted file mode 100644 index 8021b8d..0000000 --- a/pj_base/tests/markers_api_test.cpp +++ /dev/null @@ -1,261 +0,0 @@ -// Copyright 2026 Davide Faconti -// SPDX-License-Identifier: Apache-2.0 - -#include - -#include -#include -#include -#include - -#include "pj_base/plugin_data_api.h" -#include "pj_base/sdk/plugin_data_api.hpp" -#include "pj_base/span.hpp" - -namespace PJ { -namespace { - -// Fake host: records the last create/remove call and serves list/config from -// host-owned storage (so the borrowed-string lifetime contract can be tested). -struct FakeMarkersHost { - bool create_called = false; - bool create_should_fail = false; - std::string last_id; - std::vector last_inputs; - std::string last_output; - std::string last_script; - std::string last_params; - - std::string removed_id; - - std::vector stored_ids; // host storage the listed views point into - std::string recipe_storage; // host storage the recipe view points into - - bool preview_set = false; - bool preview_cleared = false; - std::string last_preview_script; - std::string preview_topic_storage = "__markers__/__preview__/anomaly"; // host-owned, borrowed back -}; - -bool mkCreate( - void* ctx, PJ_string_view_t id, const PJ_string_view_t* inputs, uint64_t input_count, - PJ_string_view_t output_marker_topic, PJ_string_view_t script, PJ_string_view_t params_json, - PJ_error_t* out_error) noexcept { - auto* self = static_cast(ctx); - if (self->create_should_fail) { - if (out_error != nullptr) { - sdk::fillError(out_error, 1, "markers", "create boom"); - } - return false; - } - self->create_called = true; - self->last_id = std::string(sdk::toStringView(id)); - self->last_inputs.clear(); - for (uint64_t i = 0; i < input_count; ++i) { - self->last_inputs.emplace_back(sdk::toStringView(inputs[i])); - } - self->last_output = std::string(sdk::toStringView(output_marker_topic)); - self->last_script = std::string(sdk::toStringView(script)); - self->last_params = std::string(sdk::toStringView(params_json)); - return true; -} - -bool mkRemove(void* ctx, PJ_string_view_t id, PJ_error_t* /*out_error*/) noexcept { - static_cast(ctx)->removed_id = std::string(sdk::toStringView(id)); - return true; -} - -bool mkList( - void* ctx, PJ_string_view_t* out_ids, uint64_t capacity, uint64_t* out_count, PJ_error_t* /*out_error*/) noexcept { - auto* self = static_cast(ctx); - *out_count = self->stored_ids.size(); - const uint64_t n = std::min(capacity, self->stored_ids.size()); - for (uint64_t i = 0; i < n; ++i) { - out_ids[i] = sdk::toAbiString(self->stored_ids[i]); - } - return true; -} - -bool mkConfig( - void* ctx, PJ_string_view_t /*id*/, PJ_string_view_t* out_recipe_json, PJ_error_t* /*out_error*/) noexcept { - out_recipe_json[0] = sdk::toAbiString(static_cast(ctx)->recipe_storage); - return true; -} - -bool mkSetPreview( - void* ctx, const PJ_string_view_t* /*inputs*/, uint64_t /*input_count*/, PJ_string_view_t script, - PJ_string_view_t /*params_json*/, PJ_string_view_t* out_preview_object_topic, PJ_error_t* /*out_error*/) noexcept { - auto* self = static_cast(ctx); - self->preview_set = true; - self->last_preview_script = std::string(sdk::toStringView(script)); - out_preview_object_topic[0] = sdk::toAbiString(self->preview_topic_storage); - return true; -} - -bool mkClearPreview(void* ctx, PJ_error_t* /*out_error*/) noexcept { - static_cast(ctx)->preview_cleared = true; - return true; -} - -PJ_markers_host_vtable_t makeVtable() { - return PJ_markers_host_vtable_t{ - .protocol_version = 1, - .struct_size = sizeof(PJ_markers_host_vtable_t), - .create_marker_generator = mkCreate, - .remove_marker_generator = mkRemove, - .list_marker_generator_ids = mkList, - .marker_generator_config = mkConfig, - .set_marker_preview = mkSetPreview, - .clear_marker_preview = mkClearPreview, - }; -} - -TEST(MarkersApiTest, CreateForwardsAllArgsIntact) { - FakeMarkersHost host; - const auto vtable = makeVtable(); - sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); - - const std::string_view inputs[] = {"imu/accel/x", "imu/accel/y"}; - auto status = view.createMarkerGenerator( - "vib_detect", PJ::Span(inputs), "__global__", "-- pj-script: lua\nreturn {}", - R"({"threshold":3.0})"); - - ASSERT_TRUE(status) << status.error(); - EXPECT_TRUE(host.create_called); - EXPECT_EQ(host.last_id, "vib_detect"); - ASSERT_EQ(host.last_inputs.size(), 2u); - EXPECT_EQ(host.last_inputs[0], "imu/accel/x"); - EXPECT_EQ(host.last_inputs[1], "imu/accel/y"); - EXPECT_EQ(host.last_output, "__global__"); - EXPECT_EQ(host.last_script, "-- pj-script: lua\nreturn {}"); - EXPECT_EQ(host.last_params, R"({"threshold":3.0})"); -} - -TEST(MarkersApiTest, CreateFailureSurfacesError) { - FakeMarkersHost host; - host.create_should_fail = true; - const auto vtable = makeVtable(); - sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); - - auto status = view.createMarkerGenerator("x", PJ::Span{}, "topic", "s", "{}"); - - EXPECT_FALSE(status); - EXPECT_NE(status.error().find("create boom"), std::string::npos); - EXPECT_FALSE(host.create_called); -} - -TEST(MarkersApiTest, RemoveForwardsId) { - FakeMarkersHost host; - const auto vtable = makeVtable(); - sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); - - ASSERT_TRUE(view.remove("vib_detect")); - EXPECT_EQ(host.removed_id, "vib_detect"); -} - -TEST(MarkersApiTest, ListCountThenFillReturnsOwnedCopies) { - FakeMarkersHost host; - host.stored_ids = {"alpha", "beta", "gamma"}; - const auto vtable = makeVtable(); - sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); - - auto ids = view.list(); - ASSERT_TRUE(ids) << ids.error(); - ASSERT_EQ(ids->size(), 3u); - EXPECT_EQ((*ids)[0], "alpha"); - EXPECT_EQ((*ids)[2], "gamma"); - - host.stored_ids[0] = "clobbered"; - EXPECT_EQ((*ids)[0], "alpha"); // owned copy -} - -TEST(MarkersApiTest, RecipeOfCopiesBorrowedJson) { - FakeMarkersHost host; - host.recipe_storage = R"({"inputs":["a"],"output":"__global__","params":{}})"; - const auto vtable = makeVtable(); - sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); - - auto recipe = view.recipeOf("id"); - ASSERT_TRUE(recipe) << recipe.error(); - const std::string expected = host.recipe_storage; - - host.recipe_storage = "CLOBBERED"; - EXPECT_EQ(*recipe, expected); // owned copy -} - -TEST(MarkersApiTest, UnboundViewReportsNotBound) { - sdk::MarkersHostView view; // default-constructed = not bound - EXPECT_FALSE(view.valid()); - - auto status = view.createMarkerGenerator("x", PJ::Span{}, "topic", "s", "{}"); - - EXPECT_FALSE(status); - EXPECT_NE(status.error().find("not bound"), std::string::npos); -} - -// The view fails fast on an empty output marker topic: a generator must address -// exactly one target (global or per-series). The guard lives in the view so a -// misuse never reaches the vtable (the host enforces it authoritatively too). -TEST(MarkersApiTest, CreateRejectsEmptyOutputTopic) { - FakeMarkersHost host; - const auto vtable = makeVtable(); - sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); - - auto status = - view.createMarkerGenerator("x", PJ::Span{}, "", "-- pj-script: lua\nreturn {}", "{}"); - - EXPECT_FALSE(status); - EXPECT_NE(status.error().find("output"), std::string::npos); - EXPECT_FALSE(host.create_called); // rejected before the ABI call -} - -// Regression guard: `script` / `params_json` are PJ_string_view_t {data,size} -- -// binary-safe, NOT NUL-terminated. A payload carrying embedded NULs and the WASM -// "\0asm" magic round-trips byte-for-byte (keeps the WASM/Python backend door open). -TEST(MarkersApiTest, BinarySafePayloadRoundTrips) { - FakeMarkersHost host; - const auto vtable = makeVtable(); - sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); - - const char wasm_bytes[] = {'\0', 'a', 's', 'm', '\x01', '\0', '\0', '\0', 'X', 'Y'}; - const std::string blob(wasm_bytes, sizeof(wasm_bytes)); // 10 bytes, 3 embedded NULs - ASSERT_EQ(blob.size(), 10u); - - auto status = view.createMarkerGenerator( - "fft", PJ::Span{}, "__global__", std::string_view(blob.data(), blob.size()), "{}"); - - ASSERT_TRUE(status) << status.error(); - EXPECT_EQ(host.last_script.size(), blob.size()); // not truncated at the first NUL - EXPECT_EQ(host.last_script, blob); -} - -// The preview slot forwards the script and returns the (borrowed) preview object -// topic name as an owned copy for read-back. -TEST(MarkersApiTest, SetPreviewForwardsScriptAndReturnsTopic) { - FakeMarkersHost host; - const auto vtable = makeVtable(); - sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); - - const std::string_view inputs[] = {"in"}; - auto topic = view.setPreview(PJ::Span(inputs), "createEvent(0.0)", "{}"); - ASSERT_TRUE(topic) << topic.error(); - EXPECT_TRUE(host.preview_set); - EXPECT_EQ(host.last_preview_script, "createEvent(0.0)"); - EXPECT_EQ(*topic, host.preview_topic_storage); - - const std::string expected = host.preview_topic_storage; - host.preview_topic_storage = "CLOBBERED"; // owned copy survives - EXPECT_EQ(*topic, expected); -} - -TEST(MarkersApiTest, ClearPreviewForwards) { - FakeMarkersHost host; - const auto vtable = makeVtable(); - sdk::MarkersHostView view(PJ_markers_host_t{.ctx = &host, .vtable = &vtable}); - - ASSERT_TRUE(view.clearPreview()); - EXPECT_TRUE(host.preview_cleared); -} - -} // namespace -} // namespace PJ diff --git a/recipe.yaml b/recipe.yaml index 28c93f1..9a92d7b 100644 --- a/recipe.yaml +++ b/recipe.yaml @@ -1,7 +1,7 @@ schema_version: 1 context: - version: "0.12.0" + version: "0.13.0" package: name: plotjuggler_sdk From 360dd87ee3672a7dbb7415ee294b335612e41b04 Mon Sep 17 00:00:00 2001 From: alvvm Date: Sat, 27 Jun 2026 10:20:09 +0200 Subject: [PATCH 09/11] style(sdk): clang-format the unified data-processors API surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applies the pre-commit clang-format pass that the merge commit (bf7eb0f) skipped via --no-verify. Formatting only — no semantic change. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01FHPK8VduUcoenjL8AtoSVd --- pj_base/include/pj_base/builtin/plot_markers.hpp | 16 ++++++++-------- pj_base/include/pj_base/plugin_data_api.h | 12 ++++++------ pj_base/include/pj_base/sdk/plugin_data_api.hpp | 5 ++--- .../include/pj_plugins/sdk/widget_data.hpp | 6 +++--- 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/pj_base/include/pj_base/builtin/plot_markers.hpp b/pj_base/include/pj_base/builtin/plot_markers.hpp index 217836e..03a0328 100644 --- a/pj_base/include/pj_base/builtin/plot_markers.hpp +++ b/pj_base/include/pj_base/builtin/plot_markers.hpp @@ -73,18 +73,18 @@ struct PlotMarker { MarkerKind kind = MarkerKind::kRegion; // --- anchor (interpret by kind; irrelevant fields ignored) --- - Timestamp t_start = 0; ///< Region start · Event/Label time · (ValueBand: ignored). - Timestamp t_end = 0; ///< Region end · (others: ignored). - double value_low = 0.0; ///< ValueBand low · Event point value · (others: ignored). - double value_high = 0.0; ///< ValueBand high · (others: ignored). - bool has_value = false; ///< Event: `value_low` is a meaningful point value. + Timestamp t_start = 0; ///< Region start · Event/Label time · (ValueBand: ignored). + Timestamp t_end = 0; ///< Region end · (others: ignored). + double value_low = 0.0; ///< ValueBand low · Event point value · (others: ignored). + double value_high = 0.0; ///< ValueBand high · (others: ignored). + bool has_value = false; ///< Event: `value_low` is a meaningful point value. // --- semantics / presentation (shared by every kind) --- MarkerStatus status = MarkerStatus::kNone; MarkerSeverity severity = MarkerSeverity::kInfo; - std::string category; ///< e.g. "overspeed", "state_transition". - std::string label; ///< Short title (tooltip / panel / Label content). - std::string description; ///< Optional longer text. + std::string category; ///< e.g. "overspeed", "state_transition". + std::string label; ///< Short title (tooltip / panel / Label content). + std::string description; ///< Optional longer text. ColorRGBA color = {0, 0, 0, 0}; ///< a=0 → derive color from `severity`. std::vector metadata; diff --git a/pj_base/include/pj_base/plugin_data_api.h b/pj_base/include/pj_base/plugin_data_api.h index 5f6347d..3831101 100644 --- a/pj_base/include/pj_base/plugin_data_api.h +++ b/pj_base/include/pj_base/plugin_data_api.h @@ -749,8 +749,8 @@ typedef struct PJ_object_read_host_vtable_t { * several loaded datasets, so the name-only `lookup_topic` above is ambiguous * across datasets. This dataset-scoped form returns the topic owned by * `dataset_id`, or {id=0} on miss. ABI-APPENDED slot: gate via struct_size. */ - PJ_object_topic_handle_t (*lookup_topic_on_dataset)( - void* ctx, uint32_t dataset_id, PJ_string_view_t topic_name) PJ_NOEXCEPT; + PJ_object_topic_handle_t (*lookup_topic_on_dataset)(void* ctx, uint32_t dataset_id, PJ_string_view_t topic_name) + PJ_NOEXCEPT; } PJ_object_read_host_vtable_t; /* ABI-FROZEN: fat pointer layout permanent. */ @@ -858,10 +858,10 @@ typedef struct PJ_data_processors_host_vtable_t { * failure no partial state is left AND any previously published output for this id * is preserved. All string arguments are borrowed for the duration of the call. */ bool (*create_data_processor)( - void* ctx, PJ_string_view_t id, PJ_string_view_t kind, PJ_string_view_t language, - const PJ_string_view_t* inputs, uint64_t input_count, const PJ_string_view_t* outputs, uint64_t output_count, - PJ_string_view_t script, PJ_string_view_t params_json, uint32_t flags, PJ_string_view_t* out_topics, - uint64_t out_topics_capacity, uint64_t* out_topics_count, PJ_error_t* out_error) PJ_NOEXCEPT; + void* ctx, PJ_string_view_t id, PJ_string_view_t kind, PJ_string_view_t language, const PJ_string_view_t* inputs, + uint64_t input_count, const PJ_string_view_t* outputs, uint64_t output_count, PJ_string_view_t script, + PJ_string_view_t params_json, uint32_t flags, PJ_string_view_t* out_topics, uint64_t out_topics_capacity, + uint64_t* out_topics_count, PJ_error_t* out_error) PJ_NOEXCEPT; /* [main-thread] Remove a node by id (persistent or ephemeral). Unknown id is an error. */ bool (*remove_data_processor)(void* ctx, PJ_string_view_t id, PJ_error_t* out_error) PJ_NOEXCEPT; diff --git a/pj_base/include/pj_base/sdk/plugin_data_api.hpp b/pj_base/include/pj_base/sdk/plugin_data_api.hpp index c787479..3fc027c 100644 --- a/pj_base/include/pj_base/sdk/plugin_data_api.hpp +++ b/pj_base/include/pj_base/sdk/plugin_data_api.hpp @@ -2,10 +2,10 @@ // Copyright 2026 Davide Faconti // SPDX-License-Identifier: Apache-2.0 +#include #include #include #include -#include #include #include #include @@ -1272,8 +1272,7 @@ class ToolboxHostView { return unexpected("toolbox host is not bound"); } if (!hasTailSlot( - offsetof(PJ_toolbox_host_vtable_t, set_object_topic_retention), - host_.vtable->set_object_topic_retention)) { + offsetof(PJ_toolbox_host_vtable_t, set_object_topic_retention), host_.vtable->set_object_topic_retention)) { return unexpected("toolbox host does not support object retention (older host)"); } PJ_error_t err{}; diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp index c13485e..94e53e8 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp @@ -36,7 +36,7 @@ struct ChartSeries { /// X/Y are in the chart's own units (e.g. seconds-from-start), so the producer must /// rebase to match the series points it pushes alongside. struct ChartMarker { - std::string kind; // "event" | "region" | "value_band" | "label" + std::string kind; // "event" | "region" | "value_band" | "label" double x0 = 0.0; double x1 = 0.0; double y0 = 0.0; @@ -51,8 +51,8 @@ struct ChartMarker { [[nodiscard]] inline nlohmann::json chartMarkersToJson(const std::vector& marks) { nlohmann::json arr = nlohmann::json::array(); for (const auto& m : marks) { - nlohmann::json jm = {{"kind", m.kind}, {"x0", m.x0}, {"x1", m.x1}, - {"y0", m.y0}, {"y1", m.y1}, {"has_value", m.has_value}}; + nlohmann::json jm = {{"kind", m.kind}, {"x0", m.x0}, {"x1", m.x1}, + {"y0", m.y0}, {"y1", m.y1}, {"has_value", m.has_value}}; if (!m.color.empty()) { jm["color"] = m.color; } From 21895cc07056b5368f2ab3233bc8bf7b6b285029 Mon Sep 17 00:00:00 2001 From: alvvm Date: Sat, 27 Jun 2026 10:21:39 +0200 Subject: [PATCH 10/11] style(sdk): clang-format remaining branch files (proto + dialog typed) Completes the formatting pass over the full PR diff (PlotMarkers.proto column alignment + dialog_plugin_typed.hpp signature wrap) that the narrowly-scoped previous pass missed. Formatting only. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01FHPK8VduUcoenjL8AtoSVd --- pj_base/proto/pj/PlotMarkers.proto | 12 ++++++------ .../include/pj_plugins/sdk/dialog_plugin_typed.hpp | 3 +-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/pj_base/proto/pj/PlotMarkers.proto b/pj_base/proto/pj/PlotMarkers.proto index b5cc3a2..ef471ac 100644 --- a/pj_base/proto/pj/PlotMarkers.proto +++ b/pj_base/proto/pj/PlotMarkers.proto @@ -44,11 +44,11 @@ enum MarkerSeverity { message PlotMarker { // Anchor (interpret by kind; irrelevant fields are ignored). MarkerKind kind = 1; - int64 t_start = 2; // ns; Region start / Event / Label time - int64 t_end = 3; // ns; Region end - double value_low = 4; // ValueBand low / Event point value - double value_high = 5; // ValueBand high - bool has_value = 6; // Event: value_low is a meaningful point value + int64 t_start = 2; // ns; Region start / Event / Label time + int64 t_end = 3; // ns; Region end + double value_low = 4; // ValueBand low / Event point value + double value_high = 5; // ValueBand high + bool has_value = 6; // Event: value_low is a meaningful point value // Semantics / presentation (shared by every kind). MarkerStatus status = 7; @@ -56,7 +56,7 @@ message PlotMarker { string category = 9; string label = 10; string description = 11; - PJ.Color color = 12; // alpha 0 → derive from severity + PJ.Color color = 12; // alpha 0 → derive from severity repeated PJ.KeyValuePair metadata = 13; } diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_typed.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_typed.hpp index dee5b38..64161c9 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_typed.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_typed.hpp @@ -104,8 +104,7 @@ class DialogPluginTyped : public DialogPluginBase { } /// MarkerTimeline: a mark was moved, resized, or deleted. `marks` is the full set. - virtual bool onMarkerTimelineChanged( - std::string_view /*widget_name*/, const std::vector& /*marks*/) { + virtual bool onMarkerTimelineChanged(std::string_view /*widget_name*/, const std::vector& /*marks*/) { return false; } From a3d151321453e6840c586d482a6f3f1ab8bb6681 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20I=C3=B1igo=20Blasco?= Date: Mon, 29 Jun 2026 08:56:23 +0200 Subject: [PATCH 11/11] feat(dialog): add save-file picker to the dialog SDK (#138) Add WidgetData::setSaveFilePicker and the matching WidgetDataView accessors so a plugin can turn a button into a native "save as" chooser. The chosen path is delivered through the existing onFileSelected handler, so no new event type or typed handler is required. This is a backward-compatible API addition, so bump the SDK to 0.13.0 and bring recipe.yaml back in sync with the other version sources. Contents: - setSaveFilePicker(name, text, filter, title, default_suffix) -> "save_file_picker" action - WidgetDataView: isSaveFilePicker / saveFilePickerFilter / Title / DefaultSuffix - version 0.13.0 in conanfile.py, CMakeLists.txt, recipe.yaml + docstring example - docs: dialog-sdk-reference.md and dialog-plugin-guide.md --- docs/dialog-sdk-reference.md | 4 ++-- .../pj_plugins/host/widget_data_view.hpp | 5 ++++ .../include/pj_plugins/sdk/widget_data.hpp | 7 ++++-- pj_plugins/docs/dialog-plugin-guide.md | 23 +++++++++++++++++++ 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/docs/dialog-sdk-reference.md b/docs/dialog-sdk-reference.md index 6f05f7c..c7f8f58 100644 --- a/docs/dialog-sdk-reference.md +++ b/docs/dialog-sdk-reference.md @@ -57,7 +57,7 @@ For the full tutorial, see [dialog-plugin-guide.md](../pj_plugins/docs/dialog-pl | `setButtonIconNamed(name, icon_id)` | Set a button icon by id, resolved from the host's themed icon set (consistent tinting; unknown id → no icon) | | `setShortcut(name, key_sequence)` | Assign keyboard shortcut (e.g. `"Ctrl+A"`) | | `setFilePicker(name, text, filter, title)` | Turn into an **open** file picker (existing file) | -| `setSaveFilePicker(name, text, filter, title)` | Turn into a **save-as** file picker (navigate + type a new filename); reports via `onFileSelected` | +| `setSaveFilePicker(name, text, filter, title, default_suffix="")` | Turn into a **save-as** file picker (navigate + type a new filename); reports via `onFileSelected`. `default_suffix` is appended when the typed name has none | | `setFolderPicker(name, text, title)` | Turn into folder picker | ### QListWidget @@ -150,7 +150,7 @@ Override these in your `DialogPluginTyped` subclass. Return `true` when state ch | `onValueChanged(name, int)` | QSpinBox | New integer value | | `onValueChanged(name, double)` | QDoubleSpinBox | New double value | | `onClicked(name)` | QPushButton | (no payload) | -| `onFileSelected(name, path)` | QPushButton (file picker) | Selected file path | +| `onFileSelected(name, path)` | QPushButton (file picker or save-file picker) | Selected file path | | `onFolderSelected(name, path)` | QPushButton (folder picker) | Selected folder path | | `onSelectionChanged(name, items)` | QListWidget, QTableWidget | Vector of selected item texts | | `onItemDoubleClicked(name, index)` | QListWidget, QTableWidget | Row index of double-clicked item | diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp index ec8334d..f9c12c3 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp @@ -322,6 +322,11 @@ class WidgetDataView { return it != w->end() && it->is_string() && it->get() == "save_file_picker"; } + /// The extension appended to a save-file picker's typed name when it has none. + [[nodiscard]] std::optional saveFilePickerDefaultSuffix(std::string_view name) const { + return getString(name, "default_suffix"); + } + [[nodiscard]] std::optional filePickerFilter(std::string_view name) const { return getString(name, "filter"); } diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp index 94e53e8..c79c5b8 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp @@ -418,14 +418,17 @@ class WidgetData { /// (which opens an existing file), this lets the user navigate to a directory AND /// type a new filename, so a not-yet-existing rule/config file can be created. The /// chosen path is reported through the same onFileSelected(name, path) event — the - /// plugin distinguishes save from open by the widget name. + /// plugin distinguishes save from open by the widget name. `default_suffix` is + /// appended when the typed name carries no extension. WidgetData& setSaveFilePicker( - std::string_view name, std::string_view button_text, std::string_view filter, std::string_view title) { + std::string_view name, std::string_view button_text, std::string_view filter, std::string_view title, + std::string_view default_suffix = "") { auto& e = entry(name); e["button_text"] = button_text; e["action"] = "save_file_picker"; e["filter"] = filter; e["title"] = title; + e["default_suffix"] = default_suffix; return *this; } diff --git a/pj_plugins/docs/dialog-plugin-guide.md b/pj_plugins/docs/dialog-plugin-guide.md index 7c09f8d..b13312e 100644 --- a/pj_plugins/docs/dialog-plugin-guide.md +++ b/pj_plugins/docs/dialog-plugin-guide.md @@ -354,6 +354,7 @@ work like polling a server for available topics. | QDoubleSpinBox | `setValue(double)` | `onValueChanged(name, double)` | | QPushButton | `setButtonText` | `onClicked(name)` | | QPushButton (file picker) | `setFilePicker` | `onFileSelected(name, path)` | +| QPushButton (save-file picker) | `setSaveFilePicker` | `onFileSelected(name, path)` | | QPushButton (folder picker) | `setFolderPicker` | `onFolderSelected(name, path)` | | QLabel | `setLabel` | (none — display only) | | QListWidget | `setListItems`, `setSelectedItems` | `onSelectionChanged(name, items)`, `onItemDoubleClicked(name, index)` | @@ -457,6 +458,28 @@ bool onFileSelected(std::string_view name, std::string_view path) override { } ``` +### Save-file picker + +For an *export* button, use `setSaveFilePicker()` instead. The host shows a +native save dialog (the user types a name and picks a location) and delivers the +chosen path through the same `onFileSelected()` handler — distinguish the button +by its `objectName`. The optional `default_suffix` is appended when the typed +name carries no extension. + +```cpp +// In widget_data(): +wd.setSaveFilePicker("export_btn", "Export...", "*.json", "Export Library", "json"); + +// Same handler as the file picker, routed by name: +bool onFileSelected(std::string_view name, std::string_view path) override { + if (name == "export_btn") { + writeLibraryTo(path); + return true; + } + return false; +} +``` + ### Dynamic visibility and enabled state Control widget visibility and enabled state from `widget_data()` to build