diff --git a/data_load_mcap/CMakeLists.txt b/data_load_mcap/CMakeLists.txt index e596d34f..6609d363 100644 --- a/data_load_mcap/CMakeLists.txt +++ b/data_load_mcap/CMakeLists.txt @@ -44,6 +44,23 @@ target_include_directories(mcap_helpers_test SYSTEM PRIVATE ${CMAKE_CURRENT_SOUR target_link_libraries(mcap_helpers_test PRIVATE LZ4::lz4_static zstd::libzstd_static GTest::gtest_main) add_test(NAME mcap_helpers_test COMMAND mcap_helpers_test) +# --- Unit test for MCAP dialog model/config behavior --- +add_executable(mcap_dialog_test tests/mcap_dialog_test.cpp) +target_compile_features(mcap_dialog_test PRIVATE cxx_std_20) +target_compile_options(mcap_dialog_test PRIVATE ${PJ_WARNING_FLAGS}) +target_compile_definitions(mcap_dialog_test PRIVATE + MCAP_TEST_DATA_DIR="${CMAKE_CURRENT_SOURCE_DIR}/test_data" +) +target_include_directories(mcap_dialog_test PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR} + ${CMAKE_CURRENT_BINARY_DIR}/generated +) +target_include_directories(mcap_dialog_test SYSTEM PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/contrib) +target_link_libraries(mcap_dialog_test PRIVATE + plotjuggler_sdk::plugin_sdk nlohmann_json::nlohmann_json LZ4::lz4_static zstd::libzstd_static GTest::gtest_main +) +add_test(NAME mcap_dialog_test COMMAND mcap_dialog_test) + # --- Unit test for the MessageByteStore (hot/cold lazy byte layer) --- find_package(Threads REQUIRED) add_executable(message_byte_store_test tests/message_byte_store_test.cpp) diff --git a/data_load_mcap/README.md b/data_load_mcap/README.md index 722e8da5..b100db34 100644 --- a/data_load_mcap/README.md +++ b/data_load_mcap/README.md @@ -75,6 +75,19 @@ so the user can select which topics to import. Per-import options: - **Embedded timestamp** — extract from message headers when supported by the parser (e.g. ROS `header.stamp`). +## Always-included channels + +Channels whose `(schema name, encoding)` matches a small built-in whitelist +are always loaded, pre-checked and locked in the picker, regardless of the +user's selection — because PJ4's Scene3D needs them to place other 3D +objects (`PointCloud`/`Mesh3D`) and silently breaks without them. Today the +whitelist covers ROS 2's transform tree (`tf2_msgs/msg/TFMessage` over +`cdr`) and Foxglove's (`foxglove.FrameTransform` over `protobuf`). The match +is on message type, not topic name — Foxglove's convention has no fixed +topic name the way ROS fixes `/tf`. This keeps the loader itself +format-agnostic: the whitelist is just data (`kAlwaysIncludeRules` in +`mcap_dialog.hpp`), not a ROS-specific code path. + ## How parsers see this loader Because the loader speaks only `pushMessage` with a fetcher, any diff --git a/data_load_mcap/mcap_dialog.hpp b/data_load_mcap/mcap_dialog.hpp index 4113b76b..faf24274 100644 --- a/data_load_mcap/mcap_dialog.hpp +++ b/data_load_mcap/mcap_dialog.hpp @@ -24,6 +24,33 @@ struct ChannelInfo { uint64_t msg_count = 0; }; +struct AlwaysIncludeRule { + std::string schema_name; + std::string encoding; +}; + +/// Channels whose (schema, encoding) match a rule here are always loaded, +/// regardless of user selection -- 3D rendering (Scene3D's transform tree) +/// silently breaks without them. Keyed on message type rather than topic +/// name because topic naming isn't consistent across producers: Foxglove's +/// own foxglove.FrameTransform has no fixed topic-name convention the way +/// ROS fixes /tf (real Foxglove-recorded files use topic names like plain +/// "tf", no leading slash, or something else entirely). The mechanism here +/// is generic -- this table is just its (currently two-entry) data. +const std::vector kAlwaysIncludeRules = { + {"tf2_msgs/msg/TFMessage", "cdr"}, + {"foxglove.FrameTransform", "protobuf"}, +}; + +bool isAlwaysIncluded(const ChannelInfo& ch) { + for (const auto& rule : kAlwaysIncludeRules) { + if (ch.schema == rule.schema_name && ch.encoding == rule.encoding) { + return true; + } + } + return false; +} + class McapDialog : public PJ::DialogPluginTyped { using PJ::DialogPluginTyped::onValueChanged; @@ -92,7 +119,7 @@ class McapDialog : public PJ::DialogPluginTyped { wd.setTableHeaders("tableWidget", headers); std::vector> rows; - std::vector selected_row_indices; + std::vector selected_topic_names; std::vector disabled_row_indices; rows.reserve(filtered.size()); @@ -100,15 +127,23 @@ class McapDialog : public PJ::DialogPluginTyped { const auto& ch = *filtered[i]; rows.push_back({ch.topic, ch.schema, ch.encoding, std::to_string(ch.msg_count)}); if (selected_topics_.count(ch.topic) > 0) { - selected_row_indices.push_back(static_cast(i)); + selected_topic_names.push_back(ch.topic); } - if (ch.msg_count == 0) { + bool locked_always_included = ch.msg_count > 0 && isAlwaysIncluded(ch); + if (ch.msg_count == 0 || locked_always_included) { disabled_row_indices.push_back(static_cast(i)); } + if (locked_always_included) { + wd.setCellTooltip( + "tableWidget", static_cast(i), 0, "Always loaded — required for 3D transform rendering."); + } } wd.setTableRows("tableWidget", rows); wd.setDisabledRows("tableWidget", disabled_row_indices); - wd.setSelectedRows("tableWidget", selected_row_indices); + // Restore selection by first-column text (the topic name), not row index: + // the host matches items by text, which is sort-agnostic, so the selection + // survives the table's built-in column sorting (sortingEnabled=true). + wd.setSelectedItems("tableWidget", selected_topic_names); wd.setShortcut("btnSelectAll", "Ctrl+A"); wd.setShortcut("btnDeselectAll", "Ctrl+Shift+A"); @@ -185,6 +220,7 @@ class McapDialog : public PJ::DialogPluginTyped { for (const auto& topic : selected) { selected_topics_.insert(topic); } + reassertAlwaysIncluded(); return true; // update OK button state } return false; @@ -202,6 +238,7 @@ class McapDialog : public PJ::DialogPluginTyped { } if (widget_name == "btnDeselectAll") { selected_topics_.clear(); + reassertAlwaysIncluded(); return true; } return false; @@ -332,6 +369,20 @@ class McapDialog : public PJ::DialogPluginTyped { } } } + + reassertAlwaysIncluded(); + } + + /// Channels matching kAlwaysIncludeRules are always loaded: 3D rendering + /// depends on them even when the user narrows their selection to a handful + /// of unrelated topics. Idempotent -- safe to call after any mutation of + /// selected_topics_. + void reassertAlwaysIncluded() { + for (const auto& ch : all_channels_) { + if (ch.msg_count > 0 && isAlwaysIncluded(ch)) { + selected_topics_.insert(ch.topic); + } + } } std::vector filteredChannels() const { diff --git a/data_load_mcap/test_data/generate_verification_mcaps.py b/data_load_mcap/test_data/generate_verification_mcaps.py index d1e6af1a..68c82204 100644 --- a/data_load_mcap/test_data/generate_verification_mcaps.py +++ b/data_load_mcap/test_data/generate_verification_mcaps.py @@ -7,6 +7,9 @@ Files generated: test_publish_vs_log_time.mcap -- Test 1: publish_time != log_time (offset 500ms) test_embedded_timestamp.mcap -- Test 2: Header.stamp differs from MCAP timestamp (+2s) + test_dialog_whitelist.mcap -- Test: always-include whitelist (tf2_msgs/TFMessage + + foxglove.FrameTransform matches, empty channel, near + miss, and ordinary channel) test_large_arrays.mcap -- Test 3: float64[1000] arrays for clamp/skip testing test_empty_channel.mcap -- Test 6: one channel with 0 messages test_unsupported_encoding.mcap -- Test 7: channel with encoding "protobuf" (no parser) @@ -69,6 +72,33 @@ def cdr_float64_fixed_array(values: list) -> bytes: b"string frame_id\n" ) +SCHEMA_TF2_MSGS = ( + b"geometry_msgs/TransformStamped[] transforms\n" + b"================\n" + b"MSG: geometry_msgs/TransformStamped\n" + b"Header header\n" + b"string child_frame_id\n" + b"Transform transform\n" + b"================\n" + b"MSG: std_msgs/Header\n" + b"int32 sec\n" + b"uint32 nanosec\n" + b"string frame_id\n" + b"================\n" + b"MSG: geometry_msgs/Transform\n" + b"Vector3 translation\n" + b"Quaternion rotation\n" +) + +SCHEMA_FOXGLOVE_FRAME_TRANSFORM_PROTO = ( + b'syntax = "proto3";\n' + b"package foxglove;\n" + b"message FrameTransform {\n" + b" string parent_frame_id = 2;\n" + b" string child_frame_id = 3;\n" + b"}\n" +) + OUT = Path(__file__).parent @@ -145,6 +175,72 @@ def gen_embedded_timestamp(): print(f"[OK] {path.name:45s} {path.stat().st_size:>8} bytes") +# --------------------------------------------------------------------------- +# Test: always-include whitelist (schema + encoding based, not topic name) +# --------------------------------------------------------------------------- + +def gen_dialog_whitelist(): + path = OUT / "test_dialog_whitelist.mcap" + n_msgs = 5 + dt_ns = 100_000_000 + + with open(path, "wb") as f: + w = Writer(f) + w.start(profile="ros2", library="pj-verification") + + # 1. ROS 2 /tf -- whitelisted (schema tf2_msgs/msg/TFMessage, encoding cdr). + schema_tf2 = w.register_schema( + name="tf2_msgs/msg/TFMessage", encoding="ros2msg", data=SCHEMA_TF2_MSGS, + ) + ch_tf = w.register_channel(topic="/tf", message_encoding="cdr", schema_id=schema_tf2) + + # 2. Foxglove transform under a non-conventional topic name -- whitelisted + # (schema foxglove.FrameTransform, encoding protobuf). Proves the match + # is on message type, not on topic naming: real Foxglove-recorded files + # use topic names like plain "tf" (no leading slash) or arbitrary names. + schema_foxglove = w.register_schema( + name="foxglove.FrameTransform", encoding="protobuf", + data=SCHEMA_FOXGLOVE_FRAME_TRANSFORM_PROTO, + ) + ch_foxglove = w.register_channel( + topic="transforms", message_encoding="protobuf", schema_id=schema_foxglove, + ) + + # 3. /tf_static -- same whitelisted schema/encoding, but zero messages: + # must NOT be force-included (nothing to load). + _ = w.register_channel(topic="/tf_static", message_encoding="cdr", schema_id=schema_tf2) + + # 4. Near miss: right schema name, wrong encoding -- must NOT be force-included. + schema_tf2_json = w.register_schema( + name="tf2_msgs/msg/TFMessage", encoding="jsonschema", data=SCHEMA_TF2_MSGS, + ) + ch_near_miss_encoding = w.register_channel( + topic="/near_miss_encoding", message_encoding="json", schema_id=schema_tf2_json, + ) + + # 5. Ordinary channel, unaffected by the whitelist. + schema_float = w.register_schema( + name="std_msgs/Float64", encoding="ros2msg", data=SCHEMA_FLOAT64, + ) + ch_ordinary = w.register_channel( + topic="/sensor/value2", message_encoding="cdr", schema_id=schema_float, + ) + + for i in range(n_msgs): + ts = i * dt_ns + w.add_message(channel_id=ch_tf, log_time=ts, publish_time=ts, data=cdr_float64(1.0)) + # Dummy protobuf bytes -- doesn't need to be valid, the dialog never + # decodes message content, only reads channel/schema metadata. + w.add_message(channel_id=ch_foxglove, log_time=ts, publish_time=ts, data=b"\x00" * 8) + w.add_message( + channel_id=ch_near_miss_encoding, log_time=ts, publish_time=ts, data=b'{"x":1}', + ) + w.add_message(channel_id=ch_ordinary, log_time=ts, publish_time=ts, data=cdr_float64(2.0)) + + w.finish() + print(f"[OK] {path.name:45s} {path.stat().st_size:>8} bytes") + + # --------------------------------------------------------------------------- # Test 3: Large arrays # --------------------------------------------------------------------------- @@ -354,6 +450,7 @@ def gen_large_file(): gen_publish_vs_log_time() gen_embedded_timestamp() + gen_dialog_whitelist() gen_large_arrays() gen_empty_channel() gen_unsupported_encoding() diff --git a/data_load_mcap/test_data/test_dialog_whitelist.mcap b/data_load_mcap/test_data/test_dialog_whitelist.mcap new file mode 100644 index 00000000..a7441380 Binary files /dev/null and b/data_load_mcap/test_data/test_dialog_whitelist.mcap differ diff --git a/data_load_mcap/tests/mcap_dialog_test.cpp b/data_load_mcap/tests/mcap_dialog_test.cpp new file mode 100644 index 00000000..4d71874e --- /dev/null +++ b/data_load_mcap/tests/mcap_dialog_test.cpp @@ -0,0 +1,161 @@ +#define MCAP_IMPLEMENTATION +#include "mcap_dialog.hpp" + +#include + +#include +#include +#include +#include + +namespace { + +TEST(McapDialogTest, DefaultConstructs) { + McapDialog dialog; + EXPECT_TRUE(dialog.selectedTopics().empty()); + EXPECT_TRUE(dialog.analyzeError().empty()); +} + +TEST(McapDialogTest, WidgetDataSelectsRowsByTextNotIndex) { + McapDialog dialog; + nlohmann::json cfg; + cfg["filepath"] = std::string(MCAP_TEST_DATA_DIR) + "/test_publish_vs_log_time.mcap"; + ASSERT_TRUE(dialog.loadConfig(cfg.dump())); + + const auto data = nlohmann::json::parse(dialog.widget_data()); + ASSERT_TRUE(data.contains("tableWidget")); + const auto& tbl = data["tableWidget"]; + + ASSERT_TRUE(tbl.contains("selected_items")); + std::vector selected_items = tbl["selected_items"].get>(); + EXPECT_EQ(selected_items, (std::vector{"/sensor/value"})); + + EXPECT_FALSE(tbl.contains("selected_rows")); +} + +TEST(AlwaysIncludeRuleTest, MatchesRos2Tf) { + ChannelInfo ch; + ch.topic = "/tf"; + ch.schema = "tf2_msgs/msg/TFMessage"; + ch.encoding = "cdr"; + ch.msg_count = 10; + EXPECT_TRUE(isAlwaysIncluded(ch)); +} + +TEST(AlwaysIncludeRuleTest, MatchesFoxgloveFrameTransformUnderAnyTopicName) { + ChannelInfo ch; + ch.topic = "transforms"; // deliberately not "/tf" + ch.schema = "foxglove.FrameTransform"; + ch.encoding = "protobuf"; + ch.msg_count = 10; + EXPECT_TRUE(isAlwaysIncluded(ch)); +} + +TEST(AlwaysIncludeRuleTest, RejectsRightSchemaWrongEncoding) { + ChannelInfo ch; + ch.topic = "/tf"; + ch.schema = "tf2_msgs/msg/TFMessage"; + ch.encoding = "json"; + ch.msg_count = 10; + EXPECT_FALSE(isAlwaysIncluded(ch)); +} + +TEST(AlwaysIncludeRuleTest, RejectsRightEncodingWrongSchema) { + ChannelInfo ch; + ch.topic = "/sensor/value"; + ch.schema = "std_msgs/Float64"; + ch.encoding = "cdr"; + ch.msg_count = 10; + EXPECT_FALSE(isAlwaysIncluded(ch)); +} + +TEST(McapDialogTest, ForceIncludesOnlyWhitelistedNonEmptyChannels) { + McapDialog dialog; + nlohmann::json cfg; + cfg["filepath"] = std::string(MCAP_TEST_DATA_DIR) + "/test_dialog_whitelist.mcap"; + // Saved selection predates this feature and omits everything except one + // ordinary topic -- as if loading an old layout. + cfg["selected_topics"] = std::vector{"/sensor/value2"}; + + ASSERT_TRUE(dialog.loadConfig(cfg.dump())); + + // Pin the fixture: all five channels must be present in the analyzed table, + // otherwise the negative assertions below would pass vacuously. + const auto data = nlohmann::json::parse(dialog.widget_data()); + std::set row_topics; + for (const auto& row : data["tableWidget"]["rows"]) { + row_topics.insert(row[0].get()); + } + EXPECT_EQ( + row_topics, (std::set{"/near_miss_encoding", "/sensor/value2", "/tf", "/tf_static", "transforms"})); + + const auto& selected = dialog.selectedTopics(); + EXPECT_TRUE(selected.count("/tf") > 0); // whitelisted, has messages -> forced back in + EXPECT_TRUE(selected.count("transforms") > 0); // whitelisted (any topic name) -> forced back in + EXPECT_EQ(selected.count("/tf_static"), 0u); // whitelisted schema/encoding, zero messages -> NOT forced + EXPECT_EQ(selected.count("/near_miss_encoding"), 0u); // right schema, wrong encoding -> NOT forced + EXPECT_TRUE(selected.count("/sensor/value2") > 0); // explicitly saved by the user -> stays +} + +TEST(McapDialogTest, WhitelistedRowsAreDisabledAndTooltippedInWidgetData) { + McapDialog dialog; + nlohmann::json cfg; + cfg["filepath"] = std::string(MCAP_TEST_DATA_DIR) + "/test_dialog_whitelist.mcap"; + ASSERT_TRUE(dialog.loadConfig(cfg.dump())); + + const auto data = nlohmann::json::parse(dialog.widget_data()); + const auto& rows = data["tableWidget"]["rows"]; + + int tf_row = -1; + int ordinary_row = -1; + for (size_t i = 0; i < rows.size(); ++i) { + if (rows[i][0] == "/tf") { + tf_row = static_cast(i); + } + if (rows[i][0] == "/sensor/value2") { + ordinary_row = static_cast(i); + } + } + ASSERT_NE(tf_row, -1); + ASSERT_NE(ordinary_row, -1); + + const auto& disabled = data["tableWidget"]["disabled_rows"]; + EXPECT_NE(std::find(disabled.begin(), disabled.end(), tf_row), disabled.end()); + EXPECT_EQ(std::find(disabled.begin(), disabled.end(), ordinary_row), disabled.end()); + + ASSERT_TRUE(data["tableWidget"].contains("cell_tooltips")); + const auto& tooltips = data["tableWidget"]["cell_tooltips"]; + EXPECT_TRUE(tooltips.contains(std::to_string(tf_row) + ",0")); +} + +TEST(McapDialogTest, DeselectAllKeepsWhitelistedTopicsSelected) { + McapDialog dialog; + nlohmann::json cfg; + cfg["filepath"] = std::string(MCAP_TEST_DATA_DIR) + "/test_dialog_whitelist.mcap"; + ASSERT_TRUE(dialog.loadConfig(cfg.dump())); + + EXPECT_TRUE(dialog.onClicked("btnDeselectAll")); + + const auto& selected = dialog.selectedTopics(); + EXPECT_TRUE(selected.count("/tf") > 0); + EXPECT_TRUE(selected.count("transforms") > 0); + EXPECT_EQ(selected.count("/sensor/value2"), 0u); +} + +TEST(McapDialogTest, HostReportedSelectionOmittingWhitelistIsOverridden) { + McapDialog dialog; + nlohmann::json cfg; + cfg["filepath"] = std::string(MCAP_TEST_DATA_DIR) + "/test_dialog_whitelist.mcap"; + ASSERT_TRUE(dialog.loadConfig(cfg.dump())); + + // Simulate the host reporting a selection that omits /tf and transforms -- + // the dialog must add them back regardless. + EXPECT_TRUE(dialog.onSelectionChanged("tableWidget", {"/sensor/value2"})); + + const auto& selected = dialog.selectedTopics(); + EXPECT_TRUE(selected.count("/tf") > 0); + EXPECT_TRUE(selected.count("transforms") > 0); + EXPECT_TRUE(selected.count("/sensor/value2") > 0); +} + +} // namespace