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.

Sign up for GitHub

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

Jump to bottom

[Noetic] Added Ignition common profiler to gazebo_plugins #1139

Merged
merged 2 commits into from
Jul 28, 2020
Merged

[Noetic] Added Ignition common profiler to gazebo_plugins #1139

merged 2 commits into from
Jul 28, 2020

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Jul 23, 2020

This PR is related to this other in Gazebo gazebosim/gazebo-classic#2776

It will allow to profile the methods in gazebo_plugins which are called periodically

To compile with the profile enable you need add this option -DENABLE_PROFILER=1. For example:

catkin_make_isolated --pkg gazebo_plugins --cmake-args -DENABLE_PROFILER=1

Signed-off-by: ahcorde ahcorde@gmail.com

@ahcorde
Added Ignition common profiler
e1cab28 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde self-assigned this Jul 23, 2020
iche033
iche033 approved these changes Jul 27, 2020
View reviewed changes
Copy link
Contributor

@iche033 iche033 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. Just one minor typo.

gazebo_plugins/src/gazebo_ros_prosilica.cpp Outdated
this->PutCameraData(_image, sensor_update_time);
IGN_PROFILE_END();
IGN_PROFILE_BEGIN("PutCameraData");
Copy link
Contributor

Choose a reason for hiding this comment

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

PublishCameraInfo

@ahcorde
Fixed typo
71c5fb9 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Contributor Author

ahcorde commented Jul 28, 2020

This test is failing _ctest_gazebo_plugins_rostest_test_range_range_plugin.test (Failed). It looks unrelated. Can we merge this @iche033 ?

@iche033
Copy link
Contributor

iche033 commented Jul 28, 2020

looks like that test also failed before. Could be flaky

@iche033 iche033 merged commit 585af82 into noetic-devel Jul 28, 2020
@iche033 iche033 deleted the ahcorde/noetic/profiler branch July 28, 2020 18:17
@eborghi10
Copy link

@ahcorde is there a reason why this was not added to ROS Melodic?

@ahcorde
Copy link
Contributor Author

ahcorde commented Jun 25, 2021

@eborghi10 not really, I will be happy to review a the PR if you want to contribute

@eborghi10
Copy link

@eborghi10 not really, I will be happy to review a the PR if you want to contribute

@ahcorde, thanks for answering quickly. I tried to make it work yesterday but I didn't see the information from the new plugins in the Profiler so I wondered if it's related to how gazebo/common works in comparison to Ignition Common.

At least for now, I opened a PR in Gazebo to use the profiler since it wasn't being installed from source: gazebosim/gazebo-classic#3032.

I'll try next week to create a PR with the changes. It shouldn't be hard.

cohen39 pushed a commit to cohen39/gazebo_ros_pkgs that referenced this pull request Nov 15, 2021
@ahcorde
[Noetic] Added Ignition common profiler to gazebo_plugins (ros-simula…
044a9d4 
…tion#1139)

* Added Ignition common profiler

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

* Fixed typo

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
Reviewers

@iche033 iche033 iche033 approved these changes

@chapulina chapulina Awaiting requested review from chapulina

Assignees

@ahcorde ahcorde

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ahcorde @iche033 @eborghi10