Make double parsing locale independent #921
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
requested review from
bmagyar,
destogl,
erickisos,
harderthan,
jaron-l,
mhubii and
VX792
henningkayser
force-pushed
the
pr/locale_independent_doubles
branch
from
fcbda9b to
958be1c
Compare
std::from_chars uses dot notation for doubles and is locale independent
henningkayser
force-pushed
the
pr/locale_independent_doubles
branch
from
af611d8 to
1a427bc
Compare
henningkayser
changed the title
destogl
requested changes
Jan 23, 2023
bmagyar
approved these changes
Jan 24, 2023
|
@Mergifyio backport to humble |
mergify bot
pushed a commit
that referenced
this pull request
Jan 24, 2023
(cherry picked from commit 78bb0ca)
✅ Backports have been created
|
bmagyar
pushed a commit
that referenced
this pull request
Jan 25, 2023
(cherry picked from commit 78bb0ca) Co-authored-by: Henning Kayser <[email protected]>
bmagyar
added a commit
to bmagyar/ros2_control
that referenced
this pull request
Mar 8, 2023
This reverts commit 78bb0ca.
bmagyar
added a commit
that referenced
this pull request
Mar 9, 2023
christophfroehlich
pushed a commit
to christophfroehlich/ros2_control
that referenced
this pull request
Aug 22, 2023
This reverts commit 78bb0ca.
|
Usage of std::from_chars requires C++17. Which is not available on all target hardware platforms. :( |
pac48
pushed a commit
to pac48/ros2_control
that referenced
this pull request
Jan 26, 2024
Open
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A recent comment in a MoveIt issue made me curious about locale-dependent bugs and runtime flakiness in general. Turns out, parsing doubles with
std::stodis locale dependent and will silently remove decimals if the parsed text doesn't fit the configured locale. This has been found out and fixed a while ago (see ros/urdfdom#98 for instance), but it still seems to be an issue in several packages,ros2_controlis just a prominent one and a great example for a start. See this MoveIt issue for other examples moveit/moveit2#1882.Solution: use https://en.cppreference.com/w/cpp/utility/from_chars which implements locale default "C" using dot notation.
(I just pushed 958be1c to demonstrate the issue by making the test fail)