Repository navigation
E2E latency measurements - #50
stephen-derosa wants to merge 4 commits into
Conversation
BOT-471 Latency Measurement
Goal: calculate and determine internal bridge latency. The goal is very very small so we get to hang our hat on the same number the cpp sdk uses. single computer, two bridges custom ROS message: T0= pub to bridge T1 = bridge receives T2 = bridge forwards over livekit T3 = livekit receives T4 = bridge pubs T5 = sub receives |
862a8e7 to
ec681f3
Compare
There was a problem hiding this comment.
Pull request overview
Adds an end-to-end latency measurement workflow for ros2_livekit_bridge, including a typed latency probe topic that is specially forwarded (with T1–T4 stamping) and supporting tooling to publish probes, compute rolling percentile stats, and run a CI regression guard.
Changes:
- Introduces
measure_latencyconfig support and a dedicated typed forwarding path for/ros2_livekit_bridge/latency/timestamp→/ros2_livekit_bridge/latency/timestamp_rx. - Adds latency probe publisher/subscriber utilities + a helper launch file for running a full measurement across two ROS domains.
- Adds new message definitions and an integration test to guard against bridge-internal latency regressions; updates docs/config accordingly.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/waveshare_launch/config/waveshare_livekit_robot.yaml | Enables measure_latency for waveshare robot config. |
| src/test/waveshare_launch/config/waveshare_livekit_controller.yaml | Enables measure_latency for waveshare controller config. |
| src/test/test_utilities/scripts/latency_probe_subscriber.py | New node to compute rolling latency percentiles and publish LatencyStats. |
| src/test/test_utilities/scripts/latency_probe_publisher.py | New node to publish LatencyTimestamps probes (T0 end). |
| src/test/test_utilities/package.xml | Updates runtime dependencies for new latency tooling. |
| src/test/test_utilities/launch/latency_measurement.launch.py | New launch file to run two bridges + probes in separate ROS domains. |
| src/test/test_utilities/CMakeLists.txt | Installs the new probe scripts. |
| src/ros2_livekit_bridge/test/integration/latency_e2e_test.cpp | New integration regression guard for bridge-internal latency. |
| src/ros2_livekit_bridge/test/integration/bridge_e2e_fixture.hpp | Adds measure_latency option support to the integration fixture YAML generator. |
| src/ros2_livekit_bridge/src/topic_forwarder.cpp | Implements the dedicated typed latency probe forwarding path (stamp + republish). |
| src/ros2_livekit_bridge/src/ros2_livekit_bridge.cpp | Plumbs measure_latency config into TopicForwarder options. |
| src/ros2_livekit_bridge/README.md | Documents how to characterize and regress latency using the new probes. |
| src/ros2_livekit_bridge/launch/livekit_bridge_local.launch.py | Minor formatting change (currently introduces whitespace noise). |
| src/ros2_livekit_bridge/include/ros2_livekit_bridge/topic_forwarder.hpp | Adds latency topic constants and option flag documentation/fields. |
| src/ros2_livekit_bridge/include/ros2_livekit_bridge/ros2_livekit_bridge.hpp | Stores measure_latency_ in the bridge node. |
| src/ros2_livekit_bridge_msgs/package.xml | Adds message package dependencies for new msg types. |
| src/ros2_livekit_bridge_msgs/msg/LatencyTimestamps.msg | New probe message carrying T0–T5 and payload padding. |
| src/ros2_livekit_bridge_msgs/msg/LatencyStats.msg | New stats message for rolling summaries. |
| src/ros2_livekit_bridge_msgs/msg/LatencyMetric.msg | New per-segment percentile summary message. |
| src/ros2_livekit_bridge_msgs/CMakeLists.txt | Generates the new msg interfaces. |
| src/ros2_livekit_bridge_config/schema/ros2_livekit_bridge_config.schema.json | Adds measure_latency config field (description currently out of sync with implementation). |
| docs/configuration.md | Documents measure_latency behavior and links to the package README. |
Comments suppressed due to low confidence (1)
src/ros2_livekit_bridge_msgs/msg/LatencyStats.msg:37
- Typo in field comment: "receceives" -> "receives" (and the message name should match
LatencyTimestamps).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a666e5d to
37eaaf4
Compare
| } | ||
| } | ||
|
|
||
| void TopicForwarder::readInboundLatencyTrack(std::shared_ptr<InboundDataTrackState> state) { |
There was a problem hiding this comment.
id really like a more isolated place for this latency work to live
bdb3240 to
3501754
Compare
5672ccd to
b02e22d
Compare
| float64 p50_ms | ||
| float64 p90_ms | ||
| float64 p95_ms | ||
| float64 p99_ms |
There was a problem hiding this comment.
I assume these are percentiles? might be good to add some # comments to the fields
| /// @brief ROS topic the bridge subscribes to (and the LiveKit data-track name it | ||
| /// forwards on) for latency probes, created only when measure_latency is set. | ||
| inline constexpr const char* kLatencyTimestampTopic = "/ros2_livekit_bridge/latency/timestamp"; | ||
| /// @brief ROS type string for latency probe messages. | ||
| inline constexpr const char* kLatencyTimestampMsgType = "ros2_livekit_bridge_msgs/msg/LatencyTimestamps"; | ||
| /// @brief ROS topic the receiving bridge republishes inbound latency probes on. | ||
| /// Deliberately distinct from @ref kLatencyTimestampTopic so the outbound | ||
| /// subscription never re-forwards a republished probe (no cross-bridge loop). | ||
| inline constexpr const char* kLatencyTimestampRxTopic = "/ros2_livekit_bridge/latency/timestamp_rx"; | ||
|
|
There was a problem hiding this comment.
This isn't a hard blocker, but I feel a little iffy about a custom topic for latency measurement, for a couple reasons:
- Specfic topics and types hard coded into the bridge
- Additional logic in the bridge just for this use case (this PR has a decent chunk of bridge changes which gives me pause)
- Latency can heavily depend on message sizes and serialization complexity, the latency .msg as defined is pretty flat and easy
If I were at this, I would go with tracing tools (like chromium tracing) or even RCLCPP_TRACE log statements that could be compiled in/out, each statement having a sequence ID that could be traced, then some sort of orchestration script that runs two bridges and monitors the log output to aggregate results. I get that's more complex, but then we could measure image topics, custom topics, ROS topics, etc. and see how things perform and where the bottlenecks are.
Given we only really control the bridge latency, the ROS latency isn't our domain and livekit latencies are measurable elsewhere in our codebases, an E2E number + log trace statements for our bridge portions seem good enough to me without custom config/logic for a specific topic.
Thoughts?
f740366 to
d66d8bf
Compare
5b0e2a7 to
406f5b0
Compare
|
|
||
| ### Regression budgets | ||
|
|
||
| The analyzer can act as a CI gate: |
There was a problem hiding this comment.
this is a bit of AI overkill i was planning to remove, but thought id leave it in to see if you see value in having this as a cI gate @alan-george-lk
0e97d6a to
b4e73f9
Compare
| ) | ||
|
|
||
|
|
||
| def _probe_publisher(context): |
There was a problem hiding this comment.
ros2 topic pub is the workload generator -- it creates the source-side rmw_publish event used as T0 and gives the sending bridge messages to forward.
| ] | ||
|
|
||
|
|
||
| def _probe_subscriber(_context): |
There was a problem hiding this comment.
ros2 topic hz is the destination subscriber -- It creates the final rmw_take event used as T5. It also confirms that messages are arriving at the expected rate.
860cc76 to
2eef12a
Compare
4476666 to
4327011
Compare
|
discussed in sync: going to use compile time macros such as North star would be to create create reports nightly |
4327011 to
ff84101
Compare
ff84101 to
971dc02
Compare
0380169 to
5e55f44
Compare
Overview
measure_latencyand its custom ROS pub/sub path with lightweight LTTng tracepoints in the normal forwarding path.Testing
Example results
50 Hz Docker smoke test with 1,113 complete records and 1,093 measured samples: