gz-sim icon indicating copy to clipboard operation
gz-sim copied to clipboard

Add support for specifying log record period

Open iche033 opened this issue 1 year ago • 3 comments

Signed-off-by: Ian Chen [email protected]

🎉 New feature

Summary

Current log recorder records states on every update. By specifying the log record period, the user is able to control how often the simulator records states. A new command line arg --record-period is added (gazebo-classic also has the same arg name)

Test it

Run the INTEGRATION_log_system test.

You can also try enabling log recording through the command line, e.g.

# record at 10ms instead of the default 1ms and save the log file in a `log_test` dir
ign gazebo -v 4 -r --record --record-path ./log_test --record-period 0.01  pendulum_links.sdf

then playback the log file:

ign gazebo -v 4 --playback  ./log_test

You should see that the playback is now choppy due to reduced recording rate.

Checklist

  • [x] Signed all commits for DCO
  • [x] 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.

iche033 avatar Aug 09 '22 05:08 iche033

Codecov Report

Merging #1636 (f4728cc) into ign-gazebo6 (8c9489d) will increase coverage by 0.07%. The diff coverage is 94.11%.

@@               Coverage Diff               @@
##           ign-gazebo6    #1636      +/-   ##
===============================================
+ Coverage        64.40%   64.48%   +0.07%     
===============================================
  Files              320      320              
  Lines            25892    25935      +43     
===============================================
+ Hits             16677    16723      +46     
+ Misses            9215     9212       -3     
Impacted Files Coverage Δ
include/ignition/gazebo/EntityComponentManager.hh 100.00% <ø> (ø)
include/ignition/gazebo/ServerConfig.hh 100.00% <ø> (ø)
src/ign.cc 64.83% <80.00%> (+0.22%) :arrow_up:
src/systems/scene_broadcaster/SceneBroadcaster.cc 92.16% <83.33%> (-0.27%) :arrow_down:
src/EntityComponentManager.cc 89.38% <100.00%> (+0.02%) :arrow_up:
src/ServerConfig.cc 90.99% <100.00%> (+0.30%) :arrow_up:
src/ServerPrivate.cc 82.96% <100.00%> (+0.15%) :arrow_up:
src/systems/log/LogRecord.cc 79.15% <100.00%> (+1.01%) :arrow_up:
src/SimulationRunner.cc 91.89% <0.00%> (+0.95%) :arrow_up:

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

codecov[bot] avatar Aug 09 '22 18:08 codecov[bot]

Just for testing I tried 0.1 and 1 record-periods. I was expecting very choppy results but I think they playback stops for a lot of time. Not sure if there's something going on.

I can reproduce the issue. Just dumping my findings here: Found that it's to do with the scene_broadcaster in playback mode. It's not able to pick up periodic changed states and send them to the gui. The logic has implicit assumption that states are updated often enough so that every time it's about to send the states to the gui, there are state changes available. However, log playback now updates at lower rate. You will only see the pendulum move if by some timing coincidence that log playback updates at the same iteration as when scenebroadcaster is about to publish its states.

Unthrottling the scene_broadcaster makes it work but I haven't found a good fix yet

iche033 avatar Aug 12 '22 05:08 iche033

playback scene broadcaster issue should be fixed in f4728cc. Now if no periodic changes are available when scene broadcaster is publishing, I force it to do an offcycle state update the next time it sees periodic changes. This should ensure all changed states from log playback are published to the GUI.

iche033 avatar Aug 12 '22 19:08 iche033