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

Apply low-pass filter to RTF display #529

Merged
merged 3 commits into from
Apr 11, 2023
Merged

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Apr 7, 2023

🎉 New feature

Follow-up from gazebosim/gz-sim#1942

Summary

This adds a more aggressive low-pass filter than what was in gz-sim such that the RTF display is more stable and reflects the RTF of a longer window of time.

Test it

Run gz sim camera_sensor.sdf. The RTF will display is much more stable than before. After a while, remove the image display and watch the RTF increase.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey azeey requested a review from jennuine as a code owner April 7, 2023 18:22
@azeey azeey requested a review from iche033 April 7, 2023 18:22
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Apr 7, 2023
@azeey
Copy link
Contributor Author

azeey commented Apr 7, 2023

@iche033 I've made the filter less aggressive than what I had gazebosim/gz-sim#1942 so that it is responsive to changes like the image display being removed while still providing a stable RTF reading.

@codecov
Copy link

codecov bot commented Apr 7, 2023

Codecov Report

Merging #529 (063b43c) into gz-gui7 (bf42b84) will increase coverage by 0.05%.
The diff coverage is 91.30%.

❗ Current head 063b43c differs from pull request most recent head d70596f. Consider uploading reports for the commit d70596f to get more accurate results

@@             Coverage Diff             @@
##           gz-gui7     #529      +/-   ##
===========================================
+ Coverage    69.13%   69.19%   +0.05%     
===========================================
  Files           44       44              
  Lines         4921     4937      +16     
===========================================
+ Hits          3402     3416      +14     
- Misses        1519     1521       +2     
Impacted Files Coverage Δ
src/plugins/world_stats/WorldStats.cc 94.11% <91.30%> (-0.89%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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.

nice, looks good to me. Just minor comment about doc

@@ -57,6 +57,9 @@ namespace plugins

/// \brief Holds iterations
public: QString iterations;

public: std::optional<double> simTimeDelayed;
public: std::optional<double> realTimeDelayed;
Copy link
Contributor

Choose a reason for hiding this comment

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

add doxy comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 09fd320

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey azeey merged commit 1084ba2 into gazebosim:gz-gui7 Apr 11, 2023
@azeey azeey deleted the filter_rtf branch April 11, 2023 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants