diff --git a/CLAUDE.md b/CLAUDE.md index c7285c9..b741737 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -197,7 +197,7 @@ Then, for each message in the (compressed) payload: ## Testing -### Test Count: 174 unit tests across 10 test suites +### Test Count: 176 unit tests across 11 test suites ### Commands ```bash @@ -232,7 +232,7 @@ pre-commit run -a port: 9090 # WebSocket port publish_rate: 50.0 # Hz session_timeout: 10.0 # seconds -strip_large_messages: true # Strip Image/PointCloud2/etc data fields +strip_large_messages: false # Opt-in: strip Image/PointCloud2/etc data fields ``` ### RTI (via CLI flags): @@ -263,5 +263,5 @@ pj_bridge_fastdds --domains 0 1 --port 9090 --publish-rate 50 --session-timeout **Last Updated**: 2026-07-02 **Project Phase**: Unified multi-backend architecture -**Test Status**: 174 unit tests passing (all sanitizers clean) +**Test Status**: 176 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 7a207ef..1fc0143 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -249,6 +249,7 @@ if(BUILD_TESTING AND ament_cmake_FOUND) 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 ) diff --git a/README.md b/README.md index f7dfd17..752f8a6 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ independently. - **High Performance**: 50 Hz message aggregation with ZSTD compression. The original message timestamp is preserved, and less bandwidth is used. - **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**: Automatic stripping of large array fields (Image, PointCloud2, LaserScan, OccupancyGrid) to reduce bandwidth while preserving metadata +- **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 ## CI Status @@ -38,7 +38,7 @@ independently. | `port` | int | 9090 | WebSocket server port | | `publish_rate` | double | 50.0 | Aggregation publish rate in Hz | | `session_timeout` | double | 10.0 | Client timeout duration in seconds | -| `strip_large_messages` | bool | true | Strip large arrays from Image, PointCloud2, LaserScan, OccupancyGrid messages | +| `strip_large_messages` | bool | false | Opt-in: strip large arrays from Image, PointCloud2, LaserScan, OccupancyGrid messages | ## Just "Download and Run" diff --git a/ros2/include/pj_bridge_ros2/ros2_subscription_manager.hpp b/ros2/include/pj_bridge_ros2/ros2_subscription_manager.hpp index 510cbcf..d3f6f95 100644 --- a/ros2/include/pj_bridge_ros2/ros2_subscription_manager.hpp +++ b/ros2/include/pj_bridge_ros2/ros2_subscription_manager.hpp @@ -37,7 +37,7 @@ namespace pj_bridge { */ class Ros2SubscriptionManager : public SubscriptionManagerInterface { public: - explicit Ros2SubscriptionManager(rclcpp::Node::SharedPtr node, bool strip_large_messages = true); + explicit Ros2SubscriptionManager(rclcpp::Node::SharedPtr node, bool strip_large_messages = false); Ros2SubscriptionManager(const Ros2SubscriptionManager&) = delete; Ros2SubscriptionManager& operator=(const Ros2SubscriptionManager&) = delete; diff --git a/ros2/src/main.cpp b/ros2/src/main.cpp index 6cc8607..53900db 100644 --- a/ros2/src/main.cpp +++ b/ros2/src/main.cpp @@ -39,7 +39,7 @@ int main(int argc, char** argv) { node->declare_parameter("port", 9090); node->declare_parameter("publish_rate", 50.0); node->declare_parameter("session_timeout", 10.0); - node->declare_parameter("strip_large_messages", true); + node->declare_parameter("strip_large_messages", false); int port = node->get_parameter("port").as_int(); double publish_rate = node->get_parameter("publish_rate").as_double(); diff --git a/tests/unit/test_ros2_subscription_manager.cpp b/tests/unit/test_ros2_subscription_manager.cpp new file mode 100644 index 0000000..ace2f2d --- /dev/null +++ b/tests/unit/test_ros2_subscription_manager.cpp @@ -0,0 +1,106 @@ +/* + * 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 +#include + +#include +#include +#include + +#include "pj_bridge_ros2/ros2_subscription_manager.hpp" + +using namespace pj_bridge; + +namespace { + +// 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")); + auto manager = use_default_config ? std::make_shared(node) + : std::make_shared(node, strip_large_messages); + + std::atomic received_size{0}; + manager->set_message_callback( + [&received_size](const std::string&, std::shared_ptr> data, uint64_t) { + received_size = data->size(); + }); + + const std::string topic = "/strip_test_image_" + std::string(node->get_name()); + auto publisher = node->create_publisher(topic, rclcpp::QoS(10)); + EXPECT_TRUE(manager->subscribe(topic, "sensor_msgs/msg/Image")); + + sensor_msgs::msg::Image img; + img.width = 100; + img.height = 100; + img.step = 300; + 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)); + } + + manager->unsubscribe_all(); + return received_size.load(); +} + +constexpr size_t kImagePayloadBytes = 100 * 300; + +} // namespace + +class Ros2SubscriptionManagerTest : public ::testing::Test { + protected: + void SetUp() override { + rclcpp::init(0, nullptr); + } + + void TearDown() override { + rclcpp::shutdown(); + } +}; + +// --------------------------------------------------------------------------- +// Stripping is opt-in: by default, large data fields are forwarded intact. +// --------------------------------------------------------------------------- +TEST_F(Ros2SubscriptionManagerTest, DataFieldsIncludedByDefault) { + size_t received = roundtrip_image_bytes(false, /*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. +// --------------------------------------------------------------------------- +TEST_F(Ros2SubscriptionManagerTest, OptInStrippingRemovesData) { + size_t received = roundtrip_image_bytes(/*strip_large_messages=*/true, /*use_default_config=*/false); + ASSERT_GT(received, 0u) << "no message received"; + EXPECT_LT(received, kImagePayloadBytes) << "opt-in stripping did not remove the data payload"; +}