moveit / srdfdom

Semantic Robot Description Format
BSD 3-Clause "New" or "Revised" License
12 stars 68 forks source link

Parse decimals in a locale-independent way #108

Closed AndyZe closed 1 year ago

AndyZe commented 1 year ago

Motivated by these MoveIt and ros2_control issues:

https://github.com/ros-planning/moveit2/issues/1882

https://github.com/ros-controls/ros2_control/pull/921/files

AndyZe commented 1 year ago

I think galactic-testing is failing because this usage of std::to_chars requires C++17

https://stackoverflow.com/a/63964295

simonschmeisser commented 1 year ago

You need a pretty recent gcc version for floating point support in std::to_char

AndyZe commented 1 year ago

Alright, thanks. What do you recommend to do here @simonschmeisser ?

simonschmeisser commented 1 year ago

First I'd verify the gcc version as the source of the problem, what does galactic mean in terms of Ubuntu? Then probably copy paste the helper function from MoveIt core helpers

codecov[bot] commented 1 year ago

Codecov Report

Base: 67.83% // Head: 67.09% // Decreases project coverage by -0.74% :warning:

Coverage data is based on head (4bb945b) compared to base (91d7db8). Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## ros2 #108 +/- ## ========================================== - Coverage 67.83% 67.09% -0.74% ========================================== Files 3 3 Lines 634 641 +7 ========================================== Hits 430 430 - Misses 204 211 +7 ``` | [Impacted Files](https://codecov.io/gh/ros-planning/srdfdom/pull/108?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-planning) | Coverage Δ | | |---|---|---| | [src/model.cpp](https://codecov.io/gh/ros-planning/srdfdom/pull/108?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-planning#diff-c3JjL21vZGVsLmNwcA==) | `58.77% <0.00%> (-1.03%)` | :arrow_down: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-planning). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-planning)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

simonschmeisser commented 1 year ago

So it seems a very ugly hack (by actually switching the locale, which is global in c, temporarily) to support floating points in std::from_char was added for gcc 11.1.0, Ubuntu 20.04 defaults to 9.4.0. Proper support seems to have been implemented by now but is not actually released yet ( https://github.com/gcc-mirror/gcc/commits/master/libstdc%2B%2B-v3/src/c%2B%2B17/floating_from_chars.cc ).