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

Add clock plugin with basic functionality #70

Merged
merged 14 commits into from
Jan 29, 2024
Merged

Conversation

xela-95
Copy link
Member

@xela-95 xela-95 commented Jan 25, 2024

This PR adds to the gz-sim-yarp-plugins a clock plugin, with the only basic functionality of writing the current simulation time on the yarp /clock port.

Closes #60 , #73

@xela-95 xela-95 requested a review from traversaro January 25, 2024 11:37
@xela-95 xela-95 self-assigned this Jan 25, 2024
@xela-95 xela-95 linked an issue Jan 25, 2024 that may be closed by this pull request
Copy link

codecov bot commented Jan 25, 2024

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (dab8a73) 83.19% compared to head (404f77b) 83.36%.

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

Files Patch % Lines
plugins/clock/Clock.cc 80.76% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #70      +/-   ##
==========================================
+ Coverage   83.19%   83.36%   +0.17%     
==========================================
  Files          14       16       +2     
  Lines         964     1004      +40     
==========================================
+ Hits          802      837      +35     
- Misses        162      167       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xela-95 xela-95 force-pushed the feature/plugin-clock branch from 8f6967f to e7426c7 Compare January 26, 2024 13:42
@xela-95
Copy link
Member Author

xela-95 commented Jan 26, 2024

@traversaro I tried to develop a first basic unit test for the clock plugin, here:

https://github.com/robotology/gz-sim-yarp-plugins/blob/4a06b08fa297f69f3959402f0ecd1d0dd23963c7/tests/clock/ClockTest.cc

The unit test instantiates a gazebo server fixture and reads the /clock port after a certain number of simulation steps have been ran. Then it checks whether the elapsed simulation time received on the port is the one expected (number of timesteps times delta T).

Next I will try to add test to check other expected behaviors, like the pause and reset of simulation.

In this way it is possible to avoid desctruction and re-construction of the plugin object with consequent closing of  the yarp port.
@xela-95
Copy link
Member Author

xela-95 commented Jan 29, 2024

In commit 5acde4f I've implemented the ISystemReset interface. In this way, every time the user requests a simulation reset through Gazebo UI this method is called instead of destructing and re-constructing the plugin with the consequent closing of the yarp port. Now for a subscriber to the port the reset is transparent and it will just see the simulation time starting back from zero.

Unfortunately I've been not able to implement a unit test since the Server object does not expose a Reset() method that I can use to inject such event.

Some issues and PRs talking about reset:

@xela-95 xela-95 marked this pull request as ready for review January 29, 2024 11:52
{
if (_info.paused)
{
// yDebug() << "Simulation pause, skipping clock update";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// yDebug() << "Simulation pause, skipping clock update";
// yDebug() << "Simulation pause, skipping clock update";

Similarly, if a message is commented, we can just remove it from the code to keep code simple.

Copy link
Member

@traversaro traversaro left a comment

Choose a reason for hiding this comment

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

Just a small comment regarding unnecessary debug messages. Beside that, I think it is good to go, remember to squash commits!

xela-95 and others added 2 commits January 29, 2024 14:29
@xela-95 xela-95 merged commit 4a0ff1d into main Jan 29, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port basic clock plugin functionality from gazebo-yarp-plugins
2 participants