diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 3fa9230..0448c05 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,27 @@ Changelog for package pj_ros_bridge ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +0.7.0 (2026-07-06) +------------------ +* Foxglove-parity feature set, closing the gap with foxglove_bridge's + topic-subscription features: + - Topic whitelist: full-match regex filtering of visible/subscribable + topics (``topic_whitelist`` / ``--topic-whitelist``) + - QoS depth heuristics (ROS2): KEEP_LAST subscription depth derived from + discovered publisher depths, clamped to ``min_qos_depth``/``max_qos_depth`` + - Opt-in pushed topic advertisement: ``subscribe_topic_updates`` / + ``unsubscribe_topic_updates`` commands and a ``topics_changed`` + notification, polled at ``topic_poll_interval`` + - Slow-client backpressure: bounded, drop-oldest per-client send queue + (``client_backlog_size``) instead of blocking the publish loop or + disconnecting the client + - Latched topic replay (ROS2): new subscribers to a ``TRANSIENT_LOCAL`` + topic (e.g. ``/tf_static``) immediately receive the retained last message + - TLS (``wss://``): optional server-certificate TLS via OpenSSL + (``tls``/``certfile``/``keyfile``, CMake option ``PJ_BRIDGE_TLS``) +* All changes are additive; ``protocol_version`` remains ``1`` +* 231 unit tests passing (TSAN/ASAN clean) + 0.1.0 (2026-02-11) ------------------ * **License changed to AGPL-3.0** (was Apache-2.0 in development) diff --git a/CLAUDE.md b/CLAUDE.md index b741737..66143af 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -197,7 +197,7 @@ Then, for each message in the (compressed) payload: ## Testing -### Test Count: 176 unit tests across 11 test suites +### Test Count: 231 unit tests across 11 test suites ### Commands ```bash @@ -229,22 +229,38 @@ pre-commit run -a ### ROS2 (via `--ros-args -p`): ```yaml -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 +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 +topic_whitelist: [".*"] # Full-match regex patterns restricting visible/subscribable topics +min_qos_depth: 1 # Minimum KEEP_LAST subscription depth after aggregating publisher depths +max_qos_depth: 100 # Maximum KEEP_LAST subscription depth after aggregating publisher depths +topic_poll_interval: 1.0 # Seconds between topics_changed notification polls; 0 disables polling +client_backlog_size: 100 # Max frames queued per slow client before dropping the oldest (must be > 0) +tls: false # Enable TLS (wss://); requires certfile and keyfile +certfile: "" # TLS server certificate file +keyfile: "" # TLS private key file ``` ### RTI (via CLI flags): ```bash -pj_bridge_rti --domains 0 1 --port 9090 --publish-rate 50 --session-timeout 10 +pj_bridge_rti --domains 0 1 --port 9090 --publish-rate 50 --session-timeout 10 \ + --topic-whitelist ".*" --topic-poll-interval 1.0 --client-backlog-size 100 \ + --certfile cert.pem --keyfile key.pem ``` ### FastDDS (via CLI flags): ```bash -pj_bridge_fastdds --domains 0 1 --port 9090 --publish-rate 50 --session-timeout 10 +pj_bridge_fastdds --domains 0 1 --port 9090 --publish-rate 50 --session-timeout 10 \ + --topic-whitelist ".*" --topic-poll-interval 1.0 --client-backlog-size 100 \ + --certfile cert.pem --keyfile key.pem ``` +See `docs/API.md` for full semantics of each option (topic whitelist matching rules, +QoS depth heuristics, `topics_changed` notification, backpressure policy, latched +replay, and TLS setup). + ## Important Design Decisions 1. **Backend-agnostic core via interfaces**: `TopicSourceInterface` and `SubscriptionManagerInterface` allow the same `BridgeServer` to work with ROS2 or RTI DDS. @@ -261,7 +277,7 @@ pj_bridge_fastdds --domains 0 1 --port 9090 --publish-rate 50 --session-timeout --- -**Last Updated**: 2026-07-02 +**Last Updated**: 2026-07-06 **Project Phase**: Unified multi-backend architecture -**Test Status**: 176 unit tests passing (all sanitizers clean) +**Test Status**: 231 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 1fc0143..41c34a7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -53,6 +53,8 @@ find_package(ZSTD REQUIRED) include(FetchContent) # IXWebSocket — try system/conda first, fall back to FetchContent +option(PJ_BRIDGE_TLS "Build IXWebSocket with TLS (OpenSSL) support" ON) + find_package(ixwebsocket QUIET) if(NOT ixwebsocket_FOUND) message(STATUS "IXWebSocket not found — fetching via FetchContent") @@ -61,7 +63,10 @@ if(NOT ixwebsocket_FOUND) URL https://github.com/machinezone/IXWebSocket/archive/refs/tags/v11.4.6.tar.gz URL_HASH SHA256=c024334f8e45980836c67008979a884d6dcc5ef067dd2eb1fa7241f4c17ddc32 ) - set(USE_TLS OFF CACHE BOOL "" FORCE) + set(USE_TLS ${PJ_BRIDGE_TLS} CACHE BOOL "" FORCE) + if(PJ_BRIDGE_TLS) + set(USE_OPEN_SSL ON CACHE BOOL "" FORCE) + endif() set(USE_ZLIB ON CACHE BOOL "" FORCE) set(IXWEBSOCKET_INSTALL OFF CACHE BOOL "" FORCE) FetchContent_MakeAvailable(ixwebsocket) @@ -93,6 +98,7 @@ add_library(pj_bridge_app STATIC app/src/message_serializer.cpp app/src/bridge_server.cpp app/src/standalone_event_loop.cpp + app/src/whitelist_filter.cpp ) target_include_directories(pj_bridge_app PUBLIC @@ -241,11 +247,13 @@ if(BUILD_TESTING AND ament_cmake_FOUND) ament_add_gtest(${PROJECT_NAME}_tests tests/unit/test_websocket_middleware.cpp + tests/unit/test_bounded_frame_queue.cpp tests/unit/test_message_buffer.cpp tests/unit/test_session_manager.cpp tests/unit/test_message_serializer.cpp tests/unit/test_bridge_server.cpp tests/unit/test_protocol_constants.cpp + tests/unit/test_whitelist_filter.cpp tests/unit/test_topic_discovery.cpp tests/unit/test_schema_extractor.cpp tests/unit/test_generic_subscription_manager.cpp diff --git a/README.md b/README.md index 752f8a6..0913a82 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,12 @@ 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 +- **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` +- **Slow-Client Backpressure**: A bounded, drop-oldest per-client send queue (`client_backlog_size`) keeps a lagging client from blocking the publish loop or growing an unbounded backlog, instead of disconnecting it +- **Latched Topic Replay** (ROS2 only): New subscribers to a `TRANSIENT_LOCAL` topic (e.g. `/tf_static`) immediately receive the retained last message instead of waiting for the next publish +- **TLS / `wss://`** (opt-in): Serve the WebSocket endpoint over TLS with a server certificate + private key (`tls`/`--certfile`+`--keyfile`); clients connect via `wss://` instead of `ws://` ## CI Status @@ -33,12 +39,39 @@ independently. ## Configuration Parameters +### ROS2 (via `--ros-args -p`) + | Parameter | Type | Default | Description | |-----------|------|---------|-------------| | `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 | false | Opt-in: strip large arrays from Image, PointCloud2, LaserScan, OccupancyGrid messages | +| `topic_whitelist` | string array | `[".*"]` | Full-match regex patterns (ECMAScript) restricting visible/subscribable topics | +| `min_qos_depth` | int | 1 | 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 | +| `topic_poll_interval` | double | 1.0 | Seconds between `topics_changed` notification polls; `0` disables polling | +| `client_backlog_size` | int | 100 | Max binary frames queued per slow client before the oldest is dropped (must be `> 0`) | +| `tls` | bool | false | Enable TLS (`wss://`); requires `certfile` and `keyfile` | +| `certfile` | string | `""` | TLS server certificate file | +| `keyfile` | string | `""` | TLS private key file | + +### FastDDS / RTI (via CLI flags) + +| Flag | Type | Default | Description | +|------|------|---------|-------------| +| `--domains`, `-d` | int list | (required) | DDS domain IDs | +| `--port`, `-p` | 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 | +| `--log-level` | string | `info` | Log level (trace, debug, info, warn, error) | +| `--stats` | flag | off | Print statistics every 5 seconds | +| `--topic-whitelist` | string list | `.*` | Full-match regex patterns (ECMAScript), repeatable | +| `--topic-poll-interval` | double | 1.0 | Seconds between `topics_changed` notification polls; `0` disables polling | +| `--client-backlog-size` | int | 100 | Max binary frames queued per slow client before the oldest is dropped (range `1`-`1000000`) | +| `--certfile` | string | (none) | TLS server certificate file; enables `wss://`, requires `--keyfile` | +| `--keyfile` | string | (none) | TLS private key file; enables `wss://`, requires `--certfile` | +| `--qos-profile` | string | (none) | RTI only: QoS profile XML file path | ## Just "Download and Run" @@ -73,6 +106,8 @@ chmod +x pj_bridge_ros2-humble-x86_64.AppImage All dependencies (spdlog, nlohmann_json, ZSTD) are provided by the dependency manager. IXWebSocket is resolved via `find_package` first, with a FetchContent fallback for colcon builds. Only `tl::expected` is vendored. +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. + ### 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/bridge_server.hpp b/app/include/pj_bridge/bridge_server.hpp index 9ac03dc..35fb374 100644 --- a/app/include/pj_bridge/bridge_server.hpp +++ b/app/include/pj_bridge/bridge_server.hpp @@ -31,6 +31,7 @@ #include "pj_bridge/session_manager.hpp" #include "pj_bridge/subscription_manager_interface.hpp" #include "pj_bridge/topic_source_interface.hpp" +#include "pj_bridge/whitelist_filter.hpp" namespace pj_bridge { @@ -63,12 +64,13 @@ class BridgeServer { * @param port Server port (default: 9090) * @param session_timeout Session timeout in seconds (default: 10.0) * @param publish_rate Message aggregation publish rate in Hz (default: 50.0) + * @param whitelist Topic whitelist filter (default: matches everything) */ explicit BridgeServer( std::shared_ptr topic_source, std::shared_ptr subscription_manager, std::shared_ptr middleware, int port = 9090, double session_timeout = 10.0, - double publish_rate = 50.0); + double publish_rate = 50.0, WhitelistFilter whitelist = {}); /// Shuts down middleware before members are destroyed, preventing /// disconnect callbacks from firing into a partially destroyed object. @@ -106,6 +108,18 @@ class BridgeServer { */ void check_session_timeouts(); + /** + * @brief Check whether the (whitelist-filtered) topic set has changed and + * notify opted-in clients + * + * Called externally by the event loop (e.g. every topic_poll_interval + * seconds). The first call after construction only takes a snapshot and + * sends nothing; subsequent calls diff against that snapshot and send a + * `topics_changed` notification (added/removed) to every session that + * called `subscribe_topic_updates`, but only when the diff is non-empty. + */ + void check_topic_changes(); + /** * @brief Get the number of active client sessions */ @@ -130,11 +144,31 @@ class BridgeServer { std::string handle_heartbeat(const std::string& client_id, const nlohmann::json& request); std::string handle_pause(const std::string& client_id, const nlohmann::json& request); std::string handle_resume(const std::string& client_id, const nlohmann::json& request); + std::string handle_subscribe_topic_updates(const std::string& client_id, const nlohmann::json& request); + std::string handle_unsubscribe_topic_updates(const std::string& client_id, const nlohmann::json& request); std::string create_error_response( const std::string& error_code, const std::string& message, const nlohmann::json& request) const; void inject_response_fields(nlohmann::json& response, const nlohmann::json& request) const; void cleanup_session(const std::string& client_id); + /// Release one middleware subscription ref for @p topic_name. When the + /// last ref is gone (subscription destroyed), also clears the topic's + /// latched state in the message buffer: a stale retained sample must not + /// be replayed to the next fresh subscriber, who receives the current + /// sample from DDS transient_local redelivery instead. Call with + /// cleanup_mutex_ held (same requirement as unsubscribe itself). + void release_subscription_ref(const std::string& topic_name); + + /// Latched (transient_local) replay bookkeeping for a topic whose + /// middleware subscription ref was just (re)acquired for @p client_id — + /// shared by handle_subscribe and handle_resume. Records the topic's + /// latched flag in the message buffer and, when a retained sample is + /// available for replay, serializes it into a single-message binary frame + /// queued in pending_replays_ (flushed by process_single_request right + /// after the handler's response is sent). Call with cleanup_mutex_ held; + /// acquires only the leaf replays_mutex_ and makes no middleware calls. + void collect_latched_replay(const std::string& client_id, const std::string& topic_name); + // Backend interfaces (owned) std::shared_ptr topic_source_; std::shared_ptr subscription_manager_; @@ -148,6 +182,7 @@ class BridgeServer { int port_; double session_timeout_; double publish_rate_; + WhitelistFilter whitelist_; // State std::atomic initialized_; @@ -164,11 +199,34 @@ class BridgeServer { // cleanup_mutex_ > last_sent_mutex_ > stats_mutex_ // SessionManager::mutex_ may be acquired while holding any of these. // Never acquire a higher-order lock while holding a lower-order one. + // topics_mutex_ is a LEAF lock: it is held only to compute the + // added/removed diff and swap in the new known_topics_ snapshot. Never + // call middleware_ or session_manager_ (or acquire any other lock in this + // class) while holding topics_mutex_ — release it before sending + // notifications. + // replays_mutex_ is likewise a LEAF lock: it only guards pending_replays_ + // map accesses. Never hold it while acquiring any other lock or calling + // middleware_ — move the frames out, release, then send. std::mutex cleanup_mutex_; // Per-client per-topic last-sent timestamp (nanoseconds) for rate limiting std::unordered_map> last_sent_times_; std::mutex last_sent_mutex_; + + // Whitelist-filtered topic name -> type snapshot, used by check_topic_changes() + // to detect additions/removals/type-changes. LEAF lock (see comment above). + std::unordered_map known_topics_; + bool topics_snapshot_taken_{false}; + std::mutex topics_mutex_; + + // Latched (transient_local) replay frames queued by handle_subscribe and + // handle_resume (via collect_latched_replay), flushed by + // process_single_request right AFTER the handler's response is sent — the + // client must receive the response (which carries the topic's schema on + // subscribe) before the binary frame that needs it. LEAF lock (see + // comment above). + std::unordered_map>> pending_replays_; + std::mutex replays_mutex_; }; } // namespace pj_bridge diff --git a/app/include/pj_bridge/message_buffer.hpp b/app/include/pj_bridge/message_buffer.hpp index 2ef2406..f5aa04f 100644 --- a/app/include/pj_bridge/message_buffer.hpp +++ b/app/include/pj_bridge/message_buffer.hpp @@ -25,8 +25,10 @@ #include #include #include +#include #include #include +#include #include namespace pj_bridge { @@ -78,12 +80,40 @@ class MessageBuffer { /// Return the total number of messages across all topics. size_t size() const; + /// Mark (or unmark) @p topic_name as latched (transient_local). While a + /// topic is latched, add_message() additionally retains its newest sample + /// in a TTL-exempt, one-entry-per-topic store so that later subscribers to + /// an already-shared subscription can be replayed the current value even + /// after it has aged out of the normal buffer. Setting latched to false + /// erases the retained sample. + void set_latched(const std::string& topic_name, bool latched); + + /// Return the retained latched sample for @p topic_name, or nullopt if the + /// topic is not latched or no sample has been retained yet. + std::optional get_latched(const std::string& topic_name) const; + + /// Like get_latched(), but additionally returns nullopt while the topic + /// still has messages in the normal buffer. In that case the upcoming + /// publish cycle will deliver the sample to the new subscriber anyway — + /// replaying it would produce a duplicate. Replay is only needed for + /// samples that already left the normal buffer (drained by + /// move_messages() or TTL-evicted). + std::optional get_latched_for_replay(const std::string& topic_name) const; + private: mutable std::mutex mutex_; std::unordered_map> topic_buffers_; uint64_t max_message_age_ns_; ClockFn clock_fn_; + // Latched (transient_local) topic support: topics currently flagged as + // latched, and the single newest retained sample per latched topic. + // Bounded at one message per latched topic; entries otherwise persist for + // the lifetime of the process (not TTL-cleaned, not drained by + // move_messages()). + std::unordered_set latched_topics_; + std::unordered_map latched_last_; + uint64_t now_ns() const; void cleanup_old_messages(); }; diff --git a/app/include/pj_bridge/middleware/bounded_frame_queue.hpp b/app/include/pj_bridge/middleware/bounded_frame_queue.hpp new file mode 100644 index 0000000..aa8b657 --- /dev/null +++ b/app/include/pj_bridge/middleware/bounded_frame_queue.hpp @@ -0,0 +1,93 @@ +/* + * 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 + +namespace pj_bridge { + +/// A FIFO queue of binary frames bounded to at most `max_frames` entries. +/// +/// When a push would exceed the capacity, the OLDEST queued frame is dropped +/// to make room for the new one (never the newest), matching foxglove_bridge's +/// slow-client backpressure policy: recent data is prioritized over stale +/// backlog for interactive plotting/visualization use cases. +/// +/// NOT thread-safe by design: this is a plain data structure. Callers are +/// expected to hold their own lock (e.g. WebSocketMiddleware's +/// `clients_mutex_`) around every call. +class BoundedFrameQueue { + public: + explicit BoundedFrameQueue(size_t max_frames) : max_frames_(max_frames) {} + + /// Pushes a frame, dropping the oldest queued frame first if the queue is + /// already at capacity. Returns the number of frames dropped as a result of + /// this call (0 or 1). A queue with `max_frames == 0` drops every push + /// immediately and never grows. + size_t push(std::vector frame) { + if (max_frames_ == 0) { + ++dropped_total_; + return 1; + } + + size_t dropped = 0; + if (frames_.size() >= max_frames_) { + frames_.pop_front(); + ++dropped_total_; + dropped = 1; + } + frames_.push_back(std::move(frame)); + return dropped; + } + + /// Removes and returns the oldest frame, or nullopt if the queue is empty. + std::optional> pop_front() { + if (frames_.empty()) { + return std::nullopt; + } + std::optional> front = std::move(frames_.front()); + frames_.pop_front(); + return front; + } + + bool empty() const { + return frames_.empty(); + } + + size_t size() const { + return frames_.size(); + } + + /// Cumulative number of frames dropped over the lifetime of this queue. + uint64_t dropped_total() const { + return dropped_total_; + } + + private: + size_t max_frames_; + std::deque> frames_; + uint64_t dropped_total_{0}; +}; + +} // namespace pj_bridge diff --git a/app/include/pj_bridge/middleware/middleware_interface.hpp b/app/include/pj_bridge/middleware/middleware_interface.hpp index abda272..ec62b2e 100644 --- a/app/include/pj_bridge/middleware/middleware_interface.hpp +++ b/app/include/pj_bridge/middleware/middleware_interface.hpp @@ -68,6 +68,11 @@ class MiddlewareInterface { /// @return true if the message was sent, false if the client is gone. virtual bool send_binary(const std::string& client_identity, const std::vector& data) = 0; + /// Discard any queued outbound data for this client (e.g. when its session + /// is destroyed server-side while the socket stays open). Default no-op for + /// implementations without per-client outbound queues. + virtual void drop_pending(const std::string& /*client_identity*/) {} + /// @return true if initialize() succeeded and shutdown() has not been called. virtual bool is_ready() const = 0; diff --git a/app/include/pj_bridge/middleware/websocket_middleware.hpp b/app/include/pj_bridge/middleware/websocket_middleware.hpp index 5cf96a6..f1c1da4 100644 --- a/app/include/pj_bridge/middleware/websocket_middleware.hpp +++ b/app/include/pj_bridge/middleware/websocket_middleware.hpp @@ -22,21 +22,33 @@ #include #include +#include +#include #include #include +#include #include #include #include #include #include +#include "pj_bridge/middleware/bounded_frame_queue.hpp" #include "pj_bridge/middleware/middleware_interface.hpp" namespace pj_bridge { +/// Server-side TLS configuration (certificate + private key paths). Both +/// files are validated for existence/readability in initialize(), before any +/// IXWebSocket TLS API is touched. +struct TlsConfig { + std::string certfile; + std::string keyfile; +}; + class WebSocketMiddleware : public MiddlewareInterface { public: - WebSocketMiddleware(); + explicit WebSocketMiddleware(size_t client_backlog_size = 100, std::optional tls = std::nullopt); ~WebSocketMiddleware() override; WebSocketMiddleware(const WebSocketMiddleware&) = delete; @@ -53,6 +65,11 @@ class WebSocketMiddleware : public MiddlewareInterface { bool is_ready() const override; void set_on_connect(ConnectionCallback callback) override; void set_on_disconnect(ConnectionCallback callback) override; + void drop_pending(const std::string& client_identity) override; + + /// Total number of frames dropped due to slow-client backpressure, summed + /// across all clients (currently connected and already disconnected). + uint64_t dropped_frame_count() const; private: struct IncomingRequest { @@ -66,8 +83,24 @@ class WebSocketMiddleware : public MiddlewareInterface { mutable std::mutex queue_mutex_; std::unordered_map> clients_; + // Per-client backlog of frames withheld while the client's socket buffer is + // over the high watermark, and the steady-clock time each client's drop + // warning was last logged. Both are guarded by `clients_mutex_` (the same + // lock as `clients_`), not a separate mutex, so client lifecycle and + // pending-queue lifecycle stay consistent with each other. + std::unordered_map pending_frames_; + std::unordered_map last_drop_warn_; mutable std::mutex clients_mutex_; + // Sum of dropped_total() from clients that have since disconnected (their + // BoundedFrameQueue entry is erased from pending_frames_ on disconnect, so + // its count is folded in here to keep dropped_frame_count() a lifetime + // total). Guarded by clients_mutex_. + uint64_t dropped_from_disconnected_{0}; + + size_t client_backlog_size_; + std::optional tls_; + ConnectionCallback on_connect_; ConnectionCallback on_disconnect_; @@ -78,6 +111,13 @@ class WebSocketMiddleware : public MiddlewareInterface { static constexpr int kShutdownTimeoutSeconds = 3; static constexpr size_t kMaxIncomingQueueSize = 1024; + + // Lossy-send policy adapted from foxglove_bridge (MIT License, Copyright + // (c) Foxglove Technologies Inc): once a client's outgoing socket buffer + // exceeds this watermark, new frames are queued (dropping the oldest on + // overflow) instead of blocking or disconnecting the client. + static constexpr size_t kSocketBufferHighWatermark = 1u << 20; // 1 MiB + static constexpr int kDropWarnIntervalSeconds = 30; }; } // namespace pj_bridge diff --git a/app/include/pj_bridge/session_manager.hpp b/app/include/pj_bridge/session_manager.hpp index 6f70314..43516f6 100644 --- a/app/include/pj_bridge/session_manager.hpp +++ b/app/include/pj_bridge/session_manager.hpp @@ -39,6 +39,9 @@ struct Session { std::chrono::steady_clock::time_point last_heartbeat; std::chrono::steady_clock::time_point created_at; bool paused{false}; + /// True if this session opted in to server-initiated `topics_changed` + /// notifications via the `subscribe_topic_updates` command. + bool topic_updates{false}; }; class SessionManager { @@ -59,6 +62,8 @@ class SessionManager { bool session_exists(const std::string& client_id) const; bool set_paused(const std::string& client_id, bool paused); bool is_paused(const std::string& client_id) const; + bool set_topic_updates(const std::string& client_id, bool enabled); + bool wants_topic_updates(const std::string& client_id) const; bool set_ref_held(const std::string& client_id, const std::string& topic, bool held); std::unordered_set get_ref_held_topics(const std::string& client_id) const; diff --git a/app/include/pj_bridge/standalone_event_loop.hpp b/app/include/pj_bridge/standalone_event_loop.hpp index bd119fb..d4bef91 100644 --- a/app/include/pj_bridge/standalone_event_loop.hpp +++ b/app/include/pj_bridge/standalone_event_loop.hpp @@ -32,6 +32,8 @@ struct StandaloneConfig { double publish_rate; double session_timeout; bool stats_enabled; + /// Interval in seconds between check_topic_changes() polls. 0 disables polling. + double topic_poll_interval = 1.0; }; /// Runs the standalone event loop until SIGINT/SIGTERM. diff --git a/app/include/pj_bridge/subscription_manager_interface.hpp b/app/include/pj_bridge/subscription_manager_interface.hpp index d5780d2..93f302b 100644 --- a/app/include/pj_bridge/subscription_manager_interface.hpp +++ b/app/include/pj_bridge/subscription_manager_interface.hpp @@ -74,6 +74,20 @@ class SubscriptionManagerInterface { /// Unsubscribe from all topics, destroying all underlying subscriptions. /// Called during shutdown. virtual void unsubscribe_all() = 0; + + /// True when every publisher of the topic offers TRANSIENT_LOCAL durability + /// (detected at subscribe time). Backends without this information return + /// false; the result is only meaningful while the topic is subscribed. + virtual bool is_transient_local(const std::string& /*topic_name*/) const { + return false; + } + + /// True while the topic has at least one active reference (i.e. the + /// underlying middleware subscription exists). Backends that don't track + /// this return false. + virtual bool is_subscribed(const std::string& /*topic_name*/) const { + return false; + } }; } // namespace pj_bridge diff --git a/app/include/pj_bridge/whitelist_filter.hpp b/app/include/pj_bridge/whitelist_filter.hpp new file mode 100644 index 0000000..a176d98 --- /dev/null +++ b/app/include/pj_bridge/whitelist_filter.hpp @@ -0,0 +1,57 @@ +/* + * 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 "tl/expected.hpp" + +namespace pj_bridge { + +/// Filters topic names against a set of full-match ECMAScript regex patterns. +/// +/// Mirrors foxglove_bridge's `topic_whitelist` semantics: a topic is allowed +/// if it fully matches at least one pattern (std::regex_match, not +/// regex_search — a pattern must match the entire topic name, not just a +/// prefix or substring). +/// +/// A default-constructed (or empty-pattern-list) filter matches every topic, +/// preserving today's "no whitelist" behavior. +class WhitelistFilter { + public: + /// Default: no patterns configured, matches every topic name. + WhitelistFilter() = default; + + /// Compile `patterns` as ECMAScript regexes. + /// @return the filter on success, or an error message naming the first + /// pattern that failed to compile. + static tl::expected create(const std::vector& patterns); + + /// @return true if `topic_name` fully matches any configured pattern, or + /// if no patterns are configured (match-all default). + bool matches(const std::string& topic_name) const; + + private: + std::vector patterns_; // empty = match all +}; + +} // namespace pj_bridge diff --git a/app/src/bridge_server.cpp b/app/src/bridge_server.cpp index 1846084..ffc1b34 100644 --- a/app/src/bridge_server.cpp +++ b/app/src/bridge_server.cpp @@ -60,13 +60,14 @@ double clamp_rate_hz(double rate_hz) { BridgeServer::BridgeServer( std::shared_ptr topic_source, std::shared_ptr subscription_manager, std::shared_ptr middleware, - int port, double session_timeout, double publish_rate) + int port, double session_timeout, double publish_rate, WhitelistFilter whitelist) : topic_source_(std::move(topic_source)), subscription_manager_(std::move(subscription_manager)), middleware_(std::move(middleware)), port_(port), session_timeout_(session_timeout), publish_rate_(publish_rate), + whitelist_(std::move(whitelist)), initialized_(false), total_messages_published_(0), total_bytes_published_(0), @@ -191,6 +192,10 @@ void BridgeServer::process_single_request(const std::vector& request_da response = handle_pause(client_id, request_json); } else if (command == "resume") { response = handle_resume(client_id, request_json); + } else if (command == "subscribe_topic_updates") { + response = handle_subscribe_topic_updates(client_id, request_json); + } else if (command == "unsubscribe_topic_updates") { + response = handle_unsubscribe_topic_updates(client_id, request_json); } else { response = create_error_response("UNKNOWN_COMMAND", "Unknown command: " + command, request_json); } @@ -207,7 +212,29 @@ void BridgeServer::process_single_request(const std::vector& request_da std::vector response_data(response.begin(), response.end()); if (!middleware_->send_reply(client_id, response_data)) { spdlog::warn("Failed to send response to client '{}', cleaning up session", client_id); + // cleanup_session drops any replay frames handle_subscribe queued for + // this client (they must never be delivered without the response). cleanup_session(client_id); + return; + } + + // Flush any latched-topic replay frames the handler queued for this + // client (handle_subscribe or handle_resume, via collect_latched_replay). + // Sent strictly AFTER the response so the client has the schema needed + // to decode them (docs/API.md: "right after the subscribe response"). + // Move the frames out under the leaf replays_mutex_, release it, then + // send — never call middleware_ while holding the lock. + std::vector> replay_frames; + { + std::lock_guard replays_lock(replays_mutex_); + auto it = pending_replays_.find(client_id); + if (it != pending_replays_.end()) { + replay_frames = std::move(it->second); + pending_replays_.erase(it); + } + } + for (const auto& frame : replay_frames) { + middleware_->send_binary(client_id, frame); } } @@ -225,16 +252,21 @@ std::string BridgeServer::handle_get_topics(const std::string& client_id, const response["status"] = "success"; response["topics"] = json::array(); + size_t returned_count = 0; for (const auto& topic : topics) { + if (!whitelist_.matches(topic.name)) { + continue; + } json topic_entry; topic_entry["name"] = topic.name; topic_entry["type"] = topic.type; response["topics"].push_back(topic_entry); + returned_count++; } inject_response_fields(response, request); - spdlog::info("Returning {} topics to client '{}'", topics.size(), client_id); + spdlog::info("Returning {} topics to client '{}'", returned_count, client_id); return response.dump(); } @@ -297,6 +329,15 @@ std::string BridgeServer::handle_subscribe(const std::string& client_id, const n std::unordered_map extracted_schemas; for (const auto& topic_name : topics_to_add) { + if (!whitelist_.matches(topic_name)) { + spdlog::warn("Client '{}' requested non-whitelisted topic '{}'", client_id, topic_name); + json failure; + failure["topic"] = topic_name; + failure["reason"] = "Topic not whitelisted"; + failures.push_back(failure); + continue; + } + if (topic_types.find(topic_name) == topic_types.end()) { spdlog::warn("Client '{}' requested non-existent topic '{}'", client_id, topic_name); json failure; @@ -354,7 +395,7 @@ std::string BridgeServer::handle_subscribe(const std::string& client_id, const n if (!session_manager_->add_subscription(client_id, topic_name, rate)) { spdlog::warn("Session gone for client '{}', rolling back subscribe for '{}'", client_id, topic_name); if (ref_acquired) { - subscription_manager_->unsubscribe(topic_name); + release_subscription_ref(topic_name); } continue; } @@ -365,6 +406,14 @@ std::string BridgeServer::handle_subscribe(const std::string& client_id, const n schema_obj["definition"] = extracted_schemas[topic_name]; schemas[topic_name] = schema_obj; + // Latched (transient_local) topic replay bookkeeping — only + // meaningful when we actually (re)established the middleware + // subscription this call (ref_acquired): a paused "subscribe" + // creates no subscription to query; its replay happens on resume. + if (ref_acquired) { + collect_latched_replay(client_id, topic_name); + } + spdlog::info("Client '{}' subscribed to topic '{}' (type: {})", client_id, topic_name, topic_type); } @@ -445,7 +494,7 @@ std::string BridgeServer::handle_unsubscribe(const std::string& client_id, const // Release the middleware ref only if this session actually holds one // (a paused client's refs were already released by pause). if (held_refs.count(topic_name) > 0) { - subscription_manager_->unsubscribe(topic_name); + release_subscription_ref(topic_name); } session_manager_->remove_subscription(client_id, topic_name); current_subs.erase(topic_name); @@ -507,7 +556,7 @@ std::string BridgeServer::handle_pause(const std::string& client_id, const nlohm // Release exactly the refs this session holds auto held_refs = session_manager_->get_ref_held_topics(client_id); for (const auto& topic : held_refs) { - subscription_manager_->unsubscribe(topic); + release_subscription_ref(topic); session_manager_->set_ref_held(client_id, topic, false); spdlog::debug("Released ref for topic '{}' (client '{}' paused)", topic, client_id); } @@ -574,6 +623,11 @@ std::string BridgeServer::handle_resume(const std::string& client_id, const nloh if (subscription_manager_->subscribe(topic, type_it->second)) { session_manager_->set_ref_held(client_id, topic, true); resubscribed++; + // Same latched bookkeeping as handle_subscribe: a topic subscribed + // WHILE PAUSED acquires its first ref here, and when it joins an + // already-shared subscription DDS won't redeliver — the retained + // sample (if any) is queued and flushed after the resume response. + collect_latched_replay(client_id, topic); spdlog::debug("Re-acquired ref for topic '{}' (client '{}' resumed)", topic, client_id); } else { spdlog::warn("Topic '{}' subscription failed on resume for client '{}'", topic, client_id); @@ -595,6 +649,43 @@ std::string BridgeServer::handle_resume(const std::string& client_id, const nloh return response.dump(); } +std::string BridgeServer::handle_subscribe_topic_updates(const std::string& client_id, const nlohmann::json& request) { + if (!session_manager_->session_exists(client_id)) { + session_manager_->create_session(client_id); + spdlog::info("Created new session for client '{}'", client_id); + } + + session_manager_->update_heartbeat(client_id); + session_manager_->set_topic_updates(client_id, true); + + spdlog::debug("Client '{}' subscribed to topic updates", client_id); + + json response; + response["status"] = "ok"; + response["topic_updates"] = true; + inject_response_fields(response, request); + return response.dump(); +} + +std::string BridgeServer::handle_unsubscribe_topic_updates( + const std::string& client_id, const nlohmann::json& request) { + if (!session_manager_->session_exists(client_id)) { + session_manager_->create_session(client_id); + spdlog::info("Created new session for client '{}'", client_id); + } + + session_manager_->update_heartbeat(client_id); + session_manager_->set_topic_updates(client_id, false); + + spdlog::debug("Client '{}' unsubscribed from topic updates", client_id); + + json response; + response["status"] = "ok"; + response["topic_updates"] = false; + inject_response_fields(response, request); + return response.dump(); +} + void BridgeServer::inject_response_fields(nlohmann::json& response, const nlohmann::json& request) const { response["protocol_version"] = kProtocolVersion; if (request.contains("id") && request["id"].is_string()) { @@ -626,29 +717,172 @@ void BridgeServer::check_session_timeouts() { } } -void BridgeServer::cleanup_session(const std::string& client_id) { - std::lock_guard lock(cleanup_mutex_); +void BridgeServer::check_topic_changes() { + if (!initialized_) { + return; + } - if (!session_manager_->session_exists(client_id)) { + std::unordered_map current_topics; + for (const auto& topic : topic_source_->get_topics()) { + if (!whitelist_.matches(topic.name)) { + continue; + } + current_topics[topic.name] = topic.type; + } + + json added = json::array(); + json removed = json::array(); + + { + std::lock_guard lock(topics_mutex_); + + if (!topics_snapshot_taken_) { + known_topics_ = std::move(current_topics); + topics_snapshot_taken_ = true; + return; + } + + for (const auto& [name, type] : current_topics) { + auto it = known_topics_.find(name); + if (it == known_topics_.end() || it->second != type) { + json entry; + entry["name"] = name; + entry["type"] = type; + added.push_back(entry); + } + } + + for (const auto& [name, type] : known_topics_) { + auto it = current_topics.find(name); + if (it == current_topics.end() || it->second != type) { + removed.push_back(name); + } + } + + known_topics_ = std::move(current_topics); + } // topics_mutex_ released here — never send notifications while holding it + + if (added.empty() && removed.empty()) { return; } - // Release exactly the refs this session holds. Paused clients hold none; - // there is no separate paused check to race against. - auto held_refs = session_manager_->get_ref_held_topics(client_id); - for (const auto& topic : held_refs) { - subscription_manager_->unsubscribe(topic); - spdlog::debug("Unsubscribed client '{}' from topic '{}'", client_id, topic); + json notification; + notification["notification"] = "topics_changed"; + notification["added"] = added; + notification["removed"] = removed; + notification["protocol_version"] = kProtocolVersion; + + std::vector bytes; + { + std::string text = notification.dump(); + bytes.assign(text.begin(), text.end()); } - session_manager_->remove_session(client_id); + for (const auto& client_id : session_manager_->get_active_sessions()) { + if (session_manager_->wants_topic_updates(client_id)) { + middleware_->send_reply(client_id, bytes); + } + } +} +void BridgeServer::collect_latched_replay(const std::string& client_id, const std::string& topic_name) { + // The FIRST subscriber to a latched topic gets its retained sample + // delivered naturally by DDS (transient_local redelivery on the freshly + // created subscription). Replay exists for LATER clients joining an + // already-shared subscription — via subscribe or via resume after + // subscribing while paused — whose sample may have aged out of + // MessageBuffer's normal TTL window by then. Ordering note: set_latched + // runs at (re)subscribe time, so a latched topic's messages are only + // retained once someone has subscribed — exactly the shared-subscription + // case the replay serves. + bool is_latched = subscription_manager_->is_transient_local(topic_name); + message_buffer_->set_latched(topic_name, is_latched); + if (!is_latched) { + return; + } + + // get_latched_for_replay skips samples still sitting in the normal + // buffer — those reach the new subscriber through the regular publish + // cycle, and replaying them too would duplicate. + auto latched_msg = message_buffer_->get_latched_for_replay(topic_name); + if (!latched_msg) { + return; + } + + // Do NOT send here: the handler's response (which on subscribe carries + // the topic's schema) is sent by process_single_request after the + // handler returns, and the client must receive it before any binary + // frame that needs the schema to decode. Queue a single-message frame + // (same format as regular aggregated frames, so clients need no special + // handling); process_single_request flushes it right after the response + // goes out. Does not touch last_sent_times_: the sample's old timestamp + // means it won't interfere with subsequent rate limiting. + // + // Benign race with cleanup_session between this queueing and the flush: + // if the client disconnects in between, cleanup_session erases the + // client's pending_replays_ entry (and a failed send_reply takes that + // same cleanup path), so the frames are either dropped with the dead + // session or sent to a still-connected socket — nothing strands. + AggregatedMessageSerializer serializer; + serializer.serialize_message( + topic_name, latched_msg->timestamp_ns, latched_msg->data->data(), latched_msg->data->size()); + auto frame = serializer.finalize(); + + std::lock_guard replays_lock(replays_mutex_); + pending_replays_[client_id].push_back(std::move(frame)); +} + +void BridgeServer::release_subscription_ref(const std::string& topic_name) { + subscription_manager_->unsubscribe(topic_name); + // When the last ref is released the middleware subscription is destroyed. + // Clear the topic's latched state with it: the retained sample would go + // stale, and the NEXT subscriber creates a fresh middleware subscription + // that receives the current sample via DDS transient_local redelivery — + // replaying the stale copy before it would deliver out-of-date data. + if (!subscription_manager_->is_subscribed(topic_name) && message_buffer_) { + message_buffer_->set_latched(topic_name, false); + } +} + +void BridgeServer::cleanup_session(const std::string& client_id) { { - std::lock_guard sent_lock(last_sent_mutex_); - last_sent_times_.erase(client_id); + std::lock_guard lock(cleanup_mutex_); + + // Drop any undelivered latched-replay frames regardless of session state + // (the client is gone; leaving them queued would strand them forever). + { + std::lock_guard replays_lock(replays_mutex_); + pending_replays_.erase(client_id); + } + + if (session_manager_->session_exists(client_id)) { + // Release exactly the refs this session holds. Paused clients hold + // none; there is no separate paused check to race against. + auto held_refs = session_manager_->get_ref_held_topics(client_id); + for (const auto& topic : held_refs) { + release_subscription_ref(topic); + spdlog::debug("Unsubscribed client '{}' from topic '{}'", client_id, topic); + } + + session_manager_->remove_session(client_id); + + { + std::lock_guard sent_lock(last_sent_mutex_); + last_sent_times_.erase(client_id); + } + + spdlog::info("Cleaned up session for client '{}' ({} topic refs released)", client_id, held_refs.size()); + } } - spdlog::info("Cleaned up session for client '{}' ({} topic refs released)", client_id, held_refs.size()); + // Discard any frames the middleware's slow-client backpressure queue still + // holds for this client. On a heartbeat timeout the socket stays open, so + // without this a timed-out slow client parks frames indefinitely and stale + // data could flush into a new logical session on the same connection. + // Deliberately outside the cleanup_mutex_ scope: middleware calls must not + // run under that lock. Also deliberately unconditional (even when no + // session existed): drop_pending is an idempotent no-op then. + middleware_->drop_pending(client_id); } size_t BridgeServer::get_active_session_count() const { diff --git a/app/src/message_buffer.cpp b/app/src/message_buffer.cpp index 653f80f..d0f4a27 100644 --- a/app/src/message_buffer.cpp +++ b/app/src/message_buffer.cpp @@ -41,9 +41,53 @@ void MessageBuffer::add_message( msg.received_at_ns = now_ns(); msg.data = std::move(data); + if (latched_topics_.count(topic_name) > 0) { + // Retain a copy of the newest sample (shared_ptr copy — cheap, no deep + // copy of the payload) for latched-topic replay. This store is exempt + // from TTL cleanup and from move_messages(). + latched_last_[topic_name] = msg; + } + topic_buffers_[topic_name].push_back(std::move(msg)); } +void MessageBuffer::set_latched(const std::string& topic_name, bool latched) { + std::lock_guard lock(mutex_); + if (latched) { + latched_topics_.insert(topic_name); + } else { + latched_topics_.erase(topic_name); + latched_last_.erase(topic_name); + } +} + +std::optional MessageBuffer::get_latched(const std::string& topic_name) const { + std::lock_guard lock(mutex_); + auto it = latched_last_.find(topic_name); + if (it == latched_last_.end()) { + return std::nullopt; + } + return it->second; +} + +std::optional MessageBuffer::get_latched_for_replay(const std::string& topic_name) const { + std::lock_guard lock(mutex_); + auto it = latched_last_.find(topic_name); + if (it == latched_last_.end()) { + return std::nullopt; + } + // While the topic still has messages in the normal buffer, the retained + // sample is (or is older than) one of them, and the upcoming publish + // cycle will deliver it to the new subscriber via the regular aggregated + // path — a replay would duplicate it. Empty deques are erased by both + // cleanup and move_messages(), so presence means undelivered messages. + auto buf_it = topic_buffers_.find(topic_name); + if (buf_it != topic_buffers_.end() && !buf_it->second.empty()) { + return std::nullopt; + } + return it->second; +} + void MessageBuffer::move_messages(std::unordered_map>& out_messages) { std::lock_guard lock(mutex_); out_messages = std::move(topic_buffers_); @@ -53,6 +97,8 @@ void MessageBuffer::move_messages(std::unordered_map lock(mutex_); topic_buffers_.clear(); + latched_topics_.clear(); + latched_last_.clear(); } size_t MessageBuffer::size() const { diff --git a/app/src/middleware/websocket_middleware.cpp b/app/src/middleware/websocket_middleware.cpp index d8e325d..2e920dc 100644 --- a/app/src/middleware/websocket_middleware.cpp +++ b/app/src/middleware/websocket_middleware.cpp @@ -22,11 +22,14 @@ #include #include +#include +#include #include namespace pj_bridge { -WebSocketMiddleware::WebSocketMiddleware() : initialized_(false) {} +WebSocketMiddleware::WebSocketMiddleware(size_t client_backlog_size, std::optional tls) + : client_backlog_size_(client_backlog_size), tls_(std::move(tls)), initialized_(false) {} WebSocketMiddleware::~WebSocketMiddleware() { shutdown(); @@ -51,8 +54,34 @@ tl::expected WebSocketMiddleware::initialize(uint16_t port) { return tl::unexpected("WebSocket middleware already initialized"); } + if (tls_.has_value()) { +#ifndef IXWEBSOCKET_USE_TLS + return tl::unexpected("TLS requested but IXWebSocket was built without TLS support"); +#else + std::ifstream cert_stream(tls_->certfile); + if (!cert_stream.good()) { + return tl::unexpected("TLS certificate file not found or not readable: " + tls_->certfile); + } + std::ifstream key_stream(tls_->keyfile); + if (!key_stream.good()) { + return tl::unexpected("TLS key file not found or not readable: " + tls_->keyfile); + } +#endif + } + server_ = std::make_shared(port, "0.0.0.0"); +#ifdef IXWEBSOCKET_USE_TLS + if (tls_.has_value()) { + ix::SocketTLSOptions tls_options; + tls_options.tls = true; + tls_options.certFile = tls_->certfile; + tls_options.keyFile = tls_->keyfile; + tls_options.caFile = "NONE"; + server_->setTLSOptions(tls_options); + } +#endif + server_->setOnClientMessageCallback([this]( std::shared_ptr connection_state, ix::WebSocket& web_socket, const ix::WebSocketMessagePtr& msg) { @@ -103,6 +132,12 @@ tl::expected WebSocketMiddleware::initialize(uint16_t port) { { std::lock_guard clients_lock(clients_mutex_); clients_.erase(client_id); + auto pending_it = pending_frames_.find(client_id); + if (pending_it != pending_frames_.end()) { + dropped_from_disconnected_ += pending_it->second.dropped_total(); + pending_frames_.erase(pending_it); + } + last_drop_warn_.erase(client_id); } } else if (msg->type == ix::WebSocketMessageType::Message) { @@ -206,6 +241,11 @@ void WebSocketMiddleware::shutdown() { { std::lock_guard clients_lock(clients_mutex_); clients_.clear(); + for (const auto& entry : pending_frames_) { + dropped_from_disconnected_ += entry.second.dropped_total(); + } + pending_frames_.clear(); + last_drop_warn_.clear(); } { @@ -257,6 +297,21 @@ bool WebSocketMiddleware::send_reply(const std::string& client_identity, const s } bool WebSocketMiddleware::send_binary(const std::string& client_identity, const std::vector& data) { + // Lossy-send policy adapted from foxglove_bridge (MIT License, Copyright + // (c) Foxglove Technologies Inc): a slow client never blocks the publisher + // and is never disconnected for falling behind. Instead, once its socket's + // outgoing buffer crosses kSocketBufferHighWatermark, new frames are queued + // per-client (dropping the oldest queued frame on overflow) and flushed + // once the buffer drains below the watermark again. + // + // IMPORTANT: clients_mutex_ is only ever held to look up state (the client + // handle, pending_frames_, last_drop_warn_) — never while calling into + // ix::WebSocket (bufferedAmount()/sendBinary()). Those calls take + // IXWebSocket's own internal locks, and the Open-connection handler above + // takes clients_mutex_ from an IXWebSocket-owned thread that already holds + // some of those internal locks; holding clients_mutex_ across a socket call + // here would form a lock-order cycle with that path (observed as a TSAN + // lock-order-inversion during development). std::shared_ptr ws; { std::lock_guard lock(clients_mutex_); @@ -267,9 +322,116 @@ bool WebSocketMiddleware::send_binary(const std::string& client_identity, const ws = it->second; } - std::string binary_data(reinterpret_cast(data.data()), data.size()); - auto send_info = ws->sendBinary(binary_data); - return send_info.success; + if (ws->bufferedAmount() < kSocketBufferHighWatermark) { + // Flush any backlog first, in FIFO order, bounded to the number of + // frames present when this call started so a fast producer can't spin + // forever. Each frame is popped under the lock and sent outside it. + size_t to_flush = 0; + { + std::lock_guard lock(clients_mutex_); + auto pending_it = pending_frames_.find(client_identity); + if (pending_it != pending_frames_.end()) { + to_flush = pending_it->second.size(); + } + } + + for (size_t i = 0; i < to_flush; ++i) { + std::optional> frame; + { + std::lock_guard lock(clients_mutex_); + auto pending_it = pending_frames_.find(client_identity); + if (pending_it == pending_frames_.end()) { + break; + } + frame = pending_it->second.pop_front(); + } + if (!frame) { + break; + } + std::string frame_data(reinterpret_cast(frame->data()), frame->size()); + auto send_info = ws->sendBinary(frame_data); + if (!send_info.success) { + return false; + } + } + + std::string binary_data(reinterpret_cast(data.data()), data.size()); + auto send_info = ws->sendBinary(binary_data); + return send_info.success; + } + + // Socket buffer is over the high watermark: queue the frame instead of + // sending it now. + // + // Note the backlog is only ever flushed from within send_binary itself: if + // a client's subscribed topics go quiet, queued frames sit here until the + // next send_binary call for that client (or its disconnect). This is + // deliberate — the queue is bounded, delivering stale frames on the next + // traffic burst is acceptable, and plotting clients care about fresh data, + // so a dedicated flush timer isn't worth the extra machinery. + size_t dropped; + uint64_t dropped_total_now; + { + std::lock_guard lock(clients_mutex_); + // Re-check the client still exists: between the ws lookup above (lock + // released before the socket calls) and here, the disconnect callback may + // have erased the client AND its pending_frames_ entry. Blindly + // try_emplace-ing would recreate a queue for a disconnected client, + // stranding the frame until shutdown and skewing dropped_frame_count(). + // Treat it like the top-of-function lookup miss: the client is gone. + if (clients_.find(client_identity) == clients_.end()) { + return false; + } + auto [pending_it, inserted] = pending_frames_.try_emplace(client_identity, BoundedFrameQueue(client_backlog_size_)); + (void)inserted; + dropped = pending_it->second.push(data); + dropped_total_now = pending_it->second.dropped_total(); + } + + if (dropped > 0) { + auto now = std::chrono::steady_clock::now(); + bool should_warn; + { + std::lock_guard lock(clients_mutex_); + auto warn_it = last_drop_warn_.find(client_identity); + should_warn = + warn_it == last_drop_warn_.end() || now - warn_it->second >= std::chrono::seconds(kDropWarnIntervalSeconds); + if (should_warn) { + last_drop_warn_[client_identity] = now; + } + } + if (should_warn) { + spdlog::warn( + "Slow client '{}': dropping oldest queued frame(s) ({} dropped total)", client_identity, dropped_total_now); + } + } + + // The frame was accepted for later delivery, not a failure. + return true; +} + +void WebSocketMiddleware::drop_pending(const std::string& client_identity) { + // The client's logical session was destroyed server-side (e.g. heartbeat + // timeout) while its socket may still be open. Discard its backpressure + // backlog so stale frames can't flush into a future session on the same + // connection, folding the drop count into the lifetime total. The socket + // itself is untouched: clients_ still owns it until the real disconnect. + std::lock_guard lock(clients_mutex_); + auto pending_it = pending_frames_.find(client_identity); + if (pending_it != pending_frames_.end()) { + dropped_from_disconnected_ += pending_it->second.dropped_total(); + pending_frames_.erase(pending_it); + } + last_drop_warn_.erase(client_identity); +} + +uint64_t WebSocketMiddleware::dropped_frame_count() const { + std::lock_guard lock(clients_mutex_); + uint64_t total = dropped_from_disconnected_; + for (const auto& entry : pending_frames_) { + total += entry.second.dropped_total(); + } + return total; } bool WebSocketMiddleware::publish_data(const std::vector& data) { diff --git a/app/src/session_manager.cpp b/app/src/session_manager.cpp index a8c7008..b9f24c1 100644 --- a/app/src/session_manager.cpp +++ b/app/src/session_manager.cpp @@ -222,4 +222,27 @@ bool SessionManager::is_paused(const std::string& client_id) const { return it->second.paused; } +bool SessionManager::set_topic_updates(const std::string& client_id, bool enabled) { + std::lock_guard lock(mutex_); + + auto it = sessions_.find(client_id); + if (it == sessions_.end()) { + return false; + } + + it->second.topic_updates = enabled; + return true; +} + +bool SessionManager::wants_topic_updates(const std::string& client_id) const { + std::lock_guard lock(mutex_); + + auto it = sessions_.find(client_id); + if (it == sessions_.end()) { + return false; + } + + return it->second.topic_updates; +} + } // namespace pj_bridge diff --git a/app/src/standalone_event_loop.cpp b/app/src/standalone_event_loop.cpp index 925b16e..73d6d94 100644 --- a/app/src/standalone_event_loop.cpp +++ b/app/src/standalone_event_loop.cpp @@ -49,12 +49,20 @@ void run_standalone_event_loop( spdlog::info("Bridge server initialized, entering event loop"); + if (config.topic_poll_interval > 0.0) { + // Take the silent baseline snapshot now, before any client can connect, + // so topics appearing before the first poll tick are notified rather + // than folded into the baseline. + server.check_topic_changes(); + } + using clock = std::chrono::steady_clock; auto publish_interval = std::chrono::duration_cast(std::chrono::duration(1.0 / config.publish_rate)); auto last_publish = clock::now(); auto last_timeout_check = clock::now(); auto last_stats_print = clock::now(); + auto last_topic_poll = clock::now(); while (!g_shutdown.load()) { server.process_requests(); @@ -71,6 +79,13 @@ void run_standalone_event_loop( last_timeout_check = now; } + if (config.topic_poll_interval > 0.0 && + now - last_topic_poll >= std::chrono::duration_cast( + std::chrono::duration(config.topic_poll_interval))) { + server.check_topic_changes(); + last_topic_poll = now; + } + if (config.stats_enabled && now - last_stats_print >= std::chrono::seconds(5)) { auto elapsed = std::chrono::duration(now - last_stats_print).count(); auto snapshot = server.snapshot_and_reset_stats(); @@ -88,6 +103,7 @@ void run_standalone_event_loop( fmt::format("\n Publish frequency: {:.1f} Hz", static_cast(snapshot.publish_cycles) / elapsed); stats_msg += fmt::format( "\n Sent: {:.2f} MB/s", static_cast(snapshot.total_bytes_published) / elapsed / (1024.0 * 1024.0)); + stats_msg += fmt::format("\n Dropped frames (slow clients): {}", middleware->dropped_frame_count()); spdlog::info(stats_msg); last_stats_print = now; diff --git a/app/src/whitelist_filter.cpp b/app/src/whitelist_filter.cpp new file mode 100644 index 0000000..4d0f48d --- /dev/null +++ b/app/src/whitelist_filter.cpp @@ -0,0 +1,51 @@ +/* + * 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/whitelist_filter.hpp" + +namespace pj_bridge { + +tl::expected WhitelistFilter::create(const std::vector& patterns) { + WhitelistFilter filter; + filter.patterns_.reserve(patterns.size()); + + for (const auto& pattern : patterns) { + try { + filter.patterns_.emplace_back(pattern, std::regex::ECMAScript); + } catch (const std::regex_error& e) { + return tl::unexpected("Invalid whitelist regex '" + pattern + "': " + e.what()); + } + } + + return filter; +} + +bool WhitelistFilter::matches(const std::string& topic_name) const { + if (patterns_.empty()) { + return true; + } + for (const auto& pattern : patterns_) { + if (std::regex_match(topic_name, pattern)) { + return true; + } + } + return false; +} + +} // namespace pj_bridge diff --git a/docs/API.md b/docs/API.md index ebfcc90..a6acc47 100644 --- a/docs/API.md +++ b/docs/API.md @@ -71,6 +71,10 @@ sequenceDiagram Discover available ROS2 topics. +If a `topic_whitelist` is configured on the server, topics whose name does not +fully match any whitelist pattern are omitted from the response entirely (see +[Topic Whitelist](#topic-whitelist) below). + **Request:** ```json {"command": "get_topics", "id": "gt1"} @@ -88,10 +92,60 @@ Discover available ROS2 topics. } ``` +## Topic Whitelist + +The server can be configured with a list of regex patterns restricting which +topics are visible and subscribable, mirroring foxglove_bridge's +`topic_whitelist` option: + +- **ROS2**: string-array parameter `topic_whitelist`, default `[".*"]` (match everything). +- **FastDDS / RTI**: repeatable CLI flag `--topic-whitelist`, default `.*`. + +Matching uses **full-match** ECMAScript regex semantics (`std::regex_match`, +not a substring/prefix search): a pattern must match the *entire* topic name. +For example, pattern `/cam` does **not** match `/camera`, but `/camera.*` +matches `/camera` and `/camera/image`. A topic is allowed if it fully matches +**any** configured pattern. An empty pattern list (or the default `.*`) +matches every topic. + +Non-whitelisted topics are excluded from `get_topics` responses, and +`subscribe` requests targeting them fail per-topic with reason +`"Topic not whitelisted"` (see below). + +## QoS Depth Heuristics (ROS2 only) + +When creating a subscription, the ROS2 backend picks a KEEP_LAST history +depth by summing the history depth every discovered publisher on the topic +offers (so a burst from every publisher still fits in the subscription +queue), then clamping the total to a configurable range — the same heuristic +`foxglove_bridge`'s `determineQoS()` uses: + +- **ROS2**: int parameters `min_qos_depth` (default `1`) and `max_qos_depth` + (default `100`). + +A publisher that reports depth `0` (KEEP_ALL, or an RMW such as +`rmw_fastrtps_cpp` that does not propagate history depth through discovery) +counts as `100` — the historical default — rather than `0`, so unknown +depths never shrink the queue. If no publishers are discovered yet, the +depth likewise defaults to `min(100, max_qos_depth)`. Both values must be +`>= 0` and `min_qos_depth <= max_qos_depth`; the server refuses to start +otherwise. This is independent of +the subscription's reliability/durability, which is separately adapted to +match what the discovered publishers offer (a RELIABLE subscription still +switches to BEST_EFFORT if any publisher is BEST_EFFORT, and to +TRANSIENT_LOCAL only if every publisher offers it). + ## 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. +If the server has a `topic_whitelist` configured, requests for topics that +don't fully match any whitelist pattern fail with reason +`"Topic not whitelisted"` — same failure shape as a nonexistent topic (see +[Topic Whitelist](#topic-whitelist)). If every requested topic is rejected +this way, the response is `status: "error"` with +`error_code: "ALL_SUBSCRIPTIONS_FAILED"`. + Each topic in the array can be either a plain string or an object with a `max_rate_hz` field for per-topic rate limiting. Both formats can be mixed in the same request. When `max_rate_hz` is set, the server decimates messages for that topic, sending at most one message per rate interval (the first eligible buffered message). An explicit value of `0` means unlimited (all messages forwarded). A plain string leaves the rate unspecified: new subscriptions default to unlimited, and re-subscribing to an already-subscribed topic with a plain string preserves its previously configured rate limit. @@ -151,6 +205,51 @@ The `rate_limits` field is only present when at least one topic has a non-zero r } ``` +## Latched topics (transient_local) + +*ROS2 backend only, for now.* + +When a topic's publishers all offer `TRANSIENT_LOCAL` durability (e.g. +`/tf_static`, `/robot_description`), the bridge treats it as **latched**. A +brand-new subscription receives the publisher's retained sample directly +from DDS, but the bridge's underlying middleware subscription is shared and +reference-counted across clients — a client subscribing to a topic that +another client is *already* subscribed to does not create a new DDS +subscription, so it would otherwise have to wait for the next publish (which, +for a latched topic like `/tf_static`, may never come). + +To cover this case, the bridge retains the most recent message for each +latched topic outside the normal 1-second message buffer. Immediately after +a successful `subscribe` response, the server sends one extra binary frame +per newly-subscribed latched topic containing just that retained message — +same [Binary Message Format](#binary-message-format) as regular aggregated +frames, just with a single message and the original (possibly "stale") +timestamp from when it was received. No client-side handling is required +beyond decoding it like any other binary frame. + +Only the single newest sample per latched topic is retained (bounded memory +use), and only for topics that have had at least one subscriber — the +first subscriber's DDS-native delivery is what seeds the retained copy for +later subscribers. When the last subscriber of a latched topic leaves +(unsubscribe, pause, or disconnect), the retained sample is discarded along +with the underlying subscription: the next subscriber gets the current +sample from DDS redelivery, never a stale copy. + +Replay also happens on `resume` for latched topics whose subscription +reference is re-acquired at that point (i.e. topics that were subscribed +while the client was paused): the replay frame is delivered right after the +resume response, exactly like the post-subscribe case. + +No replay frame is sent when the retained message is still pending in the +regular aggregation buffer — in that case the next aggregated frame delivers +it to the new subscriber anyway, and a replay would duplicate it. Replay +frames intentionally bypass per-topic rate limiting (`max_rate_hz`) and are +not counted in the server's publish statistics. They are, however, subject +to the same [slow-client backpressure](#slow-clients--backpressure) queueing +as regular aggregated frames: a replay destined for an already-slow client +may be delayed until the next send attempt for that client (and, like any +queued frame, can be dropped if the backlog overflows). + ## Unsubscribe Remove topics from subscription. Only removes specified topics; other subscriptions are preserved. @@ -204,6 +303,63 @@ If some subscribed topics are not currently available (publisher down) or fail t Both commands are idempotent. Smart ROS2 management: when all clients interested in a topic are paused, the ROS2 subscription is released. +## Pushed Topic Advertisement (topics_changed) + +Clients can opt in to be notified when the server's (whitelist-filtered) +topic set changes, instead of polling `get_topics`. + +**Subscribe Request:** +```json +{"command": "subscribe_topic_updates", "id": "tu1"} +``` + +**Subscribe Response:** +```json +{"status": "ok", "id": "tu1", "protocol_version": 1, "topic_updates": true} +``` + +**Unsubscribe Request:** +```json +{"command": "unsubscribe_topic_updates", "id": "tu2"} +``` + +**Unsubscribe Response:** +```json +{"status": "ok", "id": "tu2", "protocol_version": 1, "topic_updates": false} +``` + +Both commands are idempotent and create a session if the client does not +already have one (like `pause`/`resume`). + +The server periodically polls the topic graph (see `topic_poll_interval` +below) and, for every session currently opted in, sends a **notification** +(not a response to any request — it has no `id`) whenever the topic set has +changed since the last poll: + +```json +{"notification": "topics_changed", + "added": [{"name": "/t", "type": "pkg/msg/T"}], + "removed": ["/gone"], + "protocol_version": 1} +``` + +- `added` and `removed` may each be empty, but a notification is only sent + 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. +- Only whitelisted topics (see [Topic Whitelist](#topic-whitelist)) are + considered — a non-whitelisted topic appearing or disappearing never + triggers a notification. +- The very first poll after the server starts never sends a notification (it + only establishes the initial snapshot); notifications begin from the + second poll onward. +- **Clients must tolerate unknown `notification` types** appearing in future + protocol versions and ignore ones they don't recognize. + +The poll interval is configurable: +- **ROS2**: double parameter `topic_poll_interval`, default `1.0` seconds. `0` disables polling entirely. +- **FastDDS / RTI**: CLI flag `--topic-poll-interval`, default `1.0` seconds. `0` disables polling entirely. + ## Heartbeat Clients must send a heartbeat at least once per second. The default timeout is 10 seconds. @@ -234,6 +390,83 @@ All commands may return an error: Error codes: `INVALID_REQUEST`, `INVALID_JSON`, `UNKNOWN_COMMAND`, `ALL_SUBSCRIPTIONS_FAILED`, `INTERNAL_ERROR`. +## Slow clients / backpressure + +If a client can't keep up with the aggregated-message stream (e.g. a slow +network link or a busy renderer), the server never blocks the publish loop +waiting for it and never disconnects it for falling behind — matching +foxglove_bridge's slow-client policy. Instead, once a client's outgoing +socket buffer exceeds a 1 MiB high watermark, new binary frames destined for +that client are held in a small per-client queue rather than sent +immediately. If that queue is already full, the **oldest** queued frame is +dropped to make room for the newest one, so a lagging client always +eventually resumes with fresh data instead of a growing backlog of stale +frames. Queued frames are delivered, in order, on the **next send attempt** +for that client once its socket buffer has drained back below the watermark +(there is no background flush timer — if the client's topics go quiet, the +backlog waits for the next frame destined for that client). The JSON +control-plane (requests and their replies, including `heartbeat`) is never +affected — those messages are always sent immediately. If a client's session +times out server-side while its socket stays open, any backlog still queued +for it is discarded. + +Frames already queued (or already sitting in the socket's send buffer) when +an `unsubscribe` is processed may still arrive **after** the unsubscribe +response — clients must tolerate binary messages for recently-unsubscribed +topics. This is inherent to socket buffering, not specific to the backlog +queue. + +The queue depth (max frames held per client before the oldest is dropped) is +configurable: + +- **ROS2**: int parameter `client_backlog_size`, default `100`. Must be `> 0`; + the server refuses to start otherwise. +- **FastDDS / RTI**: CLI flag `--client-backlog-size`, default `100`, valid + range `1`-`1000000`. + +## TLS / wss:// + +The bridge can optionally serve the WebSocket endpoint over TLS (`wss://`) +using a server certificate + private key (OpenSSL). Client certificate +verification is not supported — this is server-authentication only. + +TLS support depends on IXWebSocket having been built with OpenSSL +(`PJ_BRIDGE_TLS=ON`, the CMake default for FetchContent builds; a +system/conda-provided IXWebSocket must likewise have been built with TLS — +check for `IXWEBSOCKET_USE_TLS` in its exported CMake target). If TLS is +requested but the linked IXWebSocket lacks TLS support, the server fails to +start with an explicit error instead of silently falling back to plaintext. + +Configuration: + +- **ROS2**: parameters `tls` (bool, default `false`), `certfile` (string, + default `""`), `keyfile` (string, default `""`). Setting `tls: true` + without both `certfile` and `keyfile` set is a startup error. +- **FastDDS / RTI**: CLI flags `--certfile ` and `--keyfile `. + Passing one without the other is a CLI11 parse error (`->needs()`); passing + both enables TLS. + +Note that only certificate/key file *readability* is validated at startup; a +mismatched certificate/key pair is not detected until clients connect, and +then only surfaces as per-connection TLS handshake failures in the server +log (IXWebSocket defers TLS setup to accept time). + +Example: generate a self-signed certificate and start the ROS2 backend with +TLS enabled: + +```bash +openssl req -x509 -newkey rsa:2048 -keyout key.pem -out cert.pem -days 365 -nodes -subj "/CN=localhost" + +ros2 run pj_bridge pj_bridge_ros2 --ros-args \ + -p tls:=true -p certfile:=cert.pem -p keyfile:=key.pem +``` + +Clients then connect via `wss://` instead of `ws://` (e.g. +`wss://127.0.0.1:9090`). With a self-signed certificate, the client must +either trust the certificate explicitly or disable peer verification (e.g. +IXWebSocket's `caFile = "NONE"`) — production deployments should use a +certificate signed by a trusted CA instead. + ## Binary Message Format Binary frames consist of a fixed 16-byte header followed by ZSTD-compressed payload. diff --git a/docs/superpowers/plans/2026-07-06-foxglove-parity.md b/docs/superpowers/plans/2026-07-06-foxglove-parity.md new file mode 100644 index 0000000..2ab1bc9 --- /dev/null +++ b/docs/superpowers/plans/2026-07-06-foxglove-parity.md @@ -0,0 +1,215 @@ +# Foxglove-parity Features Implementation Plan + +> **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:** Implement the six features in `docs/superpowers/specs/2026-07-06-foxglove-parity-design.md`: topic whitelist, QoS depth heuristics, pushed topic advertisement, slow-client backpressure, transient_local replay, TLS. + +**Architecture:** Backend-agnostic changes go in `app/` (core, no ROS deps); ROS2-specific changes in `ros2/`. All protocol changes are additive (protocol_version stays 1). One commit per task. + +**Tech Stack:** C++17, gtest, nlohmann/json, IXWebSocket 11.4.6, ZSTD, spdlog, rclcpp (Humble). + +**Build/test (from the worktree root `.worktrees/foxglove-parity`):** +```bash +pixi run -e humble build # colcon build (Release) +pixi run -e humble test # colcon test + result; baseline = 177 tests green +``` +Format before every commit: `pre-commit run -a` (clang-format). Follow `.clang-tidy` naming: classes `CamelCase`, methods `lower_case`, members `trailing_underscore_`, constants `kCamelCase`. Core code logs via `spdlog::*`, never `RCLCPP_*` (except `ros2/` adapter code). New files carry the AGPL header found at the top of every existing file (copy from `app/src/bridge_server.cpp:1-18`). + +**Key existing files (read before coding):** +- `app/include/pj_bridge/bridge_server.hpp` + `app/src/bridge_server.cpp` — orchestrator; request dispatch at `bridge_server.cpp:182-196`; subscribe validation loop at `:299-327`; publish/fan-out loop at `:677-827`; lock ordering documented at `bridge_server.hpp:163-166` (`cleanup_mutex_ > last_sent_mutex_ > stats_mutex_`). +- `app/include/pj_bridge/session_manager.hpp` — per-session state (pause flag, ref-held topics) — mimic its patterns for new per-session flags. +- `app/include/pj_bridge/message_buffer.hpp` + `.cpp` — TTL buffer; injectable `ClockFn` for tests. +- `app/include/pj_bridge/middleware/websocket_middleware.hpp` + `app/src/middleware/websocket_middleware.cpp` — IXWebSocket server; per-client `ix::WebSocket` map `clients_`. +- `ros2/src/generic_subscription_manager.cpp:28-61` — `adapt_qos()`. +- `ros2/src/main.cpp:39-47` — ROS2 param declarations; `fastdds/src/main.cpp` — CLI11 flags; `app/src/standalone_event_loop.cpp` — FastDDS/RTI event loop. +- `tests/unit/test_bridge_server.cpp` — has mock `TopicSourceInterface` / `SubscriptionManagerInterface` / `MiddlewareInterface` implementations; extend those mocks, don't invent new ones. + +--- + +### Task 1: Topic whitelist regex + +**Files:** +- Create: `app/include/pj_bridge/whitelist_filter.hpp`, `app/src/whitelist_filter.cpp` +- Create: `tests/unit/test_whitelist_filter.cpp` +- Modify: `app/include/pj_bridge/bridge_server.hpp` (ctor param + member), `app/src/bridge_server.cpp` (`handle_get_topics`, `handle_subscribe`) +- Modify: `ros2/src/main.cpp`, `fastdds/src/main.cpp`, `rti/src/main.cpp` (params/flags) +- Modify: `CMakeLists.txt` (add new .cpp to `pj_bridge_app` and test target), `docs/API.md` +- Test: `tests/unit/test_whitelist_filter.cpp`, extend `tests/unit/test_bridge_server.cpp` + +- [ ] **Step 1: Write failing tests for `WhitelistFilter`** — construction from patterns, ECMAScript `std::regex_match` (full match: pattern `/cam` must NOT match `/camera`), default-constructed filter matches everything, `create()` returns `tl::unexpected` with the offending pattern for invalid regex (e.g. `"("`). + +Class contract: +```cpp +class WhitelistFilter { + public: + WhitelistFilter() = default; // matches everything + static tl::expected create(const std::vector& patterns); + bool matches(const std::string& topic_name) const; + private: + std::vector patterns_; // empty = match all +}; +``` + +- [ ] **Step 2: Run the new test, verify it fails to compile/link.** +- [ ] **Step 3: Implement `WhitelistFilter`** (compile with `std::regex::ECMAScript`; `matches` = any pattern full-matches; add file to `pj_bridge_app` sources in CMakeLists.txt). +- [ ] **Step 4: Run tests, verify green.** +- [ ] **Step 5: Write failing BridgeServer tests**: (a) `get_topics` response omits non-whitelisted topics; (b) `subscribe` to a non-whitelisted topic yields a per-topic failure with `"reason": "Topic not whitelisted"` (and `ALL_SUBSCRIPTIONS_FAILED` when it was the only topic); (c) whitelisted topics still subscribe fine. +- [ ] **Step 6: Integrate**: add `WhitelistFilter whitelist = {}` as the **last** BridgeServer ctor param (existing call sites unaffected). In `handle_get_topics` skip non-matching topics; in `handle_subscribe` add the whitelist check at the top of the per-topic validation loop (`bridge_server.cpp:299`), before the existence check. +- [ ] **Step 7: Run tests, verify green.** +- [ ] **Step 8: Wire config**: ROS2 string-array param `topic_whitelist` default `[".*"]` (declare next to `ros2/src/main.cpp:42`); CLI11 repeatable `--topic-whitelist` default `{".*"}` in `fastdds/src/main.cpp` and `rti/src/main.cpp`. On `WhitelistFilter::create()` error: log the message and exit 1. Document the param and the full-match semantics in `docs/API.md` (get_topics + subscribe sections). +- [ ] **Step 9: Full build + test + `pre-commit run -a`.** +- [ ] **Step 10: Commit** `feat: add topic whitelist regex filtering`. + +### Task 2: QoS depth heuristics (ROS2) + +**Files:** +- Modify: `ros2/include/pj_bridge_ros2/generic_subscription_manager.hpp`, `ros2/src/generic_subscription_manager.cpp` (`adapt_qos`), `ros2/include/pj_bridge_ros2/ros2_subscription_manager.hpp` + `.cpp` (pass-through), `ros2/src/main.cpp` +- Test: extend `tests/unit/test_generic_subscription_manager.cpp` + +- [ ] **Step 1: Write failing tests**: publisher with depth 10 → subscription depth ≥ 10; two publishers depth 60 each → depth clamped to `max_qos_depth` (100); no publishers → depth = `max_qos_depth`? No — **no publishers → keep depth = max(min_qos_depth, 100 default path unchanged)**; precise rule below. Test via `node->create_publisher` with explicit `rclcpp::QoS(depth)` and inspecting the returned `rclcpp::QoS` from `adapt_qos` (make `adapt_qos` public or keep tests as friend — it is already `const` public in the header; check). + +Depth rule (adapted from foxglove_bridge `determineQoS()` — add attribution comment in the file: *"Depth aggregation adapted from foxglove_bridge (MIT License, Copyright (c) Foxglove Technologies Inc)"*): +``` +total = sum over publishers of profile.depth() // depth 0 (unknown/KEEP_ALL) contributes 0 +depth = clamp(total, min_qos_depth, max_qos_depth) +if publishers.empty(): depth = min(100, max_qos_depth) // preserve current default behavior +``` +- [ ] **Step 2: Run tests, verify failure.** +- [ ] **Step 3: Implement**: `GenericSubscriptionManager` ctor gains `size_t min_qos_depth = 1, size_t max_qos_depth = 100`; `Ros2SubscriptionManager` forwards them; reliability/durability logic at `generic_subscription_manager.cpp:39-58` unchanged. +- [ ] **Step 4: Run tests, verify green.** +- [ ] **Step 5: Wire params** `min_qos_depth` (int, 1) and `max_qos_depth` (int, 100) in `ros2/src/main.cpp`; document in `docs/API.md` config section and README parameter table. +- [ ] **Step 6: Full build + test + format. Commit** `feat: aggregate publisher QoS depths with min/max clamping`. + +### Task 3: Pushed topic advertisement (opt-in) + +**Files:** +- Modify: `app/include/pj_bridge/session_manager.hpp` + `app/src/session_manager.cpp` (per-session `wants_topic_updates` flag, default false, following the existing `paused` flag pattern) +- Modify: `app/include/pj_bridge/bridge_server.hpp` + `app/src/bridge_server.cpp` +- Modify: `ros2/src/main.cpp`, `app/include/pj_bridge/standalone_event_loop.hpp` + `app/src/standalone_event_loop.cpp`, `fastdds/src/main.cpp`, `rti/src/main.cpp` +- Modify: `docs/API.md` +- Test: extend `tests/unit/test_bridge_server.cpp`, `tests/unit/test_session_manager.cpp` + +- [ ] **Step 1: Write failing SessionManager tests** for `set_topic_updates(client_id, bool)` / `wants_topic_updates(client_id)` (default false; false for unknown client). +- [ ] **Step 2: Implement the flag. Tests green.** +- [ ] **Step 3: Write failing BridgeServer tests**: + - `{"command": "subscribe_topic_updates"}` → `{"status": "ok", "topic_updates": true, ...}`; `unsubscribe_topic_updates` → `topic_updates: false`. Both idempotent, both create a session if needed (copy the pattern from `handle_pause`). + - New public method `void check_topic_changes()`: first call snapshots silently (no notification); after the mock topic source adds `/new` and drops `/old`, the **opted-in** client receives exactly one text frame `{"notification": "topics_changed", "added": [{"name": "/new", "type": "..."}], "removed": ["/old"], "protocol_version": 1}`; a non-opted-in client receives nothing; no change → no notification; non-whitelisted topics (Task 1 filter) never appear in the diff. +- [ ] **Step 4: Implement**: dispatch the two commands in `process_single_request` (`bridge_server.cpp:182-196`); `check_topic_changes()` diffs `topic_source_->get_topics()` (whitelist-filtered) against member `std::unordered_map known_topics_` guarded by a new `topics_mutex_` (leaf lock — never held while taking any existing lock); send via `middleware_->send_reply()` to each session with the flag. +- [ ] **Step 5: Tests green.** +- [ ] **Step 6: Wire timers**: ROS2 param `topic_poll_interval` (double, default 1.0, `0` disables) + wall timer in `ros2/src/main.cpp`; add the same interval arg to `run_standalone_event_loop()` (follow how `check_session_timeouts` is scheduled there) + `--topic-poll-interval` CLI flag in `fastdds/src/main.cpp` / `rti/src/main.cpp`. Document command, notification, and param in `docs/API.md`. +- [ ] **Step 7: Full build + test + format. Commit** `feat: opt-in pushed topic advertisement (topics_changed notification)`. + +### Task 4: Slow-client backpressure (drop-oldest) + +**Files:** +- Create: `app/include/pj_bridge/middleware/bounded_frame_queue.hpp` (header-only helper) +- Create: `tests/unit/test_bounded_frame_queue.cpp` +- Modify: `app/include/pj_bridge/middleware/websocket_middleware.hpp` + `app/src/middleware/websocket_middleware.cpp` +- Modify: `ros2/src/main.cpp`, `fastdds/src/main.cpp`, `rti/src/main.cpp` (`client_backlog_size` param/flag) +- Test: `tests/unit/test_bounded_frame_queue.cpp`, extend `tests/unit/test_websocket_middleware.cpp` + +- [ ] **Step 1: Write failing tests for `BoundedFrameQueue`** (pure, no sockets — this is what makes the policy unit-testable): `push` returns number of dropped frames (0 normally); pushing past capacity drops the **oldest**; `pop_front`/`empty`/`size`; `dropped_total()` accumulates. + +```cpp +class BoundedFrameQueue { + public: + explicit BoundedFrameQueue(size_t max_frames); + size_t push(std::vector frame); // returns #dropped (0 or 1) + std::optional> pop_front(); + bool empty() const; size_t size() const; uint64_t dropped_total() const; +}; +``` +- [ ] **Step 2: Implement; tests green.** +- [ ] **Step 3: Integrate into `WebSocketMiddleware`**: ctor gains `size_t client_backlog_size = 100`; per-client `BoundedFrameQueue` map guarded by `clients_mutex_` (erased on disconnect alongside `clients_`). New `send_binary` logic, adapted from foxglove_bridge's lossy send policy (MIT attribution comment): + 1. If `ws->bufferedAmount() < kSocketBufferHighWatermark` (constant, `1u << 20` = 1 MiB): flush queued frames in order via `sendBinary`, then send the new frame. + 2. Else enqueue; on drop, warn at most once per 30 s per client (`kDropWarnIntervalSeconds = 30`), including the cumulative drop count. + - JSON replies (`send_reply`) are untouched — direct send only. + - Add `uint64_t dropped_frame_count() const` (sum across clients) to `WebSocketMiddleware` (NOT to `MiddlewareInterface`); print it in the `--stats` output of `fastdds/src/main.cpp`. +- [ ] **Step 4: Extend `test_websocket_middleware.cpp`**: normal send path still works end-to-end (existing loopback tests keep passing); `dropped_frame_count()` starts at 0. +- [ ] **Step 5: Full build + test + format. Also run TSAN** (new mutex interactions): +```bash +pixi run -e humble bash -c 'colcon build --packages-select pj_bridge --build-base build_tsan --install-base install_tsan --cmake-args -DCMAKE_BUILD_TYPE=Release -DENABLE_TSAN=ON && source install_tsan/setup.bash && TSAN_OPTIONS="suppressions=$(pwd)/tsan_suppressions.txt" setarch $(uname -m) -R build_tsan/pj_bridge/pj_bridge_tests' +``` +(exit code 66 = pre-existing suppressed IXWebSocket warnings, acceptable; NEW data-race reports are not). +- [ ] **Step 6: Commit** `feat: bounded per-client send queue with drop-oldest backpressure`. + +### Task 5: transient_local replay (ROS2, latest message) + +**Files:** +- Modify: `app/include/pj_bridge/subscription_manager_interface.hpp` (new virtual with default) +- Modify: `ros2/include/pj_bridge_ros2/generic_subscription_manager.hpp` + `.cpp`, `ros2/include/pj_bridge_ros2/ros2_subscription_manager.hpp` + `.cpp` +- Modify: `app/include/pj_bridge/message_buffer.hpp` + `app/src/message_buffer.cpp` +- Modify: `app/src/bridge_server.cpp` (`initialize` callback + `handle_subscribe`) +- Modify: `docs/API.md` +- Test: extend `tests/unit/test_message_buffer.cpp`, `tests/unit/test_bridge_server.cpp`, `tests/unit/test_generic_subscription_manager.cpp` + +- [ ] **Step 1: Interface**: add to `SubscriptionManagerInterface`: +```cpp +/// True when every publisher of the topic offers TRANSIENT_LOCAL durability +/// (detected at subscribe time). Backends without this info return false. +virtual bool is_transient_local(const std::string& /*topic_name*/) const { return false; } +``` +Non-pure with default → FastDDS/RTI/mocks unchanged. +- [ ] **Step 2: Write failing tests**: `GenericSubscriptionManager::is_transient_local` true after subscribing to a topic whose (test) publisher is transient_local, false otherwise (store the `all_transient_local` result computed in `adapt_qos` into `SubscriptionInfo` at subscribe time — `generic_subscription_manager.cpp:80-82`). +- [ ] **Step 3: Implement; tests green.** (`Ros2SubscriptionManager` forwards to the wrapped manager.) +- [ ] **Step 4: Write failing MessageBuffer tests**: `set_latched("/t", true)` → after `add_message` + TTL expiry (use injectable `ClockFn`) + `move_messages`, `get_latched("/t")` still returns the newest message; `set_latched("/t", false)` clears it; non-latched topics return `nullopt`; `clear()` clears latched storage too. + +New API on MessageBuffer: +```cpp +void set_latched(const std::string& topic_name, bool latched); +std::optional get_latched(const std::string& topic_name) const; +``` +Implementation: separate `std::unordered_map latched_last_` + `std::unordered_set latched_topics_`, updated inside `add_message` (normal queue behavior unchanged; `move_messages` does NOT touch the latched store). Retained entries persist until `set_latched(false)` or `clear()` — bounded at one message per latched topic. +- [ ] **Step 5: Implement; tests green.** +- [ ] **Step 6: Write failing BridgeServer test**: mock sub manager reports `is_transient_local("/latched") == true`; first client subscribes, a message arrives, TTL passes; a **second** client subscribes → that client immediately receives a single-message binary frame containing the retained message (decode it with the test helper already used by binary-frame assertions in `test_bridge_server.cpp`); the first client receives nothing extra. +- [ ] **Step 7: Implement in BridgeServer**: + - In `handle_subscribe`, after a successful `subscription_manager_->subscribe(...)` (`bridge_server.cpp:340-348`): call `message_buffer_->set_latched(topic, subscription_manager_->is_transient_local(topic))`. + - Still inside the success path: if latched and `get_latched(topic)` has a value, serialize that one message with `AggregatedMessageSerializer` and `middleware_->send_binary(client_id, frame)` immediately (rate-limit state untouched — replay bypasses it by construction). + - Rationale (comment it): a brand-new first subscription receives the sample from DDS naturally; the replay path covers later clients joining an already-shared subscription whose sample aged out of the TTL buffer. +- [ ] **Step 8: Full build + test + format. Commit** `feat: replay latest transient_local message to late subscribers (ROS2)`. + +### Task 6: TLS (OpenSSL, server cert only) + +**Files:** +- Modify: `CMakeLists.txt` (option `PJ_BRIDGE_TLS` default ON; FetchContent branch: `set(USE_TLS ${PJ_BRIDGE_TLS} ...)` + `set(USE_OPEN_SSL ON ...)` replacing the hardcoded `USE_TLS OFF` at `CMakeLists.txt:64`) +- Modify: `app/include/pj_bridge/middleware/websocket_middleware.hpp` + `app/src/middleware/websocket_middleware.cpp` +- Modify: `ros2/src/main.cpp`, `fastdds/src/main.cpp`, `rti/src/main.cpp`, `docs/API.md`, `.github` CI if OpenSSL dev headers are missing (pixi.toml `openssl` dependency if needed) +- Test: extend `tests/unit/test_websocket_middleware.cpp` + +- [ ] **Step 1: CMake**: flip the FetchContent flags; verify a clean configure+build passes locally (OpenSSL comes from the pixi env; add `openssl` to pixi.toml dependencies if configure fails to find it). +- [ ] **Step 2: Write failing tests**: `WebSocketMiddleware` constructed with `TlsConfig{"/nonexistent.pem", "/nonexistent.key"}` → `initialize()` returns an error mentioning the missing file; TLS round-trip test (guarded by `#ifdef IXWEBSOCKET_USE_TLS`): generate a self-signed cert in the test fixture via +```bash +openssl req -x509 -newkey rsa:2048 -keyout key.pem -out cert.pem -days 1 -nodes -subj "/CN=localhost" +``` +(into a temp dir; `std::system` in the fixture), start middleware with TLS, connect an `ix::WebSocket` client with `tlsOptions.caFile = "NONE"` (disables verification), assert echo works. +- [ ] **Step 3: Implement**: +```cpp +struct TlsConfig { std::string certfile; std::string keyfile; }; +explicit WebSocketMiddleware(size_t client_backlog_size = 100, std::optional tls = std::nullopt); +``` +In `initialize()`: if TLS requested — fail with a clear message when the files don't exist/aren't readable, or when built without TLS (`#ifndef IXWEBSOCKET_USE_TLS` → error "IXWebSocket built without TLS support"); otherwise `ix::SocketTLSOptions opts; opts.tls = true; opts.certFile = ...; opts.keyFile = ...; opts.caFile = "NONE"; server_->setTLSOptions(opts);` before `listen()`. +- [ ] **Step 4: Tests green.** +- [ ] **Step 5: Wire config**: ROS2 params `tls` (bool, false), `certfile` (""), `keyfile` (""); `tls=true` with missing params → error + exit 1. CLI: `--certfile` + `--keyfile` (both or neither; CLI11 `needs()`), presence implies TLS. Document in `docs/API.md` + README (`wss://` URL note). +- [ ] **Step 6: Full build + test + format. Commit** `feat: optional TLS (wss://) via OpenSSL server certificate`. + +### Task 7: Documentation & changelog sweep + +**Files:** +- Modify: `README.md` (feature list + parameter tables), `CLAUDE.md` (config section, test count), `CHANGELOG.rst`, `docs/API.md` (final consistency pass) + +- [ ] **Step 1**: Update README parameter tables (all new params/flags for all backends), CLAUDE.md configuration + test-count sections, CHANGELOG.rst entry for the six features. +- [ ] **Step 2**: Re-read `docs/API.md` top-to-bottom for consistency (every new command/notification/param documented exactly once, examples valid JSON). +- [ ] **Step 3: Full build + test + TSAN + ASAN**: +```bash +pixi run -e humble bash -c 'colcon build --packages-select pj_bridge --build-base build_asan --install-base install_asan --cmake-args -DCMAKE_BUILD_TYPE=Release -DENABLE_ASAN=ON && source install_asan/setup.bash && ASAN_OPTIONS="new_delete_type_mismatch=0" LSAN_OPTIONS="suppressions=$(pwd)/asan_suppressions.txt" build_asan/pj_bridge/pj_bridge_tests' +``` +- [ ] **Step 4: Commit** `docs: document foxglove-parity features`. + +--- + +## Verification (whole plan) + +1. `pixi run -e humble build && pixi run -e humble test` — all tests green (baseline 177 + new). +2. TSAN and ASAN runs clean (modulo documented pre-existing suppressions). +3. `git log --oneline origin/main..HEAD` shows one commit per task + spec/plan docs. +4. `docs/API.md` documents: `topic_whitelist`, `min_qos_depth`/`max_qos_depth`, `subscribe_topic_updates`/`unsubscribe_topic_updates` + `topics_changed`, `client_backlog_size`, `topic_poll_interval`, `tls`/`certfile`/`keyfile`. diff --git a/docs/superpowers/specs/2026-07-06-foxglove-parity-design.md b/docs/superpowers/specs/2026-07-06-foxglove-parity-design.md new file mode 100644 index 0000000..4a15838 --- /dev/null +++ b/docs/superpowers/specs/2026-07-06-foxglove-parity-design.md @@ -0,0 +1,123 @@ +# Foxglove-parity features for pj_bridge — Design + +**Date:** 2026-07-06 +**Status:** Approved +**Scope:** Six features closing the gap with foxglove_bridge on the topic-subscription +feature set, identified in a side-by-side comparison of the two bridges. + +## Background + +pj_bridge leads foxglove_bridge on bandwidth efficiency (ZSTD-compressed aggregated +frames, per-topic rate limiting, subscription-group fan-out) but lacks several +robustness/deployment features foxglove_bridge has. This design adds them. Where logic +is adapted from foxglove_bridge (MIT license), the affected file carries a copyright +attribution notice; MIT is compatible with this repo's AGPL-3.0. + +All protocol changes are **additive**: `protocol_version` stays `1`. Old clients are +unaffected (no new unsolicited traffic unless a client opts in). + +## Feature 1 — Topic whitelist regex + +- New `WhitelistFilter` class in `app/` holding a list of compiled `std::regex` + (ECMAScript grammar, `std::regex_match` = full match, mirroring foxglove's + `isWhitelisted`). +- `BridgeServer` receives the filter via its config and applies it in two places: + 1. `get_topics` responses — non-matching topics are omitted. + 2. `subscribe` requests — non-whitelisted topics fail with reason + `"Topic not whitelisted"` (per-topic failure, consistent with the existing + partial_success model). +- Config: ROS2 string-array param `topic_whitelist` (default `[".*"]`); + FastDDS/RTI CLI flag `--topic-whitelist` (repeatable, default `.*`). +- Invalid regex at startup = fatal error with a clear message. + +## Feature 2 — QoS depth heuristics (ROS2 only) + +- Extend `adapt_qos()` in `GenericSubscriptionManager`: + `depth = clamp(sum(publisher depths), min_qos_depth, max_qos_depth)`. + Publishers reporting depth 0 (unknown/KEEP_ALL) contribute 0, as in foxglove. +- Defaults: `min_qos_depth = 1`, `max_qos_depth = 100` (preserves the current + effective ceiling of `rclcpp::QoS(100)`). +- Reliability and durability heuristics unchanged. +- Logic adapted from foxglove_bridge `determineQoS()` — attribution comment in file. +- Config: ROS2 params `min_qos_depth`, `max_qos_depth`. + +## Feature 3 — Pushed topic advertisement (opt-in per client) + +- New commands: + - `{"command": "subscribe_topic_updates"}` → `{"status": "ok", ...}`; sets a + per-session flag. + - `{"command": "unsubscribe_topic_updates"}` → clears it. Both idempotent. +- New server-initiated notification, sent **only** to opted-in clients whenever the + topic set changes: + + ```json + {"notification": "topics_changed", + "added": [{"name": "/t", "type": "pkg/msg/T"}], + "removed": ["/gone"], + "protocol_version": 1} + ``` + + `added`/`removed` may be empty arrays; a notification is sent only when at least + one of them is non-empty. The whitelist (Feature 1) is applied before diffing. +- Detection is backend-agnostic: `BridgeServer::check_topic_changes()` diffs + `TopicSourceInterface::get_topics()` against the previous snapshot. Entry points + drive it on a timer: new param/flag `topic_poll_interval` (seconds, default `1.0`; + `0` disables polling and the feature). + +## Feature 4 — Slow-client backpressure (foxglove-style) + +Policy per user decision: bounded per-client queue, **drop-oldest** on overflow, +throttled warnings, never disconnect for data-plane pressure. + +- Implemented entirely in `WebSocketMiddleware`; interfaces unchanged. +- Per-client bounded `std::deque` of pending **binary** frames in front of the socket: + - `send_binary()`: if the client's `ix::WebSocket::bufferedAmount()` is below a + 1 MiB high-watermark (`kSocketBufferHighWatermark`), flush the pending queue in + order, then send the new frame; otherwise enqueue the frame. + - On enqueue past `client_backlog_size` (default 100 frames ≈ 2 s at 50 Hz), drop + the **oldest** queued frame and increment a per-client dropped-frame counter. + - Warning log throttled to one per 30 s per client (foxglove's cadence). +- JSON replies (`send_reply`) are control-plane: direct send, never queued or dropped. +- Config: ROS2 param / CLI flag `client_backlog_size`. +- Dropped-frame totals exposed via the existing stats path. + +## Feature 5 — transient_local replay (ROS2, latest message) + +- `adapt_qos()` already detects the all-publishers-transient_local case; the + subscription manager records it and exposes + `bool is_transient_local(const std::string& topic) const` on + `SubscriptionManagerInterface`. FastDDS/RTI implementations return `false` (defer). +- `MessageBuffer` gains a per-topic *latched* mode (`set_latched(topic, bool)`): + the most recent message for a latched topic is exempt from TTL eviction — a + one-slot durable cache. It is cleared when the topic's subscription is destroyed. +- On a client `subscribe` to a latched topic, the retained message is force-included + in that client's next binary frame, bypassing the per-topic rate-limit timestamp + check exactly once (so `/tf_static`-style topics appear immediately even though + their message timestamp is old). + +## Feature 6 — TLS (OpenSSL, server cert only) + +- CMake: new option `PJ_BRIDGE_TLS` (default `ON`). The IXWebSocket FetchContent path + sets `USE_TLS=ON` and `USE_OPEN_SSL=ON`. If a system IXWebSocket without TLS is + found and `tls` is requested at runtime, startup fails with a clear error. +- `WebSocketMiddleware` constructor takes `std::optional{certfile, keyfile}` + → mapped to `ix::SocketTLSOptions`. `MiddlewareInterface` unchanged. +- Config: ROS2 params `tls` (bool, default false), `certfile`, `keyfile`; + FastDDS/RTI CLI `--certfile`/`--keyfile` (presence of both implies TLS). +- Missing or unreadable cert/key files fail startup with a clear error. +- Tests generate a self-signed cert via the `openssl` CLI in test setup. + +## Cross-cutting + +- `docs/API.md`: document the new commands, notification, and params. +- README/CLAUDE.md configuration tables updated. +- Every feature lands with unit tests in the existing gtest suites' style; the full + suite must stay TSAN/ASAN clean. +- One commit per feature on branch `feature/foxglove-parity`; single PR. + +## Non-goals + +- FastDDS/RTI transient_local replay (interface stub only). +- mTLS / client certificates. +- Per-publisher history-depth replay caches (latest-message only). +- Any change to the binary frame format or protocol version. diff --git a/fastdds/src/main.cpp b/fastdds/src/main.cpp index ce00885..3e04920 100644 --- a/fastdds/src/main.cpp +++ b/fastdds/src/main.cpp @@ -20,11 +20,13 @@ #include #include +#include #include #include #include "pj_bridge/middleware/websocket_middleware.hpp" #include "pj_bridge/standalone_event_loop.hpp" +#include "pj_bridge/whitelist_filter.hpp" #include "pj_bridge_fastdds/fastdds_subscription_manager.hpp" #include "pj_bridge_fastdds/fastdds_topic_source.hpp" @@ -37,6 +39,11 @@ int main(int argc, char* argv[]) { double session_timeout = 10.0; std::string log_level = "info"; bool stats_enabled = false; + std::vector topic_whitelist{".*"}; + double topic_poll_interval = 1.0; + int client_backlog_size = 100; + std::string certfile; + std::string keyfile; app.add_option("--domains,-d", domain_ids, "DDS domain IDs")->required()->expected(1, -1); app.add_option("--port,-p", port, "WebSocket port")->default_val(9090)->check(CLI::Range(1, 65535)); @@ -44,9 +51,31 @@ int main(int argc, char* argv[]) { app.add_option("--session-timeout", session_timeout, "Session timeout in seconds")->default_val(10.0); app.add_option("--log-level", log_level, "Log level (trace, debug, info, warn, error)")->default_val("info"); app.add_flag("--stats", stats_enabled, "Print statistics every 5 seconds"); + app.add_option( + "--topic-poll-interval", topic_poll_interval, + "Interval in seconds between topics_changed notification polls (0 disables)") + ->default_val(1.0); + app.add_option( + "--client-backlog-size", client_backlog_size, + "Max frames queued per slow client before dropping the oldest (backpressure)") + ->default_val(100) + ->check(CLI::Range(1, 1000000)); + // Bound variable is already initialized to {".*"} (match everything); CLI11 + // leaves it untouched if the flag is not passed, so no default_val() is + // needed (and default_val() on a vector would round-trip through a + // single joined string, which is not what we want here). + app.add_option("--topic-whitelist", topic_whitelist, "Topic whitelist regex patterns (full-match, ECMAScript)"); + + auto certfile_opt = + app.add_option("--certfile", certfile, "TLS server certificate file (enables wss://, requires --keyfile)"); + auto keyfile_opt = app.add_option("--keyfile", keyfile, "TLS private key file (enables wss://, requires --certfile)"); + certfile_opt->needs(keyfile_opt); + keyfile_opt->needs(certfile_opt); CLI11_PARSE(app, argc, argv); + bool tls_enabled = !certfile.empty(); + spdlog::set_level(spdlog::level::from_str(log_level)); spdlog::info("pj_bridge (FastDDS backend) starting..."); @@ -54,16 +83,38 @@ int main(int argc, char* argv[]) { spdlog::info(" Port: {}", port); spdlog::info(" Publish rate: {:.1f} Hz", publish_rate); spdlog::info(" Session timeout: {:.1f} s", session_timeout); + spdlog::info(" Topic whitelist: {}", fmt::join(topic_whitelist, ", ")); + spdlog::info(" Topic poll interval: {:.1f} s", topic_poll_interval); + spdlog::info(" Client backlog size: {}", client_backlog_size); + spdlog::info(" TLS: {}", tls_enabled ? "enabled" : "disabled"); + + auto whitelist_result = pj_bridge::WhitelistFilter::create(topic_whitelist); + if (!whitelist_result) { + spdlog::error("Invalid --topic-whitelist: {}", whitelist_result.error()); + return 1; + } + + if (topic_poll_interval < 0.0) { + spdlog::error("Invalid --topic-poll-interval: {:.1f} (must be >= 0; 0 disables polling)", topic_poll_interval); + return 1; + } try { auto topic_source = std::make_shared(domain_ids); auto sub_manager = std::make_shared(*topic_source); - auto middleware = std::make_shared(); + std::optional tls_config; + if (tls_enabled) { + tls_config = pj_bridge::TlsConfig{certfile, keyfile}; + } + auto middleware = + std::make_shared(static_cast(client_backlog_size), tls_config); - pj_bridge::BridgeServer server(topic_source, sub_manager, middleware, port, session_timeout, publish_rate); + pj_bridge::BridgeServer server( + topic_source, sub_manager, middleware, port, session_timeout, publish_rate, + std::move(whitelist_result.value())); pj_bridge::run_standalone_event_loop( - server, sub_manager, middleware, {port, publish_rate, session_timeout, stats_enabled}); + server, sub_manager, middleware, {port, publish_rate, session_timeout, stats_enabled, topic_poll_interval}); } catch (const std::exception& e) { spdlog::error("Fatal error: {}", e.what()); return 1; diff --git a/package.xml b/package.xml index bb417a4..d3f1e2c 100644 --- a/package.xml +++ b/package.xml @@ -27,6 +27,10 @@ ament_cmake_gtest geometry_msgs + + rmw_cyclonedds_cpp ament_cmake diff --git a/ros2/include/pj_bridge_ros2/generic_subscription_manager.hpp b/ros2/include/pj_bridge_ros2/generic_subscription_manager.hpp index 8343555..cff1572 100644 --- a/ros2/include/pj_bridge_ros2/generic_subscription_manager.hpp +++ b/ros2/include/pj_bridge_ros2/generic_subscription_manager.hpp @@ -47,7 +47,8 @@ using Ros2MessageCallback = */ class GenericSubscriptionManager { public: - explicit GenericSubscriptionManager(rclcpp::Node::SharedPtr node); + explicit GenericSubscriptionManager( + rclcpp::Node::SharedPtr node, size_t min_qos_depth = 1, size_t max_qos_depth = 100); bool subscribe(const std::string& topic_name, const std::string& topic_type, Ros2MessageCallback callback); bool unsubscribe(const std::string& topic_name); @@ -55,15 +56,26 @@ class GenericSubscriptionManager { size_t get_reference_count(const std::string& topic_name) const; void unsubscribe_all(); - private: + /// True when the subscription for `topic_name` was created with + /// TRANSIENT_LOCAL durability (i.e. every publisher discovered at + /// subscribe time offered it). Returns false if the topic is not + /// currently subscribed. + bool is_transient_local(const std::string& topic_name) const; + + // Exposed publicly for testability; computes the QoS a subscription should + // use for `topic_name` based on the publishers currently discovered on it. rclcpp::QoS adapt_qos(const std::string& topic_name) const; + private: struct SubscriptionInfo { std::shared_ptr subscription; size_t reference_count; + bool transient_local; ///< durability actually used for this subscription }; rclcpp::Node::SharedPtr node_; + size_t min_qos_depth_; + size_t max_qos_depth_; mutable std::mutex mutex_; std::unordered_map subscriptions_; }; diff --git a/ros2/include/pj_bridge_ros2/ros2_subscription_manager.hpp b/ros2/include/pj_bridge_ros2/ros2_subscription_manager.hpp index d3f6f95..2ab8b2b 100644 --- a/ros2/include/pj_bridge_ros2/ros2_subscription_manager.hpp +++ b/ros2/include/pj_bridge_ros2/ros2_subscription_manager.hpp @@ -37,7 +37,9 @@ namespace pj_bridge { */ class Ros2SubscriptionManager : public SubscriptionManagerInterface { public: - explicit Ros2SubscriptionManager(rclcpp::Node::SharedPtr node, bool strip_large_messages = false); + explicit Ros2SubscriptionManager( + rclcpp::Node::SharedPtr node, bool strip_large_messages = false, size_t min_qos_depth = 1, + size_t max_qos_depth = 100); Ros2SubscriptionManager(const Ros2SubscriptionManager&) = delete; Ros2SubscriptionManager& operator=(const Ros2SubscriptionManager&) = delete; @@ -46,6 +48,8 @@ class Ros2SubscriptionManager : public SubscriptionManagerInterface { bool subscribe(const std::string& topic_name, const std::string& topic_type) override; bool unsubscribe(const std::string& topic_name) override; void unsubscribe_all() override; + bool is_transient_local(const std::string& topic_name) const override; + bool is_subscribed(const std::string& topic_name) const override; private: GenericSubscriptionManager inner_manager_; diff --git a/ros2/src/generic_subscription_manager.cpp b/ros2/src/generic_subscription_manager.cpp index 9e27b73..22cfca3 100644 --- a/ros2/src/generic_subscription_manager.cpp +++ b/ros2/src/generic_subscription_manager.cpp @@ -19,11 +19,15 @@ #include "pj_bridge_ros2/generic_subscription_manager.hpp" +#include + #include "pj_bridge/time_utils.hpp" namespace pj_bridge { -GenericSubscriptionManager::GenericSubscriptionManager(rclcpp::Node::SharedPtr node) : node_(node) {} +GenericSubscriptionManager::GenericSubscriptionManager( + rclcpp::Node::SharedPtr node, size_t min_qos_depth, size_t max_qos_depth) + : node_(node), min_qos_depth_(min_qos_depth), max_qos_depth_(max_qos_depth) {} rclcpp::QoS GenericSubscriptionManager::adapt_qos(const std::string& topic_name) const { // Match the QoS the publishers actually offer (same policy as rosbag2): @@ -31,13 +35,36 @@ rclcpp::QoS GenericSubscriptionManager::adapt_qos(const std::string& topic_name) // topics!) and a VOLATILE one misses latched (TRANSIENT_LOCAL) samples. rclcpp::QoS qos(100); + // Historical default depth, also used as the per-publisher fallback when a + // publisher's depth is unknown (see below). + constexpr size_t kFallbackDepth = 100; + auto publishers = node_->get_publishers_info_by_topic(topic_name); if (publishers.empty()) { + // No info yet → same fallback the per-publisher rule below uses. + qos.keep_last(std::min(kFallbackDepth, max_qos_depth_)); return qos; } bool any_best_effort = false; bool all_transient_local = true; + // Depth aggregation adapted from foxglove_bridge (MIT License, + // Copyright (c) Foxglove Technologies Inc): + // sum the publishers' history depths so a burst from every publisher + // still fits, then clamp to [min_qos_depth, max_qos_depth]. + // + // Deviations from foxglove_bridge: + // - A publisher reporting depth 0 means the RMW didn't propagate depth + // through discovery (e.g. rmw_fastrtps_cpp reports 0 for everyone) or + // the publisher uses KEEP_ALL. It contributes kFallbackDepth instead of + // 0 — assuming the historical generous default is safer than shrinking + // the queue and dropping messages on high-rate topics. + // - The sum is saturating: each contribution is capped at the remaining + // headroom below max_qos_depth_ (total_depth <= max_qos_depth_ is a loop + // invariant), so the addition is structurally incapable of wrapping + // size_t no matter how many publishers report huge depths — even for + // extreme max_qos_depth_ values passed via the constructor API. + size_t total_depth = 0; for (const auto& info : publishers) { const auto& profile = info.qos_profile(); if (profile.reliability() == rclcpp::ReliabilityPolicy::BestEffort) { @@ -46,7 +73,11 @@ rclcpp::QoS GenericSubscriptionManager::adapt_qos(const std::string& topic_name) if (profile.durability() != rclcpp::DurabilityPolicy::TransientLocal) { all_transient_local = false; } + const size_t publisher_depth = profile.depth() > 0 ? profile.depth() : kFallbackDepth; + const size_t headroom = max_qos_depth_ - total_depth; // total_depth <= max_qos_depth_ invariant + total_depth += std::min(publisher_depth, headroom); } + qos.keep_last(std::clamp(total_depth, min_qos_depth_, max_qos_depth_)); // BEST_EFFORT matches both kinds of publisher; TRANSIENT_LOCAL only // matches if every publisher offers it. @@ -77,9 +108,12 @@ bool GenericSubscriptionManager::subscribe( callback(topic_name, msg, receive_time); }; - auto subscription = node_->create_generic_subscription(topic_name, topic_type, adapt_qos(topic_name), sub_callback); + rclcpp::QoS qos = adapt_qos(topic_name); + bool transient_local = (qos.durability() == rclcpp::DurabilityPolicy::TransientLocal); + + auto subscription = node_->create_generic_subscription(topic_name, topic_type, qos, sub_callback); - subscriptions_[topic_name] = SubscriptionInfo{subscription, 1}; + subscriptions_[topic_name] = SubscriptionInfo{subscription, 1, transient_local}; return true; } catch (const std::exception& e) { @@ -132,4 +166,13 @@ void GenericSubscriptionManager::unsubscribe_all() { subscriptions_.clear(); } +bool GenericSubscriptionManager::is_transient_local(const std::string& topic_name) const { + std::lock_guard lock(mutex_); + auto it = subscriptions_.find(topic_name); + if (it == subscriptions_.end()) { + return false; + } + return it->second.transient_local; +} + } // namespace pj_bridge diff --git a/ros2/src/main.cpp b/ros2/src/main.cpp index 53900db..4ae111d 100644 --- a/ros2/src/main.cpp +++ b/ros2/src/main.cpp @@ -21,10 +21,14 @@ #include #include +#include #include +#include +#include #include "pj_bridge/bridge_server.hpp" #include "pj_bridge/middleware/websocket_middleware.hpp" +#include "pj_bridge/whitelist_filter.hpp" #include "pj_bridge_ros2/ros2_subscription_manager.hpp" #include "pj_bridge_ros2/ros2_topic_source.hpp" @@ -40,25 +44,88 @@ 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>("topic_whitelist", {".*"}); + node->declare_parameter("min_qos_depth", 1); + node->declare_parameter("max_qos_depth", 100); + node->declare_parameter("topic_poll_interval", 1.0); + node->declare_parameter("client_backlog_size", 100); + node->declare_parameter("tls", false); + node->declare_parameter("certfile", ""); + node->declare_parameter("keyfile", ""); int port = node->get_parameter("port").as_int(); 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::vector topic_whitelist = node->get_parameter("topic_whitelist").as_string_array(); + int64_t min_qos_depth = node->get_parameter("min_qos_depth").as_int(); + int64_t max_qos_depth = node->get_parameter("max_qos_depth").as_int(); + double topic_poll_interval = node->get_parameter("topic_poll_interval").as_double(); + int64_t client_backlog_size = node->get_parameter("client_backlog_size").as_int(); + bool tls_enabled = node->get_parameter("tls").as_bool(); + std::string certfile = node->get_parameter("certfile").as_string(); + std::string keyfile = node->get_parameter("keyfile").as_string(); RCLCPP_INFO( node->get_logger(), - "Configuration: port=%d, publish_rate=%.1f Hz, session_timeout=%.1f s, strip_large_messages=%s", port, - publish_rate, session_timeout, strip_large_messages ? "true" : "false"); + "Configuration: port=%d, publish_rate=%.1f Hz, session_timeout=%.1f s, strip_large_messages=%s, " + "min_qos_depth=%ld, max_qos_depth=%ld, 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, + 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"); + rclcpp::shutdown(); + return 1; + } + + if (topic_poll_interval < 0.0) { + RCLCPP_ERROR( + node->get_logger(), "Invalid topic_poll_interval: %.1f (must be >= 0; 0 disables polling)", + topic_poll_interval); + rclcpp::shutdown(); + return 1; + } + + if (client_backlog_size <= 0) { + RCLCPP_ERROR(node->get_logger(), "Invalid client_backlog_size: %ld (must be > 0)", client_backlog_size); + rclcpp::shutdown(); + return 1; + } + + auto whitelist_result = pj_bridge::WhitelistFilter::create(topic_whitelist); + if (!whitelist_result) { + RCLCPP_ERROR(node->get_logger(), "Invalid topic_whitelist: %s", whitelist_result.error().c_str()); + rclcpp::shutdown(); + return 1; + } + + if (min_qos_depth < 0 || max_qos_depth < 0 || min_qos_depth > max_qos_depth) { + RCLCPP_ERROR( + node->get_logger(), + "Invalid QoS depth configuration: min_qos_depth=%ld, max_qos_depth=%ld (both must be >= 0 and " + "min_qos_depth <= max_qos_depth)", + min_qos_depth, max_qos_depth); + rclcpp::shutdown(); + return 1; + } try { // Create backend components auto topic_source = std::make_shared(node); - auto sub_manager = std::make_shared(node, strip_large_messages); - auto middleware = std::make_shared(); + auto sub_manager = std::make_shared( + node, strip_large_messages, static_cast(min_qos_depth), static_cast(max_qos_depth)); + std::optional tls_config; + if (tls_enabled) { + tls_config = pj_bridge::TlsConfig{certfile, keyfile}; + } + auto middleware = + std::make_shared(static_cast(client_backlog_size), tls_config); // Create bridge server - pj_bridge::BridgeServer server(topic_source, sub_manager, middleware, port, session_timeout, publish_rate); + pj_bridge::BridgeServer server( + topic_source, sub_manager, middleware, port, session_timeout, publish_rate, + std::move(whitelist_result.value())); if (!server.initialize()) { RCLCPP_ERROR(node->get_logger(), "Failed to initialize bridge server"); @@ -81,6 +148,18 @@ int main(int argc, char** argv) { auto timeout_timer = node->create_wall_timer(1s, [&server]() { server.check_session_timeouts(); }); + // topic_poll_interval == 0 disables the pushed topic-advertisement poll. + rclcpp::TimerBase::SharedPtr topic_poll_timer; + if (topic_poll_interval > 0.0) { + // Take the silent baseline snapshot now, before any client can connect, + // so topics appearing before the first timer tick are notified rather + // than folded into the baseline. + server.check_topic_changes(); + topic_poll_timer = node->create_wall_timer( + std::chrono::duration_cast(std::chrono::duration(topic_poll_interval)), + [&server]() { server.check_topic_changes(); }); + } + // Spin until shutdown. spin() blocks waiting for work and returns when // rclcpp::shutdown() runs (e.g. on SIGINT) — unlike spin_some() in a // loop, which returns immediately when idle and busy-spins a full core. @@ -95,6 +174,9 @@ int main(int argc, char** argv) { request_timer->cancel(); publish_timer->cancel(); timeout_timer->cancel(); + if (topic_poll_timer) { + topic_poll_timer->cancel(); + } executor.remove_node(node); // Clear the subscription manager callback before server destruction diff --git a/ros2/src/ros2_subscription_manager.cpp b/ros2/src/ros2_subscription_manager.cpp index 12636b9..fd6de59 100644 --- a/ros2/src/ros2_subscription_manager.cpp +++ b/ros2/src/ros2_subscription_manager.cpp @@ -26,8 +26,9 @@ namespace pj_bridge { -Ros2SubscriptionManager::Ros2SubscriptionManager(rclcpp::Node::SharedPtr node, bool strip_large_messages) - : inner_manager_(node), strip_large_messages_(strip_large_messages) {} +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) {} void Ros2SubscriptionManager::set_message_callback(MessageCallback callback) { std::lock_guard lock(callback_mutex_); @@ -77,4 +78,12 @@ void Ros2SubscriptionManager::unsubscribe_all() { inner_manager_.unsubscribe_all(); } +bool Ros2SubscriptionManager::is_transient_local(const std::string& topic_name) const { + return inner_manager_.is_transient_local(topic_name); +} + +bool Ros2SubscriptionManager::is_subscribed(const std::string& topic_name) const { + return inner_manager_.is_subscribed(topic_name); +} + } // namespace pj_bridge diff --git a/rti/src/main.cpp b/rti/src/main.cpp index e069fab..396fdd8 100644 --- a/rti/src/main.cpp +++ b/rti/src/main.cpp @@ -20,11 +20,13 @@ #include #include +#include #include #include #include "pj_bridge/middleware/websocket_middleware.hpp" #include "pj_bridge/standalone_event_loop.hpp" +#include "pj_bridge/whitelist_filter.hpp" #include "pj_bridge_rti/dds_subscription_manager.hpp" #include "pj_bridge_rti/dds_topic_discovery.hpp" @@ -38,6 +40,11 @@ int main(int argc, char* argv[]) { std::string qos_profile; std::string log_level = "info"; bool stats_enabled = false; + std::vector topic_whitelist{".*"}; + double topic_poll_interval = 1.0; + int client_backlog_size = 100; + std::string certfile; + std::string keyfile; app.add_option("--domains,-d", domain_ids, "DDS domain IDs")->required()->expected(1, -1); app.add_option("--port,-p", port, "WebSocket port")->default_val(9090)->check(CLI::Range(1, 65535)); @@ -46,9 +53,31 @@ int main(int argc, char* argv[]) { app.add_option("--qos-profile", qos_profile, "QoS profile XML file path"); app.add_option("--log-level", log_level, "Log level (trace, debug, info, warn, error)")->default_val("info"); app.add_flag("--stats", stats_enabled, "Print statistics every 5 seconds"); + app.add_option( + "--topic-poll-interval", topic_poll_interval, + "Interval in seconds between topics_changed notification polls (0 disables)") + ->default_val(1.0); + app.add_option( + "--client-backlog-size", client_backlog_size, + "Max frames queued per slow client before dropping the oldest (backpressure)") + ->default_val(100) + ->check(CLI::Range(1, 1000000)); + // Bound variable is already initialized to {".*"} (match everything); CLI11 + // leaves it untouched if the flag is not passed, so no default_val() is + // needed (and default_val() on a vector would round-trip through a + // single joined string, which is not what we want here). + app.add_option("--topic-whitelist", topic_whitelist, "Topic whitelist regex patterns (full-match, ECMAScript)"); + + auto certfile_opt = + app.add_option("--certfile", certfile, "TLS server certificate file (enables wss://, requires --keyfile)"); + auto keyfile_opt = app.add_option("--keyfile", keyfile, "TLS private key file (enables wss://, requires --certfile)"); + certfile_opt->needs(keyfile_opt); + keyfile_opt->needs(certfile_opt); CLI11_PARSE(app, argc, argv); + bool tls_enabled = !certfile.empty(); + spdlog::set_level(spdlog::level::from_str(log_level)); spdlog::info("pj_bridge (RTI DDS backend) starting..."); @@ -59,16 +88,38 @@ int main(int argc, char* argv[]) { if (!qos_profile.empty()) { spdlog::info(" QoS profile: {}", qos_profile); } + spdlog::info(" Topic whitelist: {}", fmt::join(topic_whitelist, ", ")); + spdlog::info(" Topic poll interval: {:.1f} s", topic_poll_interval); + spdlog::info(" Client backlog size: {}", client_backlog_size); + spdlog::info(" TLS: {}", tls_enabled ? "enabled" : "disabled"); + + auto whitelist_result = pj_bridge::WhitelistFilter::create(topic_whitelist); + if (!whitelist_result) { + spdlog::error("Invalid --topic-whitelist: {}", whitelist_result.error()); + return 1; + } + + if (topic_poll_interval < 0.0) { + spdlog::error("Invalid --topic-poll-interval: {:.1f} (must be >= 0; 0 disables polling)", topic_poll_interval); + return 1; + } try { auto topic_source = std::make_shared(domain_ids, qos_profile); auto sub_manager = std::make_shared(*topic_source); - auto middleware = std::make_shared(); + std::optional tls_config; + if (tls_enabled) { + tls_config = pj_bridge::TlsConfig{certfile, keyfile}; + } + auto middleware = + std::make_shared(static_cast(client_backlog_size), tls_config); - pj_bridge::BridgeServer server(topic_source, sub_manager, middleware, port, session_timeout, publish_rate); + pj_bridge::BridgeServer server( + topic_source, sub_manager, middleware, port, session_timeout, publish_rate, + std::move(whitelist_result.value())); pj_bridge::run_standalone_event_loop( - server, sub_manager, middleware, {port, publish_rate, session_timeout, stats_enabled}); + server, sub_manager, middleware, {port, publish_rate, session_timeout, stats_enabled, topic_poll_interval}); } catch (const std::exception& e) { spdlog::error("Fatal error: {}", e.what()); return 1; diff --git a/tests/unit/test_bounded_frame_queue.cpp b/tests/unit/test_bounded_frame_queue.cpp new file mode 100644 index 0000000..5d5a842 --- /dev/null +++ b/tests/unit/test_bounded_frame_queue.cpp @@ -0,0 +1,98 @@ +/* + * 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 "pj_bridge/middleware/bounded_frame_queue.hpp" + +using namespace pj_bridge; + +namespace { +std::vector make_frame(uint8_t value) { + return {value}; +} +} // namespace + +TEST(BoundedFrameQueueTest, PushUnderCapacityReturnsZeroAndGrows) { + BoundedFrameQueue queue(2); + + EXPECT_EQ(queue.push(make_frame(1)), 0u); + EXPECT_EQ(queue.size(), 1u); + EXPECT_FALSE(queue.empty()); + + EXPECT_EQ(queue.push(make_frame(2)), 0u); + EXPECT_EQ(queue.size(), 2u); +} + +TEST(BoundedFrameQueueTest, PushPastCapacityDropsOldest) { + BoundedFrameQueue queue(2); + + EXPECT_EQ(queue.push(make_frame(1)), 0u); + EXPECT_EQ(queue.push(make_frame(2)), 0u); + // Queue is now {1, 2}; pushing {3} should drop the OLDEST frame (1), not + // the newest, leaving {2, 3}. + EXPECT_EQ(queue.push(make_frame(3)), 1u); + EXPECT_EQ(queue.size(), 2u); + + auto first = queue.pop_front(); + ASSERT_TRUE(first.has_value()); + EXPECT_EQ(*first, make_frame(2)); + + auto second = queue.pop_front(); + ASSERT_TRUE(second.has_value()); + EXPECT_EQ(*second, make_frame(3)); + + EXPECT_TRUE(queue.empty()); +} + +TEST(BoundedFrameQueueTest, PopFrontOnEmptyReturnsNullopt) { + BoundedFrameQueue queue(2); + EXPECT_EQ(queue.pop_front(), std::nullopt); +} + +TEST(BoundedFrameQueueTest, DroppedTotalAccumulatesAcrossPushes) { + BoundedFrameQueue queue(1); + + EXPECT_EQ(queue.dropped_total(), 0u); + EXPECT_EQ(queue.push(make_frame(1)), 0u); + EXPECT_EQ(queue.dropped_total(), 0u); + + EXPECT_EQ(queue.push(make_frame(2)), 1u); + EXPECT_EQ(queue.dropped_total(), 1u); + + EXPECT_EQ(queue.push(make_frame(3)), 1u); + EXPECT_EQ(queue.dropped_total(), 2u); +} + +TEST(BoundedFrameQueueTest, ZeroCapacityDropsEveryPushAndStaysEmpty) { + BoundedFrameQueue queue(0); + + EXPECT_EQ(queue.push(make_frame(1)), 1u); + EXPECT_TRUE(queue.empty()); + EXPECT_EQ(queue.size(), 0u); + EXPECT_EQ(queue.dropped_total(), 1u); + + EXPECT_EQ(queue.push(make_frame(2)), 1u); + EXPECT_TRUE(queue.empty()); + EXPECT_EQ(queue.dropped_total(), 2u); + + EXPECT_EQ(queue.pop_front(), std::nullopt); +} diff --git a/tests/unit/test_bridge_server.cpp b/tests/unit/test_bridge_server.cpp index 781eee0..2f00921 100644 --- a/tests/unit/test_bridge_server.cpp +++ b/tests/unit/test_bridge_server.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -32,9 +33,11 @@ #include #include "pj_bridge/bridge_server.hpp" +#include "pj_bridge/message_serializer.hpp" #include "pj_bridge/middleware/middleware_interface.hpp" #include "pj_bridge/subscription_manager_interface.hpp" #include "pj_bridge/topic_source_interface.hpp" +#include "pj_bridge/whitelist_filter.hpp" #include "tl/expected.hpp" using json = nlohmann::json; @@ -69,6 +72,7 @@ class MockMiddleware : public MiddlewareInterface { } bool send_reply(const std::string& client_identity, const std::vector& data) override { + log_send(SendKind::kText, client_identity); std::lock_guard lock(reply_mutex_); replies_.emplace_back(client_identity, data); return true; @@ -79,6 +83,7 @@ class MockMiddleware : public MiddlewareInterface { } bool send_binary(const std::string& client_identity, const std::vector& data) override { + log_send(SendKind::kBinary, client_identity); std::lock_guard lock(binary_mutex_); binary_sends_.emplace_back(client_identity, data); return true; @@ -98,8 +103,19 @@ class MockMiddleware : public MiddlewareInterface { on_disconnect_ = std::move(callback); } + void drop_pending(const std::string& client_identity) override { + std::lock_guard lock(drop_pending_mutex_); + drop_pending_calls_.push_back(client_identity); + } + // --- Test helpers --- + /// Return the client ids drop_pending() was invoked for, in call order. + std::vector get_drop_pending_calls() { + std::lock_guard lock(drop_pending_mutex_); + return drop_pending_calls_; + } + /// Push a text request as if it came from a connected client. void push_request(const std::string& client_id, const std::string& text) { std::lock_guard lock(queue_mutex_); @@ -156,7 +172,47 @@ class MockMiddleware : public MiddlewareInterface { binary_sends_.clear(); } + /// Return all replies sent to a given client, in send order (parsed JSON). + std::vector get_replies(const std::string& client_id) { + std::lock_guard lock(reply_mutex_); + std::vector result; + for (const auto& [cid, data] : replies_) { + if (cid == client_id) { + std::string text(data.begin(), data.end()); + result.push_back(json::parse(text, nullptr, false)); + } + } + return result; + } + + /// Discard all recorded replies (useful to isolate a subsequent action's output). + void clear_replies() { + std::lock_guard lock(reply_mutex_); + replies_.clear(); + } + + /// Kind of frame recorded in the combined send log. + enum class SendKind { kText, kBinary }; + + /// Combined, ordered log of every send_reply/send_binary call + /// (kind, client_id) — lets tests assert relative ordering across the + /// two send types. + std::vector> get_send_log() { + std::lock_guard lock(send_log_mutex_); + return send_log_; + } + + void clear_send_log() { + std::lock_guard lock(send_log_mutex_); + send_log_.clear(); + } + private: + void log_send(SendKind kind, const std::string& client_identity) { + std::lock_guard lock(send_log_mutex_); + send_log_.emplace_back(kind, client_identity); + } + bool ready_{false}; std::mutex queue_mutex_; @@ -168,9 +224,15 @@ class MockMiddleware : public MiddlewareInterface { std::mutex binary_mutex_; std::vector>> binary_sends_; + std::mutex send_log_mutex_; + std::vector> send_log_; + std::mutex cb_mutex_; ConnectionCallback on_connect_; ConnectionCallback on_disconnect_; + + std::mutex drop_pending_mutex_; + std::vector drop_pending_calls_; }; // --------------------------------------------------------------------------- @@ -258,13 +320,45 @@ class MockSubscriptionManager : public SubscriptionManagerInterface { ref_counts_.clear(); } + bool is_transient_local(const std::string& topic_name) const override { + std::lock_guard lock(mutex_); + return transient_local_topics_.count(topic_name) > 0; + } + // Test helpers void add_known_topic(const std::string& topic_name) { std::lock_guard lock(mutex_); known_topics_.insert(topic_name); } - bool is_subscribed(const std::string& topic_name) const { + // Mark a topic as reporting TRANSIENT_LOCAL durability from is_transient_local(). + void set_transient_local(const std::string& topic_name, bool transient_local) { + std::lock_guard lock(mutex_); + if (transient_local) { + transient_local_topics_.insert(topic_name); + } else { + transient_local_topics_.erase(topic_name); + } + } + + // Simulate a message arriving on `topic_name`, invoking the callback + // registered via set_message_callback() — exactly as the real backend + // would when a middleware message is received. + void deliver_message( + const std::string& topic_name, std::shared_ptr> data, uint64_t timestamp_ns) { + MessageCallback cb; + { + std::lock_guard lock(mutex_); + cb = callback_; + } + if (cb) { + cb(topic_name, std::move(data), timestamp_ns); + } + } + + // Overrides the interface default (false): reports true while the topic + // holds at least one ref, mirroring the real backend managers. + bool is_subscribed(const std::string& topic_name) const override { std::lock_guard lock(mutex_); auto it = ref_counts_.find(topic_name); return it != ref_counts_.end() && it->second > 0; @@ -293,9 +387,61 @@ class MockSubscriptionManager : public SubscriptionManagerInterface { MessageCallback callback_; std::unordered_set known_topics_; std::unordered_map ref_counts_; + std::unordered_set transient_local_topics_; bool underflow_detected_{false}; }; +// --------------------------------------------------------------------------- +// decode_frame — test helper to parse a binary aggregated-message frame +// (16-byte header + ZSTD-compressed payload) back into its messages, mirroring +// the wire format documented in docs/API.md and produced by +// AggregatedMessageSerializer::finalize(). +// --------------------------------------------------------------------------- +struct DecodedMessage { + std::string topic; + uint64_t timestamp_ns; + std::vector data; +}; + +std::vector decode_frame(const std::vector& frame) { + std::vector result; + if (frame.size() < 16) { + return result; + } + + uint32_t message_count = 0; + std::memcpy(&message_count, frame.data() + 4, sizeof(message_count)); + + std::vector compressed(frame.begin() + 16, frame.end()); + std::vector payload; + AggregatedMessageSerializer::decompress_zstd(compressed, payload); + + size_t offset = 0; + for (uint32_t i = 0; i < message_count; ++i) { + uint16_t topic_len = 0; + std::memcpy(&topic_len, payload.data() + offset, sizeof(topic_len)); + offset += sizeof(topic_len); + + std::string topic(payload.begin() + offset, payload.begin() + offset + topic_len); + offset += topic_len; + + uint64_t timestamp_ns = 0; + std::memcpy(×tamp_ns, payload.data() + offset, sizeof(timestamp_ns)); + offset += sizeof(timestamp_ns); + + uint32_t data_len = 0; + std::memcpy(&data_len, payload.data() + offset, sizeof(data_len)); + offset += sizeof(data_len); + + std::vector data(payload.begin() + offset, payload.begin() + offset + data_len); + offset += data_len; + + result.push_back(DecodedMessage{std::move(topic), timestamp_ns, std::move(data)}); + } + + return result; +} + // --------------------------------------------------------------------------- // Test fixture // --------------------------------------------------------------------------- @@ -423,6 +569,44 @@ TEST_F(BridgeServerTest, CleanupSessionIdempotent) { EXPECT_EQ(server_->get_active_session_count(), 0u); } +// --------------------------------------------------------------------------- +// 6b. CleanupSessionDropsPendingMiddlewareFrames +// +// When a session is destroyed server-side (heartbeat timeout or disconnect), +// cleanup_session must tell the middleware to discard any outbound frames +// still parked in its slow-client backpressure queue for that client — +// otherwise a timed-out client whose socket stays open leaks its backlog, +// and stale frames could flush into a new logical session on the same +// connection. +// --------------------------------------------------------------------------- +TEST_F(BridgeServerTest, CleanupSessionDropsPendingMiddlewareFrames) { + ASSERT_TRUE(server_->initialize()); + + // Create a session via heartbeat. + json hb; + hb["command"] = "heartbeat"; + mock_->push_request("client_DP", hb.dump()); + server_->process_requests(); + ASSERT_EQ(server_->get_active_session_count(), 1u); + EXPECT_TRUE(mock_->get_drop_pending_calls().empty()); + + // Disconnect triggers cleanup_session, which must invoke drop_pending for + // exactly this client. + mock_->simulate_disconnect("client_DP"); + EXPECT_EQ(server_->get_active_session_count(), 0u); + + auto calls = mock_->get_drop_pending_calls(); + ASSERT_EQ(calls.size(), 1u); + EXPECT_EQ(calls[0], "client_DP"); + + // cleanup_session for a client without a session still drops pending + // middleware state (idempotent, matches drop_pending's no-op contract). + mock_->simulate_disconnect("client_DP"); + calls = mock_->get_drop_pending_calls(); + ASSERT_EQ(calls.size(), 2u); + EXPECT_EQ(calls[1], "client_DP"); +} + // --------------------------------------------------------------------------- // 7. PerClientFiltering // @@ -2003,3 +2187,623 @@ TEST_F(BridgeServerTest, SubscribeAcceptsEmptySchemaDefinition) { EXPECT_EQ(response["schemas"]["/trigger"]["definition"], ""); EXPECT_EQ(mock_sub_manager_->ref_count("/trigger"), 1); } + +// --------------------------------------------------------------------------- +// Topic whitelist filtering +// --------------------------------------------------------------------------- + +// GetTopicsOmitsNonWhitelistedTopics +TEST_F(BridgeServerTest, GetTopicsOmitsNonWhitelistedTopics) { + auto whitelist_result = WhitelistFilter::create({"/allowed.*"}); + ASSERT_TRUE(whitelist_result.has_value()); + server_ = std::make_unique( + mock_topic_source_, mock_sub_manager_, mock_, 19999, 10.0, 50.0, whitelist_result.value()); + ASSERT_TRUE(server_->initialize()); + + mock_topic_source_->set_topics({{"/allowed/foo", "std_msgs/msg/String"}, {"/blocked", "std_msgs/msg/String"}}); + + json req; + req["command"] = "get_topics"; + mock_->push_request("client_wl_get_topics", req.dump()); + server_->process_requests(); + + json reply = mock_->pop_reply("client_wl_get_topics"); + ASSERT_FALSE(reply.is_discarded()); + EXPECT_EQ(reply["status"], "success"); + ASSERT_TRUE(reply["topics"].is_array()); + ASSERT_EQ(reply["topics"].size(), 1u); + EXPECT_EQ(reply["topics"][0]["name"], "/allowed/foo"); +} + +// SubscribeToNonWhitelistedTopicFailsAllSubscriptionsFailed +TEST_F(BridgeServerTest, SubscribeToNonWhitelistedTopicFailsAllSubscriptionsFailed) { + auto whitelist_result = WhitelistFilter::create({"/allowed.*"}); + ASSERT_TRUE(whitelist_result.has_value()); + server_ = std::make_unique( + mock_topic_source_, mock_sub_manager_, mock_, 19999, 10.0, 50.0, whitelist_result.value()); + ASSERT_TRUE(server_->initialize()); + + mock_topic_source_->set_topics({{"/blocked", "std_msgs/msg/String"}}); + mock_sub_manager_->add_known_topic("/blocked"); + + json sub; + sub["command"] = "subscribe"; + sub["topics"] = json::array({"/blocked"}); + mock_->push_request("client_wl_blocked", sub.dump()); + server_->process_requests(); + + json reply = mock_->pop_reply("client_wl_blocked"); + ASSERT_FALSE(reply.is_discarded()); + EXPECT_EQ(reply["status"], "error"); + EXPECT_EQ(reply["error_code"], "ALL_SUBSCRIPTIONS_FAILED"); + ASSERT_TRUE(reply.contains("failures")); + ASSERT_FALSE(reply["failures"].empty()); + EXPECT_EQ(reply["failures"][0]["topic"], "/blocked"); + EXPECT_EQ(reply["failures"][0]["reason"], "Topic not whitelisted"); + EXPECT_EQ(mock_sub_manager_->ref_count("/blocked"), 0); +} + +// SubscribeMixedWhitelistedAndNonWhitelistedTopicsPartialSuccess +TEST_F(BridgeServerTest, SubscribeMixedWhitelistedAndNonWhitelistedTopicsPartialSuccess) { + auto whitelist_result = WhitelistFilter::create({"/allowed.*"}); + ASSERT_TRUE(whitelist_result.has_value()); + server_ = std::make_unique( + mock_topic_source_, mock_sub_manager_, mock_, 19999, 10.0, 50.0, whitelist_result.value()); + ASSERT_TRUE(server_->initialize()); + + mock_topic_source_->set_topics({{"/allowed/foo", "std_msgs/msg/String"}, {"/blocked", "std_msgs/msg/String"}}); + mock_sub_manager_->add_known_topic("/allowed/foo"); + mock_sub_manager_->add_known_topic("/blocked"); + + json sub; + sub["command"] = "subscribe"; + sub["topics"] = json::array({"/allowed/foo", "/blocked"}); + mock_->push_request("client_wl_mixed", sub.dump()); + server_->process_requests(); + + json reply = mock_->pop_reply("client_wl_mixed"); + ASSERT_FALSE(reply.is_discarded()); + EXPECT_EQ(reply["status"], "partial_success"); + ASSERT_TRUE(reply["schemas"].contains("/allowed/foo")); + ASSERT_TRUE(reply.contains("failures")); + + bool found_blocked_failure = false; + for (const auto& failure : reply["failures"]) { + if (failure["topic"] == "/blocked") { + EXPECT_EQ(failure["reason"], "Topic not whitelisted"); + found_blocked_failure = true; + } + } + EXPECT_TRUE(found_blocked_failure); + EXPECT_EQ(mock_sub_manager_->ref_count("/allowed/foo"), 1); + EXPECT_EQ(mock_sub_manager_->ref_count("/blocked"), 0); +} + +// --------------------------------------------------------------------------- +// Pushed topic advertisement (subscribe/unsubscribe_topic_updates, check_topic_changes) +// --------------------------------------------------------------------------- + +// SubscribeTopicUpdatesReturnsOkTrue +TEST_F(BridgeServerTest, SubscribeTopicUpdatesReturnsOkTrue) { + ASSERT_TRUE(server_->initialize()); + + json req; + req["command"] = "subscribe_topic_updates"; + req["id"] = "tu1"; + mock_->push_request("client_tu_1", req.dump()); + server_->process_requests(); + + json reply = mock_->pop_reply("client_tu_1"); + ASSERT_FALSE(reply.is_discarded()); + EXPECT_EQ(reply["status"], "ok"); + EXPECT_TRUE(reply["topic_updates"].get()); + EXPECT_EQ(reply["id"], "tu1"); + EXPECT_TRUE(reply.contains("protocol_version")); + + // Repeated call is idempotent — same result. + mock_->push_request("client_tu_1", req.dump()); + server_->process_requests(); + json reply2 = mock_->pop_reply("client_tu_1"); + ASSERT_FALSE(reply2.is_discarded()); + EXPECT_EQ(reply2["status"], "ok"); + EXPECT_TRUE(reply2["topic_updates"].get()); +} + +// UnsubscribeTopicUpdatesReturnsOkFalse +TEST_F(BridgeServerTest, UnsubscribeTopicUpdatesReturnsOkFalse) { + ASSERT_TRUE(server_->initialize()); + + json req; + req["command"] = "unsubscribe_topic_updates"; + req["id"] = "tu2"; + mock_->push_request("client_tu_2", req.dump()); + server_->process_requests(); + + json reply = mock_->pop_reply("client_tu_2"); + ASSERT_FALSE(reply.is_discarded()); + EXPECT_EQ(reply["status"], "ok"); + EXPECT_FALSE(reply["topic_updates"].get()); + EXPECT_EQ(reply["id"], "tu2"); + EXPECT_TRUE(reply.contains("protocol_version")); + + // Repeated call is idempotent — same result. + mock_->push_request("client_tu_2", req.dump()); + server_->process_requests(); + json reply2 = mock_->pop_reply("client_tu_2"); + ASSERT_FALSE(reply2.is_discarded()); + EXPECT_EQ(reply2["status"], "ok"); + EXPECT_FALSE(reply2["topic_updates"].get()); +} + +// CheckTopicChangesFirstCallSendsNothing +TEST_F(BridgeServerTest, CheckTopicChangesFirstCallSendsNothing) { + ASSERT_TRUE(server_->initialize()); + mock_topic_source_->set_topics({{"/a", "std_msgs/msg/String"}}); + + json sub; + sub["command"] = "subscribe_topic_updates"; + mock_->push_request("client_first", sub.dump()); + server_->process_requests(); + mock_->clear_replies(); + + server_->check_topic_changes(); + + auto replies = mock_->get_replies("client_first"); + EXPECT_TRUE(replies.empty()); +} + +// CheckTopicChangesNotifiesAddedTopicOnlyToOptedInClient +TEST_F(BridgeServerTest, CheckTopicChangesNotifiesAddedTopicOnlyToOptedInClient) { + ASSERT_TRUE(server_->initialize()); + mock_topic_source_->set_topics({{"/a", "std_msgs/msg/String"}}); + + json sub; + sub["command"] = "subscribe_topic_updates"; + mock_->push_request("client_opted_in", sub.dump()); + server_->process_requests(); + + json hb; + hb["command"] = "heartbeat"; + mock_->push_request("client_plain", hb.dump()); + server_->process_requests(); + + server_->check_topic_changes(); // first call: snapshot only, sends nothing + mock_->clear_replies(); + + mock_topic_source_->set_topics({{"/a", "std_msgs/msg/String"}, {"/b", "sensor_msgs/msg/Imu"}}); + server_->check_topic_changes(); + + auto opted_in_replies = mock_->get_replies("client_opted_in"); + ASSERT_EQ(opted_in_replies.size(), 1u); + json notif = opted_in_replies[0]; + EXPECT_EQ(notif["notification"], "topics_changed"); + EXPECT_TRUE(notif.contains("protocol_version")); + ASSERT_TRUE(notif["removed"].empty()); + ASSERT_EQ(notif["added"].size(), 1u); + EXPECT_EQ(notif["added"][0]["name"], "/b"); + EXPECT_EQ(notif["added"][0]["type"], "sensor_msgs/msg/Imu"); + + auto plain_replies = mock_->get_replies("client_plain"); + EXPECT_TRUE(plain_replies.empty()); +} + +// CheckTopicChangesNotifiesRemovedTopic +TEST_F(BridgeServerTest, CheckTopicChangesNotifiesRemovedTopic) { + ASSERT_TRUE(server_->initialize()); + mock_topic_source_->set_topics({{"/a", "std_msgs/msg/String"}, {"/b", "std_msgs/msg/String"}}); + + json sub; + sub["command"] = "subscribe_topic_updates"; + mock_->push_request("client_opted_in", sub.dump()); + server_->process_requests(); + + server_->check_topic_changes(); // snapshot + mock_->clear_replies(); + + mock_topic_source_->remove_topic("/b"); + server_->check_topic_changes(); + + auto replies = mock_->get_replies("client_opted_in"); + ASSERT_EQ(replies.size(), 1u); + json notif = replies[0]; + ASSERT_TRUE(notif["added"].empty()); + ASSERT_EQ(notif["removed"].size(), 1u); + EXPECT_EQ(notif["removed"][0], "/b"); +} + +// CheckTopicChangesNoChangeSendsNothing +TEST_F(BridgeServerTest, CheckTopicChangesNoChangeSendsNothing) { + ASSERT_TRUE(server_->initialize()); + mock_topic_source_->set_topics({{"/a", "std_msgs/msg/String"}}); + + json sub; + sub["command"] = "subscribe_topic_updates"; + mock_->push_request("client_opted_in", sub.dump()); + server_->process_requests(); + + server_->check_topic_changes(); // snapshot + mock_->clear_replies(); + + server_->check_topic_changes(); // nothing changed + + auto replies = mock_->get_replies("client_opted_in"); + EXPECT_TRUE(replies.empty()); +} + +// CheckTopicChangesIgnoresNonWhitelistedTopics +TEST_F(BridgeServerTest, CheckTopicChangesIgnoresNonWhitelistedTopics) { + auto whitelist_result = WhitelistFilter::create({"/allowed.*"}); + ASSERT_TRUE(whitelist_result.has_value()); + server_ = std::make_unique( + mock_topic_source_, mock_sub_manager_, mock_, 19999, 10.0, 50.0, whitelist_result.value()); + ASSERT_TRUE(server_->initialize()); + + mock_topic_source_->set_topics({{"/allowed/foo", "std_msgs/msg/String"}}); + + json sub; + sub["command"] = "subscribe_topic_updates"; + mock_->push_request("client_wl", sub.dump()); + server_->process_requests(); + + server_->check_topic_changes(); // snapshot + mock_->clear_replies(); + + mock_topic_source_->set_topics({{"/allowed/foo", "std_msgs/msg/String"}, {"/blocked", "std_msgs/msg/String"}}); + server_->check_topic_changes(); + + auto replies = mock_->get_replies("client_wl"); + EXPECT_TRUE(replies.empty()); +} + +// CheckTopicChangesTypeChangeAppearsInBothAddedAndRemoved +TEST_F(BridgeServerTest, CheckTopicChangesTypeChangeAppearsInBothAddedAndRemoved) { + ASSERT_TRUE(server_->initialize()); + mock_topic_source_->set_topics({{"/a", "std_msgs/msg/String"}}); + + json sub; + sub["command"] = "subscribe_topic_updates"; + mock_->push_request("client_opted_in", sub.dump()); + server_->process_requests(); + + server_->check_topic_changes(); // snapshot + mock_->clear_replies(); + + mock_topic_source_->set_topics({{"/a", "std_msgs/msg/Header"}}); + server_->check_topic_changes(); + + auto replies = mock_->get_replies("client_opted_in"); + ASSERT_EQ(replies.size(), 1u); + json notif = replies[0]; + ASSERT_EQ(notif["removed"].size(), 1u); + EXPECT_EQ(notif["removed"][0], "/a"); + ASSERT_EQ(notif["added"].size(), 1u); + EXPECT_EQ(notif["added"][0]["name"], "/a"); + EXPECT_EQ(notif["added"][0]["type"], "std_msgs/msg/Header"); +} + +// UnsubscribeTopicUpdatesStopsNotifications +TEST_F(BridgeServerTest, UnsubscribeTopicUpdatesStopsNotifications) { + ASSERT_TRUE(server_->initialize()); + mock_topic_source_->set_topics({{"/a", "std_msgs/msg/String"}}); + + json sub; + sub["command"] = "subscribe_topic_updates"; + mock_->push_request("client_x", sub.dump()); + server_->process_requests(); + + server_->check_topic_changes(); // snapshot + + json unsub; + unsub["command"] = "unsubscribe_topic_updates"; + mock_->push_request("client_x", unsub.dump()); + server_->process_requests(); + mock_->clear_replies(); + + mock_topic_source_->set_topics({{"/a", "std_msgs/msg/String"}, {"/b", "std_msgs/msg/String"}}); + server_->check_topic_changes(); + + auto replies = mock_->get_replies("client_x"); + EXPECT_TRUE(replies.empty()); +} + +// --------------------------------------------------------------------------- +// Latched (transient_local) topic replay +// +// A late subscriber joining an already-shared middleware subscription for a +// latched topic must be immediately replayed the most recent message, even +// after it has aged out of the normal 1-second message buffer. The FIRST +// subscriber gets its sample naturally from DDS (transient_local +// redelivery), so this replay path exists specifically for LATER clients. +// --------------------------------------------------------------------------- + +// LatchedTopicReplaysToLateSubscriberOnly +TEST_F(BridgeServerTest, LatchedTopicReplaysToLateSubscriberOnly) { + ASSERT_TRUE(server_->initialize()); + mock_topic_source_->set_topics({{"/latched", "std_msgs/msg/String"}}); + mock_sub_manager_->add_known_topic("/latched"); + mock_sub_manager_->set_transient_local("/latched", true); + + json sub; + sub["command"] = "subscribe"; + sub["topics"] = json::array({"/latched"}); + + // Client A subscribes first (the "first subscriber" — no retained sample + // exists yet, so no replay frame is sent). + mock_->push_request("client_a", sub.dump()); + server_->process_requests(); + json resp_a = mock_->pop_reply("client_a"); + ASSERT_FALSE(resp_a.is_discarded()); + EXPECT_EQ(resp_a["status"], "success"); + EXPECT_TRUE(mock_->get_binary_sends().empty()); + + // A message arrives on the latched topic. + std::vector payload = {10, 20, 30, 40}; + auto data = std::make_shared>(payload.size()); + for (size_t i = 0; i < payload.size(); ++i) { + (*data)[i] = static_cast(payload[i]); + } + mock_sub_manager_->deliver_message("/latched", data, 12345); + + // Drain the normal message buffer, simulating the message aging out of + // the regular publish cycle (it is delivered to client_a here). + server_->publish_aggregated_messages(); + mock_->clear_binary_sends(); + mock_->clear_send_log(); + + // Client B subscribes later, joining the already-shared subscription. + mock_->push_request("client_b", sub.dump()); + server_->process_requests(); + json resp_b = mock_->pop_reply("client_b"); + ASSERT_FALSE(resp_b.is_discarded()); + EXPECT_EQ(resp_b["status"], "success"); + + // Exactly one binary frame, sent only to client_b, containing the + // retained message. + auto binary_sends = mock_->get_binary_sends(); + ASSERT_EQ(binary_sends.size(), 1u); + EXPECT_EQ(binary_sends[0].first, "client_b"); + + auto messages = decode_frame(binary_sends[0].second); + ASSERT_EQ(messages.size(), 1u); + EXPECT_EQ(messages[0].topic, "/latched"); + EXPECT_EQ(messages[0].timestamp_ns, 12345u); + EXPECT_EQ(messages[0].data, payload); + + // Ordering: the subscribe RESPONSE (text, carrying the schema needed to + // decode) must reach client_b BEFORE the replay binary frame. + auto send_log = mock_->get_send_log(); + std::vector b_sends; + for (const auto& [kind, cid] : send_log) { + if (cid == "client_b") { + b_sends.push_back(kind); + } + } + ASSERT_EQ(b_sends.size(), 2u); + EXPECT_EQ(b_sends[0], MockMiddleware::SendKind::kText); + EXPECT_EQ(b_sends[1], MockMiddleware::SendKind::kBinary); +} + +// SubscribeToNonLatchedTopicSendsNoImmediateFrame +TEST_F(BridgeServerTest, SubscribeToNonLatchedTopicSendsNoImmediateFrame) { + ASSERT_TRUE(server_->initialize()); + mock_topic_source_->set_topics({{"/plain", "std_msgs/msg/String"}}); + mock_sub_manager_->add_known_topic("/plain"); + // Not marked transient_local. + + json sub; + sub["command"] = "subscribe"; + sub["topics"] = json::array({"/plain"}); + + mock_->push_request("client_a", sub.dump()); + server_->process_requests(); + mock_->pop_reply("client_a"); + + std::vector payload = {1, 2, 3}; + auto data = std::make_shared>(payload.size()); + for (size_t i = 0; i < payload.size(); ++i) { + (*data)[i] = static_cast(payload[i]); + } + mock_sub_manager_->deliver_message("/plain", data, 999); + server_->publish_aggregated_messages(); + mock_->clear_binary_sends(); + + // A second, late client subscribing to a non-latched topic must not + // receive any immediate replay frame. + mock_->push_request("client_b", sub.dump()); + server_->process_requests(); + json resp_b = mock_->pop_reply("client_b"); + ASSERT_FALSE(resp_b.is_discarded()); + EXPECT_EQ(resp_b["status"], "success"); + + EXPECT_TRUE(mock_->get_binary_sends().empty()); +} + +// LatchedReplaySkippedWhileSampleStillBuffered +// +// If the retained sample is still sitting in the normal message buffer (it +// arrived recently and has not been drained yet), the regular publish cycle +// will deliver it to the new subscriber anyway — a replay frame would +// duplicate it, so none must be sent. +TEST_F(BridgeServerTest, LatchedReplaySkippedWhileSampleStillBuffered) { + ASSERT_TRUE(server_->initialize()); + mock_topic_source_->set_topics({{"/latched", "std_msgs/msg/String"}}); + mock_sub_manager_->add_known_topic("/latched"); + mock_sub_manager_->set_transient_local("/latched", true); + + json sub; + sub["command"] = "subscribe"; + sub["topics"] = json::array({"/latched"}); + + // Client A subscribes, then a message arrives — but the buffer is NOT + // drained before client B subscribes. + mock_->push_request("client_a", sub.dump()); + server_->process_requests(); + mock_->pop_reply("client_a"); + + std::vector payload = {7, 8, 9}; + auto data = std::make_shared>(payload.size()); + for (size_t i = 0; i < payload.size(); ++i) { + (*data)[i] = static_cast(payload[i]); + } + mock_sub_manager_->deliver_message("/latched", data, 555); + mock_->clear_binary_sends(); + + // Client B subscribes while the sample is still in the normal buffer: + // no replay frame. + mock_->push_request("client_b", sub.dump()); + server_->process_requests(); + json resp_b = mock_->pop_reply("client_b"); + ASSERT_FALSE(resp_b.is_discarded()); + EXPECT_EQ(resp_b["status"], "success"); + EXPECT_TRUE(mock_->get_binary_sends().empty()); + + // The regular publish cycle then delivers the sample to client B (and A) + // through the normal aggregated path — exactly once, no duplicate. + server_->publish_aggregated_messages(); + bool b_received_via_normal_path = false; + for (const auto& [cid, frame] : mock_->get_binary_sends()) { + if (cid != "client_b") { + continue; + } + auto messages = decode_frame(frame); + ASSERT_EQ(messages.size(), 1u); + EXPECT_EQ(messages[0].topic, "/latched"); + EXPECT_EQ(messages[0].data, payload); + EXPECT_FALSE(b_received_via_normal_path) << "duplicate frame for client_b"; + b_received_via_normal_path = true; + } + EXPECT_TRUE(b_received_via_normal_path); +} + +// LastRefReleaseClearsLatchedSample +// +// When the last reference to a latched topic is released, the middleware +// subscription is destroyed and the retained sample goes stale. It must be +// cleared: the NEXT subscriber creates a fresh middleware subscription that +// receives the current sample via DDS transient_local redelivery, and must +// NOT be replayed the stale copy first. +TEST_F(BridgeServerTest, LastRefReleaseClearsLatchedSample) { + ASSERT_TRUE(server_->initialize()); + mock_topic_source_->set_topics({{"/latched", "std_msgs/msg/String"}}); + mock_sub_manager_->add_known_topic("/latched"); + mock_sub_manager_->set_transient_local("/latched", true); + + json sub; + sub["command"] = "subscribe"; + sub["topics"] = json::array({"/latched"}); + + // Client A subscribes; a message arrives and is drained (retained copy + // now lives only in the latched store). + mock_->push_request("client_a", sub.dump()); + server_->process_requests(); + mock_->pop_reply("client_a"); + + auto data = std::make_shared>(3, std::byte{42}); + mock_sub_manager_->deliver_message("/latched", data, 111); + server_->publish_aggregated_messages(); + + // Client A unsubscribes — last ref, subscription destroyed, latched + // state cleared. + json unsub; + unsub["command"] = "unsubscribe"; + unsub["topics"] = json::array({"/latched"}); + mock_->push_request("client_a", unsub.dump()); + server_->process_requests(); + mock_->pop_reply("client_a"); + ASSERT_EQ(mock_sub_manager_->ref_count("/latched"), 0); + mock_->clear_binary_sends(); + + // A NEW subscriber gets NO replay frame (the retained sample was + // cleared; DDS redelivery on the fresh subscription covers it). + mock_->push_request("client_b", sub.dump()); + server_->process_requests(); + json resp_b = mock_->pop_reply("client_b"); + ASSERT_FALSE(resp_b.is_discarded()); + EXPECT_EQ(resp_b["status"], "success"); + EXPECT_TRUE(mock_->get_binary_sends().empty()); +} + +// ResumeReplaysLatchedSampleForTopicSubscribedWhilePaused +// +// A client that subscribes to a latched topic WHILE PAUSED acquires no +// middleware ref (and gets no replay frame). On resume it joins the +// already-shared subscription, where DDS does not redeliver — the retained +// sample must be replayed then: resume RESPONSE first, then exactly one +// binary frame with the retained message. +TEST_F(BridgeServerTest, ResumeReplaysLatchedSampleForTopicSubscribedWhilePaused) { + ASSERT_TRUE(server_->initialize()); + mock_topic_source_->set_topics({{"/latched", "std_msgs/msg/String"}}); + mock_sub_manager_->add_known_topic("/latched"); + mock_sub_manager_->set_transient_local("/latched", true); + + json sub; + sub["command"] = "subscribe"; + sub["topics"] = json::array({"/latched"}); + + // Client A subscribes (seeds the latched retention), a message arrives + // and is drained — the retained sample has "aged out" of the normal + // buffer and lives only in the latched store. + mock_->push_request("client_a", sub.dump()); + server_->process_requests(); + mock_->pop_reply("client_a"); + + std::vector payload = {3, 1, 4, 1, 5}; + auto data = std::make_shared>(payload.size()); + for (size_t i = 0; i < payload.size(); ++i) { + (*data)[i] = static_cast(payload[i]); + } + mock_sub_manager_->deliver_message("/latched", data, 777); + server_->publish_aggregated_messages(); + ASSERT_EQ(mock_sub_manager_->ref_count("/latched"), 1); + + // Client P pauses, then subscribes to the latched topic while paused: + // no ref acquired, no replay frame. + json pause_req; + pause_req["command"] = "pause"; + mock_->push_request("client_p", pause_req.dump()); + server_->process_requests(); + mock_->pop_reply("client_p"); + mock_->clear_binary_sends(); + + mock_->push_request("client_p", sub.dump()); + server_->process_requests(); + json sub_resp = mock_->pop_reply("client_p"); + ASSERT_FALSE(sub_resp.is_discarded()); + EXPECT_EQ(sub_resp["status"], "success"); + EXPECT_TRUE(mock_->get_binary_sends().empty()); + ASSERT_EQ(mock_sub_manager_->ref_count("/latched"), 1); // still only client A + + // Resume: client P joins the already-shared subscription and must be + // replayed the retained sample — resume response FIRST, then the frame. + mock_->clear_send_log(); + json resume_req; + resume_req["command"] = "resume"; + mock_->push_request("client_p", resume_req.dump()); + server_->process_requests(); + json resume_resp = mock_->pop_reply("client_p"); + ASSERT_FALSE(resume_resp.is_discarded()); + EXPECT_EQ(resume_resp["status"], "ok"); + EXPECT_FALSE(resume_resp["paused"].get()); + EXPECT_EQ(mock_sub_manager_->ref_count("/latched"), 2); + + // Exactly one binary frame, to client_p only, with the retained message. + auto binary_sends = mock_->get_binary_sends(); + ASSERT_EQ(binary_sends.size(), 1u); + EXPECT_EQ(binary_sends[0].first, "client_p"); + auto messages = decode_frame(binary_sends[0].second); + ASSERT_EQ(messages.size(), 1u); + EXPECT_EQ(messages[0].topic, "/latched"); + EXPECT_EQ(messages[0].timestamp_ns, 777u); + EXPECT_EQ(messages[0].data, payload); + + // Ordering: resume response (text) before the replay frame (binary). + std::vector p_sends; + for (const auto& [kind, cid] : mock_->get_send_log()) { + if (cid == "client_p") { + p_sends.push_back(kind); + } + } + ASSERT_EQ(p_sends.size(), 2u); + EXPECT_EQ(p_sends[0], MockMiddleware::SendKind::kText); + EXPECT_EQ(p_sends[1], MockMiddleware::SendKind::kBinary); +} diff --git a/tests/unit/test_generic_subscription_manager.cpp b/tests/unit/test_generic_subscription_manager.cpp index 46c40e6..70d6c15 100644 --- a/tests/unit/test_generic_subscription_manager.cpp +++ b/tests/unit/test_generic_subscription_manager.cpp @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -29,6 +30,46 @@ using namespace pj_bridge; +namespace { +// History depth is a purely local resource policy, not one of the +// requested/offered QoS policies DDS discovery is required to propagate +// (unlike reliability/durability, which affect wire compatibility). This +// workspace's default RMW (rmw_fastrtps_cpp) does not report it — publishers +// discovered via get_publishers_info_by_topic() always come back with depth +// 0 / history UNKNOWN — whereas CycloneDDS does propagate it. The QoS depth +// aggregation tests below need a real (non-mocked) discovered depth to +// exercise adapt_qos(), so force CycloneDDS for this test binary. +// +// The RMW implementation is selected once per process and cached on first +// use, so this must happen before any rclcpp::init() call anywhere in the +// binary. Static objects across all translation units are guaranteed to be +// constructed before main() runs (and therefore before any TEST_F body), +// so a file-scope static works regardless of test execution order. An +// operator's own explicit RMW_IMPLEMENTATION is still respected. +struct ForceCycloneDdsForQosDepthTests { + ForceCycloneDdsForQosDepthTests() { + if (std::getenv("RMW_IMPLEMENTATION") == nullptr) { + setenv("RMW_IMPLEMENTATION", "rmw_cyclonedds_cpp", 1); + } + } +}; +const ForceCycloneDdsForQosDepthTests force_cyclonedds_for_qos_depth_tests; +} // namespace + +namespace { +// Publisher discovery is asynchronous even within a single process, so tests +// must poll the graph rather than assume a freshly created publisher is +// immediately visible via get_publishers_info_by_topic(). +void wait_for_publisher_count( + const rclcpp::Node::SharedPtr& node, const std::string& topic_name, size_t expected_count) { + auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + while (node->get_publishers_info_by_topic(topic_name).size() < expected_count && + std::chrono::steady_clock::now() < deadline) { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } +} +} // namespace + class GenericSubscriptionManagerTest : public ::testing::Test { protected: void SetUp() override { @@ -43,6 +84,19 @@ class GenericSubscriptionManagerTest : public ::testing::Test { rclcpp::shutdown(); } + // Runtime probe: returns true if the active RMW propagates publisher + // history depth through discovery. The static initializer above forces + // CycloneDDS (which does), but a harness exporting its own + // RMW_IMPLEMENTATION overrides it — in that case the depth-assertion + // tests must be skipped rather than fail on discovery values the RMW + // simply never reports. + bool rmw_propagates_depth() { + auto probe = node_->create_publisher("/qos_depth_probe_topic", rclcpp::QoS(7)); + wait_for_publisher_count(node_, "/qos_depth_probe_topic", 1); + auto infos = node_->get_publishers_info_by_topic("/qos_depth_probe_topic"); + return !infos.empty() && infos[0].qos_profile().depth() == 7; + } + rclcpp::Node::SharedPtr node_; std::unique_ptr manager_; }; @@ -226,6 +280,9 @@ TEST_F(GenericSubscriptionManagerTest, MultipleTopicsIndependent) { // --------------------------------------------------------------------------- TEST_F(GenericSubscriptionManagerTest, AdaptsQosToBestEffortPublisher) { auto publisher = node_->create_publisher("/qos_be_topic", rclcpp::SensorDataQoS()); + // Wait for discovery before subscribing so adapt_qos() sees the publisher + // (discovery is asynchronous and its latency varies by RMW implementation). + wait_for_publisher_count(node_, "/qos_be_topic", 1); std::atomic received{0}; auto callback = [&received](const std::string&, const std::shared_ptr&, uint64_t) { @@ -247,3 +304,134 @@ TEST_F(GenericSubscriptionManagerTest, AdaptsQosToBestEffortPublisher) { EXPECT_GT(received.load(), 0) << "RELIABLE subscription never matches a BEST_EFFORT publisher"; } + +// --------------------------------------------------------------------------- +// QoS depth aggregation +// +// Depth aggregation adapted from foxglove_bridge (MIT License, Copyright (c) +// Foxglove Technologies Inc): each publisher contributes its history depth +// (or the fallback of 100 when it reports 0 — unknown/KEEP_ALL), the sum is +// saturating (each contribution capped at max_qos_depth, running total +// clamped to max_qos_depth), and the result is clamped to +// [min_qos_depth, max_qos_depth]. +// +// The size_t-overflow case is not practically constructible with real +// publishers; it is covered by the saturation logic itself (the +// per-contribution cap makes wrap impossible), so no test exists for it. +// +// (See the ForceCycloneDdsForQosDepthTests note above for why this test +// binary forces CycloneDDS. Each depth-assertion test additionally probes +// the active RMW at runtime and skips if it doesn't propagate depths.) +// --------------------------------------------------------------------------- + +#define SKIP_IF_RMW_HIDES_DEPTH() \ + if (!rmw_propagates_depth()) { \ + GTEST_SKIP() << "Active RMW does not propagate publisher history depth through " \ + "discovery; depth aggregation cannot be observed with real values."; \ + } + +TEST_F(GenericSubscriptionManagerTest, AdaptQosDepthMatchesSinglePublisher) { + SKIP_IF_RMW_HIDES_DEPTH(); + auto publisher = node_->create_publisher("/qos_depth_topic1", rclcpp::QoS(10)); + wait_for_publisher_count(node_, "/qos_depth_topic1", 1); + + rclcpp::QoS qos = manager_->adapt_qos("/qos_depth_topic1"); + + EXPECT_EQ(qos.depth(), 10u); +} + +TEST_F(GenericSubscriptionManagerTest, AdaptQosDepthSumsAcrossPublishersClampedToMax) { + SKIP_IF_RMW_HIDES_DEPTH(); + auto publisher1 = node_->create_publisher("/qos_depth_topic2", rclcpp::QoS(60)); + auto publisher2 = node_->create_publisher("/qos_depth_topic2", rclcpp::QoS(60)); + wait_for_publisher_count(node_, "/qos_depth_topic2", 2); + + rclcpp::QoS qos = manager_->adapt_qos("/qos_depth_topic2"); + + // 60 + 60 = 120, clamped to the default max_qos_depth of 100. + EXPECT_EQ(qos.depth(), 100u); +} + +TEST_F(GenericSubscriptionManagerTest, AdaptQosDepthClampedToConfiguredMinimum) { + SKIP_IF_RMW_HIDES_DEPTH(); + auto manager_with_min = std::make_unique(node_, /*min_qos_depth=*/30); + auto publisher = node_->create_publisher("/qos_depth_topic3", rclcpp::QoS(5)); + wait_for_publisher_count(node_, "/qos_depth_topic3", 1); + + rclcpp::QoS qos = manager_with_min->adapt_qos("/qos_depth_topic3"); + + EXPECT_EQ(qos.depth(), 30u); +} + +TEST_F(GenericSubscriptionManagerTest, AdaptQosDepthDefaultsTo100WhenNoPublishers) { + // RMW-independent: no publishers are involved, so no skip guard needed. + rclcpp::QoS qos = manager_->adapt_qos("/qos_depth_no_publishers"); + + EXPECT_EQ(qos.depth(), 100u); +} + +TEST_F(GenericSubscriptionManagerTest, AdaptQosDepthFallsBackTo100WhenDepthUnknown) { + SKIP_IF_RMW_HIDES_DEPTH(); + // A KEEP_ALL publisher reports history depth 0 through discovery — the + // same value RMWs that don't propagate depth at all (e.g. rmw_fastrtps_cpp) + // report for every publisher. Depth 0 means "unknown", so the publisher + // contributes the fallback of 100 instead of 0 — not clamped up to + // min_qos_depth (a depth-1 queue would drop messages on high-rate topics). + auto publisher = + node_->create_publisher("/qos_depth_keep_all_topic", rclcpp::QoS(rclcpp::KeepAll())); + wait_for_publisher_count(node_, "/qos_depth_keep_all_topic", 1); + + ASSERT_EQ(node_->get_publishers_info_by_topic("/qos_depth_keep_all_topic")[0].qos_profile().depth(), 0u) + << "Precondition: discovery must report depth 0 for a KEEP_ALL publisher"; + + rclcpp::QoS qos = manager_->adapt_qos("/qos_depth_keep_all_topic"); + + EXPECT_EQ(qos.depth(), 100u); +} + +TEST_F(GenericSubscriptionManagerTest, AdaptQosDepthMixedKnownAndUnknownPublishers) { + SKIP_IF_RMW_HIDES_DEPTH(); + // One KEEP_ALL publisher (reports 0 → contributes the fallback of 100) + // plus one depth-10 publisher: 100 + 10 saturates at the default + // max_qos_depth of 100. + auto publisher_keep_all = + node_->create_publisher("/qos_depth_mixed_topic", rclcpp::QoS(rclcpp::KeepAll())); + auto publisher_depth_10 = node_->create_publisher("/qos_depth_mixed_topic", rclcpp::QoS(10)); + wait_for_publisher_count(node_, "/qos_depth_mixed_topic", 2); + + rclcpp::QoS qos = manager_->adapt_qos("/qos_depth_mixed_topic"); + + EXPECT_EQ(qos.depth(), 100u); +} + +// --------------------------------------------------------------------------- +// is_transient_local() +// +// Durability (unlike history depth) is one of the QoS policies DDS discovery +// is required to propagate, since it affects wire compatibility — every RMW +// reports it correctly, so these tests need no SKIP_IF_RMW_HIDES_DEPTH guard. +// --------------------------------------------------------------------------- + +TEST_F(GenericSubscriptionManagerTest, IsTransientLocalTrueWhenPublisherOffersIt) { + auto publisher = node_->create_publisher("/latched_topic", rclcpp::QoS(1).transient_local()); + wait_for_publisher_count(node_, "/latched_topic", 1); + + auto callback = [](const std::string&, const std::shared_ptr&, uint64_t) {}; + ASSERT_TRUE(manager_->subscribe("/latched_topic", "sensor_msgs/msg/Imu", callback)); + + EXPECT_TRUE(manager_->is_transient_local("/latched_topic")); +} + +TEST_F(GenericSubscriptionManagerTest, IsTransientLocalFalseForVolatilePublisher) { + auto publisher = node_->create_publisher("/volatile_topic", rclcpp::QoS(1)); + wait_for_publisher_count(node_, "/volatile_topic", 1); + + auto callback = [](const std::string&, const std::shared_ptr&, uint64_t) {}; + ASSERT_TRUE(manager_->subscribe("/volatile_topic", "sensor_msgs/msg/Imu", callback)); + + EXPECT_FALSE(manager_->is_transient_local("/volatile_topic")); +} + +TEST_F(GenericSubscriptionManagerTest, IsTransientLocalFalseForUnsubscribedTopic) { + EXPECT_FALSE(manager_->is_transient_local("/never_subscribed_topic")); +} diff --git a/tests/unit/test_message_buffer.cpp b/tests/unit/test_message_buffer.cpp index 31ab4a4..6e7a8e1 100644 --- a/tests/unit/test_message_buffer.cpp +++ b/tests/unit/test_message_buffer.cpp @@ -397,3 +397,116 @@ TEST_F(MessageBufferTest, ExpiredMessagesStillPurgedWithInjectedClock) { // The first message is stale and must be gone; the new one remains EXPECT_EQ(buffer.size(), 1); } + +// --------------------------------------------------------------------------- +// Latched (transient_local) topic support +// +// A latched topic retains its newest sample outside the normal TTL/move_messages +// path so that late subscribers to an already-shared subscription can be +// replayed the current value even after it has aged out of the buffer. +// --------------------------------------------------------------------------- + +TEST_F(MessageBufferTest, LatchedMessageSurvivesTtlAndMoveMessages) { + uint64_t fake_now = 10'000'000'000ULL; + MessageBuffer buffer(MessageBuffer::kDefaultMaxMessageAgeNs, [&fake_now]() { return fake_now; }); + + buffer.set_latched("/t", true); + + std::vector data = {1, 2, 3}; + buffer.add_message("/t", 1, create_test_data(data)); + + // Advance the clock well past the 1 s TTL and add a message on another + // topic to trigger cleanup of the (non-latched) deque path. + fake_now += 2'000'000'000ULL; + buffer.add_message("/other", 2, create_test_data({9})); + + // Drain everything via move_messages — the latched store must not be + // touched by this call. + std::unordered_map> messages; + buffer.move_messages(messages); + + auto latched = buffer.get_latched("/t"); + ASSERT_TRUE(latched.has_value()); + EXPECT_EQ(extract_data(latched->data), data); +} + +TEST_F(MessageBufferTest, GetLatchedReturnsNewestMessage) { + buffer_.set_latched("/t", true); + + buffer_.add_message("/t", 1, create_test_data({1, 1, 1})); + buffer_.add_message("/t", 2, create_test_data({2, 2, 2})); + + auto latched = buffer_.get_latched("/t"); + ASSERT_TRUE(latched.has_value()); + EXPECT_EQ(extract_data(latched->data), (std::vector{2, 2, 2})); + EXPECT_EQ(latched->timestamp_ns, 2u); +} + +TEST_F(MessageBufferTest, SetLatchedFalseClearsRetainedEntry) { + buffer_.set_latched("/t", true); + buffer_.add_message("/t", 1, create_test_data({1, 2, 3})); + ASSERT_TRUE(buffer_.get_latched("/t").has_value()); + + buffer_.set_latched("/t", false); + EXPECT_FALSE(buffer_.get_latched("/t").has_value()); + + // Once unlatched, further messages are not retained either. + buffer_.add_message("/t", 2, create_test_data({4, 5, 6})); + EXPECT_FALSE(buffer_.get_latched("/t").has_value()); +} + +TEST_F(MessageBufferTest, NonLatchedTopicHasNoRetainedMessage) { + buffer_.add_message("/plain", 1, create_test_data({1, 2, 3})); + EXPECT_FALSE(buffer_.get_latched("/plain").has_value()); +} + +TEST_F(MessageBufferTest, ClearAlsoClearsLatchedStorage) { + buffer_.set_latched("/t", true); + buffer_.add_message("/t", 1, create_test_data({1, 2, 3})); + ASSERT_TRUE(buffer_.get_latched("/t").has_value()); + + buffer_.clear(); + EXPECT_FALSE(buffer_.get_latched("/t").has_value()); + + // Latching config was cleared too: a message added post-clear (without a + // fresh set_latched call) is not retained. + buffer_.add_message("/t", 2, create_test_data({4, 5, 6})); + EXPECT_FALSE(buffer_.get_latched("/t").has_value()); +} + +TEST_F(MessageBufferTest, GetLatchedForReplaySkipsWhileSampleStillBuffered) { + buffer_.set_latched("/t", true); + buffer_.add_message("/t", 1, create_test_data({1, 2, 3})); + + // The sample is still in the normal buffer: the regular publish cycle + // will deliver it, so the replay accessor must return nothing (a replay + // now would duplicate the message). The raw accessor still sees it. + EXPECT_FALSE(buffer_.get_latched_for_replay("/t").has_value()); + EXPECT_TRUE(buffer_.get_latched("/t").has_value()); + + // Once drained by move_messages, the retained copy becomes replayable. + std::unordered_map> messages; + buffer_.move_messages(messages); + auto replay = buffer_.get_latched_for_replay("/t"); + ASSERT_TRUE(replay.has_value()); + EXPECT_EQ(extract_data(replay->data), (std::vector{1, 2, 3})); +} + +TEST_F(MessageBufferTest, GetLatchedForReplayAvailableAfterTtlEviction) { + uint64_t fake_now = 10'000'000'000ULL; + MessageBuffer buffer(MessageBuffer::kDefaultMaxMessageAgeNs, [&fake_now]() { return fake_now; }); + + buffer.set_latched("/t", true); + buffer.add_message("/t", 1, create_test_data({4, 5, 6})); + EXPECT_FALSE(buffer.get_latched_for_replay("/t").has_value()); + + // TTL eviction (triggered by an add on another topic) removes the sample + // from the normal buffer without draining it to any client — the + // retained copy must then be available for replay. + fake_now += 2'000'000'000ULL; + buffer.add_message("/other", 2, create_test_data({9})); + + auto replay = buffer.get_latched_for_replay("/t"); + ASSERT_TRUE(replay.has_value()); + EXPECT_EQ(extract_data(replay->data), (std::vector{4, 5, 6})); +} diff --git a/tests/unit/test_ros2_subscription_manager.cpp b/tests/unit/test_ros2_subscription_manager.cpp index ace2f2d..b21fb53 100644 --- a/tests/unit/test_ros2_subscription_manager.cpp +++ b/tests/unit/test_ros2_subscription_manager.cpp @@ -19,11 +19,10 @@ #include -#include -#include - #include #include +#include +#include #include #include "pj_bridge_ros2/ros2_subscription_manager.hpp" diff --git a/tests/unit/test_session_manager.cpp b/tests/unit/test_session_manager.cpp index a27c537..43e4f4d 100644 --- a/tests/unit/test_session_manager.cpp +++ b/tests/unit/test_session_manager.cpp @@ -324,6 +324,30 @@ TEST_F(SessionManagerTest, PausedStatePreservedAcrossHeartbeats) { EXPECT_TRUE(manager_.is_paused("client1")); } +TEST_F(SessionManagerTest, SessionStartsWithTopicUpdatesDisabled) { + EXPECT_TRUE(manager_.create_session("client1")); + + EXPECT_FALSE(manager_.wants_topic_updates("client1")); +} + +TEST_F(SessionManagerTest, SetTopicUpdatesChangesState) { + EXPECT_TRUE(manager_.create_session("client1")); + + EXPECT_TRUE(manager_.set_topic_updates("client1", true)); + EXPECT_TRUE(manager_.wants_topic_updates("client1")); + + EXPECT_TRUE(manager_.set_topic_updates("client1", false)); + EXPECT_FALSE(manager_.wants_topic_updates("client1")); +} + +TEST_F(SessionManagerTest, SetTopicUpdatesReturnsFalseForNonexistentSession) { + EXPECT_FALSE(manager_.set_topic_updates("nonexistent", true)); +} + +TEST_F(SessionManagerTest, WantsTopicUpdatesReturnsFalseForNonexistentSession) { + EXPECT_FALSE(manager_.wants_topic_updates("nonexistent")); +} + TEST_F(SessionManagerTest, ThreadSafety) { SessionManager thread_manager(10.0); constexpr int kNumThreads = 10; diff --git a/tests/unit/test_websocket_middleware.cpp b/tests/unit/test_websocket_middleware.cpp index 407a7ad..573a73a 100644 --- a/tests/unit/test_websocket_middleware.cpp +++ b/tests/unit/test_websocket_middleware.cpp @@ -22,6 +22,8 @@ #include #include +#include +#include #include #include #include @@ -440,6 +442,227 @@ TEST_F(WebSocketMiddlewareTest, IncomingQueueBounded) { std::this_thread::sleep_for(std::chrono::milliseconds(100)); } +// --------------------------------------------------------------------------- +// Task 4 — Slow-client backpressure (bounded queue, drop-oldest) +// +// The drop-oldest policy itself is exercised by BoundedFrameQueue's own unit +// tests (test_bounded_frame_queue.cpp), since forcing a real socket's +// bufferedAmount() over the 1 MiB watermark is not realistically achievable +// in a fast unit test. These tests instead cover the parts that only make +// sense with a live WebSocketMiddleware: normal traffic never counts as +// dropped, and the constructor accepts an explicit backlog size. +// --------------------------------------------------------------------------- +TEST_F(WebSocketMiddlewareTest, DroppedFrameCountZeroAfterNormalTraffic) { + auto result = middleware_->initialize(18096); + ASSERT_TRUE(result.has_value()); + + EXPECT_EQ(middleware_->dropped_frame_count(), 0u); + + ix::WebSocket client; + client.setUrl("ws://127.0.0.1:18096"); + client.setOnMessageCallback([](const ix::WebSocketMessagePtr& /*msg*/) {}); + client.start(); + + ASSERT_TRUE(wait_for_client_open(client)) << "Client failed to connect"; + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + + client.send("register"); + std::vector data; + std::string client_id; + ASSERT_TRUE(poll_receive_request(*middleware_, data, client_id)); + ASSERT_FALSE(client_id.empty()); + + std::vector payload = {1, 2, 3, 4}; + for (int i = 0; i < 10; ++i) { + EXPECT_TRUE(middleware_->send_binary(client_id, payload)); + } + + EXPECT_EQ(middleware_->dropped_frame_count(), 0u); + + client.stop(); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); +} + +// Regression guard for the disconnect-race fix in send_binary's enqueue path: +// a client that is not (or no longer) in clients_ must never get a +// pending_frames_ entry created for it. The over-watermark branch itself +// cannot be reached from a unit test (we cannot force a real socket's +// bufferedAmount() past 1 MiB), so this verifies the shared contract via the +// public API: send_binary for an id absent from clients_ returns false and +// leaves no pending state behind — dropped_frame_count() stays 0 even after +// repeated attempts. The in-branch re-check under clients_mutex_ enforces the +// same "client gone -> return false, no queue created" rule; only the actual +// interleaving with a concurrent disconnect is not exercised here. +TEST_F(WebSocketMiddlewareTest, SendBinaryToAbsentClientCreatesNoPendingState) { + auto result = middleware_->initialize(18098); + ASSERT_TRUE(result.has_value()); + + std::vector data = {1, 2, 3}; + for (int i = 0; i < 5; ++i) { + EXPECT_FALSE(middleware_->send_binary("never-connected", data)); + } + EXPECT_EQ(middleware_->dropped_frame_count(), 0u); + + // Shutdown folds any (erroneously) surviving per-client queues into the + // disconnected-clients accumulator — the count must still be zero after. + middleware_->shutdown(); + EXPECT_EQ(middleware_->dropped_frame_count(), 0u); +} + +// drop_pending discards a client's backpressure backlog when its session is +// destroyed server-side (heartbeat timeout) while the socket stays open. A +// unit test cannot force the over-watermark path that populates the backlog, +// so this verifies the reachable contract: drop_pending is a safe idempotent +// no-op for clients with no pending state (never connected, or connected but +// never backlogged), never skews dropped_frame_count(), and leaves a live +// client's socket fully usable — send_binary still succeeds afterwards. +TEST_F(WebSocketMiddlewareTest, DropPendingIsSafeNoOpAndKeepsSocketUsable) { + auto result = middleware_->initialize(18099); + ASSERT_TRUE(result.has_value()); + + // No such client: must not crash or create state. + middleware_->drop_pending("never-connected"); + middleware_->drop_pending("never-connected"); + EXPECT_EQ(middleware_->dropped_frame_count(), 0u); + + // Connected client with an empty backlog: drop_pending must not touch the + // socket itself. + ix::WebSocket client; + client.setUrl("ws://127.0.0.1:18099"); + client.setOnMessageCallback([](const ix::WebSocketMessagePtr& /*msg*/) {}); + client.start(); + + ASSERT_TRUE(wait_for_client_open(client)) << "Client failed to connect"; + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + + client.send("register"); + std::vector data; + std::string client_id; + ASSERT_TRUE(poll_receive_request(*middleware_, data, client_id)); + ASSERT_FALSE(client_id.empty()); + + middleware_->drop_pending(client_id); + EXPECT_EQ(middleware_->dropped_frame_count(), 0u); + + std::vector payload = {1, 2, 3, 4}; + EXPECT_TRUE(middleware_->send_binary(client_id, payload)); + EXPECT_EQ(middleware_->dropped_frame_count(), 0u); + + client.stop(); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); +} + +TEST(WebSocketMiddlewareBacklogConstructionTest, ExplicitBacklogSizeConstructsAndInitializes) { + WebSocketMiddleware middleware(5); + EXPECT_EQ(middleware.dropped_frame_count(), 0u); + + auto result = middleware.initialize(18097); + ASSERT_TRUE(result.has_value()); + EXPECT_TRUE(middleware.is_ready()); + + middleware.shutdown(); +} + +// --------------------------------------------------------------------------- +// Task 6 — TLS (wss://) via OpenSSL, server certificate only +// --------------------------------------------------------------------------- + +TEST(WebSocketMiddlewareTlsTest, MissingCertFileFailsInitializeWithPathInMessage) { + pj_bridge::TlsConfig tls{"/nonexistent/cert.pem", "/nonexistent/key.pem"}; + WebSocketMiddleware middleware(100, tls); + + auto result = middleware.initialize(18099); + ASSERT_FALSE(result.has_value()); + EXPECT_NE(result.error().find("/nonexistent/cert.pem"), std::string::npos) + << "Error message should mention the missing cert file path: " << result.error(); +} + +#ifdef IXWEBSOCKET_USE_TLS +namespace { + +// Generates a throwaway self-signed cert/key pair under `dir` using the +// system `openssl` binary. Returns true on success; the caller should +// GTEST_SKIP() when this fails (missing openssl binary, sandboxed +// environment, etc.) rather than fail the test outright. +bool generate_self_signed_cert(const std::string& certfile, const std::string& keyfile) { + std::string cmd = "openssl req -x509 -newkey rsa:2048 -keyout " + keyfile + " -out " + certfile + + " -days 1 -nodes -subj /CN=localhost 2>/dev/null"; + int rc = std::system(cmd.c_str()); + return rc == 0 && std::filesystem::exists(certfile) && std::filesystem::exists(keyfile); +} + +} // namespace + +TEST(WebSocketMiddlewareTlsTest, TlsRoundTrip) { + auto dir = std::filesystem::temp_directory_path() / "pj_bridge_tls_test"; + std::filesystem::create_directories(dir); + std::string certfile = (dir / "cert.pem").string(); + std::string keyfile = (dir / "key.pem").string(); + + if (!generate_self_signed_cert(certfile, keyfile)) { + GTEST_SKIP() << "Could not generate a self-signed certificate (openssl binary unavailable or failed)"; + } + + pj_bridge::TlsConfig tls{certfile, keyfile}; + WebSocketMiddleware middleware(100, tls); + + auto result = middleware.initialize(18199); + ASSERT_TRUE(result.has_value()) << (result.has_value() ? "" : result.error()); + ASSERT_TRUE(middleware.is_ready()); + + ix::WebSocket client; + client.setUrl("wss://127.0.0.1:18199"); + + ix::SocketTLSOptions client_tls_options; + client_tls_options.caFile = "NONE"; // self-signed cert: disable peer verification + client.setTLSOptions(client_tls_options); + + std::mutex msg_mutex; + std::condition_variable msg_cv; + std::string received_message; + bool message_received = false; + + client.setOnMessageCallback([&](const ix::WebSocketMessagePtr& msg) { + if (msg->type == ix::WebSocketMessageType::Message && !msg->binary) { + std::lock_guard lock(msg_mutex); + received_message = msg->str; + message_received = true; + msg_cv.notify_one(); + } + }); + client.start(); + + ASSERT_TRUE(wait_for_client_open(client)) << "TLS client failed to connect"; + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + + const std::string test_message = "hello over wss"; + client.send(test_message); + + std::vector data; + std::string client_id; + ASSERT_TRUE(poll_receive_request(middleware, data, client_id)) << "receive_request() never returned the message"; + std::string received(data.begin(), data.end()); + EXPECT_EQ(received, test_message); + + const std::string reply_text = "reply over wss"; + std::vector reply_data(reply_text.begin(), reply_text.end()); + EXPECT_TRUE(middleware.send_reply(client_id, reply_data)); + + { + std::unique_lock lock(msg_mutex); + ASSERT_TRUE(msg_cv.wait_for(lock, std::chrono::seconds(2), [&] { return message_received; })) + << "TLS client never received the reply"; + } + EXPECT_EQ(received_message, reply_text); + + client.stop(); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + middleware.shutdown(); + + std::filesystem::remove_all(dir); +} +#endif // IXWEBSOCKET_USE_TLS + int main(int argc, char** argv) { testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/tests/unit/test_whitelist_filter.cpp b/tests/unit/test_whitelist_filter.cpp new file mode 100644 index 0000000..d7074cb --- /dev/null +++ b/tests/unit/test_whitelist_filter.cpp @@ -0,0 +1,87 @@ +/* + * 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 "pj_bridge/whitelist_filter.hpp" + +namespace pj_bridge { +namespace { + +TEST(WhitelistFilterTest, DefaultConstructedMatchesEverything) { + WhitelistFilter filter; + EXPECT_TRUE(filter.matches("/anything")); + EXPECT_TRUE(filter.matches("/foo/bar")); + EXPECT_TRUE(filter.matches("")); +} + +TEST(WhitelistFilterTest, CreateSucceedsForValidPatterns) { + auto result = WhitelistFilter::create({"/camera.*", "/imu"}); + ASSERT_TRUE(result.has_value()); +} + +TEST(WhitelistFilterTest, CreateFailsForInvalidRegexAndNamesOffendingPattern) { + auto result = WhitelistFilter::create({"/valid", "("}); + ASSERT_FALSE(result.has_value()); + EXPECT_NE(result.error().find("("), std::string::npos); +} + +TEST(WhitelistFilterTest, FullMatchSemanticsRejectPartialMatch) { + auto result = WhitelistFilter::create({"/cam"}); + ASSERT_TRUE(result.has_value()); + WhitelistFilter filter = result.value(); + + // "/cam" must NOT match "/camera" — full match, not prefix match. + EXPECT_FALSE(filter.matches("/camera")); + EXPECT_TRUE(filter.matches("/cam")); +} + +TEST(WhitelistFilterTest, WildcardPatternMatchesSubPaths) { + auto result = WhitelistFilter::create({"/camera.*"}); + ASSERT_TRUE(result.has_value()); + WhitelistFilter filter = result.value(); + + EXPECT_TRUE(filter.matches("/camera")); + EXPECT_TRUE(filter.matches("/camera/image")); + EXPECT_FALSE(filter.matches("/other")); +} + +TEST(WhitelistFilterTest, MatchesIfAnyPatternMatches) { + auto result = WhitelistFilter::create({"/foo", "/bar.*"}); + ASSERT_TRUE(result.has_value()); + WhitelistFilter filter = result.value(); + + EXPECT_TRUE(filter.matches("/foo")); + EXPECT_TRUE(filter.matches("/bar/baz")); + EXPECT_FALSE(filter.matches("/qux")); +} + +TEST(WhitelistFilterTest, EmptyPatternListMatchesEverything) { + auto result = WhitelistFilter::create({}); + ASSERT_TRUE(result.has_value()); + WhitelistFilter filter = result.value(); + + EXPECT_TRUE(filter.matches("/anything")); +} + +} // namespace +} // namespace pj_bridge