moveit / moveit_setup_assistant

THIS REPO HAS MOVED TO https://github.com/ros-planning/moveit
8 stars 20 forks source link

Setup Assistant stores wrong formated doubles for group_states in the .srdf #51

Closed e1000i closed 8 years ago

e1000i commented 11 years ago

When defining a joint state for a group_state in decimal notation setup assistant stores it as:

<group_state name="Deposit" group="Arm">     
    <joint name="hand" value="0" />
    <joint name="prisma1" value="0,4449"/>
    <joint name="xaxis" value="0" />
    <joint name="yaxis" value="2,15" />
</group_state>

But seems the parser throws an error when loading the robot model, notice the ','. This particular pose appears available, but not work, and not give any error message when trying to define it.

[ERROR] [1374223441.626850398]: Unable to parse joint value '0,4449'
[ERROR] [1374223441.626921239]: Unable to parse joint value '2,15'

So I had to manually change on this joints to all ',' for '.' :

<group_state name="Deposit" group="Arm">     
    <joint name="hand" value="0" />
    <joint name="prisma1" value="0.4449"/>
    <joint name="xaxis" value="0" />
    <joint name="yaxis" value="2.15" />
</group_state> 

and with that, no error is prompt when the robot model is loaded.

So probably just need to check that the values are saved with the correct '.' always in any situation. Even in the GUI always are points. So not understand why writes ','.

I'm using groovy desktop from ubuntu 12.04 repository with a custom robot. We thought it could be our locale system settings that mess when the assistant writes the file. But I changed to UK local and still saving comas. No one else noticed?

isucan commented 11 years ago

I do not know what your locale is, but this is a locale issue. The unfortunate bit i that seems to be used in your version of TinyXML, but not by boost::lexical_cast, which is what the srdfdom parser calls. Essentially this comes down to an inconsistency in locale settings between boost and tinyxml.

hersh commented 10 years ago

This bug has showed up again here: https://github.com/ros-planning/moveit_commander/issues/11 (incorrectly).

@isucan why did you close this? Surely this can be fixed in moveit_setup_assistant somehow.

isucan commented 10 years ago

I agree that locale is a problem, as mentioned on the mailing list. However, I think the fix is not to support locale in the SRDF. We should always use POSIX, so we can transfer SRDF files between countries :). I think the bug is not fixed though, I do not remember why I closed it. To me it seems like TinyXML should allow overriding the locale when saving, and we should use defaults.

skohlbr commented 9 years ago

Just wanted to note I also encountered this bug. Don't really have time to investigate in-depth myself, but if there are any more ideas for fix I could try them.

simonschmeisser commented 9 years ago

How about using boost::lexical_cast and then writing the attribute as a string?

carlosjoserg commented 8 years ago

I'm also experimenting this issue, I also believe it is a locale issue. There was a similar issue in gazebo, more critical though there, and the temporary solution was to do:

export LC_NUMERIC=C

This worked for me in the setup assistant context. You can add that to your bash or something.

I wished I could help you with a PR... the best I can do is to point you to Rivero's solution, which might help you solve it here as well.

rhaschke commented 8 years ago

Fixed with #123.