Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xacro doesn't properly escape -- in urdf paths #344

Open
dkogan opened this issue May 30, 2024 · 2 comments
Open

xacro doesn't properly escape -- in urdf paths #344

dkogan opened this issue May 30, 2024 · 2 comments

Comments

@dkogan
Copy link

dkogan commented May 30, 2024

I have this python source:

#!/usr/bin/python3
import xacro
urdf_path = '/tmp/tst--1.xacro'
urdf      = xacro.process_file(urdf_path)
x         = urdf.toxml()

It does this:

$ ./tst.py
Traceback (most recent call last):
  File "colcon-workspace/./tst.py", line 10, in <module>
    x = urdf.toxml()
  File "/usr/lib/python3.10/xml/dom/minidom.py", line 47, in toxml
    return self.toprettyxml("", "", encoding, standalone)
  File "/usr/lib/python3.10/xml/dom/minidom.py", line 60, in toprettyxml
    self.writexml(writer, "", indent, newl, encoding, standalone)
  File "/usr/lib/python3.10/xml/dom/minidom.py", line 1828, in writexml
    node.writexml(writer, indent, addindent, newl)
  File "/usr/lib/python3.10/xml/dom/minidom.py", line 1211, in writexml
    raise ValueError("'--' is not allowed in a comment node")
ValueError: '--' is not allowed in a comment node

The main issue is that the comment xacro adds here:

" | This document was autogenerated by xacro from %-30s | " % input_file_name,

is not properly escaped. The -- in comments is apparently disallowed by the xml parser, and it doesn't like the -- in the urdf filename.

A secondary (and more important) issue is that ros tends to wrap everything in many layers of cruft, each layer swallowing each other's errors, making debugging extremely difficult. For me, this was part of a launch file doing stuff, and the only error message was the '--' is not allowed in a comment node without any indication of what component was complainint about what. Short of decruftifying ros, can we make each individual error message more descriptive? If the error message here was FILE:LINE: xacro reading file FILE and parsing into xml produced error: ValueError: .....

Thanks

@rhaschke
Copy link
Contributor

Thanks for pointing out this issue. Unfortunately, I didn't find a way to escape -- correctly. Do you have any suggestion?
Regarding the utility of the error message: the backtrace includes file and line number information and clearly hints you at xacro, doesn't it?

@dkogan
Copy link
Author

dkogan commented Jun 1, 2024

Hi. Thanks for responding. I have no suggestions; sorry. You can find '--' and change the comment to remove it manually. That's terrible, though so I don't know.

As for the error message, the backtrace is indeed wonderful, and if it was available, there would be no problem. But this is ros. You don't actually get the backtrace usually. Here's what I see:

$ ros2 launch thing.py

[INFO] [launch]: All log files can be found below /home/dima/.ros/log/whatever
[INFO] [launch]: Default logging verbosity is set to INFO
LOTS AND LOTS OF STUFF. NO IDEA WHAT IS OR IS NOT RELATED
when instantiating macro: AAA (.../BBB.xacro)
in file: .../CCC.xacro
[ERROR] [launch]: Caught exception in launch (see debug for traceback): '--' is not allowed in a comment node

Where's the backtrace? It's not in /home/dima/.ros/log/whatever either. Is this log the "debug for traceback", or is that someplace else that I don't know about? When I was debugging this I didn't realize that the notes about "BBB.xacro" or "CCC.xacro" were actually related to the failure (because the console output doesn't have any indication they are), but that probably wouldn't have made this any easier anyway. This isn't your bug clearly, but fixing ros isn't possible, and it would be nice if your one error message had more context in it to make debuggability possible despite ros's numerous failures.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants