Use Rviz as a component #1140
Use Rviz as a component #1140
Conversation
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
|
From my point of view, launching it always as a component (really, a component host) shall be fine. |
|
What was the logic before, with the name of Rviz node? |
|
@ahcorde can this one be merged for Jazzy? |
|
Friendly ping @ahcorde :) |
|
Any updates? |
| @@ -399,6 +400,7 @@ private Q_SLOTS: | |||
| rclcpp::executors::SingleThreadedExecutor::SharedPtr executor_; | |||
| ros_integration::RosNodeAbstractionIface::WeakPtr rviz_ros_node_; | |||
| rviz_common::transformation::TransformationManager * transformation_manager_; | |||
| std::shared_ptr<ros_integration::ComponentManager> mgr_; | |||
There was a problem hiding this comment.
Explicitly initialize to nullptr in constructors' initialization list?
| @@ -225,7 +226,15 @@ VisualizationManager::VisualizationManager( | |||
|
|
|||
| connect(this, SIGNAL(timeJumped()), this, SLOT(resetTime())); | |||
|
|
|||
| executor_->add_node(rviz_ros_node_.lock()->get_raw_node()); | |||
| if (!mgr_) { | |||
| mgr_ = std::make_shared<ros_integration::ComponentManager>(executor_); | |||
There was a problem hiding this comment.
Should we allow users to configure the name of the component manager and pass it along as an argument here?
There could be name clashes with other component containers running. It would also make adding nodes to this container less ambiguious by being able to specify the right name.
| using rclcpp_components::ComponentManager::ComponentManager; | ||
|
|
||
| void | ||
| add_node_to_executor( |
| auto node_id = unique_id_++; | ||
| node_wrappers_[node_id] = node; | ||
| if (auto exec = this->executor_.lock()) { | ||
| exec->add_node(node.get_node_base_interface(), true); |
There was a problem hiding this comment.
Drop true since it is the default arg.
| exec->add_node(node.get_node_base_interface(), true); | |
| exec->add_node(node.get_node_base_interface()); |
| @@ -44,7 +44,7 @@ namespace ros_integration | |||
| { | |||
|
|
|||
| RosNodeAbstraction::RosNodeAbstraction(const std::string & node_name) | |||
| : raw_node_(rclcpp::Node::make_shared(node_name)) | |||
| : raw_node_(rclcpp::Node::make_shared(node_name, rclcpp::NodeOptions().use_intra_process_comms(true))) | |||
There was a problem hiding this comment.
Make use_intra_process_comms a configurable parameter for users to specify?
Related with this issue #1114
TODOs