Added tests for the AnimationTime component

[adlarkin]

I added missing tests for a std::chrono::steady_clock::duration component after doing something similar in #401.

Signed-off-by: Ashton Larkin [email protected]

[adlarkin]

A few other things to consider:

1. Should we use uint64_t instead of int64_t for de-serialization (relevant code snippet below), or can std::chrono::duration::count() return a negative value if a jump back in time occurs? (if negative count() return values are possible, I'll update Logical audio sensor plugin #401 to use int64_t)

https://github.com/ignitionrobotics/ign-gazebo/blob/aea1accff6d8daee0358ae9025e2a9797d292844/include/ignition/gazebo/components/Actor.hh#L58-L71

2. The serialization/de-serialization methods are only as accurate as milliseconds, since that is what we are currently casting the durations to. If we want to serialize time that is more precise than milliseconds, then the information that exceeds millisecond precision will be lost. Should we use a more precise duration cast? For example, something like std::chrono::nanoseconds?

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (ign-gazebo4@6f32258). Click here to learn what that means.
The diff coverage is n/a.

@@              Coverage Diff               @@
##             ign-gazebo4     #433   +/-   ##
==============================================
  Coverage               ?   77.26%           
==============================================
  Files                  ?      208           
  Lines                  ?    11192           
  Branches               ?        0           
==============================================
  Hits                   ?     8648           
  Misses                 ?     2544           
  Partials               ?        0           

---

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 6f32258...02ad2db. Read the comment docs.

[chapulina]

can std::chrono::duration::count() return a negative value if a jump back in time occurs?

The specific duration type we're using is std::chrono::steady_clock::duration, which is equivalent to std::chrono::nanoseconds, which under the hood is std::chrono::duration<int64_t, std::nano>. Note the int64_t there. So it can be negative.

You're right that we use negative durations for jumps back in time. For example, dt will be negative when a log playback is rewinded:

https://github.com/ignitionrobotics/ign-gazebo/blob/da2ddbc14bc487d53e49de5b9fdea3272797698c/src/systems/log/LogPlayback.cc#L484-L486

Now, will it ever be negative for AnimationTime? I don't think it should. But I was trying to be general when implementing the serialization.

Should we use a more precise duration cast?

Yeah I think that's a good idea. For the animation it probably isn't necessary, but I think it's good to be general.

[adlarkin]

I went ahead and modified the serialization to use nanoseconds instead of milliseconds in 02ad2db. This should be ready for another review now.

[chapulina]

Nice, thanks for the test!