-
Notifications
You must be signed in to change notification settings - Fork 65
refactor: delegate receive_message logic to ROS2TopicAPI and implement _destroy_subscribers flag #776
Description
Summary
The current implementation of receive_message in ROS2BaseConnector (src/rai_core/rai/communication/ros2/connectors/base.py) contains logic that belongs in ROS2TopicAPI. Additionally, the _destroy_subscribers flag stored in ROS2TopicAPI.__init__ is never used.
Backlink: #775 (comment)
Requested by: @maciejmajek
Current Issues
1. Mixed responsibilities in receive_message
ROS2BaseConnector.receive_message currently:
- Maintains its own
last_msg: Dict[str, T]cache at the connector level - Directly calls
_topic_api.create_subscriber()with internal connector callbacks (partial(self.general_callback, source)) - Registers an additional
_last_message_callbackviaself.register_callback()to track last messages - Polls
self.last_msgin awhileloop to wait for a message within the timeout
This logic should live inside ROS2TopicAPI, keeping the connector as a thin delegation layer.
2. Unused _destroy_subscribers flag
ROS2TopicAPI accepts and stores a destroy_subscribers: bool parameter (currently stored as self._destroy_subscribers) but never references it anywhere in the class. The flag's intended behavior (per the existing TODO comment and class docstring) is to destroy a subscriber immediately after a message is received, in order to reduce throughput overhead from keeping idle subscribers alive. This is unrelated to the shutdown() cleanup path.
Proposed Changes
- Move subscriber creation, message caching, timeout polling, and optional post-receive subscriber destruction into
ROS2TopicAPIas areceive_messagemethod. - Use
self._destroy_subscribersinside that new method to conditionally destroy the subscription after the first message is received. - Simplify
ROS2BaseConnector.receive_messageto delegate directly to_topic_api.receive_message(...). - Ensure compatibility with rclpy issue #1142 (known crash on subscriber destruction while executor is spinning) — the flag should remain
Falseby default.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status