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] Publish performance metrics #2819

Merged
merged 10 commits into from
Sep 25, 2020

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Aug 10, 2020

This PR create a new message called performanceMetrics.proto. This message contains

  • simulation real time factor
  • sim update sensor rate

Which other field are relevant to publish in this message?

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

gazebo/Server.cc Outdated Show resolved Hide resolved
gazebo/Server.cc Outdated Show resolved Hide resolved
gazebo/Server.cc Outdated Show resolved Hide resolved
gazebo/Server.cc Outdated Show resolved Hide resolved
gazebo/Server.cc Outdated Show resolved Hide resolved
@ahcorde ahcorde changed the title Publish performance metrics [Gazebo 9] Publish performance metrics Aug 28, 2020
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde force-pushed the ahcorde/publish/performance_metrics branch from 6e099dd to 0657d38 Compare September 1, 2020 08:28
@ahcorde
Copy link
Contributor Author

ahcorde commented Sep 1, 2020

Moved logic to SensorManager.cc

@ahcorde ahcorde requested a review from iche033 September 1, 2020 08:48
ahcorde and others added 2 commits September 2, 2020 12:37
Signed-off-by: ahcorde <ahcorde@gmail.com>
… mutex

Signed-off-by: Ian Chen <ichen@osrfoundation.org>
@iche033
Copy link
Contributor

iche033 commented Sep 2, 2020

I made some changes in fd995c1:

  • move variables to top and add comments
  • protect the PublishPerformanceMetrics function with mutex
  • prevent publishing if sim time diff < 0
  • style fixes

other than that, the metrics data published look to me

Copy link

@samuelgundry samuelgundry left a comment

Choose a reason for hiding this comment

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

Thanks for posting this. Sam from AWS RoboMaker. A couple of clarifying questions w.r.t FPS otherwise looks good. A few comments w.r.t code styles, but these depends on your existing preferences so feel free to ignore/push back.

gazebo/sensors/SensorManager.cc Outdated Show resolved Hide resolved
gazebo/sensors/SensorManager.cc Outdated Show resolved Hide resolved
gazebo/sensors/SensorManager.cc Outdated Show resolved Hide resolved
gazebo/sensors/SensorManager.cc Outdated Show resolved Hide resolved
gazebo/msgs/performance_metrics.proto Show resolved Hide resolved
gazebo/msgs/performance_metrics.proto Outdated Show resolved Hide resolved
gazebo/msgs/performance_metrics.proto Show resolved Hide resolved
gazebo/msgs/CMakeLists.txt Outdated Show resolved Hide resolved
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
@iche033
Copy link
Contributor

iche033 commented Sep 17, 2020

I made some minor tweaks to address a couple unresolved comments. 13f57fe

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.

latest changes look good to me

@ahcorde
Copy link
Contributor Author

ahcorde commented Sep 17, 2020

@osrf-jenkins retest this please

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.

Just one last comment then I think this is good to go!

gazebo/msgs/performance_metrics.proto Outdated Show resolved Hide resolved
Signed-off-by: ahcorde <ahcorde@gmail.com>
@chapulina chapulina merged commit 7adba90 into gazebo9 Sep 25, 2020
@chapulina chapulina deleted the ahcorde/publish/performance_metrics branch September 25, 2020 19:57
ahcorde added a commit that referenced this pull request Sep 28, 2020
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>

Co-authored-by: Ian Chen <ichen@osrfoundation.org>
ahcorde added a commit that referenced this pull request Sep 28, 2020
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>

Co-authored-by: Ian Chen <ichen@osrfoundation.org>
chapulina pushed a commit that referenced this pull request Sep 30, 2020
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Co-authored-by: Ian Chen <ichen@osrfoundation.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
9 Gazebo 9 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants