diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 914a3d6..faa6b02 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,38 @@ Changelog for package pj_ros_bridge ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- +* Message transforms (ROS2 only): operator-configured, off by default, a + topic is transformed for all subscribers or none — no per-client + negotiation. New ``transform_profile`` parameter points to a JSON file of + ordered per-topic rules (``match_type`` exact and required, optionally + narrowed by a ``match_topic`` full-match regex; first match wins; a transform + paired with a type it cannot handle is a startup error). A transformed topic is advertised + with its transform's output type/schema, plus an optional ``source_type`` + on ``get_topics`` entries (not yet on ``topics_changed`` entries); new + ``message_transforms`` server capability. +* New ``cloudini`` transform: ``sensor_msgs/msg/PointCloud2`` -> + ``point_cloud_interfaces/msg/CompressedPointCloud2`` using `Cloudini + `_ point cloud compression + (lossy at a configurable resolution; its own second compression stage is + disabled since the bridge already ZSTD-compresses every frame). Only + built when ``cloudini_lib`` is available (``find_package``, else fetched + at configure time unless ``-DPJ_BRIDGE_FETCH_CLOUDINI=OFF``); a profile + naming ``cloudini`` on a build without it fails at startup. +* ``strip_large_messages`` is now sugar for a ``strip`` transform rule + appended after the profile's own rules (an explicit profile rule for the + same type wins). +* **Behavior change:** a transform (including ``strip``) that fails on a + message now drops the sample instead of forwarding it — previously + ``strip_large_messages`` forwarded the original, unstripped message on + failure. +* Per-topic transform statistics (samples, drops, compression ratio, + microseconds/sample) logged with the final statistics at shutdown. +* The project now requires **C++20** (``std::span``, and ``cloudini_lib``'s + PUBLIC ``cxx_std_20`` requirement). +* 304 unit tests passing. + 0.10.0 (2026-09-20) ------------------- * WebSocket permessage-deflate declined server-side (redundant with our own diff --git a/CLAUDE.md b/CLAUDE.md index 5bf8581..94b8f0d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -69,6 +69,7 @@ make -j$(nproc) ### Code Quality +- **C++ Standard**: C++20 (required for `std::span`, and because `cloudini_lib` exports `cxx_std_20` as a PUBLIC requirement) - **Thread Safety**: Use mutexes, document thread safety in class comments - **Testing**: Unit tests required for all core components (gtest) - **Code Formatting**: `pre-commit run -a` before committing (clang-format) @@ -123,7 +124,7 @@ BridgeServer does NOT own timers. The entry point (`main.cpp`) drives the event 6. **Ros2TopicSource** (`ros2/`) — Wraps `TopicDiscovery` + `SchemaExtractor`. Schema encoding: `"ros2msg"`. -7. **Ros2SubscriptionManager** (`ros2/`) — Wraps `GenericSubscriptionManager` + optional `MessageStripper`. Converts `rclcpp::SerializedMessage` → `shared_ptr>` via memcpy. +7. **Ros2SubscriptionManager** (`ros2/`) — Wraps `GenericSubscriptionManager`. On `subscribe()`, resolves the topic's `TransformSet` binding (if any) to find the real source type and subscribes with it; in the message callback, a bound transform's `apply()` runs on the rcl buffer before forwarding (replacing the old inline `MessageStripper` call — `strip` is now a transform, see item 13). Converts `rclcpp::SerializedMessage` → `shared_ptr>` via memcpy (or via the transform's output when one is bound). 8. **RtiTopicSource** (`rti/`) — Wraps `DdsTopicDiscovery`. Schema encoding: `"omgidl"`. (Build disabled) @@ -133,6 +134,10 @@ BridgeServer does NOT own timers. The entry point (`main.cpp`) drives the event 11. **FastDdsSubscriptionManager** (`fastdds/`) — Directly implements `SubscriptionManagerInterface`. Creates `DataReader`s with `DynamicPubSubType`, deserializes into `DynamicData` and re-serializes to extract CDR bytes. +12. **TransformSet** (`app/`) — Owns the ordered `transform_profile` rules, the transform name → `TransformFactory` map, and the per-topic `BoundTransform` instances (created lazily on first match, `first match wins`). Thread-safe: written from the request thread (`get_topics`/`subscribe`), read from the ingest thread. Also tracks per-topic transform statistics (samples, drops, compression ratio, µs/sample), logged at shutdown. + +13. **TransformingTopicSource** (`app/`) — Decorator over a `TopicSourceInterface`. Rewrites `get_topics()`/`get_schema()` to the transform's output type/schema for matched topics (recording `source_type`) and passes everything else through unchanged. `strip` and `cloudini` are the two built-in transforms (`ros2/src/strip_transform.cpp`, `app/src/cloudini_transform.cpp`); `strip` replaces the old inline `MessageStripper` call in `Ros2SubscriptionManager` (see item 7). + ### Communication Pattern **WebSocket** (single port, default 9090): @@ -181,6 +186,9 @@ Then, for each message in the (compressed) payload: - **nlohmann/json** — JSON (`find_package(REQUIRED)`) - **tl::expected** — error handling (header-only, vendored in 3rdparty/) +### Core (optional) +- **Cloudini** (`cloudini_lib`) — point cloud compression for the `cloudini` transform. `find_package(cloudini_lib)` first, else fetched at configure time (pinned tag `1.3.1`, static) unless `-DPJ_BRIDGE_FETCH_CLOUDINI=OFF`. A build with neither simply lacks the `cloudini` transform (`PJ_BRIDGE_HAS_CLOUDINI` undefined); `strip` and the rest of the transform machinery are unaffected. + ### ROS2 Backend - `rclcpp`, `ament_index_cpp`, `ament_cmake` - `sensor_msgs`, `nav_msgs` (for message stripper) @@ -197,7 +205,7 @@ Then, for each message in the (compressed) payload: ## Testing -### Test Count: 231 unit tests across 11 test suites +### Test Count: 304 unit tests ### Commands ```bash @@ -233,6 +241,7 @@ port: 9090 # WebSocket port publish_rate: 50.0 # Hz session_timeout: 10.0 # seconds strip_large_messages: false # Opt-in: strip Image/PointCloud2/etc data fields +transform_profile: "" # Path to a per-topic message-transform profile JSON (e.g. Cloudini); empty disables topic_whitelist: [".*"] # Full-match regex patterns restricting visible/subscribable topics min_qos_depth: 10 # Minimum KEEP_LAST subscription depth after aggregating publisher depths max_qos_depth: 100 # Maximum KEEP_LAST subscription depth after aggregating publisher depths @@ -282,5 +291,5 @@ identity/capabilities object, and TLS setup). **Last Updated**: 2026-07-06 **Project Phase**: Unified multi-backend architecture -**Test Status**: 231 unit tests passing (all sanitizers clean) +**Test Status**: 304 unit tests passing (all sanitizers clean) **Executables**: `pj_bridge_ros2` (ROS2), `pj_bridge_rti` (RTI DDS, disabled), `pj_bridge_fastdds` (FastDDS) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5e75a07..4da834c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -15,7 +15,7 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") add_compile_options(-Wall -Wextra -Wpedantic) endif() -set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) # Sanitizer options @@ -88,6 +88,25 @@ else() message(STATUS "Using system IXWebSocket") endif() +# Cloudini (optional) — point cloud compression transform +option(PJ_BRIDGE_FETCH_CLOUDINI "Fetch Cloudini when cloudini_lib is not installed" ON) +# 1.3.1 validates malformed clouds itself and can be embedded without workarounds. +find_package(cloudini_lib 1.3.1 QUIET) +if(NOT cloudini_lib_FOUND AND PJ_BRIDGE_FETCH_CLOUDINI) + message(STATUS "cloudini_lib not found — fetching via FetchContent") + FetchContent_Declare( + cloudini + URL https://github.com/facontidavide/cloudini/archive/refs/tags/1.3.1.tar.gz + URL_HASH SHA256=d8c0e265bb841f59002e996bc957b4f0845b5399deb62b3247dc11115b48b4d9 + SOURCE_SUBDIR cloudini_lib + ) + # Reuse the zstd found above instead of a second, vendored copy in the same binary. + set(CLOUDINI_FORCE_VENDORED_DEPS OFF CACHE BOOL "" FORCE) + FetchContent_MakeAvailable(cloudini) + set_target_properties(cloudini_lib PROPERTIES POSITION_INDEPENDENT_CODE ON) + set(cloudini_lib_FOUND TRUE) +endif() + # spdlog — available via rosdep (libspdlog-dev) or pixi (conda-forge) find_package(spdlog REQUIRED) @@ -113,6 +132,8 @@ add_library(pj_bridge_app STATIC app/src/bridge_server.cpp app/src/standalone_event_loop.cpp app/src/whitelist_filter.cpp + app/src/transform_set.cpp + app/src/transforming_topic_source.cpp ) target_include_directories(pj_bridge_app PUBLIC @@ -127,6 +148,21 @@ target_link_libraries(pj_bridge_app PUBLIC nlohmann_json::nlohmann_json ) +if(cloudini_lib_FOUND) + # A standalone install exports cloudini::cloudini_lib, an ament install cloudini_lib::cloudini_lib. + if(TARGET cloudini::cloudini_lib) + set(_pj_cloudini_target cloudini::cloudini_lib) + else() + set(_pj_cloudini_target cloudini_lib::cloudini_lib) + endif() + target_sources(pj_bridge_app PRIVATE app/src/cloudini_transform.cpp) + target_link_libraries(pj_bridge_app PRIVATE ${_pj_cloudini_target}) + target_compile_definitions(pj_bridge_app PUBLIC PJ_BRIDGE_HAS_CLOUDINI) + message(STATUS "Cloudini transform: enabled") +else() + message(STATUS "Cloudini transform: disabled (cloudini_lib not found)") +endif() + # ============================================================================ # pj_bridge_ros2 — ROS2 backend (only if ament_cmake found) # ============================================================================ @@ -142,6 +178,7 @@ if(ament_cmake_FOUND) ros2/src/schema_extractor.cpp ros2/src/generic_subscription_manager.cpp ros2/src/message_stripper.cpp + ros2/src/strip_transform.cpp ros2/src/ros2_topic_source.cpp ros2/src/ros2_subscription_manager.cpp ) @@ -266,11 +303,14 @@ if(BUILD_TESTING AND ament_cmake_FOUND) tests/unit/test_bridge_server.cpp tests/unit/test_protocol_constants.cpp tests/unit/test_whitelist_filter.cpp + tests/unit/test_transform_set.cpp + tests/unit/test_transforming_topic_source.cpp tests/unit/test_topic_discovery.cpp tests/unit/test_schema_extractor.cpp tests/unit/test_generic_subscription_manager.cpp tests/unit/test_ros2_subscription_manager.cpp tests/unit/test_message_stripper.cpp + tests/unit/test_cloudini_transform.cpp ) target_link_libraries(${PROJECT_NAME}_tests diff --git a/README.md b/README.md index c066cdb..c481cf6 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ independently. - **Multi-Client Support**: Multiple clients can connect simultaneously with shared subscriptions - **Runtime Schema Discovery**: Automatic extraction of message schemas from installed ROS2 packages on the server side. - **Large Message Stripping** (opt-in): Optional stripping of large array fields (Image, PointCloud2, LaserScan, OccupancyGrid) to reduce bandwidth while preserving metadata. Disabled by default — full message data is forwarded; enable with `strip_large_messages:=true` for low-bandwidth links +- **Message Transforms** (opt-in, ROS2 only): Operator-configured per-topic transforms (e.g. `cloudini` point cloud compression) applied before buffering, replacing a topic's payload and advertised type/schema for all subscribers. Off by default (`transform_profile:=""`); see [docs/API.md](docs/API.md#message-transforms-ros2-only) - **Topic Whitelist**: Restrict which topics are visible/subscribable via full-match regex patterns (`topic_whitelist` / `--topic-whitelist`), mirroring foxglove_bridge's option of the same name - **QoS Depth Heuristics** (ROS2 only): KEEP_LAST subscription depth is derived from the discovered publishers' depths and clamped to a configurable `[min_qos_depth, max_qos_depth]` range - **Pushed Topic Advertisement** (opt-in): Clients can subscribe to a `topics_changed` notification instead of polling `get_topics`, at a configurable `topic_poll_interval` @@ -49,6 +50,7 @@ independently. | `publish_rate` | double | 50.0 | Aggregation publish rate in Hz | | `session_timeout` | double | 10.0 | Client timeout duration in seconds | | `strip_large_messages` | bool | false | Opt-in: strip large arrays from Image, PointCloud2, LaserScan, OccupancyGrid messages | +| `transform_profile` | string | `""` | ROS2 only: path to a JSON profile of per-topic message transforms (e.g. Cloudini point cloud compression); empty disables the feature | | `topic_whitelist` | string array | `[".*"]` | Full-match regex patterns (ECMAScript) restricting visible/subscribable topics | | `min_qos_depth` | int | 10 | ROS2 only: minimum KEEP_LAST subscription depth after aggregating publisher depths | | `max_qos_depth` | int | 100 | ROS2 only: maximum KEEP_LAST subscription depth after aggregating publisher depths | @@ -113,6 +115,8 @@ All dependencies (spdlog, nlohmann_json, ZSTD) are provided by the dependency ma TLS (`wss://`) support depends on IXWebSocket being built with OpenSSL. The CMake option `PJ_BRIDGE_TLS` (default `ON`) controls this for the FetchContent path (`-DPJ_BRIDGE_TLS=OFF` to disable); a system/conda-provided IXWebSocket must likewise have been built with TLS. See [docs/API.md](docs/API.md#tls--wss) for details. +Cloudini (the `cloudini` message transform) is optional: `find_package(cloudini_lib)` is tried first (version >= 1.3.1), otherwise it is fetched at configure time and linked statically. Builds without network access must either have `cloudini_lib` installed or pass `-DPJ_BRIDGE_FETCH_CLOUDINI=OFF`, which builds the bridge without that transform. + ### ROS2 — Pixi [Pixi](https://pixi.sh) manages the full toolchain including ROS2 via [RoboStack](https://robostack.github.io/). diff --git a/app/include/pj_bridge/cloudini_transform.hpp b/app/include/pj_bridge/cloudini_transform.hpp new file mode 100644 index 0000000..247e70b --- /dev/null +++ b/app/include/pj_bridge/cloudini_transform.hpp @@ -0,0 +1,34 @@ +/* + * Copyright (C) 2026 Davide Faconti + * + * This file is part of pj_bridge. + * + * pj_bridge is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * pj_bridge is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with pj_bridge. If not, see . + */ + +#pragma once + +#include "pj_bridge/message_transform.hpp" + +namespace pj_bridge { + +/// `cloudini` transform: sensor_msgs PointCloud2 -> point_cloud_interfaces +/// CompressedPointCloud2. Works on CDR bytes, so it serves any backend whose +/// PointCloud2 is CDR-encoded. Only declared when Cloudini is available. +/// +/// Params: resolution (float, default 0.001), fields ({name: resolution}, 0 +/// removes the field), viz_preprocessing (bool, default false). +TransformFactory make_cloudini_transform_factory(); + +} // namespace pj_bridge diff --git a/app/include/pj_bridge/message_transform.hpp b/app/include/pj_bridge/message_transform.hpp new file mode 100644 index 0000000..bfdc32e --- /dev/null +++ b/app/include/pj_bridge/message_transform.hpp @@ -0,0 +1,65 @@ +/* + * Copyright (C) 2026 Davide Faconti + * + * This file is part of pj_bridge. + * + * pj_bridge is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * pj_bridge is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with pj_bridge. If not, see . + */ + +#pragma once + +#include +#include +#include +#include +#include +#include +#include + +#include "tl/expected.hpp" + +namespace pj_bridge { + +/// One instance per topic, called from a single thread (the ingest executor). +/// Implementations SHOULD keep codec contexts and scratch buffers as members. +class MessageTransform { + public: + virtual ~MessageTransform() = default; + + /// `in` views the middleware's buffer and is valid only during the call. + /// `out` is the final storage later held by MessageBuffer: clear and fill it. + /// On error the caller DROPS the sample. It must never forward `in` instead: + /// the client was told the output type at subscribe time. + virtual tl::expected apply(std::span in, std::vector& out) = 0; + + /// Type name advertised to clients in place of `source_type`. Default: unchanged. + virtual std::string output_type(const std::string& source_type) const { + return source_type; + } + + /// Schema advertised to clients, given the untransformed one. Default: unchanged. + virtual std::string output_schema(const std::string& source_schema) const { + return source_schema; + } +}; + +struct TransformFactory { + /// Can this transform handle messages of `source_type`? + std::function accepts; + /// Reject unknown or malformed params. Called once per rule at startup. + std::function(const nlohmann::json& params)> check_params; + std::function(const std::string& source_type, const nlohmann::json& params)> create; +}; + +} // namespace pj_bridge diff --git a/app/include/pj_bridge/protocol_constants.hpp b/app/include/pj_bridge/protocol_constants.hpp index af57f3c..7342c9d 100644 --- a/app/include/pj_bridge/protocol_constants.hpp +++ b/app/include/pj_bridge/protocol_constants.hpp @@ -65,6 +65,7 @@ inline constexpr const char* kServerCapabilities[] = { "topics_changed", // pushed topic advertisement (subscribe_topic_updates) "per_topic_rate_limit", // subscribe entries accept {name, max_rate_hz} "size_class_frames", // large topics isolated into own frames (header flag bit0 = heavy) + "message_transforms", // topics may be advertised with a transformed type (+ source_type) }; } // namespace pj_bridge diff --git a/app/include/pj_bridge/topic_source_interface.hpp b/app/include/pj_bridge/topic_source_interface.hpp index 947cadf..3185056 100644 --- a/app/include/pj_bridge/topic_source_interface.hpp +++ b/app/include/pj_bridge/topic_source_interface.hpp @@ -26,8 +26,9 @@ namespace pj_bridge { /// A discovered topic with its fully-qualified name and message type. struct TopicInfo { - std::string name; ///< e.g. "/sensor/imu" - std::string type; ///< e.g. "sensor_msgs/msg/Imu" + std::string name; ///< e.g. "/sensor/imu" + std::string type; ///< e.g. "sensor_msgs/msg/Imu" + std::string source_type{}; ///< set only for transformed topics: the type before the transform }; /// Abstract interface for discovering topics and retrieving their schemas. diff --git a/app/include/pj_bridge/transform_set.hpp b/app/include/pj_bridge/transform_set.hpp new file mode 100644 index 0000000..0a92d16 --- /dev/null +++ b/app/include/pj_bridge/transform_set.hpp @@ -0,0 +1,97 @@ +/* + * Copyright (C) 2026 Davide Faconti + * + * This file is part of pj_bridge. + * + * pj_bridge is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * pj_bridge is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with pj_bridge. If not, see . + */ + +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "pj_bridge/message_transform.hpp" + +namespace pj_bridge { + +/// A transform bound to one topic, plus its counters. +/// run() is called from the ingest thread only; counters are read from others. +struct BoundTransform { + std::string source_type; + std::string transform_name; + std::string output_type; + std::unique_ptr transform; + + std::atomic samples{0}; + std::atomic drops{0}; + std::atomic in_bytes{0}; + std::atomic out_bytes{0}; + std::atomic micros{0}; + + /// Applies the transform and updates counters. False = drop the sample. + bool run(const std::string& topic, std::span in, std::vector& out); +}; + +/// Ordered transform rules plus the per-topic bindings created from them. +/// Thread safety: all public methods are safe to call concurrently. +class TransformSet { + public: + using FactoryMap = std::map; + + /// Parse and validate a profile. Any problem is a startup error. + static tl::expected, std::string> create( + const nlohmann::json& profile, FactoryMap factories); + + /// Append a `match_type` rule after the profile's rules (strip_large_messages sugar). + tl::expected append_type_rule(const std::string& match_type, const std::string& transform); + + /// First matching rule wins. Creates and remembers the binding on first sight. + /// Returns nullptr when no rule applies to this topic. + std::shared_ptr bind(const std::string& topic, const std::string& source_type); + + /// Binding previously created by bind(), or nullptr. + std::shared_ptr find(const std::string& topic) const; + + /// One line per bound topic that processed at least one sample. + std::string stats_summary() const; + + private: + struct Rule { + std::string match_type; // mandatory + std::optional match_topic; + std::string transform; + nlohmann::json params = nlohmann::json::object(); + }; + + TransformSet() = default; + tl::expected add_rule(Rule rule); + + FactoryMap factories_; + std::vector rules_; + mutable std::mutex mutex_; + std::unordered_map> bindings_; + std::unordered_set setup_failed_; // topics whose setup error was already reported +}; + +} // namespace pj_bridge diff --git a/app/include/pj_bridge/transforming_topic_source.hpp b/app/include/pj_bridge/transforming_topic_source.hpp new file mode 100644 index 0000000..a556a26 --- /dev/null +++ b/app/include/pj_bridge/transforming_topic_source.hpp @@ -0,0 +1,45 @@ +/* + * Copyright (C) 2026 Davide Faconti + * + * This file is part of pj_bridge. + * + * pj_bridge is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * pj_bridge is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with pj_bridge. If not, see . + */ + +#pragma once + +#include + +#include "pj_bridge/topic_source_interface.hpp" +#include "pj_bridge/transform_set.hpp" + +namespace pj_bridge { + +/// Advertises transformed topics with their OUTPUT type and schema, so clients +/// never see the source type. Thread safety: same as the wrapped source. +class TransformingTopicSource : public TopicSourceInterface { + public: + TransformingTopicSource(std::shared_ptr inner, std::shared_ptr transforms); + + std::vector get_topics() override; + std::string get_schema(const std::string& topic_name) override; + std::string schema_encoding() const override; + bool is_transient_local(const std::string& topic_name) const override; + + private: + std::shared_ptr inner_; + std::shared_ptr transforms_; +}; + +} // namespace pj_bridge diff --git a/app/src/bridge_server.cpp b/app/src/bridge_server.cpp index fe3551e..b1fce8f 100644 --- a/app/src/bridge_server.cpp +++ b/app/src/bridge_server.cpp @@ -279,6 +279,9 @@ std::string BridgeServer::handle_get_topics(const std::string& client_id, const json topic_entry; topic_entry["name"] = topic.name; topic_entry["type"] = topic.type; + if (!topic.source_type.empty()) { + topic_entry["source_type"] = topic.source_type; + } attach_latched_badge(topic_entry, topic.name); if (include_schemas) { attach_schema_fields(topic_entry, topic.name); diff --git a/app/src/cloudini_transform.cpp b/app/src/cloudini_transform.cpp new file mode 100644 index 0000000..70356e6 --- /dev/null +++ b/app/src/cloudini_transform.cpp @@ -0,0 +1,120 @@ +/* + * Copyright (C) 2026 Davide Faconti + * + * This file is part of pj_bridge. + * + * pj_bridge is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * pj_bridge is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with pj_bridge. If not, see . + */ + +#include "pj_bridge/cloudini_transform.hpp" + +#include +#include +// Defines non-inline `const char*` globals: include in this translation unit ONLY. +#include + +namespace pj_bridge { +namespace { + +constexpr const char* kPointCloud2 = "sensor_msgs/msg/PointCloud2"; + +class CloudiniTransform : public MessageTransform { + public: + explicit CloudiniTransform(const nlohmann::json& params) + : resolution_(params.value("resolution", 0.001f)), viz_preprocessing_(params.value("viz_preprocessing", false)) { + if (params.contains("fields")) { + for (auto it = params["fields"].begin(); it != params["fields"].end(); ++it) { + profile_[it.key()] = it->get(); + } + } + } + + tl::expected apply(std::span in, std::vector& out) override { + try { + const Cloudini::ConstBufferView raw(reinterpret_cast(in.data()), in.size()); + auto cloud = cloudini_ros::getDeserializedPointCloudMessage(raw); + cloudini_ros::applyResolutionProfile(profile_, cloud.fields, resolution_); + if (viz_preprocessing_) { + cloudini_ros::applyVizLossyPreprocessing(cloud); + } + auto info = cloudini_ros::toEncodingInfo(cloud); + // The frame is zstd-compressed by the bridge: a second stage here would compress twice. + info.compression_opt = Cloudini::CompressionOption::NONE; + info.use_threads = false; + + // Cloudini writes CDR into a vector; scratch_ keeps its capacity across calls. + cloudini_ros::convertPointCloud2ToCompressedCloud(cloud, info, scratch_); + const auto* bytes = reinterpret_cast(scratch_.data()); + out.assign(bytes, bytes + scratch_.size()); + return {}; + } catch (const std::exception& e) { + return tl::make_unexpected(std::string(e.what())); + } + } + + std::string output_type(const std::string&) const override { + return compressed_schema_name; + } + + std::string output_schema(const std::string&) const override { + return compressed_schema_data; + } + + private: + float resolution_; + bool viz_preprocessing_; + cloudini_ros::ResolutionProfile profile_; + std::vector scratch_; +}; + +tl::expected check_cloudini_params(const nlohmann::json& params) { + for (auto it = params.begin(); it != params.end(); ++it) { + const auto& key = it.key(); + if (key == "resolution") { + if (!it->is_number() || it->get() <= 0.0) { + return tl::make_unexpected(std::string("'resolution' must be a number > 0")); + } + } else if (key == "viz_preprocessing") { + if (!it->is_boolean()) { + return tl::make_unexpected(std::string("'viz_preprocessing' must be a boolean")); + } + } else if (key == "fields") { + if (!it->is_object()) { + return tl::make_unexpected(std::string("'fields' must be an object of {name: resolution}")); + } + for (auto f = it->begin(); f != it->end(); ++f) { + if (!f->is_number() || f->get() < 0.0) { + return tl::make_unexpected("'fields." + f.key() + "' must be a number >= 0 (0 removes the field)"); + } + } + } else { + return tl::make_unexpected("unknown param '" + key + "'"); + } + } + return {}; +} + +} // namespace + +TransformFactory make_cloudini_transform_factory() { + TransformFactory f; + f.accepts = [](const std::string& type) { return type == kPointCloud2; }; + f.check_params = check_cloudini_params; + f.create = [](const std::string&, const nlohmann::json& params) { + return std::make_unique(params); + }; + return f; +} + +} // namespace pj_bridge diff --git a/app/src/transform_set.cpp b/app/src/transform_set.cpp new file mode 100644 index 0000000..e803a8d --- /dev/null +++ b/app/src/transform_set.cpp @@ -0,0 +1,211 @@ +/* + * Copyright (C) 2026 Davide Faconti + * + * This file is part of pj_bridge. + * + * pj_bridge is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * pj_bridge is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with pj_bridge. If not, see . + */ + +#include "pj_bridge/transform_set.hpp" + +#include + +#include +#include + +namespace pj_bridge { + +namespace { +// regex_match can throw at match time (error_complexity / error_stack): treat as no match. +bool topic_matches(const std::string& topic, const std::regex& pattern) { + try { + return std::regex_match(topic, pattern); + } catch (const std::regex_error&) { + return false; + } +} +} // namespace + +bool BoundTransform::run(const std::string& topic, std::span in, std::vector& out) { + const auto start = std::chrono::steady_clock::now(); + tl::expected result; + try { + result = transform->apply(in, out); + } catch (const std::exception& e) { // a codec throwing on bad input costs one sample, not the bridge + result = tl::make_unexpected(std::string("exception: ") + e.what()); + } + if (!result) { + const uint64_t n = drops.fetch_add(1) + 1; + if ((n & (n - 1)) == 0) { // 1st, 2nd, 4th, 8th... keeps a broken topic from flooding the log + spdlog::warn("Transform '{}' failed on '{}' ({} drops so far): {}", transform_name, topic, n, result.error()); + } + return false; + } + const auto elapsed = std::chrono::duration_cast(std::chrono::steady_clock::now() - start); + samples.fetch_add(1); + in_bytes.fetch_add(in.size()); + out_bytes.fetch_add(out.size()); + micros.fetch_add(static_cast(elapsed.count())); + return true; +} + +tl::expected, std::string> TransformSet::create( + const nlohmann::json& profile, FactoryMap factories) { + if (!profile.is_object() || !profile.contains("transforms") || !profile["transforms"].is_array()) { + return tl::make_unexpected(std::string("transform profile must be an object with a 'transforms' array")); + } + for (auto it = profile.begin(); it != profile.end(); ++it) { + if (it.key() != "transforms") { + return tl::make_unexpected("transform profile: unknown key '" + it.key() + "'"); + } + } + + std::shared_ptr set(new TransformSet()); + set->factories_ = std::move(factories); + + size_t index = 0; + for (const auto& entry : profile["transforms"]) { + const std::string where = "rule " + std::to_string(index++) + ": "; + if (!entry.is_object()) { + return tl::make_unexpected(where + "must be an object"); + } + Rule rule; + for (auto it = entry.begin(); it != entry.end(); ++it) { + const auto& key = it.key(); + if (key == "match_type" && it->is_string()) { + rule.match_type = it->get(); + } else if (key == "match_topic" && it->is_string()) { + try { + rule.match_topic.emplace(it->get(), std::regex::ECMAScript); + } catch (const std::regex_error& e) { + return tl::make_unexpected(where + "invalid match_topic regex: " + e.what()); + } + } else if (key == "transform" && it->is_string()) { + rule.transform = it->get(); + } else if (key == "params" && it->is_object()) { + rule.params = *it; + } else { + return tl::make_unexpected(where + "key '" + key + "' is unknown or has the wrong JSON type"); + } + } + if (auto added = set->add_rule(std::move(rule)); !added) { + return tl::make_unexpected(where + added.error()); + } + } + return set; +} + +tl::expected TransformSet::add_rule(Rule rule) { + // Mandatory, so that a transform applied to a type it cannot handle is a + // startup error: topic types are only known once topics are discovered. + if (rule.match_type.empty()) { + return tl::make_unexpected(std::string("needs a non-empty 'match_type' ('match_topic' can only narrow it)")); + } + if (rule.transform.empty()) { + return tl::make_unexpected(std::string("missing 'transform'")); + } + auto factory = factories_.find(rule.transform); + if (factory == factories_.end()) { + return tl::make_unexpected("unknown transform '" + rule.transform + "' (not built in, or misspelled)"); + } + if (auto ok = factory->second.check_params(rule.params); !ok) { + return tl::make_unexpected("transform '" + rule.transform + "': " + ok.error()); + } + if (!factory->second.accepts(rule.match_type)) { + return tl::make_unexpected( + "transform '" + rule.transform + "' does not accept type '" + rule.match_type + + "' (use the full type name, e.g. 'sensor_msgs/msg/PointCloud2')"); + } + std::lock_guard lock(mutex_); + rules_.push_back(std::move(rule)); + return {}; +} + +tl::expected TransformSet::append_type_rule( + const std::string& match_type, const std::string& transform) { + Rule rule; + rule.match_type = match_type; + rule.transform = transform; + return add_rule(std::move(rule)); +} + +std::shared_ptr TransformSet::bind(const std::string& topic, const std::string& source_type) { + std::lock_guard lock(mutex_); + if (auto it = bindings_.find(topic); it != bindings_.end()) { + if (it->second->source_type == source_type) { + return it->second; + } + bindings_.erase(it); // the topic's type changed: match again, and report a new failure again + setup_failed_.erase(topic); + } + + // ponytail: unmatched topics re-run every rule on each call (once per topic poll); + // cache the misses too if topics x rules ever shows up in a profile. + for (const auto& rule : rules_) { + if (rule.match_type != source_type) { + continue; + } + if (rule.match_topic && !topic_matches(topic, *rule.match_topic)) { + continue; + } + const auto& factory = factories_.at(rule.transform); // accepts(match_type) was checked in add_rule() + + auto bound = std::make_shared(); + std::string error = "factory returned no transform"; + try { + bound->transform = factory.create(source_type, rule.params); + } catch (const std::exception& e) { + error = e.what(); + } + if (!bound->transform) { + if (setup_failed_.insert(topic).second) { // bind() runs on every topic poll: report once + spdlog::error( + "Transform '{}' could not be set up for '{}': {}; leaving it untransformed", rule.transform, topic, error); + } + return nullptr; + } + bound->source_type = source_type; + bound->transform_name = rule.transform; + bound->output_type = bound->transform->output_type(source_type); + bindings_[topic] = bound; + spdlog::info("Topic '{}' ({}) -> transform '{}' -> {}", topic, source_type, rule.transform, bound->output_type); + return bound; + } + return nullptr; +} + +std::shared_ptr TransformSet::find(const std::string& topic) const { + std::lock_guard lock(mutex_); + auto it = bindings_.find(topic); + return it == bindings_.end() ? nullptr : it->second; +} + +std::string TransformSet::stats_summary() const { + std::lock_guard lock(mutex_); + std::ostringstream os; + for (const auto& [topic, bound] : bindings_) { + const uint64_t n = bound->samples.load(); + const uint64_t dropped = bound->drops.load(); + if (n == 0 && dropped == 0) { + continue; + } + const double in = static_cast(bound->in_bytes.load()); + const double out = static_cast(bound->out_bytes.load()); + os << "\n " << topic << " [" << bound->transform_name << "]: " << n << " samples, " << dropped << " drops, ratio " + << (out > 0 ? in / out : 0.0) << ", " << (n ? bound->micros.load() / n : 0) << " us/sample"; + } + return os.str(); +} + +} // namespace pj_bridge diff --git a/app/src/transforming_topic_source.cpp b/app/src/transforming_topic_source.cpp new file mode 100644 index 0000000..c87c7dd --- /dev/null +++ b/app/src/transforming_topic_source.cpp @@ -0,0 +1,55 @@ +/* + * Copyright (C) 2026 Davide Faconti + * + * This file is part of pj_bridge. + * + * pj_bridge is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * pj_bridge is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with pj_bridge. If not, see . + */ + +#include "pj_bridge/transforming_topic_source.hpp" + +namespace pj_bridge { + +TransformingTopicSource::TransformingTopicSource( + std::shared_ptr inner, std::shared_ptr transforms) + : inner_(std::move(inner)), transforms_(std::move(transforms)) {} + +std::vector TransformingTopicSource::get_topics() { + auto topics = inner_->get_topics(); + for (auto& topic : topics) { + if (auto bound = transforms_->bind(topic.name, topic.type)) { + topic.source_type = topic.type; + topic.type = bound->output_type; + } + } + return topics; +} + +std::string TransformingTopicSource::get_schema(const std::string& topic_name) { + auto schema = inner_->get_schema(topic_name); + if (auto bound = transforms_->find(topic_name)) { + return bound->transform->output_schema(schema); + } + return schema; +} + +std::string TransformingTopicSource::schema_encoding() const { + return inner_->schema_encoding(); +} + +bool TransformingTopicSource::is_transient_local(const std::string& topic_name) const { + return inner_->is_transient_local(topic_name); +} + +} // namespace pj_bridge diff --git a/docs/API.md b/docs/API.md index f9beeaf..2c26afc 100644 --- a/docs/API.md +++ b/docs/API.md @@ -92,6 +92,15 @@ fully match any whitelist pattern are omitted from the response entirely (see } ``` +A topic rewritten by a [message transform](#message-transforms-ros2-only) is +advertised with its transform **output** type, plus an extra `source_type` +field carrying the original type: + +```json +{"name": "/lidar/points", "type": "point_cloud_interfaces/msg/CompressedPointCloud2", + "source_type": "sensor_msgs/msg/PointCloud2"} +``` + ### Server identity and capabilities (`server`) Every `get_topics` response carries a `server` object: @@ -100,7 +109,8 @@ Every `get_topics` response carries a `server` object: {"server": {"name": "pj_bridge", "version": "0.10.0", "capabilities": ["include_schemas", "latched_badge", "latched_replay", "topics_changed", - "per_topic_rate_limit", "size_class_frames"]}} + "per_topic_rate_limit", "size_class_frames", + "message_transforms"]}} ``` Compatibility policy for clients: @@ -243,6 +253,175 @@ the reader depth (`min_qos_depth`) covers a burst. Publish/request/session-timeout timers run on their own executor thread so a long publish (zstd-compression) cycle never delays ingest. +## Message Transforms (ROS2 only) + +The server operator can configure per-topic transforms that replace a +topic's payload — and, optionally, its type — before it is buffered and +sent. This is operator-only configuration: there is no per-client +negotiation, and a transformed topic is transformed for **all** subscribers +or none. With no profile configured and `strip_large_messages: false`, no +transform machinery is created at all and behavior/wire output are +byte-identical to a server without this feature. + +- **ROS2**: string parameter `transform_profile`, default `""` (disabled), + pointing to a JSON profile file. + +Advertised by the `message_transforms` capability (always present in +`server.capabilities`, whether or not a profile is configured). + +### Wire protocol + +Unchanged. A transformed topic is simply advertised with its transform's +**output** type instead of its real one: + +- `get_topics` entries carry the output `type`, plus an optional + `source_type` field with the original type (see + [Get Topics](#get-topics)). +- `subscribe` returns the output schema (`encoding` + `definition`) for that + topic — a client never sees the source schema. +- `topics_changed` `added` entries carry the output `type` only; they do + **not** carry `source_type` in this version (see + [Pushed Topic Advertisement](#pushed-topic-advertisement-topics_changed)). + +Everything downstream — latched replay, size-class framing, rate limiting, +the binary frame format — operates on the transformed bytes with no special +case; a transform is invisible past the subscription layer. + +Client requirement: a client that subscribes to a transformed topic needs a +parser for the **output** type. In particular, older PlotJuggler clients +without a `CompressedPointCloud2` parser see a topic they cannot decode — +enabling the `cloudini` transform on a topic is the operator's call to make +for their fleet of clients. + +### Profile format + +```json +{ + "transforms": [ + { + "match_type": "sensor_msgs/msg/PointCloud2", + "match_topic": "/lidar/.*", + "transform": "cloudini", + "params": {"resolution": 0.001, "fields": {"intensity": 0.01}, "viz_preprocessing": true} + } + ] +} +``` + +See also `docs/transform_profile.example.json`. + +Each rule in `transforms` has: + +- `match_type` (**required**) — the topic's type, matched **exactly**. +- `match_topic` (optional) — full-match ECMAScript regex against the topic + name, same semantics as [`topic_whitelist`](#topic-whitelist) + (`std::regex_match`, not a substring search). It narrows the rule to some of + the topics of that type. +- `transform` (**required**) and `params` (optional) — see below. + +Rules are evaluated in order and the **first match wins**. + +`match_type` is required so that pairing a transform with a type it cannot +handle is always a **startup error**: topic types are only known once topics +are discovered, so a rule matching by name alone could not be checked until a +mismatching topic appeared. For a transform that handles several types (such +as `strip`), write one rule per type. + +### `strip_large_messages` interaction + +`strip_large_messages: true` is sugar for one `strip` rule per strippable +type (`sensor_msgs/msg/Image`, `sensor_msgs/msg/CompressedImage`, +`sensor_msgs/msg/PointCloud2`, `sensor_msgs/msg/LaserScan`, +`nav_msgs/msg/OccupancyGrid`), appended **after** the profile's own rules — +so an explicit rule in `transform_profile` for one of those types always +wins over the boolean. + +### Startup errors + +The server refuses to start (exits with a non-zero status) on any of: + +- `transform_profile` names a file that cannot be opened/read. +- The file is not valid JSON. +- An unknown top-level key (anything other than `transforms`). +- An unknown or mistyped rule key (e.g. `params` that isn't an object). +- A rule without `match_type`. +- A rule missing `transform`. +- A rule naming an unknown transform — including `cloudini` in a build + without Cloudini compiled in (see `CLAUDE.md` / `README.md` for the build + configuration). +- An invalid `match_topic` regex. +- Params rejected by the transform (unknown key, wrong type, out-of-range + value). +- A `match_type` rule whose transform does not accept that type. + +### Runtime behavior + +If `apply()` fails for a message (bad/unexpected input, codec exception), +the sample is **dropped** — it is never forwarded untransformed, because +the client was told the output type/schema at subscribe time. Drops are +counted per topic and logged with a throttled warning (1st, 2nd, 4th, 8th… +occurrence). + +**Behavior change:** this also applies to `strip` messages. Previously, +a stripping failure forwarded the original, unstripped message; as a +transform, a `strip` failure now drops the sample instead (see +`CHANGELOG.rst`). + +Per-topic transform statistics (sample count, drop count, compression +ratio, mean microseconds/sample) are logged together with the server's +final statistics at shutdown. + +### Transforms + +#### `strip` + +Thin wrapper around the pre-existing message-stripping behavior +(`strip_large_messages`): output type and schema equal the input's, no +params accepted. + +#### `cloudini` + +`sensor_msgs/msg/PointCloud2` → `point_cloud_interfaces/msg/CompressedPointCloud2`, +using [Cloudini](https://github.com/facontidavide/cloudini) point cloud +compression. **Lossy** at the configured resolution. Only registered when +the server was built with Cloudini available; naming it in a profile on a +build without it is a startup error. The output schema is embedded in +Cloudini's own headers, so the robot does **not** need +`point_cloud_interfaces` installed. + +Cloudini's own second compression stage is disabled — the bridge already +ZSTD-compresses every outgoing frame, so a second codec-level compression +stage would waste CPU for no benefit. + +Params (all optional): + +| Key | Default | Meaning | +|---|---|---| +| `resolution` | `0.001` | Quantization resolution applied to **every** `FLOAT32` field not listed in `fields` — metres for `x`/`y`/`z`, but also e.g. `intensity` or a `FLOAT32` time field; must be `> 0` | +| `fields` | `{}` | Per-field resolution override, `{name: resolution}`; `0` removes that field entirely | +| `viz_preprocessing` | `false` | Drops NaN/inf points, voxel-deduplicates at the `xyz` resolution, and quantizes `FLOAT64` fields to 1 µs — a lossy preprocessing pass aimed at visualization, not lossless round-tripping | + +Things to know before enabling it: + +- A coarse `resolution` also coarsens non-geometry `FLOAT32` fields; give + those their own value in `fields` (for example `{"intensity": 0.001}`). +- A field removed with resolution `0` keeps its place in the point layout and + decodes as zeros. +- `viz_preprocessing` returns an unorganized cloud (`height = 1`, `width` = + points kept), even when the input was organized. +- A cloud whose `width * height * point_step` does not equal its data size, or + with a field extending past `point_step`, is dropped like any other + transform failure. +- Big-endian clouds are not supported (the flag is not inspected; they would + decode as garbage). `PointField.count` is written as 1. +- The payload uses Cloudini encoding v5: the client needs a Cloudini decoder + >= 1.2.0. + +### Backend support + +Only wired for the ROS2 backend in this version; FastDDS and RTI do not +apply transforms yet. + ## Subscribe Subscribe to one or more topics. **Breaking change:** Subscribe now uses an additive model - it only adds topics without removing existing subscriptions. Use the `unsubscribe` command to remove topics. @@ -486,6 +665,10 @@ additionally carries `encoding` + `definition` (the same fields as when at least one of them is non-empty. - A topic whose type changes (same name, different type) is reported as both removed and added. +- `added` entries do **not** carry `source_type`, unlike `get_topics` + (see [Message Transforms](#message-transforms-ros2-only)): the poll that + builds them tracks a plain name → type map. Clients that need the source + type of a transformed topic should call `get_topics`. - **Per-topic schema failure never drops a topic** (same as `get_topics`): an `added` entry whose schema cannot be extracted is delivered with `name` + `type` only, no `encoding`/`definition`. Clients must treat the schema diff --git a/docs/superpowers/plans/2026-09-20-message-transforms.md b/docs/superpowers/plans/2026-09-20-message-transforms.md new file mode 100644 index 0000000..2902053 --- /dev/null +++ b/docs/superpowers/plans/2026-09-20-message-transforms.md @@ -0,0 +1,1284 @@ +# Message Transforms (v1: Cloudini) Implementation Plan + +> **Historical.** This is the plan as written before implementation. The code diverged during review (output type/schema moved onto `MessageTransform`, `match_type` became mandatory, Cloudini 1.3.1 removed the CMake workarounds). The design doc in `docs/superpowers/specs/` and `docs/API.md` describe what was built. + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Operator-configured, per-topic payload transforms in pj_bridge, with Cloudini point cloud compression and the existing stripper as the two implementations. + +**Architecture:** A `TransformSet` (rules from a JSON profile + name→factory map) is shared by a `TransformingTopicSource` decorator (rewrites advertised type/schema) and by a hook inside `Ros2SubscriptionManager` (runs `apply()` on the rcl buffer before any copy). `BridgeServer` is untouched except for two informational fields. + +**Tech Stack:** C++20, nlohmann/json, tl::expected, spdlog, gtest, Cloudini 1.3.0 (`cloudini_lib`), rclcpp. + +**Spec:** `docs/superpowers/specs/2026-09-20-message-transforms-design.md` + +## Environment (read first) + +All builds and tests run through pixi RoboStack, from the worktree root +`~/ws_plotjuggler/plotjuggler_bridge/.worktrees/message-transforms`: + +```bash +set -o pipefail +mkdir -p install && touch install/setup.sh +B="~/.pixi/bin/pixi run -e humble" +$B colcon build --packages-select pj_bridge --cmake-args -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=ON 2>&1 | tail -20 +$B bash -c 'source install/setup.bash && build/pj_bridge/pj_bridge_tests --gtest_filter="*"' +``` + +"Build" and "Run tests ``" below mean these two commands. The baseline +is 271+ tests passing. Format with `pre-commit run -a` before every commit. +Commit messages end with the repo's standard `Co-Authored-By` trailer. + +## File map + +| File | Responsibility | +|---|---| +| `app/include/pj_bridge/message_transform.hpp` (new) | `MessageTransform`, `TransformFactory` | +| `app/include/pj_bridge/transform_set.hpp`, `app/src/transform_set.cpp` (new) | Profile parsing, rule matching, per-topic bindings, counters | +| `app/include/pj_bridge/transforming_topic_source.hpp`, `app/src/transforming_topic_source.cpp` (new) | Decorator over `TopicSourceInterface` | +| `app/include/pj_bridge/cloudini_transform.hpp`, `app/src/cloudini_transform.cpp` (new, conditional) | Cloudini factory | +| `ros2/include/pj_bridge_ros2/strip_transform.hpp`, `ros2/src/strip_transform.cpp` (new) | `strip` factory over `MessageStripper` | +| `ros2/src/ros2_subscription_manager.cpp`, its header | The hook; loses its `strip_large_messages` bool | +| `ros2/src/main.cpp` | `transform_profile` param, wiring, stats at shutdown | +| `app/include/pj_bridge/topic_source_interface.hpp` | `TopicInfo::source_type` | +| `app/src/bridge_server.cpp`, `protocol_constants.hpp` | emit `source_type`; `message_transforms` capability | +| `tests/unit/test_transform_set.cpp`, `test_transforming_topic_source.cpp`, `test_cloudini_transform.cpp` (new) | Tests | + +--- + +### Task 1: Move the project to C++20 + +**Files:** Modify `CMakeLists.txt:18` + +- [ ] **Step 1:** Change `set(CMAKE_CXX_STANDARD 17)` to `set(CMAKE_CXX_STANDARD 20)`. Run `grep -rn "cxx_std_17\|CXX_STANDARD 17\|-std=" CMakeLists.txt cmake/ 2>/dev/null` and update any other hit to 20. +- [ ] **Step 2:** Build, then run the whole suite (no filter). Expected: same pass count as before the change. C++20 typically surfaces: implicit `this` capture in `[=]` lambdas (make it `[=, this]`), and `u8""` literal type changes. Fix what the compiler reports, nothing else. +- [ ] **Step 3:** Repeat Step 2 with `-e jazzy`. +- [ ] **Step 4:** FastDDS standalone build still configures and compiles (see CLAUDE.md "FastDDS Backend"); skip if Conan cache is unavailable and say so in the commit body. +- [ ] **Step 5:** Commit: `build: require C++20`. Push the branch and confirm Humble/Jazzy/Kilted/Lyrical CI is green **before** starting Task 2 (so a compiler issue is never confused with the feature). + +--- + +### Task 2: Transform interface + +**Files:** Create `app/include/pj_bridge/message_transform.hpp` + +- [ ] **Step 1:** Write the header (license banner copied from a sibling header): + +```cpp +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace pj_bridge { + +/// One instance per topic, called from a single thread (the ingest executor). +/// Implementations SHOULD keep codec contexts and scratch buffers as members. +class MessageTransform { + public: + virtual ~MessageTransform() = default; + + /// `in` views the middleware's buffer and is valid only during the call. + /// `out` is the final storage later held by MessageBuffer: clear and fill it. + /// On error the caller DROPS the sample. It must never forward `in` instead: + /// the client was told the output type at subscribe time. + virtual tl::expected apply(std::span in, std::vector& out) = 0; +}; + +struct TransformFactory { + /// Can this transform handle messages of `source_type`? + std::function accepts; + /// Reject unknown or malformed params. Called once per rule at startup. + std::function(const nlohmann::json& params)> check_params; + /// Type name advertised to clients in place of `source_type`. + std::function output_type; + /// Schema advertised to clients. `source_schema` is the untransformed one. + std::function output_schema; + std::function(const std::string& source_type, const nlohmann::json& params)> + create; +}; + +} // namespace pj_bridge +``` + +Check the include spelling of `tl/expected.hpp` against `app/include/pj_bridge/whitelist_filter.hpp` and copy it exactly. + +- [ ] **Step 2:** Build (header is not yet included anywhere; this only checks nothing else broke). Commit: `feat(transform): MessageTransform interface`. + +--- + +### Task 3: TransformSet — profile parsing and matching + +**Files:** Create `app/include/pj_bridge/transform_set.hpp`, `app/src/transform_set.cpp`, `tests/unit/test_transform_set.cpp`. Modify `CMakeLists.txt` (add the `.cpp` to `pj_bridge_app`, the test to `${PROJECT_NAME}_tests`). + +- [ ] **Step 1: Header** + +```cpp +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "pj_bridge/message_transform.hpp" + +namespace pj_bridge { + +/// A transform bound to one topic, plus its counters. +/// run() is called from the ingest thread only; counters are read from others. +struct BoundTransform { + std::string source_type; + std::string transform_name; + std::string output_type; + std::unique_ptr transform; + + std::atomic samples{0}; + std::atomic drops{0}; + std::atomic in_bytes{0}; + std::atomic out_bytes{0}; + std::atomic micros{0}; + + /// Applies the transform and updates counters. False = drop the sample. + bool run(const std::string& topic, std::span in, std::vector& out); +}; + +/// Ordered transform rules plus the per-topic bindings created from them. +/// Thread safety: all public methods are safe to call concurrently. +class TransformSet { + public: + using FactoryMap = std::map; + + /// Parse and validate a profile. Any problem is a startup error. + static tl::expected, std::string> create( + const nlohmann::json& profile, FactoryMap factories); + + /// Append a `match_type` rule after the profile's rules (strip_large_messages sugar). + tl::expected append_type_rule(const std::string& match_type, const std::string& transform); + + /// First matching rule wins. Creates and remembers the binding on first sight. + /// Returns nullptr when no rule applies to this topic. + std::shared_ptr bind(const std::string& topic, const std::string& source_type); + + /// Binding previously created by bind(), or nullptr. + std::shared_ptr find(const std::string& topic) const; + + /// Schema to advertise for a bound topic. + std::string output_schema(const BoundTransform& bound, const std::string& source_schema) const; + + /// One line per bound topic that processed at least one sample. + std::string stats_summary() const; + + private: + struct Rule { + std::optional match_type; + std::optional match_topic; + std::string transform; + nlohmann::json params; + }; + + TransformSet() = default; + tl::expected add_rule(Rule rule); + + FactoryMap factories_; + std::vector rules_; + mutable std::mutex mutex_; + std::unordered_map> bindings_; + std::set warned_topics_; +}; + +} // namespace pj_bridge +``` + +- [ ] **Step 2: Failing tests** — `tests/unit/test_transform_set.cpp` + +```cpp +#include + +#include "pj_bridge/transform_set.hpp" + +using namespace pj_bridge; +using nlohmann::json; + +namespace { + +/// Appends one marker byte to the input. `fail` makes apply() return an error. +class FakeTransform : public MessageTransform { + public: + explicit FakeTransform(bool fail) : fail_(fail) {} + tl::expected apply(std::span in, std::vector& out) override { + if (fail_) { + return tl::make_unexpected(std::string("boom")); + } + out.assign(in.begin(), in.end()); + out.push_back(std::byte{0xAB}); + return {}; + } + + private: + bool fail_; +}; + +TransformFactory fake_factory(const std::string& accepted_type) { + TransformFactory f; + f.accepts = [accepted_type](const std::string& t) { return t == accepted_type; }; + f.check_params = [](const json& p) -> tl::expected { + for (auto it = p.begin(); it != p.end(); ++it) { + if (it.key() != "fail") { + return tl::make_unexpected("unknown param '" + it.key() + "'"); + } + } + return {}; + }; + f.output_type = [](const std::string&) { return std::string("pkg/msg/Out"); }; + f.output_schema = [](const std::string&, const std::string& s) { return "OUT:" + s; }; + f.create = [](const std::string&, const json& p) { return std::make_unique(p.value("fail", false)); }; + return f; +} + +TransformSet::FactoryMap factories() { + return {{"fake", fake_factory("pkg/msg/In")}, {"other", fake_factory("pkg/msg/Other")}}; +} + +json rule(json fields) { + return json{{"transforms", json::array({std::move(fields)})}}; +} + +} // namespace + +TEST(TransformSetTest, MatchesByType) { + auto set = TransformSet::create(rule({{"match_type", "pkg/msg/In"}, {"transform", "fake"}}), factories()); + ASSERT_TRUE(set.has_value()) << set.error(); + auto bound = (*set)->bind("/a", "pkg/msg/In"); + ASSERT_NE(bound, nullptr); + EXPECT_EQ(bound->source_type, "pkg/msg/In"); + EXPECT_EQ(bound->output_type, "pkg/msg/Out"); + EXPECT_EQ((*set)->bind("/b", "pkg/msg/Unrelated"), nullptr); + EXPECT_EQ((*set)->find("/a"), bound); + EXPECT_EQ((*set)->find("/b"), nullptr); +} + +TEST(TransformSetTest, TopicRegexIsFullMatchAndCombinesWithType) { + auto set = TransformSet::create( + rule({{"match_type", "pkg/msg/In"}, {"match_topic", "/lidar/.*"}, {"transform", "fake"}}), factories()); + ASSERT_TRUE(set.has_value()) << set.error(); + EXPECT_NE((*set)->bind("/lidar/front", "pkg/msg/In"), nullptr); + EXPECT_EQ((*set)->bind("/x/lidar/front", "pkg/msg/In"), nullptr); // full match, not search + EXPECT_EQ((*set)->bind("/lidar/rear", "pkg/msg/Unrelated"), nullptr); +} + +TEST(TransformSetTest, FirstMatchingRuleWinsAndAppendedRulesComeLast) { + json profile = {{"transforms", json::array({{{"match_topic", "/a"}, {"transform", "fake"}, {"params", {{"fail", true}}}}, + {{"match_type", "pkg/msg/In"}, {"transform", "fake"}}})}}; + auto set = TransformSet::create(profile, factories()); + ASSERT_TRUE(set.has_value()) << set.error(); + ASSERT_TRUE((*set)->append_type_rule("pkg/msg/Other", "other").has_value()); + + std::vector in{std::byte{1}}, out; + EXPECT_FALSE((*set)->bind("/a", "pkg/msg/In")->run("/a", in, out)); // rule 0 (fail=true) + EXPECT_TRUE((*set)->bind("/b", "pkg/msg/In")->run("/b", in, out)); // rule 1 + EXPECT_EQ((*set)->bind("/c", "pkg/msg/Other")->transform_name, "other"); +} + +TEST(TransformSetTest, TopicOnlyRuleWithNonAcceptedTypeLeavesTopicUntransformed) { + auto set = TransformSet::create(rule({{"match_topic", "/a"}, {"transform", "fake"}}), factories()); + ASSERT_TRUE(set.has_value()) << set.error(); + EXPECT_EQ((*set)->bind("/a", "pkg/msg/Unrelated"), nullptr); +} + +TEST(TransformSetTest, RebindsWhenSourceTypeChanges) { + auto set = TransformSet::create(rule({{"match_type", "pkg/msg/In"}, {"transform", "fake"}}), factories()); + ASSERT_TRUE(set.has_value()) << set.error(); + EXPECT_NE((*set)->bind("/a", "pkg/msg/In"), nullptr); + EXPECT_EQ((*set)->bind("/a", "pkg/msg/Unrelated"), nullptr); + EXPECT_EQ((*set)->find("/a"), nullptr); +} + +TEST(TransformSetTest, RunCountsSamplesBytesAndDrops) { + json profile = {{"transforms", json::array({{{"match_topic", "/bad"}, {"transform", "fake"}, {"params", {{"fail", true}}}}, + {{"match_type", "pkg/msg/In"}, {"transform", "fake"}}})}}; + auto set = TransformSet::create(profile, factories()); + ASSERT_TRUE(set.has_value()) << set.error(); + std::vector in(10, std::byte{7}), out; + + auto good = (*set)->bind("/good", "pkg/msg/In"); + ASSERT_TRUE(good->run("/good", in, out)); + EXPECT_EQ(out.size(), 11u); + EXPECT_EQ(good->samples.load(), 1u); + EXPECT_EQ(good->in_bytes.load(), 10u); + EXPECT_EQ(good->out_bytes.load(), 11u); + EXPECT_EQ(good->drops.load(), 0u); + + auto bad = (*set)->bind("/bad", "pkg/msg/In"); + EXPECT_FALSE(bad->run("/bad", in, out)); + EXPECT_EQ(bad->drops.load(), 1u); + EXPECT_EQ(bad->samples.load(), 0u); +} + +struct BadProfile { + const char* name; + json profile; +}; + +class TransformSetBadProfileTest : public ::testing::TestWithParam {}; + +TEST_P(TransformSetBadProfileTest, IsRejectedAtStartup) { + auto set = TransformSet::create(GetParam().profile, factories()); + EXPECT_FALSE(set.has_value()); +} + +INSTANTIATE_TEST_SUITE_P( + All, TransformSetBadProfileTest, + ::testing::Values( + BadProfile{"not_an_object", json::array()}, + BadProfile{"missing_transforms", json::object()}, + BadProfile{"unknown_top_level_key", json{{"transforms", json::array()}, {"extra", 1}}}, + BadProfile{"rule_without_matcher", rule({{"transform", "fake"}})}, + BadProfile{"rule_without_transform", rule({{"match_type", "pkg/msg/In"}})}, + BadProfile{"unknown_rule_key", rule({{"match_type", "pkg/msg/In"}, {"transform", "fake"}, {"oops", 1}})}, + BadProfile{"unknown_transform", rule({{"match_type", "pkg/msg/In"}, {"transform", "nope"}})}, + BadProfile{"invalid_regex", rule({{"match_topic", "("}, {"transform", "fake"}})}, + BadProfile{"unknown_param", rule({{"match_type", "pkg/msg/In"}, {"transform", "fake"}, {"params", {{"q", 1}}}})}, + BadProfile{"type_not_accepted", rule({{"match_type", "pkg/msg/Unrelated"}, {"transform", "fake"}})}), + [](const auto& info) { return std::string(info.param.name); }); +``` + +- [ ] **Step 3:** Add both files to `CMakeLists.txt`, build. Expected: link errors for `TransformSet::*` (not implemented). + +- [ ] **Step 4: Implementation** — `app/src/transform_set.cpp` + +```cpp +#include "pj_bridge/transform_set.hpp" + +#include + +#include +#include + +namespace pj_bridge { + +bool BoundTransform::run(const std::string& topic, std::span in, std::vector& out) { + const auto start = std::chrono::steady_clock::now(); + auto result = transform->apply(in, out); + if (!result) { + const uint64_t n = drops.fetch_add(1) + 1; + if ((n & (n - 1)) == 0) { // 1st, 2nd, 4th, 8th... keeps a broken topic from flooding the log + spdlog::warn("Transform '{}' failed on '{}' ({} drops so far): {}", transform_name, topic, n, result.error()); + } + return false; + } + const auto elapsed = std::chrono::duration_cast(std::chrono::steady_clock::now() - start); + samples.fetch_add(1); + in_bytes.fetch_add(in.size()); + out_bytes.fetch_add(out.size()); + micros.fetch_add(static_cast(elapsed.count())); + return true; +} + +tl::expected, std::string> TransformSet::create( + const nlohmann::json& profile, FactoryMap factories) { + if (!profile.is_object() || !profile.contains("transforms") || !profile["transforms"].is_array()) { + return tl::make_unexpected(std::string("transform profile must be an object with a 'transforms' array")); + } + for (auto it = profile.begin(); it != profile.end(); ++it) { + if (it.key() != "transforms") { + return tl::make_unexpected("transform profile: unknown key '" + it.key() + "'"); + } + } + + std::shared_ptr set(new TransformSet()); + set->factories_ = std::move(factories); + + size_t index = 0; + for (const auto& entry : profile["transforms"]) { + const std::string where = "transform profile rule " + std::to_string(index++) + ": "; + if (!entry.is_object()) { + return tl::make_unexpected(where + "must be an object"); + } + Rule rule; + for (auto it = entry.begin(); it != entry.end(); ++it) { + const auto& key = it.key(); + if (key == "match_type" && it->is_string()) { + rule.match_type = it->get(); + } else if (key == "match_topic" && it->is_string()) { + try { + rule.match_topic.emplace(it->get(), std::regex::ECMAScript); + } catch (const std::regex_error& e) { + return tl::make_unexpected(where + "invalid match_topic regex: " + e.what()); + } + } else if (key == "transform" && it->is_string()) { + rule.transform = it->get(); + } else if (key == "params" && it->is_object()) { + rule.params = *it; + } else { + return tl::make_unexpected(where + "unknown or mistyped key '" + key + "'"); + } + } + if (rule.params.is_null()) { + rule.params = nlohmann::json::object(); + } + if (auto added = set->add_rule(std::move(rule)); !added) { + return tl::make_unexpected(where + added.error()); + } + } + return set; +} + +tl::expected TransformSet::add_rule(Rule rule) { + if (!rule.match_type && !rule.match_topic) { + return tl::make_unexpected(std::string("needs match_type and/or match_topic")); + } + if (rule.transform.empty()) { + return tl::make_unexpected(std::string("missing 'transform'")); + } + auto factory = factories_.find(rule.transform); + if (factory == factories_.end()) { + return tl::make_unexpected("unknown transform '" + rule.transform + "' (not built in, or misspelled)"); + } + if (auto ok = factory->second.check_params(rule.params); !ok) { + return tl::make_unexpected("transform '" + rule.transform + "': " + ok.error()); + } + if (rule.match_type && !factory->second.accepts(*rule.match_type)) { + return tl::make_unexpected("transform '" + rule.transform + "' does not accept type '" + *rule.match_type + "'"); + } + std::lock_guard lock(mutex_); + rules_.push_back(std::move(rule)); + return {}; +} + +tl::expected TransformSet::append_type_rule( + const std::string& match_type, const std::string& transform) { + Rule rule; + rule.match_type = match_type; + rule.transform = transform; + rule.params = nlohmann::json::object(); + return add_rule(std::move(rule)); +} + +std::shared_ptr TransformSet::bind(const std::string& topic, const std::string& source_type) { + std::lock_guard lock(mutex_); + if (auto it = bindings_.find(topic); it != bindings_.end()) { + if (it->second->source_type == source_type) { + return it->second; + } + bindings_.erase(it); // the topic's type changed: match again + } + + for (const auto& rule : rules_) { + if (rule.match_type && *rule.match_type != source_type) { + continue; + } + if (rule.match_topic && !std::regex_match(topic, *rule.match_topic)) { + continue; + } + const auto& factory = factories_.at(rule.transform); + if (!factory.accepts(source_type)) { + if (warned_topics_.insert(topic).second) { + spdlog::warn( + "Transform '{}' matched topic '{}' but does not accept type '{}'; leaving it untransformed", + rule.transform, topic, source_type); + } + return nullptr; + } + auto bound = std::make_shared(); + bound->source_type = source_type; + bound->transform_name = rule.transform; + bound->output_type = factory.output_type(source_type); + bound->transform = factory.create(source_type, rule.params); + bindings_[topic] = bound; + spdlog::info("Topic '{}' ({}) -> transform '{}' -> {}", topic, source_type, rule.transform, bound->output_type); + return bound; + } + return nullptr; +} + +std::shared_ptr TransformSet::find(const std::string& topic) const { + std::lock_guard lock(mutex_); + auto it = bindings_.find(topic); + return it == bindings_.end() ? nullptr : it->second; +} + +std::string TransformSet::output_schema(const BoundTransform& bound, const std::string& source_schema) const { + return factories_.at(bound.transform_name).output_schema(bound.source_type, source_schema); +} + +std::string TransformSet::stats_summary() const { + std::lock_guard lock(mutex_); + std::ostringstream os; + for (const auto& [topic, bound] : bindings_) { + const uint64_t n = bound->samples.load(); + if (n == 0 && bound->drops.load() == 0) { + continue; + } + const double in = static_cast(bound->in_bytes.load()); + const double out = static_cast(bound->out_bytes.load()); + os << "\n " << topic << " [" << bound->transform_name << "]: " << n << " samples, " << bound->drops.load() + << " drops, ratio " << (out > 0 ? in / out : 0.0) << ", " << (n ? bound->micros.load() / n : 0) + << " us/sample"; + } + return os.str(); +} + +} // namespace pj_bridge +``` + +`factories_` is written only in `create()` before the object is shared, so reading it without the lock in `output_schema()` is safe. + +- [ ] **Step 5:** Build, run tests `TransformSet*`. Expected: 6 + 10 tests PASS. +- [ ] **Step 6:** Commit: `feat(transform): TransformSet profile parsing, matching and counters`. + +--- + +### Task 4: TransformingTopicSource + informational protocol fields + +**Files:** Create `app/include/pj_bridge/transforming_topic_source.hpp`, `app/src/transforming_topic_source.cpp`, `tests/unit/test_transforming_topic_source.cpp`. Modify `app/include/pj_bridge/topic_source_interface.hpp`, `app/src/bridge_server.cpp:281`, `app/include/pj_bridge/protocol_constants.hpp`, `tests/unit/test_protocol_constants.cpp`, `CMakeLists.txt`. + +- [ ] **Step 1:** Add to `TopicInfo`: + +```cpp + std::string source_type; ///< set only for transformed topics: the type before the transform +``` + +- [ ] **Step 2: Failing test** + +```cpp +#include + +#include "pj_bridge/transforming_topic_source.hpp" + +using namespace pj_bridge; + +namespace { +class StubSource : public TopicSourceInterface { + public: + std::vector get_topics() override { + return {{"/cloud", "pkg/msg/In", ""}, {"/odom", "pkg/msg/Odom", ""}}; + } + std::string get_schema(const std::string& topic) override { + return "schema-of-" + topic; + } + std::string schema_encoding() const override { + return "ros2msg"; + } + bool is_transient_local(const std::string& topic) const override { + return topic == "/cloud"; + } +}; + +class PassThrough : public MessageTransform { + public: + tl::expected apply(std::span in, std::vector& out) override { + out.assign(in.begin(), in.end()); + return {}; + } +}; + +std::shared_ptr make_set() { + TransformFactory f; + f.accepts = [](const std::string& t) { return t == "pkg/msg/In"; }; + f.check_params = [](const nlohmann::json&) -> tl::expected { return {}; }; + f.output_type = [](const std::string&) { return std::string("pkg/msg/Out"); }; + f.output_schema = [](const std::string&, const std::string& s) { return "OUT:" + s; }; + f.create = [](const std::string&, const nlohmann::json&) { return std::make_unique(); }; + nlohmann::json profile = {{"transforms", {{{"match_type", "pkg/msg/In"}, {"transform", "t"}}}}}; + return TransformSet::create(profile, {{"t", f}}).value(); +} +} // namespace + +TEST(TransformingTopicSourceTest, RewritesTypeAndSchemaOnlyForMatchedTopics) { + TransformingTopicSource source(std::make_shared(), make_set()); + + auto topics = source.get_topics(); + ASSERT_EQ(topics.size(), 2u); + EXPECT_EQ(topics[0].type, "pkg/msg/Out"); + EXPECT_EQ(topics[0].source_type, "pkg/msg/In"); + EXPECT_EQ(topics[1].type, "pkg/msg/Odom"); + EXPECT_TRUE(topics[1].source_type.empty()); + + EXPECT_EQ(source.get_schema("/cloud"), "OUT:schema-of-/cloud"); + EXPECT_EQ(source.get_schema("/odom"), "schema-of-/odom"); + EXPECT_EQ(source.schema_encoding(), "ros2msg"); + EXPECT_TRUE(source.is_transient_local("/cloud")); +} +``` + +- [ ] **Step 3:** Build. Expected: compile error, `transforming_topic_source.hpp` not found. + +- [ ] **Step 4: Implementation** + +Header: + +```cpp +#pragma once + +#include + +#include "pj_bridge/topic_source_interface.hpp" +#include "pj_bridge/transform_set.hpp" + +namespace pj_bridge { + +/// Advertises transformed topics with their OUTPUT type and schema, so clients +/// never see the source type. Thread safety: same as the wrapped source. +class TransformingTopicSource : public TopicSourceInterface { + public: + TransformingTopicSource(std::shared_ptr inner, std::shared_ptr transforms); + + std::vector get_topics() override; + std::string get_schema(const std::string& topic_name) override; + std::string schema_encoding() const override; + bool is_transient_local(const std::string& topic_name) const override; + + private: + std::shared_ptr inner_; + std::shared_ptr transforms_; +}; + +} // namespace pj_bridge +``` + +Source: + +```cpp +#include "pj_bridge/transforming_topic_source.hpp" + +namespace pj_bridge { + +TransformingTopicSource::TransformingTopicSource( + std::shared_ptr inner, std::shared_ptr transforms) + : inner_(std::move(inner)), transforms_(std::move(transforms)) {} + +std::vector TransformingTopicSource::get_topics() { + auto topics = inner_->get_topics(); + for (auto& topic : topics) { + if (auto bound = transforms_->bind(topic.name, topic.type)) { + topic.source_type = topic.type; + topic.type = bound->output_type; + } + } + return topics; +} + +std::string TransformingTopicSource::get_schema(const std::string& topic_name) { + auto schema = inner_->get_schema(topic_name); + if (auto bound = transforms_->find(topic_name)) { + return transforms_->output_schema(*bound, schema); + } + return schema; +} + +std::string TransformingTopicSource::schema_encoding() const { + return inner_->schema_encoding(); +} + +bool TransformingTopicSource::is_transient_local(const std::string& topic_name) const { + return inner_->is_transient_local(topic_name); +} + +} // namespace pj_bridge +``` + +- [ ] **Step 5:** In `app/src/bridge_server.cpp`, right after `topic_entry["type"] = topic.type;` (line ~281): + +```cpp + if (!topic.source_type.empty()) { + topic_entry["source_type"] = topic.source_type; + } +``` + +`topics_changed` entries (line ~824) are built from a name→type map and do not carry `source_type` in v1; say so in `docs/API.md` (Task 8). + +- [ ] **Step 6:** Append to `kServerCapabilities`: `"message_transforms", // topics may be advertised with a transformed type (+ source_type)`. If `test_protocol_constants.cpp` asserts the list or its size, update it. +- [ ] **Step 7:** Any aggregate initialisation `TopicInfo{name, type}` still compiles (the new member is last and defaulted). Build, run the whole suite. Expected: all PASS. +- [ ] **Step 8:** Commit: `feat(transform): advertise transformed type and schema`. + +--- + +### Task 5: ROS2 hook and the `strip` transform + +**Files:** Create `ros2/include/pj_bridge_ros2/strip_transform.hpp`, `ros2/src/strip_transform.cpp`. Modify `ros2/include/pj_bridge_ros2/message_stripper.hpp`, `ros2/src/message_stripper.cpp`, `ros2/include/pj_bridge_ros2/ros2_subscription_manager.hpp`, `ros2/src/ros2_subscription_manager.cpp`, `tests/unit/test_ros2_subscription_manager.cpp`, `CMakeLists.txt`. + +- [ ] **Step 1:** Expose the stripper's type list. In `message_stripper.hpp` add to the class: + +```cpp + static const std::unordered_set& strippable_types(); +``` + +(`#include `), and in the `.cpp`: `const std::unordered_set& MessageStripper::strippable_types() { return kStrippableTypes; }`. + +- [ ] **Step 2:** `strip_transform.hpp`: + +```cpp +#pragma once + +#include "pj_bridge/message_transform.hpp" + +namespace pj_bridge { + +/// `strip` transform: MessageStripper behind the MessageTransform interface. +/// Output type and schema equal the input's. Takes no params. +TransformFactory make_strip_transform_factory(); + +} // namespace pj_bridge +``` + +`strip_transform.cpp`: + +```cpp +#include "pj_bridge_ros2/strip_transform.hpp" + +#include + +#include "pj_bridge_ros2/message_stripper.hpp" + +namespace pj_bridge { +namespace { + +class StripTransform : public MessageTransform { + public: + explicit StripTransform(std::string type) : type_(std::move(type)) {} + + tl::expected apply(std::span in, std::vector& out) override { + try { + // MessageStripper needs a SerializedMessage: one copy of the input. + rclcpp::SerializedMessage input(in.size()); + auto& rcl_in = input.get_rcl_serialized_message(); + std::memcpy(rcl_in.buffer, in.data(), in.size()); + rcl_in.buffer_length = in.size(); + + const auto stripped = MessageStripper::strip(type_, input); + const auto& rcl_out = stripped.get_rcl_serialized_message(); + const auto* bytes = reinterpret_cast(rcl_out.buffer); + out.assign(bytes, bytes + rcl_out.buffer_length); + return {}; + } catch (const std::exception& e) { + return tl::make_unexpected(std::string(e.what())); + } + } + + private: + std::string type_; +}; + +} // namespace + +TransformFactory make_strip_transform_factory() { + TransformFactory f; + f.accepts = [](const std::string& type) { return MessageStripper::should_strip(type); }; + f.check_params = [](const nlohmann::json& params) -> tl::expected { + if (!params.empty()) { + return tl::make_unexpected(std::string("takes no params")); + } + return {}; + }; + f.output_type = [](const std::string& type) { return type; }; + f.output_schema = [](const std::string&, const std::string& schema) { return schema; }; + f.create = [](const std::string& type, const nlohmann::json&) { return std::make_unique(type); }; + return f; +} + +} // namespace pj_bridge +``` + +- [ ] **Step 3:** Change `Ros2SubscriptionManager`. Constructor becomes: + +```cpp + explicit Ros2SubscriptionManager( + rclcpp::Node::SharedPtr node, std::shared_ptr transforms = nullptr, size_t min_qos_depth = 1, + size_t max_qos_depth = 100); +``` + +Replace the member `bool strip_large_messages_;` with `std::shared_ptr transforms_;`, include `pj_bridge/transform_set.hpp`, drop the `message_stripper.hpp` include. Update the class comment: "...and applies the topic's transform, if any, on the rcl buffer before copying." + +- [ ] **Step 4: Failing test.** First read `tests/unit/test_ros2_subscription_manager.cpp`: every construction passing `true`/`false` as the second argument must change. Replace the strip-specific tests' setup with a `TransformSet` holding the strip factory, and add one new test. Helper + test (adapt the publisher/spin scaffolding to what that file already uses): + +```cpp +std::shared_ptr strip_everything() { + auto set = pj_bridge::TransformSet::create( + nlohmann::json{{"transforms", nlohmann::json::array()}}, + {{"strip", pj_bridge::make_strip_transform_factory()}}) + .value(); + for (const auto& type : pj_bridge::MessageStripper::strippable_types()) { + EXPECT_TRUE(set->append_type_rule(type, "strip").has_value()); + } + return set; +} + +TEST_F(Ros2SubscriptionManagerTest, SubscribesWithSourceTypeAndForwardsTransformedBytes) { + // A transform whose output type differs from the source type: the manager is + // asked to subscribe with the ADVERTISED type and must use the source type. + pj_bridge::TransformFactory f; + f.accepts = [](const std::string& t) { return t == "std_msgs/msg/String"; }; + f.check_params = [](const nlohmann::json&) -> tl::expected { return {}; }; + f.output_type = [](const std::string&) { return std::string("fake_msgs/msg/Out"); }; + f.output_schema = [](const std::string&, const std::string& s) { return s; }; + f.create = [](const std::string&, const nlohmann::json&) -> std::unique_ptr { + struct Marker : pj_bridge::MessageTransform { + tl::expected apply(std::span, std::vector& out) override { + out.assign(3, std::byte{0x5A}); + return {}; + } + }; + return std::make_unique(); + }; + auto set = pj_bridge::TransformSet::create( + nlohmann::json{{"transforms", {{{"match_topic", "/transform_me"}, {"transform", "marker"}}}}}, + {{"marker", f}}) + .value(); + ASSERT_NE(set->bind("/transform_me", "std_msgs/msg/String"), nullptr); // what get_topics would have done + + pj_bridge::Ros2SubscriptionManager manager(node_, set); + std::vector received; + manager.set_message_callback( + [&](const std::string&, std::shared_ptr> data, uint64_t) { received = *data; }); + + ASSERT_TRUE(manager.subscribe("/transform_me", "fake_msgs/msg/Out")); + // publish one std_msgs/msg/String on /transform_me and spin until `received` is non-empty, + // using the same publish-and-spin helper the neighbouring tests use. + publish_string_and_spin("/transform_me", "hello", [&] { return !received.empty(); }); + + EXPECT_EQ(received, std::vector(3, std::byte{0x5A})); +} +``` + +If the file has no publish-and-spin helper, add one next to the fixture: + +```cpp + void publish_string_and_spin(const std::string& topic, const std::string& text, const std::function& done) { + auto pub = node_->create_publisher(topic, 10); + std_msgs::msg::String msg; + msg.data = text; + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + while (!done() && std::chrono::steady_clock::now() < deadline) { + pub->publish(msg); + rclcpp::spin_some(node_); + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + } + } +``` + +- [ ] **Step 5:** Build. Expected: FAIL (constructor signature, behaviour). + +- [ ] **Step 6: Implement the hook.** `subscribe()` becomes: + +```cpp +bool Ros2SubscriptionManager::subscribe(const std::string& topic_name, const std::string& topic_type) { + // A transformed topic is advertised with its output type; subscribe with the real one. + std::shared_ptr bound = transforms_ ? transforms_->find(topic_name) : nullptr; + const std::string& source_type = bound ? bound->source_type : topic_type; + + Ros2MessageCallback ros2_callback = + [this, bound]( + const std::string& topic, const std::shared_ptr& msg, uint64_t receive_time_ns) { + const auto& rcl_msg = msg->get_rcl_serialized_message(); + const auto* bytes = reinterpret_cast(rcl_msg.buffer); + + std::shared_ptr> data; + if (bound) { + data = std::make_shared>(); + if (!bound->run(topic, {bytes, rcl_msg.buffer_length}, *data)) { + return; // dropped and counted; never forward the untransformed bytes + } + } else { + data = std::make_shared>(bytes, bytes + rcl_msg.buffer_length); + } + + MessageCallback cb; + { + std::lock_guard lock(callback_mutex_); + cb = callback_; + } + if (cb) { + cb(topic, std::move(data), receive_time_ns); + } + }; + + return inner_manager_.subscribe(topic_name, source_type, ros2_callback); +} +``` + +Constructor: `: inner_manager_(node, min_qos_depth, max_qos_depth), transforms_(std::move(transforms)) {}`. Remove `#include ` if now unused. + +- [ ] **Step 7:** Add `ros2/src/strip_transform.cpp` to `pj_bridge_ros2_lib`. `ros2/src/main.cpp` will not compile yet (it passes a bool): change that call temporarily to pass `nullptr` so the tree builds; Task 6 wires it properly. +- [ ] **Step 8:** Build, run tests `Ros2SubscriptionManager*:MessageStripper*`. Expected: PASS. +- [ ] **Step 9:** Commit: `feat(transform): ROS2 ingest hook; strip becomes a transform`. + +--- + +### Task 6: Wire `transform_profile` and `strip_large_messages` in main + +**Files:** Modify `ros2/src/main.cpp`. + +- [ ] **Step 1:** Next to the other `declare_parameter` calls: `node->declare_parameter("transform_profile", "");` and read it into `std::string transform_profile`. Add it to the "Configuration:" log line. +- [ ] **Step 2:** Add includes: ``, `"pj_bridge/transform_set.hpp"`, `"pj_bridge/transforming_topic_source.hpp"`, `"pj_bridge_ros2/strip_transform.hpp"`, `"pj_bridge_ros2/message_stripper.hpp"`. +- [ ] **Step 3:** Inside the `try`, replace the `topic_source` / `sub_manager` construction with: + +```cpp + std::shared_ptr topic_source = std::make_shared(node); + + std::shared_ptr transforms; + if (!transform_profile.empty() || strip_large_messages) { + nlohmann::json profile = {{"transforms", nlohmann::json::array()}}; + if (!transform_profile.empty()) { + std::ifstream file(transform_profile); + if (!file) { + RCLCPP_ERROR(node->get_logger(), "Cannot open transform_profile '%s'", transform_profile.c_str()); + rclcpp::shutdown(); + return 1; + } + profile = nlohmann::json::parse(file, nullptr, /*allow_exceptions=*/false); + if (profile.is_discarded()) { + RCLCPP_ERROR(node->get_logger(), "transform_profile '%s' is not valid JSON", transform_profile.c_str()); + rclcpp::shutdown(); + return 1; + } + } + + pj_bridge::TransformSet::FactoryMap factories; + factories["strip"] = pj_bridge::make_strip_transform_factory(); + + auto created = pj_bridge::TransformSet::create(profile, std::move(factories)); + if (!created) { + RCLCPP_ERROR(node->get_logger(), "%s", created.error().c_str()); + rclcpp::shutdown(); + return 1; + } + transforms = *created; + + if (strip_large_messages) { // after the profile's rules, so an explicit rule wins + for (const auto& type : pj_bridge::MessageStripper::strippable_types()) { + transforms->append_type_rule(type, "strip"); + } + } + topic_source = std::make_shared(topic_source, transforms); + } + + auto sub_manager = std::make_shared( + node, transforms, static_cast(min_qos_depth), static_cast(max_qos_depth)); +``` + +Check that `BridgeServer`'s constructor takes `std::shared_ptr` (it does today via implicit conversion from `shared_ptr`). + +- [ ] **Step 4:** After the "Final statistics" log: + +```cpp + if (transforms) { + const auto summary = transforms->stats_summary(); + if (!summary.empty()) { + RCLCPP_INFO(node->get_logger(), "Transform statistics:%s", summary.c_str()); + } + } +``` + +- [ ] **Step 5: Manual check.** Build. Then, in three terminals under `pixi run -e humble` with `install/setup.bash` sourced: + 1. `pj_bridge_ros2` with `--ros-args -p strip_large_messages:=true` → log shows no error; with `-p transform_profile:=/nonexistent.json` → exits 1 with "Cannot open"; with a file containing `{"transforms":[{"match_type":"sensor_msgs/msg/Image","transform":"nope"}]}` → exits 1 naming the unknown transform. + 2. Whole test suite still passes. +- [ ] **Step 6:** Commit: `feat(transform): transform_profile parameter; strip_large_messages as sugar`. + +--- + +### Task 7: Cloudini transform + +**Files:** Create `app/include/pj_bridge/cloudini_transform.hpp`, `app/src/cloudini_transform.cpp`, `tests/unit/test_cloudini_transform.cpp`. Modify `CMakeLists.txt`, `ros2/src/main.cpp`, `package.xml`. + +- [ ] **Step 1: CMake.** After the IXWebSocket block: + +```cmake +# Cloudini (optional): point cloud compression transform +option(PJ_BRIDGE_FETCH_CLOUDINI "Fetch Cloudini when it is not installed" ON) +find_package(cloudini_lib QUIET) +if(NOT cloudini_lib_FOUND AND PJ_BRIDGE_FETCH_CLOUDINI) + include(FetchContent) + set(CLOUDINI_BUILD_TOOLS OFF CACHE BOOL "" FORCE) + set(CLOUDINI_BUILD_BENCHMARKS OFF CACHE BOOL "" FORCE) + FetchContent_Declare(cloudini + URL https://github.com/facontidavide/cloudini/archive/refs/tags/1.3.0.tar.gz + URL_HASH SHA256=26c9322d5a08694b9b24bbec975b7814b05cf7d4962327f35266b38bb15b5e66 + SOURCE_SUBDIR cloudini_lib + EXCLUDE_FROM_ALL) + # Cloudini switches to a SHARED ament package (install rules, ament_package()) + # when it finds ament_cmake. Hide it for the subdirectory: we want the plain + # static library. Scoped to Cloudini's directory; our own ament state is untouched. + set(BUILD_TESTING_SAVED ${BUILD_TESTING}) + set(BUILD_TESTING OFF) + set(CMAKE_DISABLE_FIND_PACKAGE_ament_cmake ON) + FetchContent_MakeAvailable(cloudini) + set(CMAKE_DISABLE_FIND_PACKAGE_ament_cmake OFF) + set(BUILD_TESTING ${BUILD_TESTING_SAVED}) + if(NOT TARGET cloudini::cloudini_lib) + add_library(cloudini::cloudini_lib ALIAS cloudini_lib) + endif() + set_target_properties(cloudini_lib PROPERTIES POSITION_INDEPENDENT_CODE ON) + set(cloudini_lib_FOUND TRUE) +endif() +``` + +and, after `pj_bridge_app` is defined: + +```cmake +if(cloudini_lib_FOUND) + target_sources(pj_bridge_app PRIVATE app/src/cloudini_transform.cpp) + target_link_libraries(pj_bridge_app PRIVATE cloudini::cloudini_lib) + target_compile_definitions(pj_bridge_app PUBLIC PJ_BRIDGE_HAS_CLOUDINI) + message(STATUS "Cloudini transform: enabled") +else() + message(STATUS "Cloudini transform: disabled (cloudini_lib not found)") +endif() +``` + +The hash above was computed from the 1.3.0 tag tarball on 2026-09-20. After configuring, confirm in the CMake output that Cloudini did **not** take its ament branch (`grep -c "ament" build/pj_bridge/_deps/cloudini-build/CMakeCache.txt` should show no `ament_cmake_DIR` pointing to a real path) and that `libcloudini_lib.a` (static) was produced. The pixi environments ship CMake 4.x, so `EXCLUDE_FROM_ALL` in `FetchContent_Declare` (needs >= 3.28) is fine there; the Humble CI container has an older CMake — if configure fails on that keyword, remove it and wrap `FetchContent_MakeAvailable` with `set(CMAKE_SKIP_INSTALL_RULES ON)` / `OFF`. If the static `cloudini_lib` is linked PRIVATE into the static `pj_bridge_app`, the final executables still need it on their link line — CMake propagates this automatically for static libraries (`$`); confirm `pj_bridge_ros2` links. + +- [ ] **Step 2:** `cloudini_transform.hpp`: + +```cpp +#pragma once + +#include "pj_bridge/message_transform.hpp" + +namespace pj_bridge { + +/// `cloudini` transform: sensor_msgs PointCloud2 -> point_cloud_interfaces +/// CompressedPointCloud2. Works on CDR bytes, so it serves any backend whose +/// PointCloud2 is CDR-encoded. Only declared when Cloudini is available. +/// +/// Params: resolution (float, default 0.001), fields ({name: resolution}, 0 +/// removes the field), viz_preprocessing (bool, default false). +TransformFactory make_cloudini_transform_factory(); + +} // namespace pj_bridge +``` + +- [ ] **Step 3: Failing smoke test** — `tests/unit/test_cloudini_transform.cpp`, whole file inside `#ifdef PJ_BRIDGE_HAS_CLOUDINI`: + +```cpp +#ifdef PJ_BRIDGE_HAS_CLOUDINI + +#include + +#include +#include +#include + +#include "pj_bridge/cloudini_transform.hpp" + +using namespace pj_bridge; + +namespace { +std::vector make_cloud_cdr(size_t points) { + sensor_msgs::msg::PointCloud2 cloud; + cloud.header.frame_id = "lidar"; + sensor_msgs::PointCloud2Modifier modifier(cloud); + modifier.setPointCloud2FieldsByString(1, "xyz"); + modifier.resize(points); + sensor_msgs::PointCloud2Iterator x(cloud, "x"), y(cloud, "y"), z(cloud, "z"); + for (size_t i = 0; i < points; ++i, ++x, ++y, ++z) { + *x = 0.01f * static_cast(i); + *y = 1.0f; + *z = -0.5f; + } + rclcpp::SerializedMessage serialized; + rclcpp::Serialization().serialize_message(&cloud, &serialized); + const auto& rcl = serialized.get_rcl_serialized_message(); + const auto* bytes = reinterpret_cast(rcl.buffer); + return {bytes, bytes + rcl.buffer_length}; +} +} // namespace + +TEST(CloudiniTransformTest, CompressesPointCloud2) { + auto factory = make_cloudini_transform_factory(); + EXPECT_TRUE(factory.accepts("sensor_msgs/msg/PointCloud2")); + EXPECT_FALSE(factory.accepts("sensor_msgs/msg/Image")); + EXPECT_EQ(factory.output_type("sensor_msgs/msg/PointCloud2"), "point_cloud_interfaces/msg/CompressedPointCloud2"); + EXPECT_NE(factory.output_schema("sensor_msgs/msg/PointCloud2", "").find("format"), std::string::npos); + + const nlohmann::json params = {{"resolution", 0.001}}; + ASSERT_TRUE(factory.check_params(params).has_value()); + EXPECT_FALSE(factory.check_params({{"resolutoin", 0.001}}).has_value()); + EXPECT_FALSE(factory.check_params({{"resolution", -1.0}}).has_value()); + + auto transform = factory.create("sensor_msgs/msg/PointCloud2", params); + const auto in = make_cloud_cdr(10000); + std::vector out; + ASSERT_TRUE(transform->apply(in, out).has_value()); + EXPECT_FALSE(out.empty()); + EXPECT_LT(out.size(), in.size()); + + ASSERT_TRUE(transform->apply(in, out).has_value()); // instance is reusable +} + +TEST(CloudiniTransformTest, GarbageInputIsAnErrorNotACrash) { + auto transform = make_cloudini_transform_factory().create("sensor_msgs/msg/PointCloud2", nlohmann::json::object()); + const std::vector in(7, std::byte{0xFF}); + std::vector out; + EXPECT_FALSE(transform->apply(in, out).has_value()); +} + +#endif // PJ_BRIDGE_HAS_CLOUDINI +``` + +Add the file to the test target unconditionally (it is empty without the define). Build: expected link error for `make_cloudini_transform_factory`. + +- [ ] **Step 4: Implementation** — `app/src/cloudini_transform.cpp` + +```cpp +#include "pj_bridge/cloudini_transform.hpp" + +#include +#include +// Defines non-inline `const char*` globals: include in this translation unit ONLY. +#include + +namespace pj_bridge { +namespace { + +constexpr const char* kPointCloud2 = "sensor_msgs/msg/PointCloud2"; + +class CloudiniTransform : public MessageTransform { + public: + explicit CloudiniTransform(const nlohmann::json& params) + : resolution_(params.value("resolution", 0.001f)), viz_preprocessing_(params.value("viz_preprocessing", false)) { + if (params.contains("fields")) { + for (auto it = params["fields"].begin(); it != params["fields"].end(); ++it) { + profile_[it.key()] = it->get(); + } + } + } + + tl::expected apply(std::span in, std::vector& out) override { + try { + const Cloudini::ConstBufferView raw(reinterpret_cast(in.data()), in.size()); + auto cloud = cloudini_ros::getDeserializedPointCloudMessage(raw); + cloudini_ros::applyResolutionProfile(profile_, cloud.fields, resolution_); + if (viz_preprocessing_) { + cloudini_ros::applyVizLossyPreprocessing(cloud); + } + auto info = cloudini_ros::toEncodingInfo(cloud); + // The frame is zstd-compressed by the bridge: a second stage here would compress twice. + info.compression_opt = Cloudini::CompressionOption::NONE; + info.use_threads = false; + + // Cloudini writes CDR into a vector; scratch_ keeps its capacity across calls. + cloudini_ros::convertPointCloud2ToCompressedCloud(cloud, info, scratch_); + const auto* bytes = reinterpret_cast(scratch_.data()); + out.assign(bytes, bytes + scratch_.size()); + return {}; + } catch (const std::exception& e) { + return tl::make_unexpected(std::string(e.what())); + } + } + + private: + float resolution_; + bool viz_preprocessing_; + cloudini_ros::ResolutionProfile profile_; + std::vector scratch_; +}; + +tl::expected check_cloudini_params(const nlohmann::json& params) { + for (auto it = params.begin(); it != params.end(); ++it) { + const auto& key = it.key(); + if (key == "resolution") { + if (!it->is_number() || it->get() <= 0.0) { + return tl::make_unexpected(std::string("'resolution' must be a number > 0")); + } + } else if (key == "viz_preprocessing") { + if (!it->is_boolean()) { + return tl::make_unexpected(std::string("'viz_preprocessing' must be a boolean")); + } + } else if (key == "fields") { + if (!it->is_object()) { + return tl::make_unexpected(std::string("'fields' must be an object of {name: resolution}")); + } + for (auto f = it->begin(); f != it->end(); ++f) { + if (!f->is_number() || f->get() < 0.0) { + return tl::make_unexpected("'fields." + f.key() + "' must be a number >= 0 (0 removes the field)"); + } + } + } else { + return tl::make_unexpected("unknown param '" + key + "'"); + } + } + return {}; +} + +} // namespace + +TransformFactory make_cloudini_transform_factory() { + TransformFactory f; + f.accepts = [](const std::string& type) { return type == kPointCloud2; }; + f.check_params = check_cloudini_params; + f.output_type = [](const std::string&) { return std::string(compressed_schema_name); }; + f.output_schema = [](const std::string&, const std::string&) { return std::string(compressed_schema_data); }; + f.create = [](const std::string&, const nlohmann::json& params) { + return std::make_unique(params); + }; + return f; +} + +} // namespace pj_bridge +``` + +Verify against the installed headers before building: the namespace of `compressed_schema_name` / `compressed_schema_data` in `ros_message_definitions.hpp`, the spelling `Cloudini::CompressionOption::NONE`, and that `toEncodingInfo` leaves `encoding_opt` at `LOSSY`. Known cost, accepted for v1: `convertPointCloud2ToCompressedCloud` builds a `PointcloudEncoder` per call and the result is copied once (compressed size) from `scratch_` into `out`. Removing both needs a small Cloudini API addition (byte-generic output, reusable encoder) — out of scope here. + +- [ ] **Step 5:** Register it in `ros2/src/main.cpp` next to the strip factory: + +```cpp +#ifdef PJ_BRIDGE_HAS_CLOUDINI + factories["cloudini"] = pj_bridge::make_cloudini_transform_factory(); +#endif +``` + +with `#ifdef PJ_BRIDGE_HAS_CLOUDINI` / `#include "pj_bridge/cloudini_transform.hpp"` / `#endif` among the includes. Without Cloudini, a profile naming `cloudini` already fails at startup with "unknown transform 'cloudini' (not built in, or misspelled)". + +- [ ] **Step 6:** Build for Humble and Jazzy, run tests `CloudiniTransform*`. Expected: 2 PASS. Then the whole suite. +- [ ] **Step 7:** `package.xml`: do **not** add `cloudini_lib` until the key resolves on every target distro (check `rosdep resolve cloudini_lib --rosdistro humble|jazzy|lyrical`); record the result in the commit body. +- [ ] **Step 8:** Commit: `feat(transform): Cloudini point cloud compression`. + +--- + +### Task 8: Documentation + +**Files:** Modify `docs/API.md`, `README.md`, `CLAUDE.md`, `CHANGELOG.rst`. Create `docs/transform_profile.example.json`. + +- [ ] **Step 1:** `docs/transform_profile.example.json`: + +```json +{ + "transforms": [ + { + "match_type": "sensor_msgs/msg/PointCloud2", + "transform": "cloudini", + "params": {"resolution": 0.001, "viz_preprocessing": false} + } + ] +} +``` + +- [ ] **Step 2:** `docs/API.md`: new section "Message transforms" covering: operator-only and off by default; a transformed topic is advertised with its **output** type and schema and the optional `source_type` (absent from `topics_changed` entries in this version); the `message_transforms` capability; the profile format, matching rules and first-match-wins; the two transforms and their params; every startup error; runtime failures drop the sample; `strip_large_messages: true` equals strip rules appended after the profile; clients need a `CompressedPointCloud2`-capable parser; Cloudini is lossy at `resolution`. +- [ ] **Step 3:** `README.md` and `CLAUDE.md`: add `transform_profile: ""` to the ROS2 configuration block; CLAUDE.md gets C++20, the new components (TransformSet, TransformingTopicSource, the hook replacing the stripper call), Cloudini as an optional dependency, and the updated test count. +- [ ] **Step 4:** `CHANGELOG.rst`: Forthcoming section listing the feature, C++20, and the `strip` runtime-failure behaviour change (drop instead of forwarding the original). +- [ ] **Step 5:** `pre-commit run -a`; commit: `docs: message transforms`. + +--- + +### Task 9: Acceptance measurement + +Uses the pinned rig and scripts described in `docs/perf/README.md` (bridge on cores 4-7, player 12-19, client 8-11, governor `performance`, 20 s windows from `/proc//stat`). Large temporary files go on disk, never in `/tmp` (tmpfs). + +- [ ] **Step 1:** 4-lidar bag, one client subscribed to all topics, three runs each: (a) no profile, (b) `docs/transform_profile.example.json`, (c) same with `viz_preprocessing: true`. +- [ ] **Step 2:** Record per configuration: bridge CPU (% of one core), wire bytes/s from the client, client-side per-topic message counts, and the shutdown "Transform statistics" (ratio, µs/sample). +- [ ] **Step 3:** Pass criteria: per-topic message counts in (b) and (c) equal (a) within one publish period; no transform drops. If counts fall short, ingest is being starved by encode time: stop and report — the fallback (per-topic worker, 1-deep latest-wins slot) is a separate design step, do not improvise it. +- [ ] **Step 4:** End to end: PlotJuggler built from `pj-official-plugins` branch `feat/compressed-pointcloud` connects, subscribes to a lidar topic and renders the cloud. +- [ ] **Step 5:** Add the numbers to `docs/perf/README.md`; commit: `docs(perf): message transform measurements`. diff --git a/docs/superpowers/specs/2026-09-20-message-transforms-design.md b/docs/superpowers/specs/2026-09-20-message-transforms-design.md new file mode 100644 index 0000000..43a5306 --- /dev/null +++ b/docs/superpowers/specs/2026-09-20-message-transforms-design.md @@ -0,0 +1,262 @@ +# Message Transforms (v1: Cloudini) — Design + +Date: 2026-09-20. Status: implemented (ROS2 backend); see "Measured results". +Supersedes the exploratory `docs/MESSAGE_TRANSFORMS.md` (PR #14). + +## Goal + +Let the server operator replace the payload of selected topics with a +transformed one before it is buffered and sent. First transform: +[Cloudini](https://github.com/facontidavide/cloudini) point cloud compression +(`sensor_msgs/msg/PointCloud2` → `point_cloud_interfaces/msg/CompressedPointCloud2`). +The existing `MessageStripper` becomes the second implementation. + +## Decisions + +| Question | Decision | +|---|---| +| Scope | Minimal seam + Cloudini. No per-client variants, no offer/force negotiation, no worker pool. | +| Who enables it | Server operator, off by default. A topic is transformed for all subscribers or none. | +| Configuration | One parameter, `transform_profile`, pointing to a JSON file. | +| Wire protocol | Unchanged. Transformed topics are simply advertised with their output type and schema. | +| Cloudini dependency | `find_package(cloudini_lib)`, FetchContent fallback (pinned tag + SHA256, static). Feature compiled out if neither works. | +| Double compression | Cloudini second stage is `NONE`; the frame-level zstd is the general-purpose stage. | +| `strip_large_messages` | `strip` becomes a transform name; the boolean stays as sugar. | +| Threading | Synchronous in the ingest callback. Measure before adding a worker. | + +Client requirement: a PlotJuggler whose `parser_ros` decodes +`CompressedPointCloud2` (`pj-official-plugins`, branch +`feat/compressed-pointcloud`). Older clients see a topic they cannot parse; +that is the operator's responsibility when enabling the profile. The plugin +already binds its parser from the type in `get_topics` and the +`encoding`/`definition` in the subscribe response, so it needs no protocol +change. + +## Architecture + +`BridgeServer` learns types and schemas only through `TopicSourceInterface` +and message bytes only through `SubscriptionManagerInterface`. It is not +modified (except for the informational fields below). + +``` +Ros2TopicSource ──► TransformingTopicSource ──► BridgeServer + │ records topic → (source_type, rule) + ▼ + TransformSet ◄── profile JSON + name→factory map + ▲ resolve(topic) → MessageTransform* +Ros2SubscriptionManager ──┘ (calls apply() on the rcl buffer, before any copy) +``` + +- **`TransformSet`** (`app/`): owns the ordered rules, the name→factory map, + and the per-topic transform instances. `resolve(topic, source_type)` returns + the matching rule (first match wins) or nothing. Thread-safe: written from + the request thread (`get_topics`/`subscribe`), read from the ingest thread. +- **`TransformingTopicSource`** (`app/`, decorator): `get_topics()` rewrites + `type` for matched topics and records the source type in the `TransformSet`; + `get_schema()` returns the transform's output schema; everything else passes + through. +- **Backend hook**: `Ros2SubscriptionManager::subscribe(name, advertised_type)` + asks the `TransformSet` for the topic's real source type and subscribes with + it. In the message callback, if a transform is resolved for the topic, call + `apply()` on the view over the rcl buffer and forward `out`; otherwise copy + as today. FastDDS can adopt the same three lines later; v1 wires ROS2 only. +- With no profile and `strip_large_messages: false`, no `TransformSet` is + created and behavior is byte-identical to today. + +Everything downstream (latched retention, heavy-frame threshold, rate gate, +serializer, backpressure) sees post-transform bytes with no special cases. A +2 MiB cloud that compresses below `heavy_frame_threshold_bytes` rejoins normal +aggregation. + +## Interface + +`app/include/pj_bridge/message_transform.hpp`, no ROS/DDS dependencies: + +```cpp +/// One instance per topic. Called from a single thread (the ingest executor). +/// Implementations SHOULD keep codec contexts and scratch buffers as members. +class MessageTransform { + public: + virtual ~MessageTransform() = default; + /// `in` is a view over the middleware's buffer, valid only during the call. + /// `out` is the final storage (already inside the shared_ptr handed to + /// MessageBuffer): cleared and filled in place, never moved or copied. + /// On error the sample is DROPPED by the caller — never forwarded raw, since + /// the client was told the output type at subscribe time. + virtual tl::expected apply( + std::span in, std::vector& out) = 0; + /// What clients are told they receive. Defaults: unchanged (as for `strip`). + virtual std::string output_type(const std::string& source_type) const; + virtual std::string output_schema(const std::string& source_schema) const; +}; + +struct TransformFactory { + std::function accepts; + std::function(const nlohmann::json& params)> check_params; + std::function(const std::string& source_type, + const nlohmann::json& params)> create; +}; +``` + +Allocation rationale: the input is never copied for a transformed topic (the +raw cloud is read in place from the rcl buffer); the only per-message +allocation is the output vector at compressed size, which is unavoidable +because `MessageBuffer` retains it. The input cannot use move semantics: the +rcl buffer is owned by rclcpp and a `std::vector` cannot adopt it, so a +read-only span is the only zero-copy option. The hook does +`auto out = std::make_shared>(); apply(in, *out);` — +the transform writes straight into the final storage. In v1 every message gets +a fresh vector; because `out` is caller-provided, the hook can later hand out +recycled vectors (`shared_ptr` with a recycling deleter — +no downstream change) if measurement justifies it. Not built in v1. + +## Transforms + +### `cloudini` (`app/`, compiled only when Cloudini is available) + +`getDeserializedPointCloudMessage` → `applyResolutionProfile` → optional +`applyVizLossyPreprocessing` → `toEncodingInfo` → +`convertPointCloud2ToCompressedCloud`. Cloudini's CDR writer (`nanocdr`) only +targets `std::vector`, so the transform encodes into a member scratch +vector (capacity kept across calls) and copies the result — at compressed size +— into `out`. v1 accepts that copy and the per-call `PointcloudEncoder` +construction inside the convenience function; removing both needs a small +Cloudini API addition (byte-generic output, reusable encoder), tracked +separately. Second stage `NONE`. +Output schema comes from Cloudini's embedded `ros_message_definitions.hpp`, so +`point_cloud_interfaces` need not be installed. + +Params: + +| Key | Default | Meaning | +|---|---|---| +| `resolution` | `0.001` | Resolution for FLOAT32 fields not listed in `fields` (metres for xyz) | +| `fields` | `{}` | Per-field resolution; `0` removes the field | +| `viz_preprocessing` | `false` | NaN drop + voxel dedup + 1 µs FLOAT64 quantization | + +### `strip` (`ros2/`, registered by `ros2/main.cpp`) + +Thin adapter over the existing `MessageStripper`; output type = input type, +schema unchanged. It keeps its current cost (full typed deserialize / +re-serialize, plus one copy into a `SerializedMessage`). A CDR-level stripper +that avoids this is a separate follow-up. + +## Configuration + +ROS2 parameter `transform_profile` (string, default empty = disabled). + +```json +{ + "transforms": [ + { + "match_type": "sensor_msgs/msg/PointCloud2", + "match_topic": "/lidar/.*", + "transform": "cloudini", + "params": {"resolution": 0.001, "fields": {"intensity": 0.01}, "viz_preprocessing": true} + } + ] +} +``` + +- `match_type` (exact) is required; `match_topic` (full-match regex, same + semantics as `topic_whitelist`) optionally narrows it. Requiring the type + makes every transform/type mismatch a startup error. +- Ordered, first match wins. +- `strip_large_messages: true` appends one `strip` rule per strippable type + **after** the profile's rules, so an explicit profile rule wins. + +**Fail at startup** (clear error, non-zero exit): unreadable or malformed +file, unknown top-level/rule/param key, unknown transform name, `cloudini` +named in a build without Cloudini, invalid regex, params rejected by the +transform's `check_params`, a rule without `match_type`, a `match_type` the named +transform does not `accept()`. A +transform that cannot be instantiated for a topic (factory throws or returns +nothing) is logged as an error and the topic is left untransformed. + +**At runtime**, `apply()` failure → drop the sample, bump a counter, throttled +warning. This also applies to `strip` (today it forwards the original; changed +for uniformity). + +## Informational protocol additions (ignored by old clients) + +- `message_transforms` in the server capabilities list. +- Optional `"source_type"` on `get_topics` entries of transformed topics + (not on `topics_changed` entries in v1). +- Per-topic transform counters (samples, drops, compression ratio, mean µs per + sample) logged with the final statistics at shutdown; the ROS2 entry point + has no periodic stats line today. + +## Build + +The project moves from C++17 to **C++20** unconditionally: `std::span` needs +it, and `cloudini_lib` exports `cxx_std_20` as a PUBLIC requirement, so the +standard must not depend on whether the optional dependency was found. The +oldest targeted compiler is GCC 11 (Humble / Jammy), which supports it. + +```cmake +find_package(cloudini_lib QUIET) +if(NOT cloudini_lib_FOUND) # FetchContent fallback, pinned tag + URL_HASH + ... +endif() +# PJ_BRIDGE_HAS_CLOUDINI defined on pj_bridge_app when available +``` + +Buildfarm debs cannot fetch at build time, so they get the feature once +`cloudini_lib` is a resolvable rosdep key for the distro; GitHub-built debs, +conda packages and AppImages get it through the fallback. + +## Testing + +- Profile parsing: valid file; each startup failure mode. +- Rule matching and precedence, including the `strip_large_messages` sugar. +- `TransformingTopicSource` with the existing mock topic source: type and + schema rewrite, untouched topics pass through. +- Backend hook with a fake transform: source-type lookup on subscribe, output + forwarded, failure drops the sample and counts it. +- `cloudini` smoke test (only when compiled in): a synthetic PointCloud2 CDR + buffer transforms without error into a non-empty, smaller output. No + decode/round-trip test — decoding correctness belongs to Cloudini's own + suite. +- End-to-end, manual: PlotJuggler with the compressed-pointcloud parser against + the 4-lidar bag. + +## Acceptance measurement + +Pinned rig from `docs/perf/README.md`, 4-lidar bag (~52 MiB/s), 1 client, +Cloudini off vs on: bridge CPU (% of a core), bytes on the wire, and +client-side per-topic message counts (must be lossless). If encode time +starves ingest, the designed-but-unbuilt fallback is a per-topic worker with a +1-deep latest-wins input slot. + +## Measured results (2026-09-20) + +Pinned rig (bridge on cores 4-7, player 12-19, client 8-11, governor +`performance`), 90 s cut of the 4-lidar bag (4 x PointCloud2 at 10 Hz, ~52 MiB/s +raw, plus /tf, /imu, /odom), one client subscribed to everything, CPU from +`/proc//stat` over 20 s, three runs each. + +| Configuration | Bridge CPU (% of a core) | Wire | Encode | Pre-zstd ratio | +|---|---|---|---|---| +| no profile | 16.7 / 18.9 / 19.3 | 31.0 MB/s | — | — | +| `cloudini`, 1 mm | 16.7 / 14.0 / 16.8 | 14.6 MB/s | 1.4 ms/cloud | 2.7–2.9 | +| `cloudini`, 1 mm, `viz_preprocessing` | 18.3 / 17.9 / 18.3 | 13.8 MB/s | 2.9 ms/cloud | 3.1–3.5 | + +Lossless in every run: 300/300 clouds per lidar in 30 s, zero transform drops, +other topics at their nominal rates. Bandwidth halves at equal or slightly +lower CPU: the ~6% of a core spent encoding on the ingest thread is paid back +by zstd having a third of the bytes to compress. Run-to-run CPU noise is about +±2 points, so "CPU-neutral" is the defensible claim. `viz_preprocessing` doubles +the encode time for ~5% fewer bytes on this data, which is why it defaults to +off. Synchronous encoding in the ingest callback is sufficient; the per-topic +worker fallback was not needed. + +Cloudini-side check: reusing the `PointcloudEncoder` across messages instead of +the one-shot `convertPointCloud2ToCompressedCloud` measured 0% faster with +second stage `NONE` and ~5% with `ZSTD`, so the scratch-and-copy approach above +costs nothing measurable. + +## Non-goals (v1) + +Per-client transform choice or parameters; client negotiation; transform +chaining; 1→N fan-out; topic renaming; image codecs; output-buffer pooling; +FastDDS/RTI wiring; CDR-level stripper. diff --git a/docs/transform_profile.example.json b/docs/transform_profile.example.json new file mode 100644 index 0000000..d00b2c8 --- /dev/null +++ b/docs/transform_profile.example.json @@ -0,0 +1,9 @@ +{ + "transforms": [ + { + "match_type": "sensor_msgs/msg/PointCloud2", + "transform": "cloudini", + "params": {"resolution": 0.001, "viz_preprocessing": false} + } + ] +} diff --git a/ros2/include/pj_bridge_ros2/message_stripper.hpp b/ros2/include/pj_bridge_ros2/message_stripper.hpp index b418a5e..c6f38ab 100644 --- a/ros2/include/pj_bridge_ros2/message_stripper.hpp +++ b/ros2/include/pj_bridge_ros2/message_stripper.hpp @@ -21,6 +21,7 @@ #include #include +#include namespace pj_bridge { @@ -40,6 +41,9 @@ class MessageStripper { public: static bool should_strip(const std::string& message_type); static rclcpp::SerializedMessage strip(const std::string& message_type, const rclcpp::SerializedMessage& input); + + /// The message types should_strip() accepts. + static const std::unordered_set& strippable_types(); }; } // namespace pj_bridge diff --git a/ros2/include/pj_bridge_ros2/ros2_subscription_manager.hpp b/ros2/include/pj_bridge_ros2/ros2_subscription_manager.hpp index 6e1c8ef..a6b92d9 100644 --- a/ros2/include/pj_bridge_ros2/ros2_subscription_manager.hpp +++ b/ros2/include/pj_bridge_ros2/ros2_subscription_manager.hpp @@ -19,12 +19,13 @@ #pragma once +#include #include #include #include "pj_bridge/subscription_manager_interface.hpp" +#include "pj_bridge/transform_set.hpp" #include "pj_bridge_ros2/generic_subscription_manager.hpp" -#include "pj_bridge_ros2/message_stripper.hpp" namespace pj_bridge { @@ -32,13 +33,13 @@ namespace pj_bridge { * @brief ROS2 implementation of SubscriptionManagerInterface * * Wraps GenericSubscriptionManager to provide backend-agnostic subscription - * management. Handles conversion from rclcpp::SerializedMessage to - * shared_ptr> and optional message stripping. + * management. Converts rclcpp::SerializedMessage to shared_ptr> + * and applies the topic's transform, if any, on the rcl buffer before copying. */ class Ros2SubscriptionManager : public SubscriptionManagerInterface { public: explicit Ros2SubscriptionManager( - rclcpp::Node::SharedPtr node, bool strip_large_messages = false, size_t min_qos_depth = 1, + rclcpp::Node::SharedPtr node, std::shared_ptr transforms = nullptr, size_t min_qos_depth = 1, size_t max_qos_depth = 100); Ros2SubscriptionManager(const Ros2SubscriptionManager&) = delete; @@ -54,7 +55,7 @@ class Ros2SubscriptionManager : public SubscriptionManagerInterface { private: GenericSubscriptionManager inner_manager_; - bool strip_large_messages_; + std::shared_ptr transforms_; std::mutex callback_mutex_; MessageCallback callback_; diff --git a/ros2/include/pj_bridge_ros2/strip_transform.hpp b/ros2/include/pj_bridge_ros2/strip_transform.hpp new file mode 100644 index 0000000..d4a042d --- /dev/null +++ b/ros2/include/pj_bridge_ros2/strip_transform.hpp @@ -0,0 +1,34 @@ +/* + * Copyright (C) 2026 Davide Faconti + * + * This file is part of pj_bridge. + * + * pj_bridge is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * pj_bridge is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with pj_bridge. If not, see . + */ + +#pragma once + +#include "pj_bridge/transform_set.hpp" + +namespace pj_bridge { + +/// `strip` transform: MessageStripper behind the MessageTransform interface. +/// Output type and schema equal the input's. Takes no params. +TransformFactory make_strip_transform_factory(); + +/// What `strip_large_messages: true` means: one `strip` rule per strippable +/// type, appended after whatever rules `transforms` already holds. +tl::expected append_strip_rules(TransformSet& transforms); + +} // namespace pj_bridge diff --git a/ros2/src/main.cpp b/ros2/src/main.cpp index 1de482e..0f04d0d 100644 --- a/ros2/src/main.cpp +++ b/ros2/src/main.cpp @@ -20,18 +20,66 @@ #include #include +#include #include #include #include +#include #include #include #include #include "pj_bridge/bridge_server.hpp" #include "pj_bridge/middleware/websocket_middleware.hpp" +#ifdef PJ_BRIDGE_HAS_CLOUDINI +#include "pj_bridge/cloudini_transform.hpp" +#endif +#include "pj_bridge/transform_set.hpp" +#include "pj_bridge/transforming_topic_source.hpp" #include "pj_bridge/whitelist_filter.hpp" #include "pj_bridge_ros2/ros2_subscription_manager.hpp" #include "pj_bridge_ros2/ros2_topic_source.hpp" +#include "pj_bridge_ros2/strip_transform.hpp" + +namespace { + +// The rules of `profile_path` followed by the strip_large_messages sugar, or +// nullptr when neither is set. Throws on any profile error: main() exits 1. +std::shared_ptr load_transforms(const std::string& profile_path, bool strip_large_messages) { + if (profile_path.empty() && !strip_large_messages) { + return nullptr; + } + nlohmann::json profile = {{"transforms", nlohmann::json::array()}}; + if (!profile_path.empty()) { + std::ifstream file(profile_path); + if (!file) { + throw std::runtime_error("Cannot open transform_profile '" + profile_path + "'"); + } + profile = nlohmann::json::parse(file, nullptr, /*allow_exceptions=*/false); + if (profile.is_discarded()) { + throw std::runtime_error("transform_profile '" + profile_path + "' is not valid JSON"); + } + } + + pj_bridge::TransformSet::FactoryMap factories; + factories["strip"] = pj_bridge::make_strip_transform_factory(); +#ifdef PJ_BRIDGE_HAS_CLOUDINI + factories["cloudini"] = pj_bridge::make_cloudini_transform_factory(); +#endif + + auto transforms = pj_bridge::TransformSet::create(profile, std::move(factories)); + if (!transforms) { + throw std::runtime_error("transform_profile '" + profile_path + "': " + transforms.error()); + } + if (strip_large_messages) { // after the profile's rules, so an explicit rule wins + if (auto added = pj_bridge::append_strip_rules(**transforms); !added) { + throw std::runtime_error(added.error()); + } + } + return *transforms; +} + +} // namespace int main(int argc, char** argv) { rclcpp::init(argc, argv); @@ -45,6 +93,7 @@ int main(int argc, char** argv) { node->declare_parameter("publish_rate", 50.0); node->declare_parameter("session_timeout", 10.0); node->declare_parameter("strip_large_messages", false); + node->declare_parameter("transform_profile", ""); node->declare_parameter>("topic_whitelist", {".*"}); node->declare_parameter("min_qos_depth", 10); node->declare_parameter("ingest_poll_interval_ms", 5.0); @@ -60,6 +109,7 @@ int main(int argc, char** argv) { double publish_rate = node->get_parameter("publish_rate").as_double(); double session_timeout = node->get_parameter("session_timeout").as_double(); bool strip_large_messages = node->get_parameter("strip_large_messages").as_bool(); + std::string transform_profile = node->get_parameter("transform_profile").as_string(); std::vector topic_whitelist = node->get_parameter("topic_whitelist").as_string_array(); int64_t min_qos_depth = node->get_parameter("min_qos_depth").as_int(); double ingest_poll_interval_ms = node->get_parameter("ingest_poll_interval_ms").as_double(); @@ -74,10 +124,11 @@ int main(int argc, char** argv) { RCLCPP_INFO( node->get_logger(), "Configuration: port=%d, publish_rate=%.1f Hz, session_timeout=%.1f s, strip_large_messages=%s, " - "min_qos_depth=%ld, max_qos_depth=%ld, ingest_poll_interval_ms=%.1f, topic_poll_interval=%.1f s, " - "client_backlog_size=%ld, tls=%s", - port, publish_rate, session_timeout, strip_large_messages ? "true" : "false", min_qos_depth, max_qos_depth, - ingest_poll_interval_ms, topic_poll_interval, client_backlog_size, tls_enabled ? "true" : "false"); + "transform_profile='%s', min_qos_depth=%ld, max_qos_depth=%ld, ingest_poll_interval_ms=%.1f, " + "topic_poll_interval=%.1f s, client_backlog_size=%ld, tls=%s", + port, publish_rate, session_timeout, strip_large_messages ? "true" : "false", transform_profile.c_str(), + min_qos_depth, max_qos_depth, ingest_poll_interval_ms, topic_poll_interval, client_backlog_size, + tls_enabled ? "true" : "false"); if (tls_enabled && (certfile.empty() || keyfile.empty())) { RCLCPP_ERROR(node->get_logger(), "tls=true requires both 'certfile' and 'keyfile' parameters to be set"); @@ -143,9 +194,15 @@ int main(int argc, char** argv) { try { // Create backend components - auto topic_source = std::make_shared(node); + std::shared_ptr topic_source = std::make_shared(node); + + auto transforms = load_transforms(transform_profile, strip_large_messages); + if (transforms) { + topic_source = std::make_shared(topic_source, transforms); + } + auto sub_manager = std::make_shared( - node, strip_large_messages, static_cast(min_qos_depth), static_cast(max_qos_depth)); + node, transforms, static_cast(min_qos_depth), static_cast(max_qos_depth)); std::optional tls_config; if (tls_enabled) { tls_config = pj_bridge::TlsConfig{certfile, keyfile}; @@ -269,6 +326,13 @@ int main(int argc, char** argv) { "%lu heavy frames shed (congestion)", total_messages, total_bytes, middleware->dropped_frame_count(), middleware->heavy_shed_count()); + if (transforms) { + const auto summary = transforms->stats_summary(); + if (!summary.empty()) { + RCLCPP_INFO(node->get_logger(), "Transform statistics:%s", summary.c_str()); + } + } + RCLCPP_INFO(node->get_logger(), "Bridge server shutdown complete"); } catch (const std::exception& e) { diff --git a/ros2/src/message_stripper.cpp b/ros2/src/message_stripper.cpp index 09af4cb..61c0cf9 100644 --- a/ros2/src/message_stripper.cpp +++ b/ros2/src/message_stripper.cpp @@ -54,6 +54,10 @@ bool MessageStripper::should_strip(const std::string& message_type) { return kStrippableTypes.find(message_type) != kStrippableTypes.end(); } +const std::unordered_set& MessageStripper::strippable_types() { + return kStrippableTypes; +} + rclcpp::SerializedMessage MessageStripper::strip( const std::string& message_type, const rclcpp::SerializedMessage& input) { if (message_type == "sensor_msgs/msg/Image") { diff --git a/ros2/src/ros2_subscription_manager.cpp b/ros2/src/ros2_subscription_manager.cpp index 3f806b9..4b3f710 100644 --- a/ros2/src/ros2_subscription_manager.cpp +++ b/ros2/src/ros2_subscription_manager.cpp @@ -22,13 +22,12 @@ #include #include -#include namespace pj_bridge { Ros2SubscriptionManager::Ros2SubscriptionManager( - rclcpp::Node::SharedPtr node, bool strip_large_messages, size_t min_qos_depth, size_t max_qos_depth) - : inner_manager_(node, min_qos_depth, max_qos_depth), strip_large_messages_(strip_large_messages) {} + rclcpp::Node::SharedPtr node, std::shared_ptr transforms, size_t min_qos_depth, size_t max_qos_depth) + : inner_manager_(node, min_qos_depth, max_qos_depth), transforms_(std::move(transforms)) {} void Ros2SubscriptionManager::set_message_callback(MessageCallback callback) { std::lock_guard lock(callback_mutex_); @@ -36,27 +35,26 @@ void Ros2SubscriptionManager::set_message_callback(MessageCallback callback) { } bool Ros2SubscriptionManager::subscribe(const std::string& topic_name, const std::string& topic_type) { - bool needs_stripping = strip_large_messages_ && MessageStripper::should_strip(topic_type); + // A transformed topic is advertised with its output type; subscribe with the real one. + std::shared_ptr bound = transforms_ ? transforms_->find(topic_name) : nullptr; + const std::string& source_type = bound ? bound->source_type : topic_type; Ros2MessageCallback ros2_callback = - [this, topic_type, needs_stripping]( + [this, bound]( const std::string& topic, const std::shared_ptr& msg, uint64_t receive_time_ns) { - const rclcpp::SerializedMessage* msg_to_use = msg.get(); - std::optional stripped_msg; - - if (needs_stripping) { - try { - stripped_msg.emplace(MessageStripper::strip(topic_type, *msg)); - msg_to_use = &*stripped_msg; - } catch (const std::exception& e) { - spdlog::warn("Failed to strip message on topic '{}': {}. Forwarding original.", topic, e.what()); + const auto& rcl_msg = msg->get_rcl_serialized_message(); + const auto* bytes = reinterpret_cast(rcl_msg.buffer); + + std::shared_ptr> data; + if (bound) { + data = std::make_shared>(); + if (!bound->run(topic, {bytes, rcl_msg.buffer_length}, *data)) { + return; // dropped and counted; never forward the untransformed bytes } + } else { + data = std::make_shared>(bytes, bytes + rcl_msg.buffer_length); } - const auto& rcl_msg = msg_to_use->get_rcl_serialized_message(); - const auto* bytes = reinterpret_cast(rcl_msg.buffer); - auto data = std::make_shared>(bytes, bytes + rcl_msg.buffer_length); - MessageCallback cb; { std::lock_guard lock(callback_mutex_); @@ -67,7 +65,7 @@ bool Ros2SubscriptionManager::subscribe(const std::string& topic_name, const std } }; - return inner_manager_.subscribe(topic_name, topic_type, ros2_callback); + return inner_manager_.subscribe(topic_name, source_type, ros2_callback); } bool Ros2SubscriptionManager::unsubscribe(const std::string& topic_name) { diff --git a/ros2/src/strip_transform.cpp b/ros2/src/strip_transform.cpp new file mode 100644 index 0000000..36df419 --- /dev/null +++ b/ros2/src/strip_transform.cpp @@ -0,0 +1,82 @@ +/* + * Copyright (C) 2026 Davide Faconti + * + * This file is part of pj_bridge. + * + * pj_bridge is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * pj_bridge is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with pj_bridge. If not, see . + */ + +#include "pj_bridge_ros2/strip_transform.hpp" + +#include + +#include "pj_bridge_ros2/message_stripper.hpp" + +namespace pj_bridge { +namespace { + +class StripTransform : public MessageTransform { + public: + explicit StripTransform(std::string type) : type_(std::move(type)) {} + + tl::expected apply(std::span in, std::vector& out) override { + if (in.empty()) { + return tl::make_unexpected(std::string("empty message")); + } + try { + // MessageStripper needs a SerializedMessage: one copy of the input. + rclcpp::SerializedMessage input(in.size()); + auto& rcl_in = input.get_rcl_serialized_message(); + std::memcpy(rcl_in.buffer, in.data(), in.size()); + rcl_in.buffer_length = in.size(); + + const auto stripped = MessageStripper::strip(type_, input); + const auto& rcl_out = stripped.get_rcl_serialized_message(); + const auto* bytes = reinterpret_cast(rcl_out.buffer); + out.assign(bytes, bytes + rcl_out.buffer_length); + return {}; + } catch (const std::exception& e) { + return tl::make_unexpected(std::string(e.what())); + } + } + + private: + std::string type_; +}; + +} // namespace + +TransformFactory make_strip_transform_factory() { + TransformFactory f; + f.accepts = [](const std::string& type) { return MessageStripper::should_strip(type); }; + f.check_params = [](const nlohmann::json& params) -> tl::expected { + if (!params.empty()) { + return tl::make_unexpected(std::string("takes no params")); + } + return {}; + }; + f.create = [](const std::string& type, const nlohmann::json&) { return std::make_unique(type); }; + return f; +} + +tl::expected append_strip_rules(TransformSet& transforms) { + for (const auto& type : MessageStripper::strippable_types()) { + if (auto added = transforms.append_type_rule(type, "strip"); !added) { + return added; + } + } + return {}; +} + +} // namespace pj_bridge diff --git a/tests/unit/fake_transform.hpp b/tests/unit/fake_transform.hpp new file mode 100644 index 0000000..6a117fc --- /dev/null +++ b/tests/unit/fake_transform.hpp @@ -0,0 +1,80 @@ +/* + * Copyright (C) 2026 Davide Faconti + * + * This file is part of pj_bridge. + * + * pj_bridge is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * pj_bridge is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with pj_bridge. If not, see . + */ + +#pragma once + +#include +#include + +#include "pj_bridge/transform_set.hpp" + +namespace pj_bridge::test_helpers { + +/// Appends one marker byte (0xAB) to the input; advertises `output_type` and +/// "OUT:" + schema. With param {"fail": true}, apply() returns an error. +class FakeTransform : public MessageTransform { + public: + FakeTransform(bool fail, std::string output_type) : fail_(fail), output_type_(std::move(output_type)) {} + + tl::expected apply(std::span in, std::vector& out) override { + if (fail_) { + return tl::make_unexpected(std::string("boom")); + } + out.assign(in.begin(), in.end()); + out.push_back(std::byte{0xAB}); + return {}; + } + + std::string output_type(const std::string&) const override { + return output_type_; + } + + std::string output_schema(const std::string& source_schema) const override { + return "OUT:" + source_schema; + } + + private: + bool fail_; + std::string output_type_; +}; + +/// Factory accepting exactly `accepted_type`; its only param is "fail". +inline TransformFactory fake_factory(const std::string& accepted_type, const std::string& output_type = "pkg/msg/Out") { + TransformFactory f; + f.accepts = [accepted_type](const std::string& t) { return t == accepted_type; }; + f.check_params = [](const nlohmann::json& p) -> tl::expected { + for (auto it = p.begin(); it != p.end(); ++it) { + if (it.key() != "fail") { + return tl::make_unexpected("unknown param '" + it.key() + "'"); + } + } + return {}; + }; + f.create = [output_type](const std::string&, const nlohmann::json& p) { + return std::make_unique(p.value("fail", false), output_type); + }; + return f; +} + +/// Profile holding a single rule. +inline nlohmann::json rule(nlohmann::json fields) { + return nlohmann::json{{"transforms", nlohmann::json::array({std::move(fields)})}}; +} + +} // namespace pj_bridge::test_helpers diff --git a/tests/unit/test_bridge_server.cpp b/tests/unit/test_bridge_server.cpp index 7d0b62e..13d6582 100644 --- a/tests/unit/test_bridge_server.cpp +++ b/tests/unit/test_bridge_server.cpp @@ -2754,6 +2754,34 @@ TEST_F(BridgeServerTest, GetTopicsMarksLatchedTopics) { } } +// GetTopicsCarriesSourceTypeOnlyForTransformedTopics +// +// A topic whose TopicInfo::source_type is set (i.e. it was rewritten by a +// TransformingTopicSource) carries `source_type` in its get_topics entry; +// an untransformed topic (empty source_type) carries no such key. +TEST_F(BridgeServerTest, GetTopicsCarriesSourceTypeOnlyForTransformedTopics) { + ASSERT_TRUE(server_->initialize()); + mock_topic_source_->set_topics( + {{"/cloud", "point_cloud_interfaces/msg/CompressedPointCloud2", "sensor_msgs/msg/PointCloud2"}, + {"/imu", "sensor_msgs/msg/Imu"}}); + + json req; + req["command"] = "get_topics"; + mock_->push_request("client_source_type", req.dump()); + server_->process_requests(); + + auto replies = mock_->get_replies("client_source_type"); + ASSERT_EQ(replies.size(), 1u); + for (const auto& entry : replies[0]["topics"]) { + if (entry["name"] == "/cloud") { + ASSERT_TRUE(entry.contains("source_type")); + EXPECT_EQ(entry["source_type"], "sensor_msgs/msg/PointCloud2"); + } else { + EXPECT_FALSE(entry.contains("source_type")) << entry.dump(); + } + } +} + // TopicsChangedAddedEntriesCarryLatched // // A late-appearing transient-local topic is badged in the topics_changed push diff --git a/tests/unit/test_cloudini_transform.cpp b/tests/unit/test_cloudini_transform.cpp new file mode 100644 index 0000000..5db1ad1 --- /dev/null +++ b/tests/unit/test_cloudini_transform.cpp @@ -0,0 +1,103 @@ +/* + * Copyright (C) 2026 Davide Faconti + * + * This file is part of pj_bridge. + * + * pj_bridge is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * pj_bridge is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with pj_bridge. If not, see . + */ + +#ifdef PJ_BRIDGE_HAS_CLOUDINI + +#include + +#include +#include +#include +#include +#include + +#include "pj_bridge/cloudini_transform.hpp" + +using namespace pj_bridge; + +namespace { +std::vector make_cloud_cdr( + size_t points, const std::function& corrupt = nullptr) { + sensor_msgs::msg::PointCloud2 cloud; + cloud.header.frame_id = "lidar"; + sensor_msgs::PointCloud2Modifier modifier(cloud); + modifier.setPointCloud2FieldsByString(1, "xyz"); + modifier.resize(points); + sensor_msgs::PointCloud2Iterator x(cloud, "x"), y(cloud, "y"), z(cloud, "z"); + for (size_t i = 0; i < points; ++i, ++x, ++y, ++z) { + *x = 0.01f * static_cast(i); + *y = 1.0f; + *z = -0.5f; + } + if (corrupt) { + corrupt(cloud); + } + rclcpp::SerializedMessage serialized; + rclcpp::Serialization().serialize_message(&cloud, &serialized); + const auto& rcl = serialized.get_rcl_serialized_message(); + const auto* bytes = reinterpret_cast(rcl.buffer); + return {bytes, bytes + rcl.buffer_length}; +} +} // namespace + +TEST(CloudiniTransformTest, CompressesPointCloud2) { + auto factory = make_cloudini_transform_factory(); + EXPECT_TRUE(factory.accepts("sensor_msgs/msg/PointCloud2")); + EXPECT_FALSE(factory.accepts("sensor_msgs/msg/Image")); + const nlohmann::json params = {{"resolution", 0.001}}; + ASSERT_TRUE(factory.check_params(params).has_value()); + EXPECT_FALSE(factory.check_params({{"resolutoin", 0.001}}).has_value()); + EXPECT_FALSE(factory.check_params({{"resolution", -1.0}}).has_value()); + + auto transform = factory.create("sensor_msgs/msg/PointCloud2", params); + EXPECT_EQ(transform->output_type("sensor_msgs/msg/PointCloud2"), "point_cloud_interfaces/msg/CompressedPointCloud2"); + EXPECT_NE(transform->output_schema("").find("format"), std::string::npos); + const auto in = make_cloud_cdr(10000); + std::vector out; + ASSERT_TRUE(transform->apply(in, out).has_value()); + EXPECT_FALSE(out.empty()); + EXPECT_LT(out.size(), in.size()); + + ASSERT_TRUE(transform->apply(in, out).has_value()); // instance is reusable +} + +TEST(CloudiniTransformTest, GarbageInputIsAnErrorNotACrash) { + auto transform = make_cloudini_transform_factory().create("sensor_msgs/msg/PointCloud2", nlohmann::json::object()); + const std::vector in(7, std::byte{0xFF}); + std::vector out; + EXPECT_FALSE(transform->apply(in, out).has_value()); +} + +// Any DDS peer can publish a PointCloud2 whose metadata lies about its payload. +// The encoder trusts field offsets and the header repeats width/height, so these +// must be rejected here: a drop, not an over-read or an undecodable frame. +TEST(CloudiniTransformTest, InconsistentCloudMetadataIsRejected) { + auto transform = make_cloudini_transform_factory().create("sensor_msgs/msg/PointCloud2", nlohmann::json::object()); + std::vector out; + + const auto field_past_point = make_cloud_cdr(100, [](auto& c) { c.fields[2].offset = c.point_step; }); + EXPECT_FALSE(transform->apply(field_past_point, out).has_value()); + + const auto width_lies = make_cloud_cdr(100, [](auto& c) { c.width = 1000; }); + EXPECT_FALSE(transform->apply(width_lies, out).has_value()); + + EXPECT_TRUE(transform->apply(make_cloud_cdr(100), out).has_value()); // and a sane one still works +} + +#endif // PJ_BRIDGE_HAS_CLOUDINI diff --git a/tests/unit/test_ros2_subscription_manager.cpp b/tests/unit/test_ros2_subscription_manager.cpp index b21fb53..3065c03 100644 --- a/tests/unit/test_ros2_subscription_manager.cpp +++ b/tests/unit/test_ros2_subscription_manager.cpp @@ -21,23 +21,53 @@ #include #include +#include #include #include +#include #include +#include "fake_transform.hpp" +#include "pj_bridge/transform_set.hpp" #include "pj_bridge_ros2/ros2_subscription_manager.hpp" +#include "pj_bridge_ros2/strip_transform.hpp" using namespace pj_bridge; namespace { +// A TransformSet with the `strip` rule appended for every strippable type, +// as strip_large_messages=true wires it up in main.cpp. +std::shared_ptr strip_everything() { + auto set = TransformSet::create( + nlohmann::json{{"transforms", nlohmann::json::array()}}, {{"strip", make_strip_transform_factory()}}) + .value(); + EXPECT_TRUE(append_strip_rules(*set).has_value()); + return set; +} + +// Publish `msg` and spin until `done()` or 5 s have passed. +template +void publish_until( + const rclcpp::Node::SharedPtr& node, const typename rclcpp::Publisher::SharedPtr& publisher, const MsgT& msg, + const std::function& done) { + rclcpp::executors::SingleThreadedExecutor executor; + executor.add_node(node); + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + while (!done() && std::chrono::steady_clock::now() < deadline) { + publisher->publish(msg); + executor.spin_some(); + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } +} + // Publish an Image with a large data payload through the manager and return // the size of the serialized message delivered to the bridge callback. -size_t roundtrip_image_bytes(bool strip_large_messages, bool use_default_config) { - auto node = std::make_shared( - use_default_config ? "test_strip_default" : (strip_large_messages ? "test_strip_on" : "test_strip_off")); +// `transforms` is nullptr for "no transform configured". +size_t roundtrip_image_bytes(std::shared_ptr transforms, bool use_default_config) { + auto node = std::make_shared(use_default_config ? "test_strip_default" : "test_strip_on"); auto manager = use_default_config ? std::make_shared(node) - : std::make_shared(node, strip_large_messages); + : std::make_shared(node, transforms); std::atomic received_size{0}; manager->set_message_callback( @@ -46,6 +76,10 @@ size_t roundtrip_image_bytes(bool strip_large_messages, bool use_default_config) }); const std::string topic = "/strip_test_image_" + std::string(node->get_name()); + if (transforms) { + // ASSERT_* requires a void-returning function; this helper returns size_t. + EXPECT_NE(transforms->bind(topic, "sensor_msgs/msg/Image"), nullptr); // what get_topics would have done + } auto publisher = node->create_publisher(topic, rclcpp::QoS(10)); EXPECT_TRUE(manager->subscribe(topic, "sensor_msgs/msg/Image")); @@ -56,15 +90,7 @@ size_t roundtrip_image_bytes(bool strip_large_messages, bool use_default_config) img.encoding = "rgb8"; img.data.assign(static_cast(img.height) * img.step, 0xAB); - rclcpp::executors::SingleThreadedExecutor executor; - executor.add_node(node); - - auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); - while (received_size.load() == 0 && std::chrono::steady_clock::now() < deadline) { - publisher->publish(img); - executor.spin_some(); - std::this_thread::sleep_for(std::chrono::milliseconds(10)); - } + publish_until(node, publisher, img, [&] { return received_size.load() != 0; }); manager->unsubscribe_all(); return received_size.load(); @@ -86,20 +112,83 @@ class Ros2SubscriptionManagerTest : public ::testing::Test { }; // --------------------------------------------------------------------------- -// Stripping is opt-in: by default, large data fields are forwarded intact. +// Stripping is opt-in: by default, no transform is configured and large data +// fields are forwarded intact. // --------------------------------------------------------------------------- TEST_F(Ros2SubscriptionManagerTest, DataFieldsIncludedByDefault) { - size_t received = roundtrip_image_bytes(false, /*use_default_config=*/true); + size_t received = roundtrip_image_bytes(/*transforms=*/nullptr, /*use_default_config=*/true); ASSERT_GT(received, 0u) << "no message received"; EXPECT_GE(received, kImagePayloadBytes) << "image data was stripped despite default (opt-in) configuration"; } // --------------------------------------------------------------------------- -// Opting in still strips: with strip_large_messages=true the payload is -// removed and only the metadata remains. +// Opting in still strips: with a `strip` transform bound to the topic, the +// payload is removed and only the metadata remains. // --------------------------------------------------------------------------- TEST_F(Ros2SubscriptionManagerTest, OptInStrippingRemovesData) { - size_t received = roundtrip_image_bytes(/*strip_large_messages=*/true, /*use_default_config=*/false); + size_t received = roundtrip_image_bytes(strip_everything(), /*use_default_config=*/false); ASSERT_GT(received, 0u) << "no message received"; EXPECT_LT(received, kImagePayloadBytes) << "opt-in stripping did not remove the data payload"; } + +// --------------------------------------------------------------------------- +// The hook subscribes with the topic's SOURCE type (not the advertised +// output type) and forwards whatever bytes the bound transform produces. +// --------------------------------------------------------------------------- +TEST_F(Ros2SubscriptionManagerTest, SubscribesWithSourceTypeAndForwardsTransformedBytes) { + // The output type differs from the source type: the manager is asked to + // subscribe with the ADVERTISED type and must use the source type. + auto set = TransformSet::create( + test_helpers::rule( + {{"match_type", "std_msgs/msg/String"}, {"match_topic", "/transform_me"}, {"transform", "fake"}}), + {{"fake", test_helpers::fake_factory("std_msgs/msg/String", "fake_msgs/msg/Out")}}) + .value(); + ASSERT_NE(set->bind("/transform_me", "std_msgs/msg/String"), nullptr); // what get_topics would have done + + auto node = std::make_shared("test_transform_hook"); + Ros2SubscriptionManager manager(node, set); + std::vector received; + manager.set_message_callback( + [&](const std::string&, std::shared_ptr> data, uint64_t) { received = *data; }); + + // subscribe() is called with the ADVERTISED (output) type, as BridgeServer does. + ASSERT_TRUE(manager.subscribe("/transform_me", "fake_msgs/msg/Out")); + + auto publisher = node->create_publisher("/transform_me", rclcpp::QoS(10)); + std_msgs::msg::String msg; + msg.data = "hello"; + publish_until(node, publisher, msg, [&] { return !received.empty(); }); + + ASSERT_FALSE(received.empty()); + EXPECT_EQ(received.back(), std::byte{0xAB}); // FakeTransform's marker: the transform ran +} + +// The client was told the output type at subscribe time, so a sample whose +// transform fails must be dropped, never forwarded untransformed. +TEST_F(Ros2SubscriptionManagerTest, FailedTransformDropsTheSample) { + auto set = TransformSet::create( + test_helpers::rule( + {{"match_type", "std_msgs/msg/String"}, + {"match_topic", "/always_fails"}, + {"transform", "fake"}, + {"params", {{"fail", true}}}}), + {{"fake", test_helpers::fake_factory("std_msgs/msg/String", "std_msgs/msg/String")}}) + .value(); + auto bound = set->bind("/always_fails", "std_msgs/msg/String"); + ASSERT_NE(bound, nullptr); + + auto node = std::make_shared("test_transform_hook_failure"); + Ros2SubscriptionManager manager(node, set); + std::atomic forwarded{0}; + manager.set_message_callback( + [&](const std::string&, std::shared_ptr>, uint64_t) { forwarded++; }); + ASSERT_TRUE(manager.subscribe("/always_fails", "std_msgs/msg/String")); + + auto publisher = node->create_publisher("/always_fails", rclcpp::QoS(10)); + std_msgs::msg::String msg; + msg.data = "hello"; + publish_until(node, publisher, msg, [&] { return bound->drops.load() >= 3; }); + + EXPECT_GE(bound->drops.load(), 3u); + EXPECT_EQ(forwarded.load(), 0); +} diff --git a/tests/unit/test_transform_set.cpp b/tests/unit/test_transform_set.cpp new file mode 100644 index 0000000..1d259a5 --- /dev/null +++ b/tests/unit/test_transform_set.cpp @@ -0,0 +1,266 @@ +#include +#include +#include + +#include +#include +#include +#include + +#include "fake_transform.hpp" +#include "pj_bridge/transform_set.hpp" + +using namespace pj_bridge; +using namespace pj_bridge::test_helpers; +using nlohmann::json; + +namespace { + +TransformSet::FactoryMap factories() { + return {{"fake", fake_factory("pkg/msg/In")}, {"other", fake_factory("pkg/msg/Other")}}; +} + +} // namespace + +TEST(TransformSetTest, MatchesByType) { + auto set = TransformSet::create(rule({{"match_type", "pkg/msg/In"}, {"transform", "fake"}}), factories()); + ASSERT_TRUE(set.has_value()) << set.error(); + auto bound = (*set)->bind("/a", "pkg/msg/In"); + ASSERT_NE(bound, nullptr); + EXPECT_EQ(bound->source_type, "pkg/msg/In"); + EXPECT_EQ(bound->output_type, "pkg/msg/Out"); + EXPECT_EQ((*set)->bind("/b", "pkg/msg/Unrelated"), nullptr); + EXPECT_EQ((*set)->find("/a"), bound); + EXPECT_EQ((*set)->bind("/a", "pkg/msg/In"), bound); // cached: counters and codec state survive topic polls + EXPECT_EQ((*set)->find("/b"), nullptr); +} + +TEST(TransformSetTest, TopicRegexIsFullMatchAndCombinesWithType) { + auto set = TransformSet::create( + rule({{"match_type", "pkg/msg/In"}, {"match_topic", "/lidar/.*"}, {"transform", "fake"}}), factories()); + ASSERT_TRUE(set.has_value()) << set.error(); + EXPECT_NE((*set)->bind("/lidar/front", "pkg/msg/In"), nullptr); + EXPECT_EQ((*set)->bind("/x/lidar/front", "pkg/msg/In"), nullptr); // full match, not search + EXPECT_EQ((*set)->bind("/lidar/rear", "pkg/msg/Unrelated"), nullptr); +} + +TEST(TransformSetTest, FirstMatchingRuleWinsAndAppendedRulesComeLast) { + json profile = { + {"transforms", + json::array( + {{{"match_type", "pkg/msg/In"}, {"match_topic", "/a"}, {"transform", "fake"}, {"params", {{"fail", true}}}}, + {{"match_type", "pkg/msg/In"}, {"transform", "fake"}}})}}; + auto set = TransformSet::create(profile, factories()); + ASSERT_TRUE(set.has_value()) << set.error(); + ASSERT_TRUE((*set)->append_type_rule("pkg/msg/Other", "other").has_value()); + + std::vector in{std::byte{1}}, out; + EXPECT_FALSE((*set)->bind("/a", "pkg/msg/In")->run("/a", in, out)); // rule 0 (fail=true) + EXPECT_TRUE((*set)->bind("/b", "pkg/msg/In")->run("/b", in, out)); // rule 1 + EXPECT_EQ((*set)->bind("/c", "pkg/msg/Other")->transform_name, "other"); +} + +// A transform can only be paired with a type it accepts, and that is settled at +// startup: a topic of another type is simply not matched by the rule. +TEST(TransformSetTest, RuleNeverAppliesToATopicOfAnotherType) { + auto set = TransformSet::create( + rule({{"match_type", "pkg/msg/In"}, {"match_topic", "/lidar/.*"}, {"transform", "fake"}}), factories()); + ASSERT_TRUE(set.has_value()) << set.error(); + ASSERT_TRUE((*set)->append_type_rule("pkg/msg/Other", "other").has_value()); + auto bound = (*set)->bind("/lidar/image", "pkg/msg/Other"); // falls through to the later rule + ASSERT_NE(bound, nullptr); + EXPECT_EQ(bound->transform_name, "other"); + EXPECT_EQ((*set)->bind("/lidar/unrelated", "pkg/msg/Unrelated"), nullptr); +} + +TEST(TransformSetTest, FactoryReturningNullOrThrowingLeavesTopicUntransformed) { + auto broken = fake_factory("pkg/msg/In"); + broken.create = [](const std::string&, const json& p) -> std::unique_ptr { + if (p.value("fail", false)) { + throw std::runtime_error("no codec"); + } + return nullptr; + }; + json profile = { + {"transforms", json::array( + {{{"match_type", "pkg/msg/In"}, + {"match_topic", "/throws"}, + {"transform", "broken"}, + {"params", {{"fail", true}}}}, + {{"match_type", "pkg/msg/In"}, {"transform", "broken"}}})}}; + auto set = TransformSet::create(profile, {{"broken", broken}}); + ASSERT_TRUE(set.has_value()) << set.error(); + EXPECT_EQ((*set)->bind("/throws", "pkg/msg/In"), nullptr); + EXPECT_EQ((*set)->bind("/null", "pkg/msg/In"), nullptr); + EXPECT_EQ((*set)->find("/null"), nullptr); +} + +TEST(TransformSetTest, ThrowingApplyIsADropNotACrash) { + struct Thrower : MessageTransform { + tl::expected apply(std::span, std::vector&) override { + throw std::length_error("corrupt size field"); + } + }; + auto f = fake_factory("pkg/msg/In"); + f.create = [](const std::string&, const json&) { return std::make_unique(); }; + auto set = TransformSet::create(rule({{"match_type", "pkg/msg/In"}, {"transform", "t"}}), {{"t", f}}); + ASSERT_TRUE(set.has_value()) << set.error(); + auto bound = (*set)->bind("/a", "pkg/msg/In"); + std::vector in(4), out; + EXPECT_FALSE(bound->run("/a", in, out)); + EXPECT_EQ(bound->drops.load(), 1u); +} + +TEST(TransformSetTest, StatsSummaryListsOnlyTopicsThatProcessedSamples) { + auto set = TransformSet::create(rule({{"match_type", "pkg/msg/In"}, {"transform", "fake"}}), factories()); + ASSERT_TRUE(set.has_value()) << set.error(); + std::vector in(4), out; + (*set)->bind("/busy", "pkg/msg/In")->run("/busy", in, out); + (*set)->bind("/idle", "pkg/msg/In"); + const auto summary = (*set)->stats_summary(); + EXPECT_NE(summary.find("/busy"), std::string::npos); + EXPECT_EQ(summary.find("/idle"), std::string::npos); +} + +// Gives TSAN the real threading model: run() on one thread, everything else on another. +TEST(TransformSetTest, RunIsSafeConcurrentlyWithBindFindAndStats) { + auto set = TransformSet::create(rule({{"match_type", "pkg/msg/In"}, {"transform", "fake"}}), factories()); + ASSERT_TRUE(set.has_value()) << set.error(); + auto bound = (*set)->bind("/a", "pkg/msg/In"); + std::atomic stop{false}; + std::thread ingest([&] { + std::vector in(64), out; + while (!stop) { + bound->run("/a", in, out); + } + }); + // Keep going until the ingest thread has really overlapped with us: on a busy + // machine 2000 iterations can finish before it is even scheduled. + for (int i = 0; i < 2000 || bound->samples.load() < 100; ++i) { + (*set)->bind("/a", "pkg/msg/In"); + (*set)->bind("/other" + std::to_string(i % 8), "pkg/msg/In"); + (*set)->find("/a"); + (*set)->stats_summary(); + } + stop = true; + ingest.join(); + EXPECT_GT(bound->samples.load(), 0u); +} + +// bind() runs for every topic on every topic poll: a transform that cannot be +// set up must be reported once, not once per second. +TEST(TransformSetTest, SetupErrorIsLoggedOncePerTopic) { + std::ostringstream captured; + // Restored on every exit path: the sink points at `captured`, which dies with this test. + struct RestoreLogger { + std::shared_ptr previous = spdlog::default_logger(); + ~RestoreLogger() { + spdlog::set_default_logger(previous); + } + } restore_logger; + spdlog::set_default_logger( + std::make_shared("capture", std::make_shared(captured))); + + auto broken = fake_factory("pkg/msg/In"); + broken.create = [](const std::string&, const json&) -> std::unique_ptr { + throw std::runtime_error("no codec"); + }; + auto set = TransformSet::create(rule({{"match_type", "pkg/msg/In"}, {"transform", "broken"}}), {{"broken", broken}}); + ASSERT_TRUE(set.has_value()) << set.error(); + + EXPECT_EQ((*set)->bind("/a", "pkg/msg/In"), nullptr); + (*set)->bind("/a", "pkg/msg/In"); // second poll + (*set)->bind("/b", "pkg/msg/In"); // another topic is reported separately + + const std::string log = captured.str(); + size_t reports = 0; + for (size_t at = log.find("could not be set up"); at != std::string::npos; + at = log.find("could not be set up", at + 1)) { + ++reports; + } + EXPECT_EQ(reports, 2u) << log; + EXPECT_NE(log.find("'/a'"), std::string::npos) << log; + EXPECT_NE(log.find("'/b'"), std::string::npos) << log; +} + +TEST(TransformSetTest, RebindsWhenSourceTypeChanges) { + auto set = TransformSet::create(rule({{"match_type", "pkg/msg/In"}, {"transform", "fake"}}), factories()); + ASSERT_TRUE(set.has_value()) << set.error(); + EXPECT_NE((*set)->bind("/a", "pkg/msg/In"), nullptr); + EXPECT_EQ((*set)->bind("/a", "pkg/msg/Unrelated"), nullptr); + EXPECT_EQ((*set)->find("/a"), nullptr); +} + +TEST(TransformSetTest, RunCountsSamplesBytesAndDrops) { + json profile = { + {"transforms", json::array( + {{{"match_type", "pkg/msg/In"}, + {"match_topic", "/bad"}, + {"transform", "fake"}, + {"params", {{"fail", true}}}}, + {{"match_type", "pkg/msg/In"}, {"transform", "fake"}}})}}; + auto set = TransformSet::create(profile, factories()); + ASSERT_TRUE(set.has_value()) << set.error(); + std::vector in(10, std::byte{7}), out; + + auto good = (*set)->bind("/good", "pkg/msg/In"); + ASSERT_TRUE(good->run("/good", in, out)); + EXPECT_EQ(out.size(), 11u); + EXPECT_EQ(good->samples.load(), 1u); + EXPECT_EQ(good->in_bytes.load(), 10u); + EXPECT_EQ(good->out_bytes.load(), 11u); + EXPECT_EQ(good->drops.load(), 0u); + + auto bad = (*set)->bind("/bad", "pkg/msg/In"); + EXPECT_FALSE(bad->run("/bad", in, out)); + EXPECT_EQ(bad->drops.load(), 1u); + EXPECT_EQ(bad->samples.load(), 0u); +} + +struct BadProfile { + const char* name; + json profile; + const char* expected_in_error; // the operator must be told WHY, not just "no" +}; + +class TransformSetBadProfileTest : public ::testing::TestWithParam {}; + +TEST_P(TransformSetBadProfileTest, IsRejectedAtStartupWithAClearReason) { + auto set = TransformSet::create(GetParam().profile, factories()); + ASSERT_FALSE(set.has_value()); + EXPECT_NE(set.error().find(GetParam().expected_in_error), std::string::npos) << set.error(); +} + +INSTANTIATE_TEST_SUITE_P( + All, TransformSetBadProfileTest, + ::testing::Values( + BadProfile{"not_an_object", json::array(), "'transforms' array"}, + BadProfile{"missing_transforms", json::object(), "'transforms' array"}, + BadProfile{"unknown_top_level_key", json{{"transforms", json::array()}, {"extra", 1}}, "unknown key 'extra'"}, + BadProfile{"transforms_not_an_array", json{{"transforms", 5}}, "'transforms' array"}, + BadProfile{"rule_not_an_object", json{{"transforms", json::array({5})}}, "rule 0: must be an object"}, + BadProfile{ + "mistyped_rule_key", rule({{"match_type", 5}, {"transform", "fake"}}), + "key 'match_type' is unknown or has the wrong JSON type"}, + BadProfile{"empty_match_type", rule({{"match_type", ""}, {"transform", "fake"}}), "non-empty 'match_type'"}, + BadProfile{ + "topic_only_rule", rule({{"match_topic", "/lidar/.*"}, {"transform", "fake"}}), "non-empty 'match_type'"}, + BadProfile{"rule_without_matcher", rule({{"transform", "fake"}}), "non-empty 'match_type'"}, + BadProfile{"rule_without_transform", rule({{"match_type", "pkg/msg/In"}}), "missing 'transform'"}, + BadProfile{ + "unknown_rule_key", rule({{"match_type", "pkg/msg/In"}, {"transform", "fake"}, {"oops", 1}}), + "key 'oops' is unknown"}, + BadProfile{ + "unknown_transform", rule({{"match_type", "pkg/msg/In"}, {"transform", "nope"}}), + "unknown transform 'nope'"}, + BadProfile{ + "invalid_regex", rule({{"match_type", "pkg/msg/In"}, {"match_topic", "("}, {"transform", "fake"}}), + "invalid match_topic regex"}, + BadProfile{ + "unknown_param", rule({{"match_type", "pkg/msg/In"}, {"transform", "fake"}, {"params", {{"q", 1}}}}), + "unknown param 'q'"}, + // the short / ROS1 spelling is the likely real-world mistake: the message must show the expected form + BadProfile{ + "type_not_accepted", rule({{"match_type", "pkg/In"}, {"transform", "fake"}}), + "does not accept type 'pkg/In' (use the full type name, e.g. 'sensor_msgs/msg/PointCloud2')"}), + [](const auto& info) { return std::string(info.param.name); }); diff --git a/tests/unit/test_transforming_topic_source.cpp b/tests/unit/test_transforming_topic_source.cpp new file mode 100644 index 0000000..7fd8e90 --- /dev/null +++ b/tests/unit/test_transforming_topic_source.cpp @@ -0,0 +1,66 @@ +/* + * Copyright (C) 2026 Davide Faconti + * + * This file is part of pj_bridge. + * + * pj_bridge is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * pj_bridge is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with pj_bridge. If not, see . + */ + +#include + +#include "fake_transform.hpp" +#include "pj_bridge/transforming_topic_source.hpp" + +using namespace pj_bridge; + +namespace { +class StubSource : public TopicSourceInterface { + public: + std::vector get_topics() override { + return {{"/cloud", "pkg/msg/In", ""}, {"/odom", "pkg/msg/Odom", ""}}; + } + std::string get_schema(const std::string& topic) override { + return "schema-of-" + topic; + } + std::string schema_encoding() const override { + return "ros2msg"; + } + bool is_transient_local(const std::string& topic) const override { + return topic == "/cloud"; + } +}; + +std::shared_ptr make_set() { + return TransformSet::create( + test_helpers::rule({{"match_type", "pkg/msg/In"}, {"transform", "t"}}), + {{"t", test_helpers::fake_factory("pkg/msg/In")}}) + .value(); +} +} // namespace + +TEST(TransformingTopicSourceTest, RewritesTypeAndSchemaOnlyForMatchedTopics) { + TransformingTopicSource source(std::make_shared(), make_set()); + + auto topics = source.get_topics(); + ASSERT_EQ(topics.size(), 2u); + EXPECT_EQ(topics[0].type, "pkg/msg/Out"); + EXPECT_EQ(topics[0].source_type, "pkg/msg/In"); + EXPECT_EQ(topics[1].type, "pkg/msg/Odom"); + EXPECT_TRUE(topics[1].source_type.empty()); + + EXPECT_EQ(source.get_schema("/cloud"), "OUT:schema-of-/cloud"); + EXPECT_EQ(source.get_schema("/odom"), "schema-of-/odom"); + EXPECT_EQ(source.schema_encoding(), "ros2msg"); + EXPECT_TRUE(source.is_transient_local("/cloud")); +}