Allow specifying initial simulation time with a CLI argument #1801

JoanAguilar · 2022-11-18T23:15:15Z

🎉 New feature

Summary

The gz sim command now accepts a new optional paramater: initial-sim-time. This parameter controls the initial value of the simulation time.

Test it

One should be able to run gz sim --initial-sim-time [t], with [t] being a floating point value, and have the simulation time start at [t] seconds.

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: Joan Aguilar Mayans <joan@openrobotics.org>

scpeters · 2022-11-18T23:43:06Z

src/SimulationRunner.cc

@@ -122,6 +122,12 @@ SimulationRunner::SimulationRunner(const sdf::World *_world,
        static_cast<int>(this->stepSize.count() / this->desiredRtf));
  }

+  // Epoch
+  // TODO: Decide on chrono precision.


cpplint wants a user assigned to each TODO

Missing username in TODO; it should look like "// TODO(my_username): Stuff." [readability/todo] [2]

Yeah, I was hoping to decide on this before merge. In fact, seeing how updatePeriod uses nanosecond precision, I'll likely change this to the same precision and remove the TODO. That should also get CI to run a bit further.

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

src/cmd/cmdsim.rb.in

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

codecov · 2022-11-22T03:15:58Z

Codecov Report

Merging #1801 (dd58542) into gz-sim7 (3dc8c7a) will decrease coverage by 0.07%.
The diff coverage is 69.48%.

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

@@             Coverage Diff             @@
##           gz-sim7    #1801      +/-   ##
===========================================
- Coverage    64.68%   64.61%   -0.07%     
===========================================
  Files          343      347       +4     
  Lines        27430    27731     +301     
===========================================
+ Hits         17743    17919     +176     
- Misses        9687     9812     +125

Impacted Files	Coverage Δ
include/gz/sim/ServerConfig.hh	`100.00% <ø> (ø)`
include/gz/sim/Util.hh	`100.00% <ø> (ø)`
...rc/systems/ackermann_steering/AckermannSteering.hh	`100.00% <ø> (ø)`
src/systems/joint_controller/JointController.hh	`100.00% <ø> (ø)`
...int_position_controller/JointPositionController.hh	`100.00% <ø> (ø)`
src/gui/plugins/component_inspector/Inertial.cc	`11.76% <11.76%> (ø)`
src/SdfGenerator.cc	`90.33% <40.00%> (-0.62%)`	⬇️
src/systems/apply_link_wrench/ApplyLinkWrench.cc	`77.77% <46.15%> (ø)`
...int_position_controller/JointPositionController.cc	`74.13% <67.56%> (+0.56%)`	⬆️
src/Util.cc	`91.01% <71.42%> (-1.23%)`	⬇️
... and 16 more

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

…l-sim-time-2 Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

JoanAguilar · 2022-11-30T22:58:14Z

This now seems to be working fine on my machine for positive values. Negative values, on the other hand, result in a warning and the clock being reset to -1 ns every time the simulation is paused or reset:

Warning [World.cpp:139] [World] Attempting to set negative timestep. Ignoring this request because it can lead to undefined behavior.

Seeing that I couldn't find a World.cpp file or excerpts of the warning message in source, I assume that the warning (and the associated behavior) are not coming from Gazebo (DART?).

It also seems CI is not particularly happy with the changes. On my last try on my machine (Jammy) I only got one failing test: testFixture_TEST, with an error about not being able to import the python module gz.math7. Interestingly, all the Ubuntu CI jobs get other tests failing, but not this one...

Jobs:

scpeters · 2022-12-01T01:01:45Z

This now seems to be working fine on my machine for positive values. Negative values, on the other hand, result in a warning and the clock being reset to -1 ns every time the simulation is paused or reset:
Warning [World.cpp:139] [World] Attempting to set negative timestep. Ignoring this request because it can lead to undefined behavior.

I think we shouldn't worry about supporting negative timestamps

adityapande-1995

It would be nice to add some tests for this feature.

Signed-off-by: Aditya <aditya050995@gmail.com>

src/gz_TEST.cc

Signed-off-by: Aditya <aditya050995@gmail.com>

scpeters · 2023-01-07T00:19:26Z

I marked this as ready for review

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

mjcarroll · 2023-02-28T20:40:03Z

LGTM with green CI

scpeters · 2023-03-02T21:22:27Z

LGTM with green CI

some of the CI jobs are green, and those that aren't have unrelated failures. I will go ahead and merge

JoanAguilar added 6 commits November 17, 2022 15:34

Add simulation time epoch to SimulationRunner.

d28b71e

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

Add initial simulation time to ServerConfig.

573d95b

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

Add initial simulation time to gz.hh and gz.cc.

974f64b

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

Add initial simulation time to cmdsim.

9a61e20

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

Merge remote-tracking branch 'origin/gz-sim7' into initial-sim-time

f27003a

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

Increase precision of initialSimTime to double.

2d76a16

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

JoanAguilar added the enhancement New feature or request label Nov 18, 2022

JoanAguilar self-assigned this Nov 18, 2022

github-actions bot added the 🌱 garden Ignition Garden label Nov 18, 2022

scpeters reviewed Nov 18, 2022

View reviewed changes

JoanAguilar added 2 commits November 21, 2022 15:41

Change simTimeEpoch precision to nanoseconds and remove TODO.

d33e108

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

Add missing parameter in runServer call.

27bdbd9

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

scpeters reviewed Nov 22, 2022

View reviewed changes

src/cmd/cmdsim.rb.in Outdated Show resolved Hide resolved

Fix typo.

52506e2

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

JoanAguilar added 3 commits November 29, 2022 13:53

Merge remote-tracking branch 'origin/gz-sim7' into joanaguilar/initia…

53488fe

…l-sim-time-2 Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

Set currentInfo.simTime to the simTimeEpoch during construction.

8bd519f

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

Update handling of requestedSeek and requestedRunToSimTime.

5dea45c

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>

adityapande-1995 reviewed Dec 20, 2022

View reviewed changes

Added test for simtime argument

4d38a9d

Signed-off-by: Aditya <aditya050995@gmail.com>

scpeters reviewed Jan 6, 2023

View reviewed changes

src/gz_TEST.cc Outdated Show resolved Hide resolved

Test starts from t = 1000.5 sec

6c2a134

Signed-off-by: Aditya <aditya050995@gmail.com>

scpeters marked this pull request as ready for review January 7, 2023 00:19

scpeters requested a review from mjcarroll as a code owner January 7, 2023 00:19

scpeters added 2 commits February 28, 2023 11:05

Merge branch 'gz-sim7' into joanaguilar/initial-sim-time-2

1e8c37a

fix line length

26eef7b

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

mjcarroll approved these changes Feb 28, 2023

View reviewed changes

scpeters merged commit 2127f35 into gz-sim7 Mar 2, 2023

scpeters deleted the joanaguilar/initial-sim-time-2 branch March 2, 2023 21:25

scpeters mentioned this pull request Mar 5, 2023

gz_TEST: improve initial sim time test reliability #1916

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow specifying initial simulation time with a CLI argument #1801

Allow specifying initial simulation time with a CLI argument #1801

JoanAguilar commented Nov 18, 2022 •

edited by adityapande-1995

Loading

scpeters Nov 18, 2022

JoanAguilar Nov 21, 2022

codecov bot commented Nov 22, 2022 •

edited

Loading

JoanAguilar commented Nov 30, 2022

scpeters commented Dec 1, 2022

adityapande-1995 left a comment

scpeters commented Jan 7, 2023

mjcarroll commented Feb 28, 2023

scpeters commented Mar 2, 2023

Allow specifying initial simulation time with a CLI argument #1801

Allow specifying initial simulation time with a CLI argument #1801

Conversation

JoanAguilar commented Nov 18, 2022 • edited by adityapande-1995 Loading

🎉 New feature

Summary

Test it

Checklist

scpeters Nov 18, 2022

Choose a reason for hiding this comment

JoanAguilar Nov 21, 2022

Choose a reason for hiding this comment

codecov bot commented Nov 22, 2022 • edited Loading

Codecov Report

JoanAguilar commented Nov 30, 2022

scpeters commented Dec 1, 2022

adityapande-1995 left a comment

Choose a reason for hiding this comment

scpeters commented Jan 7, 2023

mjcarroll commented Feb 28, 2023

scpeters commented Mar 2, 2023

JoanAguilar commented Nov 18, 2022 •

edited by adityapande-1995

Loading

codecov bot commented Nov 22, 2022 •

edited

Loading