Encapsulate publisher logic in a ROS 2 node

[life1ess]

Summary by CodeRabbit

• Documentation

  • Added a guide for overriding ROS 2 parameters from the command line, with examples and a full list of available parameters.

• Improvements

  • Reworked the teleoperation runtime into a class-based node for more robust lifecycle, clearer parameter handling, and improved error handling and retry behavior during execution.

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors the teleop ROS2 publisher into a TeleopRos2PublisherNode class that encapsulates initialization, parameter handling, publishers, and runtime loop; adds README subsection showing how to override ROS 2 parameters via --ros-args -p (with a docker run example and parameter list).

Changes

Cohort / File(s)	Summary
Documentation examples/teleop_ros2/README.md	Added "Overriding parameters" section showing how to pass ROS 2 parameters with --ros-args -p, a docker run example including frame_id and rate_hz, and a list of available parameters (hand_topic, twist_topic, pose_topic, controller_topic, full_body_topic, frame_id, rate_hz, use_mock_operators).
Core Implementation examples/teleop_ros2/python/teleop_ros2_publisher.py	Replaced procedural script with a new TeleopRos2PublisherNode class. The class handles node initialization, parameter declaration/read, publisher creation, source/pipeline assembly, and the runtime loop via run() (publishing hand poses, twist/root pose, controller/full_body payloads). Retains main() for bootstrap and cleanup; removes previous argparse/top-level flow and adds retry/error handling for XR unavailability.

Sequence Diagram(s)

sequenceDiagram
    participant Runner
    participant Node as TeleopRos2PublisherNode
    participant XR as TeleopSession (XR)
    participant ROS as ROS2 Publishers

    Runner->>Node: start (rclpy.init() & instantiate)
    Node->>Node: declare/read parameters
    Node->>XR: create TeleopSession / assemble sources
    Node->>ROS: create publishers
    loop runtime loop
        Node->>XR: poll frames / get latest pose/twist/controller
        XR-->>Node: pose, twist, controller data
        Node->>ROS: publish hand pose, twist/root pose, controller/full_body
        alt XR unavailable / error
            Node->>Node: handle error, wait/retry
        end
    end
    Runner->>Node: shutdown -> cleanup (rclpy.shutdown())

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 A rabbit hops through refactor bright,
Classes cradle parameters tight,
Publishers hum and XR sings,
Docs show flags for runtime things,
A tiny hop, a cleaner flight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title directly and clearly describes the main architectural change: refactoring publisher logic into a ROS 2 node class.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch sgrizan/branch2

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@examples/teleop_ros2/python/teleop_ros2_publisher.py`:
- Around line 184-404: The file teleop_ros2_publisher.py was reformatted by
ruff-format in CI; run the repository's formatting hooks (e.g., pre-commit or
ruff format) locally and commit the updated file so CI no longer modifies it;
specifically run the formatter against
examples/teleop_ros2/python/teleop_ros2_publisher.py (or run pre-commit run
--all-files) and then add/commit the changes that affect
TeleopRos2PublisherNode, run(), and main() formatting to satisfy ruff-format.
- Around line 217-218: The code clamps invalid rate_hz to 1e-3 producing a 1000s
sleep; instead validate rate_hz after reading it from get_parameter and
fail-fast or apply a safe fallback: check rate_hz for <= 0 or NaN and either
raise an exception (or call self.get_logger().error and raise RuntimeError) to
stop startup, or log a warning and set a sensible default (e.g. 10.0 Hz) before
computing self._sleep_period_s; then compute self._sleep_period_s = 1.0 /
rate_hz using the validated value. Ensure you reference the same symbols:
get_parameter, rate_hz, self.get_logger(), and _sleep_period_s.
- Line 197: The parameter declaration uses a positional boolean literal in
declare_parameter which triggers Ruff FBT003; update the call to pass the
boolean as a keyword (e.g., use value=False) so it becomes explicit. Locate the
declare_parameter invocation in teleop_ros2_publisher.py (the call to
declare_parameter) and change the second argument to a named keyword argument
(value=False) to satisfy the linter and improve clarity.
- Around line 365-371: The code is creating a tuple of single-byte bytes objects
for ByteMultiArray.data which is invalid; instead use the raw bytes/sequence of
integers returned by msgpack.packb. Replace the tuple conversion where payload
is set (the lines handling controller_payload and body payload) so that payload
= msgpack.packb(controller_payload, default=mnp.encode) (and likewise for body
payload) and assign that raw bytes directly to controller_msg.data (and
body_msg.data) so ByteMultiArray receives a sequence of integers; keep using
controller_msg and body_msg as the ByteMultiArray instances and publish them as
before.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a7ee000 and e5cb28b.

📒 Files selected for processing (2)

• examples/teleop_ros2/README.md
• examples/teleop_ros2/python/teleop_ros2_publisher.py

[coderabbitai]

♻️ Duplicate comments (1)

examples/teleop_ros2/python/teleop_ros2_publisher.py (1)

377-394: ⚠️ Potential issue | 🔴 Critical

Use raw bytes for ByteMultiArray.data instead of tuple of single-byte objects.

The tuple(bytes([a]) for a in payload) conversion on lines 380 and 391 creates a tuple of bytes objects (each containing a single byte), rather than a sequence of integers (0–255) that ByteMultiArray.data expects. This will cause serialization issues or incorrect message content.

Assign the msgpack.packb() return value directly to the message's data field.

Proposed fix

                         payload = msgpack.packb(
                             controller_payload, default=mnp.encode
                         )
-                        payload = tuple(bytes([a]) for a in payload)
                         controller_msg = ByteMultiArray()
                         controller_msg.data = payload
                         self._pub_controller.publish(controller_msg)

                         body_payload = _build_full_body_payload(full_body_data)
                         payload = msgpack.packb(body_payload, default=mnp.encode)
-                        payload = tuple(bytes([a]) for a in payload)
                         body_msg = ByteMultiArray()
                         body_msg.data = payload
                         self._pub_full_body.publish(body_msg)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@examples/teleop_ros2/python/teleop_ros2_publisher.py` around lines 377 - 394,
The code currently converts the bytes returned by msgpack.packb(...) into a
tuple of single-byte bytes objects before assigning to ByteMultiArray.data,
which expects a sequence of integer byte values; instead, assign the raw packed
bytes directly. Update both places where payload is set (for controller_payload
and for body_payload produced by _build_full_body_payload) to use payload =
msgpack.packb(..., default=mnp.encode) and then set controller_msg.data =
payload and body_msg.data = payload before publishing to _pub_controller and
_pub_full_body respectively (keep using ByteMultiArray).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@examples/teleop_ros2/python/teleop_ros2_publisher.py`:
- Around line 377-394: The code currently converts the bytes returned by
msgpack.packb(...) into a tuple of single-byte bytes objects before assigning to
ByteMultiArray.data, which expects a sequence of integer byte values; instead,
assign the raw packed bytes directly. Update both places where payload is set
(for controller_payload and for body_payload produced by
_build_full_body_payload) to use payload = msgpack.packb(...,
default=mnp.encode) and then set controller_msg.data = payload and body_msg.data
= payload before publishing to _pub_controller and _pub_full_body respectively
(keep using ByteMultiArray).

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e5cb28b and 9a346cb.

📒 Files selected for processing (2)

• examples/teleop_ros2/README.md
• examples/teleop_ros2/python/teleop_ros2_publisher.py