Use stringstream instead of stod to work around locale issues.

[clalancette]

The URDF schema (https://github.com/ros/urdfdom/blob/master/xsd/urdf.xsd#L47) defines doubles in URDF as xs:double. Looking at the XSD document at https://www.w3.org/TR/xmlschema-2/#double, it says that the mantissa for a double is represented as a "decimal" number. And looking at the XSD document at https://www.w3.org/TR/xmlschema-2/#decimal, 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.

fixes #41

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

[traversaro]

This patch switches the Vector3 class to using the stream operator, which seems to ignore locale.

If I understood correctly the C++ documentation, the stringstream default locale is not affected by calls to the C-style std::setlocale function (the one called by Qt, that is actually exposing the std::stod problem). However, it is affected by calls to std::locale::global, so a call to this global function in any part of the thread or the process (this is implementation defined) could still cause a failure in the URDF parser. Adding a call to the imbue method:

ss.imbue(std::locale::classic());

should avoid this problem (see https://github.com/robotology/idyntree/issues/288#issuecomment-299447873).

[clalancette]

Adding a call to the imbue method:

Thanks for that. Done in 678ab93

[gavanderhoorn]

@clalancette: minor, but none of the three commits seem to have been made using your gh registered email, so they are not attributed to your account.

[clalancette]

@clalancette: minor, but none of the three commits seem to have been made using your gh registered email, so they are not attributed to your account.

Ah, thanks. When we got the new "@openrobotics.org" domain, I updated my .gitconfig, but I had forgotten to verify with GitHub. Done now.

[clalancette]

I've fixed up all of the includes in caf2e8c. As far as testing goes, the only thing is that there is no test infrastructure in here. I could see about adding it (gtest or similar), but then this will grow into a much larger PR. I'm inclined to kick that to another PR, but I'll let you make the final decision.

[traversaro]

As far as testing goes, the only thing is that there is no test infrastructure in here. I could see about adding it (gtest or similar), but then this will grow into a much larger PR. I'm inclined to kick that to another PR, but I'll let you make the final decision.

I am totally not a fan of the situation, but historically tests of urdfdom_headers were added in the urdfdom repo, see https://github.com/ros/urdfdom/pull/67#issuecomment-163352725 .

[clalancette]

@traversaro Thanks a lot for the pointer. I'll go ahead and add some tests over there.

[clalancette]

@scpeters Gentle ping; this PR (and the related ros/urdfdom#109) are ready for review and merging, and I'd like to get them in before Melodic. Thanks!