-
Notifications
You must be signed in to change notification settings - Fork 65
fix: get transform timeout precision issue, missing resource destructors #775
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
aebff7b
457c935
e999893
75b2fdf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,17 +68,16 @@ def __init__( | |
| # While this may lead to memory/performance issues, it's preferable to | ||
| # preventing node crashes. | ||
| self._last_msg: Dict[str, Tuple[float, Any]] = {} | ||
| self._subscriptions: Dict[str, rclpy.node.Subscription] = {} | ||
| self._destroy_subscribers: bool = destroy_subscribers | ||
| self.node = node | ||
| self.subscriptions: Dict[str, Subscription] = {} | ||
| self.publishers: Dict[str, Publisher] = {} | ||
| self._subscriptions: Dict[str, Subscription] = {} | ||
| self._publishers: Dict[str, Publisher] = {} | ||
|
|
||
| def subscriber_exists(self, topic: str) -> bool: | ||
| return topic in self.subscriptions | ||
| return topic in self._subscriptions | ||
|
|
||
| def publisher_exists(self, topic: str) -> bool: | ||
| return topic in self.publishers | ||
| return topic in self._publishers | ||
|
|
||
| def create_subscriber( | ||
| self, | ||
|
|
@@ -102,7 +101,7 @@ def create_subscriber( | |
| subscription = self.node.create_subscription( | ||
| topic=topic, msg_type=msg_cls, callback=callback, qos_profile=qos | ||
| ) | ||
| self.subscriptions[topic] = subscription | ||
| self._subscriptions[topic] = subscription | ||
| return subscription | ||
|
|
||
| def create_publisher( | ||
|
|
@@ -124,7 +123,7 @@ def create_publisher( | |
| publisher = self.node.create_publisher( | ||
| topic=topic, msg_type=msg_cls, qos_profile=qos | ||
| ) | ||
| self.publishers[topic] = publisher | ||
| self._publishers[topic] = publisher | ||
| return publisher | ||
|
|
||
| def get_topic_names_and_types( | ||
|
|
@@ -247,3 +246,6 @@ def shutdown(self) -> None: | |
| """Cleanup publishers when object is destroyed.""" | ||
| for publisher in self._publishers.values(): | ||
| publisher.destroy() | ||
|
|
||
| for subscription in self._subscriptions.values(): | ||
| subscription.destroy() | ||
|
Comment on lines
+248
to
+250
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: rclpy GitHub issue Citations:
🏁 Script executed: cd src/rai_core/rai/communication/ros2/api && cat -n topic.py | sed -n '60,75p'Repository: RobotecAI/rai Length of output: 921 🏁 Script executed: cd src/rai_core/rai/communication/ros2/api && cat -n topic.py | sed -n '240,260p'Repository: RobotecAI/rai Length of output: 617 🏁 Script executed: cd src/rai_core/rai/communication/ros2/api && rg "_destroy_subscribers" -B 2 -A 2Repository: RobotecAI/rai Length of output: 339 🏁 Script executed: cd src/rai_core/rai/communication/ros2/api && rg "destroy_subscribers" -B 3 -A 3Repository: RobotecAI/rai Length of output: 710 🏁 Script executed: cd src/rai_core/rai/communication/ros2/api && rg "_destroy_subscribers" -B 1 -A 1Repository: RobotecAI/rai Length of output: 228 🏁 Script executed: cd src/rai_core/rai/communication/ros2/api && wc -l topic.pyRepository: RobotecAI/rai Length of output: 69 🏁 Script executed: cd src/rai_core/rai/communication/ros2/api && cat topic.py | grep -n "_destroy_subscribers"Repository: RobotecAI/rai Length of output: 121 Remove the unused The 🤖 Prompt for AI Agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Uh oh, currently the logic of receive_message in ROS2BaseConnector is weird, seems like we should reduce it's logic and delegate more responsibility to the ROS2TopicAPI class. That way we could implement the _destroy_subscribers flag (it's idea is to remove the subscriber after a message has been received to minimize throughput, so nothing about the .shutdown() logic). Please create an issue for that.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainLet me first inspect the current 🏁 Script executed: Length of output: 10124 🏁 Script executed: Length of output: 4647 🏁 Script executed: Length of output: 6145 Now I have enough context to create a well-detailed issue. 🏁 Script executed: Length of output: 43
Here's what it covers based on my inspection of the code:
✏️ Learnings added
🧠 Learnings used |
||
Uh oh!
There was an error while loading. Please reload this page.