Use portable version for string-to-double conversion

[christophfroehlich]

As raised with #949 the current code is not portable to compilers not fully supporting c++17 (like on Debian 11,..) since #921

I propose using the solution of urdfdom_headers, srdfdom and MoveIt2, which fixes #949.

I created a new file and moved also the parse_bool function.

Maybe we should use this only for humble+iron, and leave the "modern" std::from_chars in rolling and for future distros? Edit: we want to keep that and add the std::from_chars with build-macros for newer compiler.

Once this is merged, I'd fix also with the then-decided solutions

• ros2_control_demos
• gazebo_ros2_control
• gz_ros2_control

[codecov]

Codecov Report

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

Comparison is base (e149646) 47.82% compared to head (560605c) 47.66%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1257      +/-   ##
==========================================
- Coverage   47.82%   47.66%   -0.16%     
==========================================
  Files          40       41       +1     
  Lines        3448     3451       +3     
  Branches     1869     1876       +7     
==========================================
- Hits         1649     1645       -4     
  Misses        455      455              
- Partials     1344     1351       +7     

Flag	Coverage Δ
unittests	47.66% <33.33%> (-0.16%)	⬇️

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

Files	Coverage Δ
...e_interface/src/mock_components/generic_system.cpp	55.01% <0.00%> (-0.09%)	⬇️
hardware_interface/src/component_parser.cpp	43.64% <37.50%> (-1.74%)	⬇️
...rface/include/hardware_interface/lexical_casts.hpp	40.00% <40.00%> (ø)

... and 1 file with indirect coverage changes

[saikishor]

Niceeee. Great job!

[christophfroehlich]

@saikishor what do you think, which version should we use from ROS-J on? std::from_chars or this version with the std::locale::classic?

[saikishor]

@saikishor what do you think, which version should we use from ROS-J on? std::from_chars or this version with the std::locale::classic?

@christophfroehlich Thanks for bringing up this. I think std::from_chars is recommended from the C++17 onwards, however, looking at the some of the main ROS repos, they are still using this locale approach in their rolling branches. For this reason, it is better to maintain this approach even in the upcoming releases. Moreover, this way we won't loose support for Debian11.

What do you think?

[saikishor]

LGTM

[destogl]

Happy, just one small comment

[destogl]

Can we add something like this?

Suggested change

	#include "hardware_interface/helpers.hpp"
	#if CPP_VER<=17
	#include "hardware_interface/helpers.hpp"
	#endif

[destogl]

#if __cplusplus > 201402L

[bmagyar]

...in a follow-up ticket

[destogl]

The goal is to add the old code of parsing with std::from_chars protected by CPP version protection. If a higher version then C++17 (this flag #if __cplusplus > 201402L) is used then we should you this standard; otherwise we use the here implemented version.

[olivier-stasse]

Thanks a lot for this !
LGTM

[saikishor]

LGTM