Skip to content

Added stepping bool to world statistics#199

Merged
nkoenig merged 3 commits intomainfrom
world_stats_stepping
Jul 26, 2022
Merged

Added stepping bool to world statistics#199
nkoenig merged 3 commits intomainfrom
world_stats_stepping

Conversation

@nkoenig
Copy link
Contributor

@nkoenig nkoenig commented Nov 4, 2021

Signed-off-by: Nate Koenig nate@openrobotics.org

🎉 New feature

Summary

Adds a stepping flag to world statistics so that we don't have to add custom headers, such as here

This PR also serves as a note to change ign-gui and ign-gazebo in Garden to avoid the statistics header:

  1. ign-gui reference
  2. ign-gazebo reference

Checklist

Note to maintainers: Remember to use Squash-Merge

Signed-off-by: Nate Koenig <nate@openrobotics.org>
@nkoenig nkoenig requested a review from adlarkin November 4, 2021 16:52
@nkoenig nkoenig requested a review from caguero as a code owner November 4, 2021 16:52
@codecov
Copy link

codecov bot commented Nov 4, 2021

Codecov Report

Merging #199 (cad8eb1) into main (24a7617) will not change coverage.
The diff coverage is n/a.

❗ Current head cad8eb1 differs from pull request most recent head 432cb99. Consider uploading reports for the commit 432cb99 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main     #199   +/-   ##
=======================================
  Coverage   84.56%   84.56%           
=======================================
  Files           7        7           
  Lines         855      855           
=======================================
  Hits          723      723           
  Misses        132      132           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24a7617...432cb99. Read the comment docs.

/// \brief Iteration step size. It's zero when paused.
Time step_size = 10;

/// \brief True is the simulator is stepping, as opposed to running.
Copy link
Member

Choose a reason for hiding this comment

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

so stepping means that it is executing a fixed number of iterations, as opposed to running for an unbounded number of iterations?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah - the idea is that this field can be used in combination with the paused field of this message to provide more context. If paused is false, then the process is running/executing, but it's not clear if it's running/executing for a bounded or unbounded number of iterations. Adding a stepping field would provide this missing information.

I think that this can be made a little clearer. Perhaps we can provide the context that I just gave above and also add the note I suggested in #199 (comment).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I can add that clarification note.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Signed-off-by: Nate Koenig <nate@openrobotics.org>
Copy link
Contributor

@adlarkin adlarkin left a comment

Choose a reason for hiding this comment

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

Newest documentation changes LGTM, thanks!

@chapulina
Copy link
Contributor

@nkoenig , do you want to leave this open until there's a downstream PR making use of it? It will still take a while to bump the versions in Garden so this can be used.

@nkoenig
Copy link
Contributor Author

nkoenig commented Nov 22, 2021

@nkoenig , do you want to leave this open until there's a downstream PR making use of it? It will still take a while to bump the versions in Garden so this can be used.

Yeah, I want to leave this open until I (or someone else) makes the downstream changes.

@chapulina chapulina added the 🌱 garden Ignition Garden label Dec 6, 2021
@chapulina chapulina added Breaking change Breaks API, ABI or behavior. Must target unstable version. enhancement New feature or request labels Jul 25, 2022
@codecov
Copy link

codecov bot commented Jul 26, 2022

Codecov Report

Merging #199 (d42abcb) into main (717d39f) will not change coverage.
The diff coverage is n/a.

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

@@           Coverage Diff           @@
##             main     #199   +/-   ##
=======================================
  Coverage   84.62%   84.62%           
=======================================
  Files          10       10           
  Lines         995      995           
=======================================
  Hits          842      842           
  Misses        153      153           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 717d39f...ce04de6. Read the comment docs.

@nkoenig
Copy link
Contributor Author

nkoenig commented Jul 26, 2022

@nkoenig nkoenig merged commit 805c3e5 into main Jul 26, 2022
@nkoenig nkoenig deleted the world_stats_stepping branch July 26, 2022 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking change Breaks API, ABI or behavior. Must target unstable version. enhancement New feature or request 🌱 garden Ignition Garden

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants