** Description changed:
- In the effort to reduce boost dependency in urdfdom and urdfdom-headers
- [2] regressions wrt locale handling were introduced. Parsing of floating
- point values in URDF (a XML language) was now locale dependent, ie on
- European systems were the decimal separator is "," instead of "."
- parsing would now fail due to std::stod expecting "1,23" instead of
- "1.23" (no, we do not localize xml files, see also [3] for a more
- technical summary). The regressed versions were released as 1.0. Shortly
- after the release (some of) the regressions were detected [3] and fixes
- merged [3][4]. Unfortunately those fixed merges were not released until
- a long time later, too late for Ubuntu 18.04 by far and potentially even
- for 20.04. More regressions were noticed and fixed later on (disclaimer:
- by me) [5].
+ [Impact]
- urdfdom is used mostly if not exclusively in the ROS community
- (including gazebo) which is a somewhat slow adopter of newer Ubuntu (and
- ros) releases but more and more people are now seeing these regressions
- in various places. To save more peoples time I propose to do a patch
- version update and pull urdfdom_headers version 1.0.3 as well as urdfdom
- version 1.0.3 from debian sid. I argue against applying a patch because
- the diff between 1.0.0 and 1.0.3 is almost identical to a set of patches
- fixing the regressions. Updating to the upstream patch release will
- therefore improve tracing of the ubuntu downstream version to the
- upstream versioning.
+ * The parsing done by the code of urdfdom truncate floating values on
+ systems that are not using the locales defined by default on English
+ systems. The users of this system are getting wrong values which are
+ specially sensible on variables with a range between 0 and 1.
- Both urdfdom_headers and urdfdom now have an extensive set of unit test
- which in the case of urdfdom are also run in a European (ie Dutch)
- locale [6]. Together with the small and homogeneous user base,
- application mostly in research and the fact that most of the library is
- (for unknown reasons) almost header-only make me estimate that scope,
- potential and impact of regressions introduced by this upgrade would be
- extremely low.
+ * The URDF schema defines doubles in URDF as xs:double. Looking at the
+ XSD document it says that the mantissa for a double is represented as a
+ "decimal" number. And looking at the XSD document a decimal number is a
+ "finite-length sequence of decimal digits (#x30-#x39) separated by a
+ period as a decimal indicator". Thus, the locale should have no effect
+ on how the document is parsed, and using std::stod is incorrect. This
+ patch switches the Vector3 class to using the stream operator, which
+ seems to ignore locale.
- 1:
https://github.com/ros/urdfdom/commit/3dc7ee812827cc69ffa457ef01fe7b9623096aed
- 2:
https://github.com/ros/urdfdom_headers/commit/9d2b421f3fcbc2a32af40b99ebd9c2cb2d088fb9
- 3: https://github.com/ros/urdfdom_headers/pull/42
- 4: https://github.com/ros/urdfdom/pull/105
- 5: https://github.com/ros/urdfdom_headers/pull/47
- 6: https://github.com/ros/urdfdom/pull/115
+ * urdfdom-headers is a key part for the largest open source robotics
+ framework, ROS (Robot Operative System). So it potentially affects a big
+ part of the robotics open source community. We (the devs and maintainers
+ of ROS have been receiving many different issues or bugs coming from
+ this problem, as Simon wrote in the initial post of this bug.
+
+ [Test Case]
+
+ * We have prepared a test case that demonstrate the problem and the fix.
Patched packages for both urdfdom and urdfdom_headers are in this ppa:
+ https://launchpad.net/~j-rivero/+archive/ubuntu/urdfdom-headers-sru2/+packages
+
+ * The dockerfile here
https://github.com/j-rivero/sru_urdfdom/blob/master/Dockerfile tries to
demonstrate the following:
+ - Setup the test case, install packages
+ - Import locale test from the latest development of urdfdom repo
+ - Run tests, failed (demonstration of the bug)
+ - Setup the PPA, install new version of packages
+ - Run test, success (demonstration of the fix)
+
+ * This is the testing build of the dockerfile
+ https://build.osrfoundation.org/job/_urdfdom_sru/15/console
+
+ [Regression Potential]
+
+ * The change affected the result of parsed values from using stof/stod
+ functions to use stringstream. Although the patch is not small, the same
+ pattern is replied on every chunk of code changed. Standard use case of
+ users setting up digits in the form of integer or double is unlikely to
+ generate big regressions. Alternative use cases of users with weird
+ values in the input would trigger an exception from the code. This is no
+ different than the exceptions thrown by previous stof/stod funtions
+ (std::invalid_argument or std::out_of_range).
+
+ * The bug was reported in Nov 2017
+ (https://github.com/ros/urdfdom_headers/issues/41) the ROS community and
+ fixes landed in upstream repositories few weeks after. Many users from
+ different countries have been using new versions or these patches since
+ them, some of them are commenting/voting in this bug. Note that we were
+ also playing with upstream test suite to build the test case and no
+ regression was found in there.
+
+ [Other Info]
+
+ * Upstream releases new patched versions with the fix that I’ve included
+ into Debian Sid and have been synced by Ubuntu starting from Disco:
+ 1.0.3-1
+
+ * debdiffs for versions from the PPA:
+ - urdfdom debdiff:
+ https://gist.github.com/j-rivero/9983dddc6dbbea99cd3c39774f1d0498
+
+ - urdfdom-headers debdiff
+ https://gist.github.com/j-rivero/bc563620802d78129d775b2d5aee69ac
+
+ https://github.com/ros/urdfdom/commit/3dc7ee812827cc69ffa457ef01fe7b9623096aed
+
https://github.com/ros/urdfdom_headers/commit/9d2b421f3fcbc2a32af40b99ebd9c2cb2d088fb9
+ https://github.com/ros/urdfdom_headers/pull/42
+ https://github.com/ros/urdfdom/pull/105
+ https://github.com/ros/urdfdom_headers/pull/47
+ https://github.com/ros/urdfdom/pull/115
See also the following upstream bug reports:
https://github.com/ros/urdfdom_headers/issues/45
https://github.com/ros/urdfdom/issues/119
and a few downstream bug reports to the ubuntu package:
https://github.com/ros-planning/moveit/issues/1333
https://github.com/ros/urdf/issues/21
https://github.com/ros-visualization/rviz/issues/1249
https://github.com/ros-visualization/rviz/issues/1151
https://github.com/ros-planning/moveit/issues/1050
https://github.com/ros-visualization/rviz/issues/1298
** Also affects: urdfdom-headers (Ubuntu Bionic)
Importance: Undecided
Status: New
** Also affects: urdfdom (Ubuntu Bionic)
Importance: Undecided
Status: New
** Changed in: urdfdom (Ubuntu)
Status: Confirmed => Fix Released
** Changed in: urdfdom-headers (Ubuntu)
Status: Confirmed => Fix Released
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1817595
Title:
[SRU] urdfdom-headers and urdfdom should not use locale dependent
parsing for floating point numbers
To manage notifications about this bug go to:
https://bugs.launchpad.net/urdfdom-headers/+bug/1817595/+subscriptions
--
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
