locale issues here as well

[simonschmeisser]

I'm sorry to bring further bad news, the ubuntu version of this package is based on 1.0.0 from 2016 which used std::stod. The issues were subsequently fixed in code but no new release was made.

This can be seen with truncated joint limits for example. (We have one linear joint inside the gripper which obviously moves less than 1.0m ... it's less visible when your revolute joints limit is reduced from 3.141 to 3.0 ...)

@scpeters can we have a new release here as well? @j-rivero can we then get this into debian (and try updating ubuntu)

(followup on ros/urdfdom_headers#45)

@gavanderhoorn so it seems necessary to grep not only the current code but the entire history of ros dependencies for std::stod usage ... (note that I have not done that)

[gavanderhoorn]

@gavanderhoorn so it seems necessary to grep not only the current code but the entire history of ros dependencies for std::stod usage ... (note that I have not done that)

You mean check the released sources for all current releases (for the different platforms)?

Yes, that would seem like a good idea.

Thanks for staying on top of this btw 👍

[traversaro]

@gavanderhoorn so it seems necessary to grep not only the current code but the entire history of ros dependencies for std::stod usage ... (note that I have not done that)

Other typical offenders for locale-related bugs are sscanf, atof, boost::lexical_cast, std::to_string and std::stringstream.

Some other locale-related bugs in projects related to robotics (I think most of this still need to be fixed):

• urdfdom usage of std::stod is error-prone to changes in system locale #98 (comment)
• https://bitbucket.org/osrf/sdformat/issues/207/error-when-starting-ign-gazebo-if

[simonschmeisser]

@scpeters ping?

[simonschmeisser]

@scpeters ping again? Import freeze for DiscoDingo/Ubuntu 19.04 is February 21st https://wiki.ubuntu.com/DiscoDingo/ReleaseSchedule

while we should also try to backport this to 18.04, releasing it now and updating Debian packages will at least fix this "for free" in 19.04 (and 20.04 LTS) @j-rivero

@clalancette @dhood can one of you ask @scpeters directly?

[scpeters]

I'll make a new release shortly.

[simonschmeisser]

@scpeters the deadline for Ubuntu 19.04 is getting closer, can we have the release soonish?

[scpeters]

working on it, bumping version number in #121

[scpeters]

@j-rivero can you help this latest release into debian?

https://github.com/ros/urdfdom/releases/tag/1.0.3

[VictorLamoine]

ping ... 🏓

[simonschmeisser]

@j-rivero apparently debian import deadline [1] for disco dingo was last Thursday (2019-02-21) but it is still at 1.0.0 [2], do you think you can still trigger this so that at least Ubuntu 19.04 has current packages?

1: https://wiki.ubuntu.com/DiscoDingo/ReleaseSchedule
2: https://packages.ubuntu.com/source/disco/urdfdom

[j-rivero]

@j-rivero apparently debian import deadline [1] for disco dingo was last Thursday (2019-02-21) but it is still at 1.0.0 [2], do you think you can still trigger this so that at least Ubuntu 19.04 has current packages?

I've uploaded the 1.0.3 to Debian testing. I will try to get a Freeze exception for getting it into Disco.

[simonschmeisser]

@VictorLamoine and others: I opened a bug report in launchpad to ask for a update in 18.04, there is a "vote" feature there! (search for something like "this bug affects me")

https://bugs.launchpad.net/ubuntu/+source/urdfdom-headers/+bug/1817595

[gavanderhoorn]

@simonschmeisser: might merit a post to ROS Discourse to mobilise as many users as possible.

This will basically affect everyone with a non-US locale, of which there are more-and-more in ROS.

[simonschmeisser]

I'm not sure I want to escalate this fully yet, lets wait for feedback from @j-rivero

[ahoarau]

I just dicovered this issue on one of my setups. Temporary fix is to set the US locale:

sudo -s

locale-gen en_US
locale-gen en_US.UTF-8
echo 'LANGUAGE=en_US.UTF-8' >> /etc/environment
echo 'LC_ALL=en_US.UTF-8' >> /etc/environment

exit

Then log out and log in or reboot.

[VictorLamoine]

Setting LC_NUMERIC should be enough.

[gavanderhoorn]

@simonschmeisser wrote:

I'm not sure I want to escalate this fully yet, lets wait for feedback from @j-rivero

would it be time to escalate this now?

It's been almost a full year since ros/urdfdom_headers#45 was first opened.

It's also still affecting a multitude of users on Melodic + Bionic that don't use a US locale, leading to many questions on ROS Answers and issues opened on trackers.

[simonschmeisser]

@j-rivero can we get your opinion either here or on the launchpad bug? I got zero reaction on launchpad so far :(

[j-rivero]

@j-rivero can we get your opinion either here or on the launchpad bug? I got zero reaction on launchpad so far :(

The SRU bug in Ubuntu looks incomplete to me, please read the strict procedure that Ubuntu has for this kind of updates. I was working on what I though was the best way to do it (see ros/urdfdom_headers#47 (comment) and follow until the last comment). That being said, there is no guarantee that we get the bug fixed in Ubuntu even if we sent the bug correct.

[gavanderhoorn]

@j-rivero: would it be possible for you to assist? I'm not speaking for @simonschmeisser, but it's clear that you have much more experience with this process.

This issue is affecting hundreds if not thousands of ROS users worldwide on Bionic.

That being said, there is no guarantee that we get the bug fixed in Ubuntu even if we sent the bug correct.

Could we then consider serving a patched version of the affected libraries from the OSRF packages repositories?

[j-rivero]

Could we then consider serving a patched version of the affected libraries from the OSRF packages repositories?

We would need to convince the ROS maintainers but I don't think that having a patched version is a crazy idea if many users are affected. The first step for me would be to demonstrate the error + fix in a docker file (as I requested in ros/urdfdom_headers#47 (comment)).

[gavanderhoorn]

@simonschmeisser: would that be something you could put together?

[simonschmeisser]

I think releasing urdfdom and co via the OSRF package repositories is the way to go! This has taken way too much time and effort already. I think releasing this to ubuntu/debian should generally be reconsidered as it seems to both slow down critical bug fixing as well as hindering progress (in combination with missing versioning of the file format).

For the docker image/ Dockerfile for the test case see this commit 1857a55 for copy pasting. I invite anybody to do this but won't be able (and willing) to work on it too soon.

[simonschmeisser]

@j-rivero I created a dockerfile here https://gist.github.com/simonschmeisser/9874bc68ac69f4529650bd9f31f72e53 could you have a quick look and if ok either me or you can add it to the SRU?

[gavanderhoorn]

Awesome. Thanks for this @simonschmeisser. Just tested and it clearly shows the problem and the solution.

# four (4) tests fail, parse* due to locale issues
RUN LANG=nl_NL.UTF-8 urdfdom_build/bin/urdf_unit_test ; exit 0

I'm not sure I like the implication here that we Dutch users are somehow the cause/related to this problem ;)

[simonschmeisser]

I chose Dutch cause I didn't want to be too self centered (ie German) and as a tribute to the amount (and quality) of your contributions! 😉

[nilsmelchert]

Yes thank you @simonschmeisser for putting this together!

How is this brought out to the convince the ROS maintainers now?

[j-rivero]

@j-rivero I created a dockerfile here https://gist.github.com/simonschmeisser/9874bc68ac69f4529650bd9f31f72e53 could you have a quick look and if ok either me or you can add it to the SRU?

Thanks. I'll do it in the next days. I'll keep you updated here.

[gavanderhoorn]

@j-rivero: you promised ;)

Did you get around this already?

[j-rivero]

Working on it. I've created patched packages cherry-picking the particular PRs/commits that I think that solve the issue. They are now hosted in this PPA. I've modified the Dockerfile to use urdfdom and urdfdom-headers directly from Ubuntu and PPA packages instead of using compiled source code (which can diverge in many aspect from binaries).

The problem is that the Dockerfile modified still present a problem with one of the tests after installing the patched packages: https://build.osrfoundation.org/job/_urdfdom_sru/11/console

[ RUN      ] URDF_UNIT_TEST.test_vector3_invalid_number
/urdfdom/urdf_parser/test/urdf_unit_test.cpp:141: Failure
Expected: vec.init("1.0 2.10.110 3.0") throws an exception of type urdf::ParseError.
  Actual: it throws nothing.

Not sure if it is a problem related to the patching of the packages (urdfdom patch, urdfdom-header patch) or with the setup of the test. Any help is welcome.

[simonschmeisser]

Thanks for having a look @j-rivero !

Shouldn't this be two patches against urdfdom-header? one fixing poses (ie xyz and rpy args in urdf and the remaining unit test) and one extending it for colors?
ros/urdfdom_headers@e7e0972 by @clalancette
and
ros/urdfdom_headers@f97cbce by me

As we have it now, this patch would fix colors and joint limits but not the geometric information (transformations). Glad we have this indirect check that is currently failing as there is no check for

urdf::Vector3 vec;
vec.init("0.1 0.2 0.3");
EXPECT_EQ(0.1, vec.x);
EXPECT_EQ(0.2, vec.y);
EXPECT_EQ(0.3, vec.z);

currently. Will open a PR.

also your version of the patch uses std::cer while the rest of the code uses consol_bridge, ie have a look here:

urdfdom/urdf_parser/src/link.cpp

Line 93 in 3ffc4ee

CONSOLE_BRIDGE_logError(std::string("Material [" + material.name + "] has malformed color rgba values: " + e.what()).c_str());

There is also a return false missing near the end of your patch (even though the return value does not seem to be checked by any of the calling functions as exceptions are intended to be used for error handling here)

(I repeat my preference for sticking with the code as released and doing a patch update to 1.0.3 in both packages)

[j-rivero]

thanks Simon for the help.

(I repeat my preference for sticking with the code as released and doing a patch update to 1.0.3 in both packages)

That would make things easier and safer in our side but my experience is that Ubuntu prefers small patches against the current versions of the packages present in the distribution instead of new versions, specially if we try to bump from 1.0.0 to 1.0.3. This is the reason why I'm cherry picking the relevant patches and inject them in the packages in Bionic.

I'll try with the patches you pointed out only against urdfdom_headers.

[j-rivero]

here we go https://bugs.launchpad.net/ubuntu/+source/urdfdom-headers/+bug/1817595/comments/6. Thanks everybody for the help. I've cc ubuntu-sponsors team so eventually someone with permissions to start the SRU process will appear.

[gavanderhoorn]

This gives hope:

description: | updated
-- | --
Changed in urdfdom (Ubuntu):
    status: | Confirmed → Fix Released
Changed in urdfdom-headers (Ubuntu):
    status: | Confirmed → Fix Released

[j-rivero]

This gives hope:

I'm afraid that 'Fix Released' means that we have fixed the bug in the upcoming version of Ubuntu, not affecting Bionic which remains in 'New' state. I'll ping some friends at Ubuntu, if that does not work we'll fix it in some other way.

Meantime, it would be great if you can use the packages in the PPA https://launchpad.net/~j-rivero/+archive/ubuntu/urdfdom-headers-sru2/+packages to detect any possible regression.

[nilsmelchert]

Thank you so much! I added your ppa and will report, if I experience any issues.

[jschleicher]

Finally the fix was released; please close here.