CM: Check if a valid time is received

[christophfroehlich]

With use_sim_time=true but nothing received on /clock topic, we get a not so nice exception
ros-controls/gz_ros2_control#439

Not sure if we should escalate and throw an error or spam the console with log output.

[codecov]

Codecov Report

Attention: Patch coverage is 22.22222% with 7 lines in your changes missing coverage. Please review.

Project coverage is 87.55%. Comparing base (89fd4ef) to head (6b400b9).
Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
controller_manager/src/controller_manager.cpp	22.22%	6 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1901      +/-   ##
==========================================
- Coverage   87.59%   87.55%   -0.04%     
==========================================
  Files         122      122              
  Lines       12766    12774       +8     
  Branches     1146     1147       +1     
==========================================
+ Hits        11182    11184       +2     
- Misses       1155     1161       +6     
  Partials      429      429              

Flag	Coverage Δ
unittests	87.55% <22.22%> (-0.04%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
controller_manager/src/controller_manager.cpp	76.31% <22.22%> (-0.30%)	⬇️

[saikishor]

Personally I would throw an error! Probably we can do that in the constructor of the CM?

[christophfroehlich]

Haven't tried that but there are SE answers that clock()->now() with sim_time is always zero in the constructor of a node.

[christophfroehlich]

We could just use the time argument instead, and maybe use a throttled/once warning?

[fmauch]

This definitively fixes the issues I recently faced, thank you for fixing that. Regarding the discussion I think it's good to be verbose about this. A throttled warning seems suitable in my opinion. But it would be good if the warning could point users to how to actually fix that problem if it occurs.

[saikishor]

This definitively fixes the issues I recently faced, thank you for fixing that. Regarding the discussion I think it's good to be verbose about this. A throttled warning seems suitable in my opinion. But it would be good if the warning could point users to how to actually fix that problem if it occurs.

I just realized that we cannot use the throttle as the clock is not working

[saikishor]

Sounds fine!

Just some minor changes

[mergify]

This pull request is in conflict. Could you fix it @christophfroehlich?

[fmauch]

Tested this locally with both, a /clock topic present and not. The result was as expected and the output is helpful in case of the missing clock topic.

[saikishor]

Some changes that might bring back the failing tests.
Sorry, it got mixed up with my Diagnostics update.

[saikishor]

Thanks