Skip to content
This repository was archived by the owner on Feb 3, 2025. It is now read-only.

[Gazebo 9] Publish performance metrics #2819

Merged
merged 10 commits into from
Sep 25, 2020
Merged

[Gazebo 9] Publish performance metrics #2819

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

@ahcorde ahcorde added enhancement New feature or request 9 Gazebo 9 labels Aug 10, 2020
@ahcorde ahcorde requested review from iche033 and chapulina August 10, 2020 17:48
@ahcorde ahcorde self-assigned this Aug 10, 2020
@ahcorde ahcorde marked this pull request as draft August 10, 2020 17:48
This was referenced Aug 11, 2020
Merged
Merged
@ahcorde ahcorde marked this pull request as ready for review August 11, 2020 22:02
iche033
iche033 reviewed Aug 14, 2020
View reviewed changes
@ahcorde ahcorde mentioned this pull request Aug 27, 2020
Merged
@ahcorde ahcorde changed the title Publish performance metrics [Gazebo 9] Publish performance metrics Aug 28, 2020
@ahcorde
Published performance metrics
0657d38 
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
@ahcorde
Calculated instantaneus real time factor
66426aa 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@iche033
style fix, add comment to var, protect PublishPerformanceMetrics with…
fd995c1 
… 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

samuelgundry
samuelgundry reviewed Sep 10, 2020
View reviewed changes
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.

chapulina
chapulina reviewed Sep 10, 2020
View reviewed changes
@ahcorde
Alphabetize msgs list
450ac53 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Added documentation to performance_metrics.proto
f7bd60f 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Included suggestions
33d4e41 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Merge branch 'ahcorde/publish/performance_metrics' of https://github.…
8386f5e 
…com/osrf/gazebo into ahcorde/publish/performance_metrics
@iche033
add more doc, user iterator
13f57fe 
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

iche033
iche033 approved these changes Sep 17, 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.

latest changes look good to me

@ahcorde
Copy link
Contributor Author

ahcorde commented Sep 17, 2020

@osrf-jenkins retest this please

chapulina
chapulina reviewed Sep 24, 2020
View reviewed changes
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!

@ahcorde
updated performance_metrics msg
8e0a4b3 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Merge branch 'ahcorde/publish/performance_metrics' of https://github.…
2e2d9ec 
…com/osrf/gazebo into ahcorde/publish/performance_metrics
@ahcorde ahcorde requested a review from chapulina September 25, 2020 11:13
@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
@ahcorde @iche033
[Gazebo 9] Publish performance metrics (#2819)
a4f4bd6 
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
@ahcorde @iche033
[Gazebo 9] Publish performance metrics (#2819)
7d00a3f 
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
@ahcorde @iche033
[Forward Port Gazebo 11] Publish performance metrics (#2819) (#2848)
b9d40cc 
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Co-authored-by: Ian Chen <ichen@osrfoundation.org>
@scpeters scpeters mentioned this pull request Dec 10, 2020
Closed
@chapulina chapulina mentioned this pull request May 6, 2021
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@samuelgundry samuelgundry samuelgundry left review comments

@iche033 iche033 iche033 approved these changes

@chapulina chapulina chapulina approved these changes

Assignees

@ahcorde ahcorde

Labels
9 Gazebo 9 enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ahcorde @iche033 @chapulina @samuelgundry