Propagate read/write rate to the HardwareInfo properly

[saikishor]

Alternate approach to #1927

Fixes #1926
Closes #1927

[christophfroehlich]

Looks way better! Just needs a test now.

[saikishor]

Looks way better! Just needs a test now.

The issue with this approach is we can't have new tests, but rely on the already existing rw_rate tests. As we are changing in the load_and_intialize_components method.

If the parsing is not working, the existing tests in hardware_interface_testing package would fail

[saikishor]

Moreover, In this approach we remove the const just to modify it, that's why I wasn't sure.

[christophfroehlich]

Ok I see now that we might also add this here to gazebo/gz_ros2_control as we call parse_control_resources_from_urdf there.

[saikishor]

Ok I see now that we might also add this here to gazebo/gz_ros2_control as we call parse_control_resources_from_urdf there.

I also thought the same earlier, but in the simulation we use the gazebo hardware directly and it runs at the rate of the simulator, so may be there it is not a big requirement

https://github.com/ros-controls/gazebo_ros2_control/blob/2e242fb7400d8a51f6b5d589d58fb358ce2b5999/gazebo_ros2_control/src/gazebo_ros2_control_plugin.cpp#L88-L98

[codecov]

Codecov Report

Attention: Patch coverage is 55.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 87.64%. Comparing base (7374c43) to head (cd2530f).

Files with missing lines	Patch %	Lines
...ace_testing/test/test_components/test_actuator.cpp	0.00%	2 Missing and 1 partial ⚠️
...rface_testing/test/test_components/test_sensor.cpp	0.00%	2 Missing and 1 partial ⚠️
...rface_testing/test/test_components/test_system.cpp	0.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1928      +/-   ##
==========================================
- Coverage   87.73%   87.64%   -0.10%     
==========================================
  Files         122      122              
  Lines       13010    13027      +17     
  Branches     1165     1169       +4     
==========================================
+ Hits        11414    11417       +3     
- Misses       1165     1175      +10     
- Partials      431      435       +4     

Flag	Coverage Δ
unittests	87.64% <55.00%> (-0.10%)	⬇️

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

Files with missing lines	Coverage Δ
hardware_interface/src/resource_manager.cpp	73.33% <100.00%> (+0.03%)	⬆️
...rface/test/mock_components/test_generic_system.cpp	99.76% <100.00%> (+<0.01%)	⬆️
hardware_interface/test/test_component_parser.cpp	98.91% <100.00%> (+0.01%)	⬆️
...ace_testing/test/test_components/test_actuator.cpp	86.27% <0.00%> (-5.40%)	⬇️
...rface_testing/test/test_components/test_sensor.cpp	57.89% <0.00%> (-10.86%)	⬇️
...rface_testing/test/test_components/test_system.cpp	84.90% <0.00%> (-5.10%)	⬇️

... and 3 files with indirect coverage changes

[christophfroehlich]

I also thought the same earlier, but in the simulation we use the gazebo hardware directly and it runs at the rate of the simulator, so may be there it is not a big requirement

The cm update rate can be lower than that of the simulator. And one could configure a hw_rate even lower: as we call the CM in the gazebo update method, this could still work?

[saikishor]

The cm update rate can be lower than that of the simulator. And one could configure a hw_rate even lower: as we call the CM in the gazebo update method, this could still work?

I believe it should work

[christophfroehlich]

Ok, but we don't use this method in the gazebo plugin, and if one changes the plugin he has to take care of that..

[saikishor]

Ok, but we don't use this method in the gazebo plugin, and if one changes the plugin he has to take care of that..

Yup!