Port MTC to ROS2 #170
Port MTC to ROS2 #170
Conversation
|
I opened the PR to see if someone else is interested in helping me porting the package |
|
fixed |
|
https://github.com/ros-planning/moveit_task_constructor/tree/ros2 need to be fast-forwarded to the main branch |
|
@rhaschke Could you please fast-forward https://github.com/ros-planning/moveit_task_constructor/tree/ros2 to the current master branch .? |
|
I forwarded ros2 to master, but this PR still lists many past commits... |
Thanks! I'll fix it now |
Codecov Report
@@ Coverage Diff @@
## ros2 #170 +/- ##
===========================================
- Coverage 52.99% 39.07% -13.92%
===========================================
Files 102 78 -24
Lines 7598 7332 -266
===========================================
- Hits 4026 2864 -1162
- Misses 3572 4468 +896
Continue to review full report at Codecov.
|
| @@ -108,16 +108,8 @@ Task createTask() { | |||
| t.add(std::move(stage)); | |||
| } | |||
|
|
|||
| { // move gripper into predefined open state | |||
There was a problem hiding this comment.
I removed these lines since we don't have a controller for the gripper, yet
There was a problem hiding this comment.
could you comment this and add a TODO instead?
| @@ -64,34 +64,74 @@ std::string getTaskId(const TaskPrivate* task) { | |||
| } | |||
| } // namespace | |||
|
|
|||
| /** | |||
| * A shared executor between all tasks' introspection, this class will keep track of the number of the added nodes and | |||
There was a problem hiding this comment.
This executor is only really used for the service, right? What's the implication of simply reusing the same node and just appending the private task namespace to the all topics? I'm pretty sure your approach works as expected, I'm just not sure if it's really necessary.
| get_solution_service_ = | ||
| nh_.advertiseService(std::string(GET_SOLUTION_SERVICE "_") + task_id_, &Introspection::getSolution, self); | ||
| task_statistics_publisher_ = node_->create_publisher<moveit_task_constructor_msgs::msg::TaskStatistics>( | ||
| STATISTICS_TOPIC, rclcpp::QoS(1).transient_local()); |
There was a problem hiding this comment.
This's how to do latching in ROS2 see
There was a problem hiding this comment.
Cool! Could you add a short note for this?
since I fast-forwarded the ros2 branch this PR has a lot of conflicts that need to be resolved first. |
|
I rebased on the latest ros2 branch, the previous branch is available here The current branch only work with rolling due to some changes in RViz which are only available there I'll clean the history tomorrow and force push again |
|
The current branch only work with rolling due to some changes in RViz which are only available there
Is that sufficient for you? I guess it's possible to `ifdef` both alternatives to get it to run at least on galactic?
|
That's possible I'll fix it today |
There was a problem hiding this comment.
It's really about time to get this merged. @JafarAbdi do you mind squashing some of the commits? Or do you prefer a squash-merge?
| @@ -58,20 +59,25 @@ class PipelinePlanner : public PlannerInterface | |||
| struct Specification | |||
| { | |||
| moveit::core::RobotModelConstPtr model; | |||
| std::string ns{ "move_group" }; | |||
| std::string ns{ "ompl" }; | |||
There was a problem hiding this comment.
Hi @JafarAbdi , I'm currently combining MTC with the Pilz_industrial_motion_planner pipeline in ROS2 and stumbled across this line. Is there a reason for switching from "move_group" to "ompl" at this place?
|
@JafarAbdi we really gotta merge this! |
|
I rebased on the latest master and cleanup the commit history, if anyone wants to use the previous branch it's still available here https://github.com/JafarAbdi/moveit_task_constructor/tree/old-ros2-5bce69d |
JafarAbdi
force-pushed
the
ros2
branch
2 times, most recently
from
4c9519f
to
a1c2c49
Compare
JafarAbdi
force-pushed
the
ros2
branch
4 times, most recently
from
04097c0
to
09593dc
Compare
This shouldn't be against master branch
Initial effort for porting MTC to ROS2, next step is to port rviz_marker_tools after that continue porting mtc core and its tests