data_load_ulog: harden raw record extraction and test the real decode path (#167)

The importer flattens each ULog MessageFormat and reads raw bytes at
resolved field offsets, rather than using ulog_cpp's typed accessors.
That offset/array/nesting logic lived inline in three hand-synchronized
traversals and was only ever validated indirectly (the tests exercised
ulog_cpp's own accessors, never the plugin's raw path).

- Extract the SDK-free traversal into ulog_flatten.hpp: a single
  forEachFlatLeaf primitive (yielding absolute offset, type, size) plus
  collectFlatFieldNames, with one isSkippedField rule. The type/value
  passes become thin visitors, so the skip/array/nested-offset logic
  exists in exactly one place instead of three.
- Bounds-check value extraction: a leaf whose offset+size exceeds the
  record size now pushes NullValue instead of reading out of bounds,
  preserving name<->value alignment on truncated/corrupt records.
- Add <cstdio>, <map>, <memory> (previously pulled in transitively).
- Add tests that drive the real traversal against a nested+array+scalar
  record with known bytes, asserting names, decoded values at the
  reported offsets, the sizeBytes() boundary, and name<->leaf alignment.

No behavior change on valid logs. All 10 tests pass.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: facontidavide

data_load_ulog/tests/ulog_source_test.cpp

@@ -1,12 +1,17 @@
 #include <gtest/gtest.h>
 
+#include <algorithm>
+#include <cstddef>
+#include <cstdint>
 #include <cstring>
 #include <memory>
 #include <string>
 #include <ulog_cpp/data_container.hpp>
 #include <ulog_cpp/reader.hpp>
 #include <vector>
 
+#include "../ulog_flatten.hpp"
+
 namespace {
 
 // Helper to build a binary ULog stream in memory.
@@ -357,4 +362,149 @@ TEST(ULogSourceTest, AllNumericTypes) {
   EXPECT_EQ(sample["flag"].as<bool>(), true);
 }
 
+// --- Tests for the plugin's own flatten/extract path (ulog_flatten.hpp) ---
+//
+// The tests above validate ulog_cpp's typed accessors. The importer does NOT use
+// those at runtime: it flattens the format and reads raw bytes at resolved
+// offsets. These tests drive that production path directly.
+
+// Decode a scalar leaf from raw bytes the same way the plugin does, returning a
+// double for comparison against known inputs.
+double decodeLeaf(const std::vector<uint8_t>& raw, size_t offset, ulog_cpp::Field::BasicType type) {
+  const uint8_t* p = raw.data() + offset;
+  switch (type) {
+    case ulog_cpp::Field::BasicType::INT8:
+      return static_cast<double>(*reinterpret_cast<const int8_t*>(p));
+    case ulog_cpp::Field::BasicType::UINT8:
+    case ulog_cpp::Field::BasicType::CHAR:
+      return static_cast<double>(*p);
+    case ulog_cpp::Field::BasicType::BOOL:
+      return static_cast<double>(*p != 0);
+    case ulog_cpp::Field::BasicType::INT16: {
+      int16_t v;
+      std::memcpy(&v, p, sizeof(v));
+      return v;
+    }
+    case ulog_cpp::Field::BasicType::UINT16: {
+      uint16_t v;
+      std::memcpy(&v, p, sizeof(v));
+      return v;
+    }
+    case ulog_cpp::Field::BasicType::INT32: {
+      int32_t v;
+      std::memcpy(&v, p, sizeof(v));
+      return v;
+    }
+    case ulog_cpp::Field::BasicType::UINT32: {
+      uint32_t v;
+      std::memcpy(&v, p, sizeof(v));
+      return v;
+    }
+    case ulog_cpp::Field::BasicType::INT64: {
+      int64_t v;
+      std::memcpy(&v, p, sizeof(v));
+      return static_cast<double>(v);
+    }
+    case ulog_cpp::Field::BasicType::UINT64: {
+      uint64_t v;
+      std::memcpy(&v, p, sizeof(v));
+      return static_cast<double>(v);
+    }
+    case ulog_cpp::Field::BasicType::FLOAT: {
+      float v;
+      std::memcpy(&v, p, sizeof(v));
+      return v;
+    }
+    case ulog_cpp::Field::BasicType::DOUBLE: {
+      double v;
+      std::memcpy(&v, p, sizeof(v));
+      return v;
+    }
+    default:
+      return 0.0;
+  }
+}
+
+// Build a message that exercises nesting + arrays + a scalar tail, then verify the
+// flattened names and the values decoded at the offsets forEachFlatLeaf reports.
+// This locks in the nested base-offset accumulation the importer relies on.
+TEST(ULogFlattenTest, NamesAndOffsetsMatchKnownRecord) {
+  ULogBuilder builder;
+  builder.writeHeader(0);
+  builder.writeFlagBits();
+  builder.writeFormat("vec3:float x;float y;float z");
+  builder.writeFormat("sample:uint64_t timestamp;vec3 pos;uint8_t mode;float[2] arr;int32_t code");
+  builder.writeSubscription(1, 0, "sample");
+
+  {
+    FieldDataBuilder fields;
+    fields.append<uint64_t>(7000000);  // timestamp (skipped)
+    fields.append<float>(1.0f);        // pos.x
+    fields.append<float>(2.0f);        // pos.y
+    fields.append<float>(3.0f);        // pos.z
+    fields.append<uint8_t>(42);        // mode
+    fields.append<float>(4.0f);        // arr.00
+    fields.append<float>(5.0f);        // arr.01
+    fields.append<int32_t>(-77);       // code
+    builder.writeData(1, fields.build());
+  }
+
+  auto container = parseBuilder(builder);
+  ASSERT_FALSE(container->hadFatalError());
+  auto sub = container->subscription("sample");
+  ASSERT_NE(sub, nullptr);
+  ASSERT_EQ(sub->size(), 1u);
+
+  // Names, in flattened order.
+  std::vector<std::string> names;
+  ulog_flatten::collectFlatFieldNames(*sub->format(), {}, names);
+  const std::vector<std::string> expected_names = {"pos.x", "pos.y", "pos.z", "mode", "arr.00", "arr.01", "code"};
+  EXPECT_EQ(names, expected_names);
+
+  // Values decoded at the offsets forEachFlatLeaf reports, in the same order.
+  const auto& raw = sub->rawSamples()[0].data();
+  std::vector<double> values;
+  size_t max_end = 0;
+  ulog_flatten::forEachFlatLeaf(
+      *sub->format(), 0, [&](size_t offset, ulog_cpp::Field::BasicType type, size_t elem_size) {
+        ASSERT_LE(offset + elem_size, raw.size());  // offsets stay inside the record
+        max_end = std::max(max_end, offset + elem_size);
+        values.push_back(decodeLeaf(raw, offset, type));
+      });
+
+  const std::vector<double> expected_values = {1.0, 2.0, 3.0, 42.0, 4.0, 5.0, -77.0};
+  ASSERT_EQ(values.size(), expected_values.size());
+  for (size_t i = 0; i < values.size(); ++i) {
+    EXPECT_DOUBLE_EQ(values[i], expected_values[i]) << "leaf " << i << " (" << names[i] << ")";
+  }
+
+  // The furthest byte any leaf reads must equal the record size, so a caller can
+  // bound-check with `offset + size <= raw_size` and a truncated record trips it.
+  EXPECT_EQ(max_end, raw.size());
+  EXPECT_EQ(max_end, static_cast<size_t>(sub->format()->sizeBytes()));
+}
+
+// Names and leaf count must stay in lockstep so the importer can zip them.
+TEST(ULogFlattenTest, NameAndLeafCountsAgree) {
+  ULogBuilder builder;
+  builder.writeHeader(0);
+  builder.writeFlagBits();
+  builder.writeFormat("vec3:float x;float y;float z");
+  builder.writeFormat("sample:uint64_t timestamp;vec3 pos;float[2] arr;int32_t code");
+  builder.writeSubscription(1, 0, "sample");
+
+  auto container = parseBuilder(builder);
+  ASSERT_FALSE(container->hadFatalError());
+  auto sub = container->subscription("sample");
+  ASSERT_NE(sub, nullptr);
+
+  std::vector<std::string> names;
+  ulog_flatten::collectFlatFieldNames(*sub->format(), {}, names);
+
+  size_t leaf_count = 0;
+  ulog_flatten::forEachFlatLeaf(*sub->format(), 0, [&](size_t, ulog_cpp::Field::BasicType, size_t) { ++leaf_count; });
+
+  EXPECT_EQ(names.size(), leaf_count);
+}
+
 }  // namespace

data_load_ulog/ulog_flatten.hpp

@@ -0,0 +1,96 @@
+#pragma once
+
+// SDK-free helpers that flatten a ulog_cpp MessageFormat into the ordered list
+// of scalar leaves PlotJuggler ingests. The traversal logic (field skipping,
+// array expansion, and the nested base-offset accumulation) lives here so it can
+// be unit-tested directly, without pulling in the plugin SDK. The SDK-coupled
+// pieces (ValueRef construction, PrimitiveType mapping) stay in ulog_source.cpp.
+
+#include <cstddef>
+#include <cstdio>
+#include <string>
+#include <ulog_cpp/messages.hpp>
+#include <vector>
+
+namespace ulog_flatten {
+
+/// Fields the importer never emits as time series: the leading message timestamp
+/// (used as the time axis) and ULog alignment padding.
+inline bool isSkippedField(const std::string& name) {
+  return name == "timestamp" || name.starts_with("_padding");
+}
+
+/// Recursively collect flattened field names for a ulog_cpp MessageFormat.
+/// Nested fields are separated by "." and array elements get ".00", ".01", etc.
+inline void collectFlatFieldNames(
+    const ulog_cpp::MessageFormat& format, const std::string& prefix, std::vector<std::string>& out) {
+  for (const auto& field_ptr : format.fields()) {
+    const auto& field = *field_ptr;
+    const std::string& name = field.name();
+    if (isSkippedField(name)) {
+      continue;
+    }
+
+    std::string new_prefix = prefix.empty() ? name : prefix + "." + name;
+
+    int arr_len = field.arrayLength();
+    int count = (arr_len < 0) ? 1 : arr_len;
+    bool is_array = (arr_len > 0);
+
+    for (int i = 0; i < count; ++i) {
+      std::string elem_name = new_prefix;
+      if (is_array) {
+        char buf[16];
+        std::snprintf(buf, sizeof(buf), ".%02d", i);
+        elem_name += buf;
+      }
+
+      if (field.type().type == ulog_cpp::Field::BasicType::NESTED) {
+        auto nested_fmt = field.nestedFormat();
+        if (nested_fmt) {
+          collectFlatFieldNames(*nested_fmt, elem_name, out);
+        }
+      } else {
+        out.push_back(elem_name);
+      }
+    }
+  }
+}
+
+/// Visit every scalar leaf of a MessageFormat in the same order as
+/// collectFlatFieldNames, invoking `visit(absolute_byte_offset, basic_type,
+/// element_size)`. `base_offset` is the byte offset of `format` within the raw
+/// record (0 at the top level); nested structs accumulate their start offset so
+/// each leaf's offset is absolute, matching ulog_cpp's own resolution scheme
+/// (MessageFormat::resolveDefinition assigns per-format 0-based field offsets).
+template <typename Visitor>
+void forEachFlatLeaf(const ulog_cpp::MessageFormat& format, size_t base_offset, Visitor&& visit) {
+  for (const auto& field_ptr : format.fields()) {
+    const auto& field = *field_ptr;
+    if (isSkippedField(field.name())) {
+      continue;
+    }
+
+    int arr_len = field.arrayLength();
+    int count = (arr_len < 0) ? 1 : arr_len;
+    size_t field_offset = base_offset + static_cast<size_t>(field.offsetInMessage());
+
+    if (field.type().type == ulog_cpp::Field::BasicType::NESTED) {
+      auto nested_fmt = field.nestedFormat();
+      if (!nested_fmt) {
+        continue;
+      }
+      auto nested_size = static_cast<size_t>(nested_fmt->sizeBytes());
+      for (size_t i = 0; i < static_cast<size_t>(count); ++i) {
+        forEachFlatLeaf(*nested_fmt, field_offset + i * nested_size, visit);
+      }
+    } else {
+      size_t elem_size = static_cast<size_t>(field.type().size);
+      for (size_t i = 0; i < static_cast<size_t>(count); ++i) {
+        visit(field_offset + i * elem_size, field.type().type, elem_size);
+      }
+    }
+  }
+}
+
+}  // namespace ulog_flatten