Skip to content

feat: uniform array-size policy across parser & streaming plugins#136

Open
pabloinigoblasco wants to merge 1 commit into
mainfrom
feat/uniform-array-size-policy
Open

feat: uniform array-size policy across parser & streaming plugins#136
pabloinigoblasco wants to merge 1 commit into
mainfrom
feat/uniform-array-size-policy

Conversation

@pabloinigoblasco
Copy link
Copy Markdown
Contributor

Summary

Every parser and streaming plugin in this repo gains a uniform "maximum
array size + clamp/skip" option, with the same defaults and JSON keys
regardless of which plugin emits the configuration. Parsers flatten each
array element into its own scalar series (field[0], field[1], …), so
an unbounded array previously meant an unbounded number of series; this
contract caps that, and the new UI lets the user pick the limit per
parser dialog.

Why

Until now each parser/stream re-implemented the option with different
member names, different JSON keys, and different defaults
(clamp_large_arrays vs discard_large_arrays). That drift made layouts
and configs non-portable between plugins and surprised users when the
same control behaved differently. A single contract removes the
divergence and lets every plugin read/write the option the same way.

Where the contract lives

The contract is header-only (struct + two JSON helpers, no SDK
dependency beyond nlohmann/json) and only plugins in this repo consume
it. It ships as a local INTERFACE library:

common/array_policy/
├── CMakeLists.txt                                 (target: pj_array_policy)
├── include/pj_array_policy/array_policy.hpp       (namespace pj::array_policy)
└── tests/array_policy_test.cpp                    (8 gtest cases)

Third-party plugins remain cross-compatible by honoring the two JSON
keys documented in the header — they do not have to link the library.

Canonical JSON keys

Key Type Default Notes
max_array_size uint32 500 0 means unlimited.
array_policy string "clamp" "clamp" or "skip".

Legacy keys are read with lower priority as a fallback, so older configs
and layouts still load:

Legacy key Maps to
discard_large_arrays array_policy skip
clamp_large_arrays array_policy clamp/skip

arrayLimitToJson always writes both the canonical and legacy keys so a
binary built before this contract can still honor the policy.

Adoption per plugin

  • parser_json, parser_protobuf, parser_ros — link the contract,
    expose the option in their dialog .ui (default clamp/500).
  • data_stream_foxglove_bridge, data_stream_pj_bridge,
    data_stream_ros2 (distro) — propagate the value through ArrayLimit
    when forwarding the configuration to the embedded parser.
  • data_load_mcap — drop a now-redundant array-size control; the
    embedded parser dialog handles it.

Testing

conan install . --output-folder=build --build=missing -s build_type=Release -s compiler.cppstd=20
cmake -S . -B build -DCMAKE_TOOLCHAIN_FILE=build/conan_toolchain.cmake
cmake --build build --target array_policy_test parser_json_plugin parser_protobuf_plugin parser_ros_plugin foxglove_source_plugin pj_bridge_source_plugin
ctest --test-dir build -R array_policy_test

All targets compile clean; array_policy_test passes 8/8 (defaults,
canonical keys, legacy fallback, zero-as-unlimited, canonical-wins-over-
legacy, round-trip).

Test plan

  • CI green on all matrix jobs.
  • Manual smoke: import a ROS2 bag with a message containing a
    large array, verify the UI shows the new control and that
    changing clamp ↔ skip updates the materialized series accordingly.
  • Manual smoke: load an existing layout written with a previous
    plugin version (using clamp_large_arrays) and verify the
    policy is restored as Clamp.

Add a shared array-size policy contract (max_array_size + clamp/skip)
adopted by every parser and streaming plugin, so the option carries the
same defaults and JSON keys regardless of which plugin emits the config.

Contract lives in common/array_policy/ as a header-only INTERFACE
library:

  common/array_policy/include/pj_array_policy/array_policy.hpp
  common/array_policy/CMakeLists.txt           (target: pj_array_policy)
  common/array_policy/tests/array_policy_test.cpp

Canonical keys read/written on every config:

  "max_array_size" : uint32 (0 means unlimited)
  "array_policy"   : "clamp" | "skip"

Legacy keys ("clamp_large_arrays", "discard_large_arrays") still work as
a fallback, so configs and layouts written by earlier plugin versions
keep loading without change.

Adoption:

  parser_json, parser_protobuf, parser_ros             — link contract,
    new UI (Qt .ui file) exposing the option in the dialog.
  data_stream_foxglove_bridge, data_stream_pj_bridge,
    data_stream_ros2 (distro)                          — route the value
    through ArrayLimit when forwarding the configuration to embedded
    parsers; default is now Clamp/500.
  data_load_mcap                                       — drop a dead
    array-size widget that is now handled by the embedded parser dialog.

The contract has no SDK dependency (only nlohmann/json) and is consumed
only by plugins in this repo, so it ships here rather than in
plotjuggler_sdk. Third-party plugins remain cross-compatible by honoring
the two JSON keys above; they do not have to link the library.

Contents:
- common/array_policy/{include,tests,CMakeLists.txt}: new shared lib + 8
  gtest cases covering canonical keys, legacy fallback, zero-as-
  unlimited, JSON round-trip.
- parser_json/json_parser{,_dialog}.{cpp,hpp} + .ui — adopt ArrayLimit,
  add UI control with default Clamp/500.
- parser_protobuf/protobuf_parser{,_dialog}.{cpp,hpp} + .ui — same.
- parser_ros/ros_parser{,_dialog}.{cpp,hpp} — same.
- data_stream_foxglove_bridge/{foxglove_dialog.hpp,foxglove_source.cpp,
  CMakeLists.txt} — propagate ArrayLimit to the inner parser config.
- data_stream_pj_bridge/{pj_bridge_dialog.hpp,pj_bridge_source.cpp,
  CMakeLists.txt} — same.
- data_stream_ros2/distro/src/ros2_dialog.hpp + CMakeLists.txt — same.
- data_load_mcap/mcap_dialog.hpp — remove the now-redundant control.
- Root CMakeLists.txt — add_subdirectory(common/array_policy) in both
  standalone and subdir modes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant