fix: six defects found in the pre-1.0 review (1.0.0)

.claude/skills/plotjuggler-plugin/SKILL.md

@@ -166,10 +166,11 @@ DataSource may add `"file_extensions": [".csv"]`.
 `min_sdk_required` declares the oldest **host** SDK contract your plugin
 functions on. It is independent of the SDK you build against: `pj_base` /
 `pj_source` are static libraries compiled into your plugin, so adopting a new
-helper (`parseIso8601Utc`, `sliderToWindow`, a codec) **never** raises the
+helper (`parseIso8601Utc`, `sliderToWindow`) **never** raises the
 floor. Only **host-contract** surfaces do — runtime-host tail slots, named
-services, wire contracts — and the authority for which release introduced each
-is `feature_floors.json`, shipped in the package at
+services, wire contracts, and emitting a builtin object type an older host
+cannot decode (`MeshMap` needs 1.0.0) — and the authority for which release
+introduced each is `feature_floors.json`, shipped in the package at
 `share/plotjuggler_sdk/feature_floors.json`.
 
 When you adopt a surface newer than your floor, pick one:

CHANGELOG.md

@@ -6,9 +6,10 @@ All notable changes to `plotjuggler_sdk` are recorded here. Versioning policy is
 ## [1.0.0] — Unreleased
 
 Host contract: extended: builtin object types kMeshMap (21) and kMeshMapUpdate (22) (floor 1.0.0).
-Builtin object types are not among the surfaces `feature_floors.json` classifies, so the floor
-checker cannot see them: a plugin that emits either type raises its `min_sdk_required` to 1.0.0
-by hand, because an older host cannot decode the object. `Log` gains an optional field and renames
+`feature_floors.json` now classifies the `PJ_builtin_object_type_t` enumerators (the types that
+shipped before 1.0.0 as baseline), so the floor checker flags a plugin that uses either new type
+with a `min_sdk_required` below 1.0.0: the floor is hard, not negotiated, because an older host
+cannot decode the object. `Log` gains an optional field and renames
 one on the same wire, and the widget-protocol changes are documentation only; `pj.playback.v1`
 already has its floor in `feature_floors.json`.
 
@@ -117,6 +118,26 @@ Removed:
 - **Breaking (source):** `PJ::sdk::MediaMetadataBuilder` (`pj_base/sdk/media_metadata.hpp`). Use
   `ObjectTopicMetadataBuilder` (`pj_base/sdk/object_topic_metadata.hpp`).
 
+Fixes:
+
+- A `PJ_string_view_t` with null data and a non-zero size is now an empty string everywhere the SDK
+  converts one: `sdk::toStringView`, the service-registry lookup, and the `saveConfig()` readers of
+  the DataSource, MessageParser and Toolbox handles. They used to read `size` bytes past a one-byte
+  literal, on views supplied by a plugin.
+- `WidgetDataView::chartMarkers()`, `markerTimelineMarks()` and `WidgetEvent::markerTimelineChanged()`
+  skip a marker entry that is not an object or has a wrong-typed field; they used to throw
+  `nlohmann::json::type_error`, so one malformed plugin update could take the host down. An integer
+  field must now hold a JSON integer: a float there (`"start": 3.0`) skips the entry instead of being
+  truncated.
+  `WidgetDataView::ChartMarkerView` is now an alias of `PJ::ChartMarker` (same fields).
+- `DialogHandle` gains `valid()`; when the plugin's `create()` returns null (its constructor threw),
+  every method returns empty / false or does nothing instead of passing the null context to the
+  plugin.
+- `WidgetData::toJson()` and `WidgetEventBuilder` replace invalid UTF-8 with U+FFFD; one non-UTF-8
+  string (a label read from a binary log) used to make the whole dialog update vanish silently.
+- The Conan package now propagates `PJ_ASSERT_THROWS` to consumers when built with
+  `assert_throws=True`; consumers compiled `PJ_ASSERT` as `assert` against archives built to throw.
+
 ## [0.40.0]
 
 Host contract: extended: pj.mcp_endpoint.v1, PJ_mcp_endpoint_host_vtable_t::acquire_endpoint,

conanfile.py

@@ -162,6 +162,10 @@ def package_info(self):
         base.set_property("cmake_target_name", "plotjuggler_sdk::base")
         base.libs = ["pj_base"]
         base.includedirs = ["include"]
+        # PUBLIC on pj_base in CMake, but CMakeDeps ignores the installed targets file:
+        # consumers must compile PJ_ASSERT the way the archives were built (ODR).
+        if self.options.assert_throws:
+            base.defines = ["PJ_ASSERT_THROWS"]
 
         # --- plugin_sdk (umbrella INTERFACE: pj_base + pj_dialog_sdk) ---
         sdk = self.cpp_info.components["plugin_sdk"]

pj_base/feature_floors.json

@@ -53,6 +53,18 @@
       },
       "acquireMcpEndpoint": "PJ_mcp_endpoint_host_vtable_t::acquire_endpoint",
       "releaseMcpEndpoint": "PJ_mcp_endpoint_host_vtable_t::release_endpoint"
+    },
+    "1.0.0": {
+      "kMeshMap": {
+        "declaration": "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_MESH_MAP",
+        "negotiated": false,
+        "aliases": ["MeshMap", "serializeMeshMap"]
+      },
+      "kMeshMapUpdate": {
+        "declaration": "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_MESH_MAP_UPDATE",
+        "negotiated": false,
+        "aliases": ["MeshMapUpdate", "serializeMeshMapUpdate"]
+      }
     }
   },
   "baseline": [
@@ -61,6 +73,25 @@
     "PJ_DESCRIPTOR_IMPORT_START_FLAG_NONE",
     "PJ_PARSER_ROUTE_FLAG_OBJECT_V1",
     "PJ_PARSER_ROUTE_FLAG_SCALAR_V1",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_CAMERA_INFO",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_COMPRESSED_POINTCLOUD",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_DEPTH_IMAGE",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_FRAME_TRANSFORMS",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_GRID_MAP",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_IMAGE",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_IMAGE_ANNOTATIONS",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_LOG",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_MESH3D",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_NONE",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_OCCUPANCY_GRID",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_OCCUPANCY_GRID_UPDATE",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_PLOT_MARKERS",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_POINTCLOUD",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_POSES_IN_FRAME",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_ROBOT_DESCRIPTION",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_SCENE_ENTITIES",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_VIDEO_FRAME",
+    "PJ_builtin_object_type_t::PJ_BUILTIN_OBJECT_TYPE_VOXEL_GRID",
     "PJ_data_processors_host_vtable_t::create_data_processor",
     "PJ_data_processors_host_vtable_t::data_processor_config",
     "PJ_data_processors_host_vtable_t::list_data_processor_ids",

pj_base/include/pj_base/sdk/plugin_data_api.hpp

@@ -320,8 +320,12 @@ namespace detail {
 
 }  // namespace detail
 
+/// Null data reads as empty whatever the size: a malformed {nullptr, size > 0} view is never dereferenced.
 [[nodiscard]] inline std::string_view toStringView(PJ_string_view_t view) {
-  return std::string_view(view.data == nullptr ? "" : view.data, view.size);
+  if (view.data == nullptr) {
+    return "";
+  }
+  return std::string_view(view.data, view.size);
 }
 
 [[nodiscard]] inline PJ_string_view_t toAbiString(std::string_view view) {

pj_base/include/pj_base/sdk/testing/parser_write_recorder.hpp

@@ -173,7 +173,7 @@ class ParserWriteRecorder {
       void* ctx, PJ_string_view_t name, PJ_primitive_type_t, PJ_field_handle_t* out_field, PJ_error_t*) noexcept {
     auto* self = static_cast<ParserWriteRecorder*>(ctx);
     uint32_t id = self->next_field_id_++;
-    self->field_names_.emplace(id, std::string(name.data == nullptr ? "" : name.data, name.size));
+    self->field_names_.emplace(id, name.data == nullptr ? std::string() : std::string(name.data, name.size));
     *out_field = PJ_field_handle_t{PJ_topic_handle_t{1}, id};
     return true;
   }

pj_base/tests/plugin_data_api_test.cpp

@@ -121,14 +121,10 @@ TEST(PluginDataApiTest, ToStringViewHandlesNullData) {
   EXPECT_EQ(sdk::toStringView(null_zero), "");
   EXPECT_EQ(sdk::toStringView(null_zero).size(), 0U);
 
-  // Null data pointer with non-zero size → still uses "" base but with the given size
-  // The SDK does: std::string_view(view.data == nullptr ? "" : view.data, view.size)
-  // With nullptr and size=5, it constructs std::string_view("", 5) which has size 5
-  // but that reads past the empty string literal — this tests current behavior
+  // Null data pointer with non-zero size (a malformed, possibly plugin-supplied view) → empty,
+  // never a view of `size` bytes past a one-byte literal.
   const PJ_string_view_t null_nonzero{nullptr, 5};
-  const auto result = sdk::toStringView(null_nonzero);
-  // The key thing: it doesn't crash (null guard prevents UB from nullptr dereference)
-  EXPECT_NE(result.data(), nullptr);
+  EXPECT_EQ(sdk::toStringView(null_nonzero).size(), 0U);
 }
 
 TEST(PluginDataApiTest, SourceWriteHostViewRejectsMissingAppendArrowStreamTailSlot) {

pj_plugins/dialog_protocol/include/pj_plugins/host/dialog_handle.hpp

@@ -68,6 +68,15 @@ class DialogHandle {
   DialogHandle(const DialogHandle&) = delete;
   DialogHandle& operator=(const DialogHandle&) = delete;
 
+  /// False when there is no vtable or no context (the plugin's create() returned
+  /// null, e.g. its constructor threw, or the handle was moved from). Every method
+  /// below then returns its "nothing" value (empty string / false / no-op) without
+  /// calling into the plugin.
+  /// @since 1.0.0
+  [[nodiscard]] bool valid() const noexcept {
+    return vt_ != nullptr && ctx_ != nullptr;
+  }
+
   /// Supply immutable host information to this dialog instance.
   ///
   /// This method owns the optional-tail-slot gate: runtime engines must call it
@@ -84,52 +93,58 @@ class DialogHandle {
   ///
   /// @since 0.21.0
   [[nodiscard]] SetHostInfoResult setHostInfo(const PJ_dialog_host_info_t& info, PJ_error_t* out_error = nullptr) {
-    if (vt_ == nullptr || ctx_ == nullptr || !PJ_HAS_TAIL_SLOT(PJ_dialog_vtable_t, vt_, set_host_info)) {
+    if (!valid() || !PJ_HAS_TAIL_SLOT(PJ_dialog_vtable_t, vt_, set_host_info)) {
       return SetHostInfoResult::Unsupported;
     }
     return vt_->set_host_info(ctx_, &info, out_error) ? SetHostInfoResult::Accepted : SetHostInfoResult::Rejected;
   }
 
-  // --- Queries ---
+  // --- Queries (empty when !valid()) ---
   [[nodiscard]] std::string manifest() const {
-    return safeString(vt_->get_manifest(ctx_));
+    return valid() ? safeString(vt_->get_manifest(ctx_)) : std::string();
   }
   [[nodiscard]] std::string ui_content() const {
-    return safeString(vt_->get_ui_content(ctx_));
+    return valid() ? safeString(vt_->get_ui_content(ctx_)) : std::string();
   }
   [[nodiscard]] std::string widget_data() const {
-    return safeString(vt_->get_widget_data(ctx_));
+    return valid() ? safeString(vt_->get_widget_data(ctx_)) : std::string();
   }
 
-  // --- Events (fallible — errors swallowed here; callers that need detail call vtable directly) ---
+  // --- Events (fallible — errors swallowed here; callers that need detail call vtable directly; false when
+  // !valid()) ---
   [[nodiscard]] bool sendEvent(std::string_view widget_name, std::string_view event_json) {
-    return vt_->on_widget_event(ctx_, std::string(widget_name).c_str(), std::string(event_json).c_str(), nullptr);
+    return valid() &&
+           vt_->on_widget_event(ctx_, std::string(widget_name).c_str(), std::string(event_json).c_str(), nullptr);
   }
 
   [[nodiscard]] bool tick() {
-    return vt_->on_tick(ctx_, nullptr);
+    return valid() && vt_->on_tick(ctx_, nullptr);
   }
 
-  // --- Dialog result ---
+  // --- Dialog result (no-ops when !valid()) ---
   void accept(std::string_view final_state_json) {
-    vt_->on_accepted(ctx_, std::string(final_state_json).c_str());
+    if (valid()) {
+      vt_->on_accepted(ctx_, std::string(final_state_json).c_str());
+    }
   }
   void reject() {
-    vt_->on_rejected(ctx_);
+    if (valid()) {
+      vt_->on_rejected(ctx_);
+    }
   }
 
-  // --- Config persistence ---
+  // --- Config persistence (empty / false when !valid()) ---
   [[nodiscard]] std::string save_config() const {
     PJ_string_view_t sv{};
-    if (!vt_->save_config(ctx_, &sv, nullptr)) {
+    if (!valid() || !vt_->save_config(ctx_, &sv, nullptr)) {
       return std::string();
     }
     return sv.data == nullptr ? std::string() : std::string(sv.data, sv.size);
   }
 
   [[nodiscard]] bool load_config(std::string_view config_json) {
     PJ_string_view_t sv{config_json.data(), config_json.size()};
-    return vt_->load_config(ctx_, sv, nullptr);
+    return valid() && vt_->load_config(ctx_, sv, nullptr);
   }
 
   // --- Escape hatch ---

pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp

@@ -18,7 +18,7 @@
 
 #include "pj_base/expected.hpp"
 #include "pj_base/number_parse.hpp"
-#include "pj_plugins/sdk/widget_data.hpp"  // TimelineMark, SceneView
+#include "pj_plugins/sdk/widget_data.hpp"  // ChartMarker, TimelineMark, SceneView
 
 namespace PJ {
 
@@ -706,20 +706,13 @@ class WidgetDataView {
     return view;
   }
 
-  /// A marker overlaid on the chart (see PJ::ChartMarker / setChartMarkers).
+  /// A marker overlaid on the chart: the plugin-side PJ::ChartMarker (see setChartMarkers),
+  /// decoded by the shared chartMarkersFromJson.
   /// 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;
-  };
+  using ChartMarkerView = ChartMarker;
 
+  /// Entries that are not objects or carry a wrong-typed field are skipped (never throws).
   [[nodiscard]] std::optional<std::vector<ChartMarkerView>> chartMarkers(std::string_view name) const {
     const nlohmann::json* w = widget(name);
     if (!w) {
@@ -729,24 +722,7 @@ class WidgetDataView {
     if (it == w->end() || !it->is_array()) {
       return std::nullopt;
     }
-    std::vector<ChartMarkerView> 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;
+    return chartMarkersFromJson(*it);
   }
 
   /// Returns whether interactive zoom is enabled on this chart widget.