ros2-ify perception pipeline #700
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Ok, I'll just switch this PR to `rolling` and continue.In Since in Getting' Started section |
130s
force-pushed
the
feature-perceptionpipeline-ros2fy
branch
2 times, most recently
from
4ea1187
to
43c9536
Compare
130s
force-pushed
the
feature-perceptionpipeline-ros2fy
branch
5 times, most recently
from
6c4ac95
to
782c455
Compare
This comment was marked as resolved.
This comment was marked as resolved.
130s
force-pushed
the
feature-perceptionpipeline-ros2fy
branch
2 times, most recently
from
1b4fc0a
to
1d8627d
Compare
This comment was marked as resolved.
This comment was marked as resolved.
130s
force-pushed
the
feature-perceptionpipeline-ros2fy
branch
from
1d8627d
to
28de005
Compare
This was referenced Jun 12, 2023
Octomap is not showing.In This doesn't seem published. pointcloud is published (from the rosbag file). So use it. These prints expected values though. Found a set of params that look suspecious. With yaml: Result on param server: Maybe I should know how a code gets these params. UPDATE: All examples of ros2 parameter .yaml files I've seen so far use:But in my case, I tried this but no good? Resulted in: With yaml: Result on param server: Maybe I should know how a code gets these params. Also noticed: Found someone already ported moveit_tutorial to ros2 answers.ros.org#389233/moveit2-add-pointcloud2-to-occupancymapmonitor/. Swapping sensor config to sensor_3d.yaml, Hm, now this.Beginning of failure seems these: Pkg does exist and is reachable by ros2: For some reasons googling by So questions:
|
|
As ticketed #708, I found changes that are merged into moveit1's tutorial but not to moveit2's. Not sure if a culprit of the issue I'm seeing (i.e. no octomap shown) but I will test those as well. |
130s
force-pushed
the
feature-perceptionpipeline-ros2fy
branch
from
28de005
to
bb901d8
Compare
This comment was marked as resolved.
This comment was marked as resolved.
130s
added a commit
to 130s/moveit_resources
that referenced
this pull request
Jun 28, 2023
130s
force-pushed
the
feature-perceptionpipeline-ros2fy
branch
3 times, most recently
from
54d5e3c
to
a432168
Compare
|
@sjahr I see you're active on this repo recently. Would you mind reviewing, or assigning appropriate reviewers? |
|
@130s Thanks for your work here! I'll take a look at it next week |
Tejas-Shastha
suggested changes
Sep 27, 2023
There was a problem hiding this comment.
First of all, thank you very much for attempting to ROS2-ify this tutorial page!
I do not yet have a rolling environment where I can test this tutorial completely, but I have commented on some things that I could find by just reading the first half related to configuration. These are some points I found a little confusing myself.
| @@ -30,19 +34,22 @@ To use the Occupancy Map Updater, it is only necessary to set the appropriate pa | |||
| YAML Configuration file (Point Cloud) | |||
| +++++++++++++++++++++++++++++++++++++ | |||
|
|
|||
| We will have to generate a YAML configuration file for configuring the 3D sensors. Please see :panda_codedir:`this example file<config/sensors_kinect_pointcloud.yaml>` for processing point clouds. | |||
| We will have to generate a YAML configuration file for configuring the 3D sensors. Please see :panda_codedir:`this example file <config/sensors_3d.yaml>` for processing point clouds. | |||
There was a problem hiding this comment.
This link is broken, as it leads to a non-existing page: https://github.com/ros-planning/panda_moveit_config/blob/rolling-devel/config/sensors_3d.yaml
| padding_offset: 0.03 | ||
| max_update_rate: 1.0 | ||
| filtered_cloud_topic: filtered_cloud | ||
| We will have to generate a YAML configuration file for configuring the 3D sensors. An :panda_codedir:`example file for processing depth images <config/sensors_3d.yaml>` can be found in the panda_moveit_config repository as well. |
There was a problem hiding this comment.
Same as above, broken link leading to a non-existing file: https://github.com/ros-planning/panda_moveit_config/blob/rolling-devel/config/sensors_3d.yaml
|
|
||
| Update the launch file | ||
| ++++++++++++++++++++++ | ||
|
|
||
| Add the YAML file to the launch script | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| You will now need to update the *sensor_manager.launch* file in the "launch" directory of your panda_moveit_config directory with this sensor information (this file is auto-generated by the Setup Assistant but is empty). You will need to add the following line into that file to configure the set of sensor sources for MoveIt to use: :: | ||
| You will now need to create a *sensor_manager.launch* file in the "launch" directory of your panda_moveit_config directory (e.g. `on github <https://github.com/ros-planning/panda_moveit_config/blob/rolling-devel/launch/sensor_manager.launch.xml>`_) with this sensor information. You will need to add the following line into that file to configure the set of sensor sources for MoveIt to use: :: |
There was a problem hiding this comment.
This section is a bit ambiguous.
- The name of the file should be
sensor_manager.launch.xml(.xml is missing) - You reference the
panda_moveit_configfrom ros-planning, but in general I believe this is not used. For the tutorials, this package from https://github.com/ros-planning/moveit_resources/tree/master/panda_moveit_config is used instead, and for the real robot, Franka's own package is used: https://github.com/frankaemika/franka_ros2/tree/humble/franka_moveit_config. - Your fork of the aforementioned repo does not yet contain this launch XML. This could of course be because the reader is expected to create it themselves, but I believe it makes for a better tutorial to show something that already exists and works.
There was a problem hiding this comment.
TLDR I'll address the issues here 👍
- You reference the panda_moveit_config from ros-planning, but in general I believe this is not used. For the tutorials, this package from https://github.com/ros-planning/moveit_resources/tree/master/panda_moveit_config is used instead, and for the real robot, Franka's own package is used: https://github.com/frankaemika/franka_ros2/tree/humble/franka_moveit_config.
Reasonbale concern. At the tiem of writing there was unclarity, but now we have it clarified #704 so I'll have to update this PR to use panda config from moveit2_resources, not from panda_moveit_config repo.
|
|
||
| If you are using depthmap change the name of the yaml file to ``sensors_kinect_depthmap.yaml``. | ||
| If you are using depthmap change the name of the yaml file to ``sensors_3d.yaml``. |
There was a problem hiding this comment.
- In both cases the name of the yaml is
sensors_3d.yaml, there is no change?
|
|
||
| If you are using depthmap change the name of the yaml file to ``sensors_kinect_depthmap.yaml``. | ||
| If you are using depthmap change the name of the yaml file to ``sensors_3d.yaml``. | ||
| Note that you will need to input the path to the right file you have created above. | ||
|
|
||
| Octomap Configuration |
There was a problem hiding this comment.
Perhaps this section needs to be refactored for ROS2 as well? In your fork of the panda moveit config, you do not use the XML launch approach mentioned above, but add this directly into the demo python launch.
There was a problem hiding this comment.
I updated that config section to include migration from .launch to .launch.py. Hope the new change makes sense.
Okay so I tried this out in my humble workspace. I was indeed able to visualize the point cloud, and even plan around the obstacles it inserts, but this was not entirely straightforward. I had to make a couple of tweaks (ex: remove world->panda_link0 static tf from launch xml because its already published, and run the rosbag play separately after starting up everything else otherwise rviz would just crash). My biggest concern is the following messages : which goes up to handle 11. We can see it comes from here and here, but I am not sure what exactly this complains about. Missing TF for robot links? I can see in TF that robot frames are well published. I increased the @130s Any thoughts? |
Removing dep on panda_moveit_config and franka_description (instead, continue to depend on moveit_resources_panda pkgs)
130s
force-pushed
the
feature-perceptionpipeline-ros2fy
branch
from
a432168
to
1962b20
Compare
…e (unverified) decision moveit#704 (comment)) Signed-off-by: Isaac Saito <130s@2000.jukuin.keio.ac.jp>
130s
added a commit
to 130s/moveit2_tutorials
that referenced
this pull request
Sep 27, 2023
…ent) Signed-off-by: Isaac Saito <130s@2000.jukuin.keio.ac.jp>
130s
added a commit
to 130s/moveit2_tutorials
that referenced
this pull request
Sep 27, 2023
…address in PR moveit#700 (comment)) Signed-off-by: Isaac Saito <130s@2000.jukuin.keio.ac.jp>
130s
commented
Sep 27, 2023
| + html_context["github_user"] | ||
| + "/moveit_resources/tree/ros2" | ||
| # + html_context["github_user"] | ||
| + "130s" |
There was a problem hiding this comment.
Reminder to revert the changes in this line and 2 lines below, once moveit/moveit_resources#181 is merged.
…ent) Signed-off-by: Isaac Saito <130s@2000.jukuin.keio.ac.jp> Remove unnecessary functionality (moveit#700 (comment)) Signed-off-by: Isaac Saito <130s@2000.jukuin.keio.ac.jp>
…address in PR moveit#700 (comment)) Signed-off-by: Isaac Saito <130s@2000.jukuin.keio.ac.jp>
130s
force-pushed
the
feature-perceptionpipeline-ros2fy
branch
from
92aca72
to
9b7e620
Compare
|
Thanks for the review and testing! @Tejas-Shastha
Ok...Not sure why it fails on your env, but I removed that static publisher.
I'm unsure what's going on. If you can spot particular error output, would you mind posting (Can that look similar to moveit/moveit_resources#170?)?
Hm, no idea yet. |
I think the entity responsible for publishing this TF changes somewhere between all the configs. I would keep whatever works for the instructions of the tutorial (I did not test the tutorial environment, rather a custom environment of my own, so I am not sure what is correct either). Apologies if this created too much confusion! I also found out that the error I mentioned above: is caused by TF issues. I guess that since the bag contains only 1 msg and it is being relaunched at a pretty high rate, it messes up TF? I recorded my own bag and tried it and got the same results. However, on a real robot with a live feed from an Intel realsense D435, I did not have this issue at all (except once when the camera dropped some frames I think, which is what led me to conclude this is a TF issue). So, as you suggested, maybe it is sufficient to declare this phenomenon as a warning and move on? A separate issue can be raised to investigate this better. Overall, I tested the configuration suggestions you made with a real Panda FER1 and it works very well! (Again, my environment is somewhat different from what is prescribed in the tutorial). I think this PR adds sufficient value that it can be merged, I'm sure it would help many moveit users! P.S. : I am apparently unable to change my review from "Changes requested" for some reason. Please consider this as approved. |
|
closes #708 |
|
@henningkayser Could you please review or push for a review of this PR? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue aimed at
Description of changes
git cherry-pickthose changes, but some PRs included changes outside of perception tutorial, which is not my scope of this PR, so I gave up and went ahead manually applying changes.Checklist
panda_moveit_configormoveit_resources_panda_moveit_config? Unclear the direction and rationale #704panda_moveit_configormoveit_resources_panda_moveit_config? Unclear the direction and rationale #704 (comment)moveit_resources?Peek.2023-06-28.14-37.webm