[Handle] Use get_optional instead of get_value<double>

[saikishor]

After some discussion with @christophfroehlich we thought get_optional might make more sense and explicit compared to get_value<double> and again get its value with .value()

[codecov]

Codecov Report

Attention: Patch coverage is 89.67742% with 16 lines in your changes missing coverage. Please review.

Project coverage is 89.46%. Comparing base (d4f74af) to head (b2d8869).
Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
...terface/include/semantic_components/gps_sensor.hpp	55.17%	5 Missing and 8 partials ⚠️
...nclude/semantic_components/force_torque_sensor.hpp	95.23%	0 Missing and 1 partial ⚠️
...terface/include/semantic_components/imu_sensor.hpp	95.45%	0 Missing and 1 partial ⚠️
...erface/include/semantic_components/pose_sensor.hpp	94.11%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2061      +/-   ##
==========================================
- Coverage   89.56%   89.46%   -0.10%     
==========================================
  Files         139      139              
  Lines       15009    15013       +4     
  Branches     1278     1284       +6     
==========================================
- Hits        13443    13432      -11     
- Misses       1088     1092       +4     
- Partials      478      489      +11     

Flag	Coverage Δ
unittests	89.46% <89.67%> (-0.10%)	⬇️

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

Files with missing lines	Coverage Δ
...mantic_components/semantic_component_interface.hpp	90.90% <100.00%> (ø)
...rface/test/test_chainable_controller_interface.cpp	100.00% <ø> (ø)
...chainable_controller/test_chainable_controller.cpp	91.76% <100.00%> (-0.10%)	⬇️
...r_manager/test/test_controller/test_controller.cpp	88.73% <100.00%> (ø)
...t_controllers_chaining_with_controller_manager.cpp	99.25% <100.00%> (ø)
...re_interface/include/hardware_interface/handle.hpp	84.00% <100.00%> (ø)
...de/hardware_interface/loaned_command_interface.hpp	52.50% <100.00%> (ø)
...lude/hardware_interface/loaned_state_interface.hpp	60.00% <100.00%> (ø)
...rface/test/mock_components/test_generic_system.cpp	99.72% <ø> (ø)
...dware_interface/test/test_component_interfaces.cpp	97.63% <100.00%> (-0.06%)	⬇️
... and 8 more

... and 2 files with indirect coverage changes

[christophfroehlich]

I like.

If we don't merge this, we should apply the changes from f958dfe also to the master.

[Juliaj]

@saikishor @christophfroehlich , just for my knowledge, could you provide some context and outline a few discussion points ? 🙇

[christophfroehlich]

see #2055 and #1976, we were not really happy with the API but hadn't a better idea.

[christophfroehlich]

and here the origin of the API discussions
ros-controls/ros2_controllers#1442
ros-controls/ros2_controllers#1467
ros-controls/ros2_controllers#1443

[christophfroehlich]

basically,
get().get_optional().value() instead of get().get_value<double>().value() in controller and hardware components 😉

[Wiktor-99]

What do you think about adding get_value_or(some_default_value)? I notice that many places currently contain a lot of boilerplate code with if (has_value()) ... else ..., and directly calling value() seems error-prone

[saikishor]

What do you think about adding get_value_or(some_default_value)? I notice that many places currently contain a lot of boilerplate code with if (has_value()) ... else ..., and directly calling value() seems error-prone

@Wiktor-99 Very good question :)

I initially thought the same thing, but upon further thinking it didn't make sense to me because in most of the use cases, you cannot assume any default value for the state or the command contextually it won't make any sense.

For instance, to obtain the position or the velocity of a joint, you cannot assume any default value and proceed with the code. Doing so might cause the control law of the controller unstable in some cases. For that reason, I didn't prefer to do it this way.

[Wiktor-99]

Yeah, I see your point in most cases there is no good default value, nevertheless I see few places where you did return some kind of default (semantic components), but I don't see better way. I'll think about it.