Skip to content
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

[Gazebo 9] Created a MarkerArray to publish normals in ROS #1047

Merged

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Feb 11, 2020

Moved this PR #1038 to noetic-devel to avoid ABI changes in melodic

We are exposing normals in Gazebo9. This PR exposes the normals as a MarkerArray and then we can visualize the normals in RVIZ

Selección_014

Steps to reproduce it:

  • Launch the test world:
    rosrun gazebo_ros gazebo test_worlds/gazebo_ros_depth_camera.world
  • Create the tf between the world and the camera:
    rosrun tf2_ros static_transform_publisher 0 0 5 0 0 -1.57 world base_link
  • Open RVIZ

@ahcorde ahcorde requested a review from iche033 February 11, 2020 10:36
@ahcorde ahcorde changed the title [Gazebo 9] Created a MarkerArray to publish normals in ROS #1038 [Gazebo 9] Created a MarkerArray to publish normals in ROS Feb 23, 2020
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't looked into why, but when I rosrun the commands on the PR description, Gazebo just hangs with a black screen. Can you reproduce it? If you can't, I can dig into my setup later. The worlds/depth_camera2.world world doesn't hang though.

gazebo_plugins/src/gazebo_ros_depth_camera.cpp Outdated Show resolved Hide resolved
gazebo_plugins/src/gazebo_ros_depth_camera.cpp Outdated Show resolved Hide resolved
gazebo_plugins/src/gazebo_ros_depth_camera.cpp Outdated Show resolved Hide resolved
gazebo_plugins/src/gazebo_ros_depth_camera.cpp Outdated Show resolved Hide resolved
@ahcorde ahcorde requested review from chapulina and iche033 February 28, 2020 10:32
@ahcorde
Copy link
Contributor Author

ahcorde commented Feb 28, 2020

You should run the example with world/gazebo_ros_depth_camera.world inside this package.

@ahcorde ahcorde requested a review from chapulina March 4, 2020 07:50
chapulina
chapulina previously approved these changes Mar 5, 2020
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me. Since @iche033 is also reviewing, let's wait for his approval before merging.

@chapulina
Copy link
Contributor

Actually, I just realized that this change will make this branch not compilable against earlier Gazebo 9 versions. @j-rivero , what's the minimum Gazebo 9 version that we need to support for Noetic? I imagine that's 9.12.0, which is going into Focal? In that case, we may need to wrap the new APIs with ifdefs.

@chapulina chapulina dismissed their stale review March 5, 2020 02:19

See comment about sompiling against 9.12.0

@iche033 iche033 requested a review from chapulina March 6, 2020 18:45
m.pose.orientation.w = q.w();

// plotting some of the normals, otherwise rviz will block it
if (index % this->reduce_normals_ == 0)
Copy link
Contributor

@iche033 iche033 Mar 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move this check earlier? We could skip creating the marker msg and doing computations when the results are not being published.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry didn't catch this earlier: index is declared later so it is not resolved here and this if block is missing curly brackects

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had this resolved in this other PR, but I can create a Pr just solving this issue

@iche033
Copy link
Contributor

iche033 commented Mar 6, 2020

looks good to me! just left one minor comment.

Signed-off-by: ahcorde <ahcorde@gmail.com>
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compiling against 9.12.0

I talked to @j-rivero about this and since the official Gazebo version for Noetic is not decided yet, let's not worry about it now. We can go back and fix it if needed.

@chapulina chapulina merged commit 832aaa9 into ros-simulation:noetic-devel Mar 6, 2020
cohen39 pushed a commit to cohen39/gazebo_ros_pkgs that referenced this pull request Nov 15, 2021
…ation#1047)

* created a MarkerArray to publish normals in ROS

* Added visualization msgs dependencie depthcamerasensor

* Added feedback

* populate point cloud when normals is activated

* argument to reduce the amount of normals to publish

* reduced computation in normals publisher

Signed-off-by: ahcorde <ahcorde@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants