sdformat: Should ensure filename / lineno is included in diagnostic mechanism

[EricCousineau-TRI]

Context: #16784 (comment)

FYI @azeey - you think this is something we can do in next ~month? (hopefully low-hanging fruit)

[jwnimmer-tri]

FYI in Drake's URDF parser, we have a sugar class to assist with reporting line numbers: detail_tinyxml2_diagnostic.h.

\CC @rpoyner-tri for any other thoughts

[rpoyner-tri]

Having not actually re-read all of the sdformat parser code, I suspect we can do something very similar, or even extend/rework some of the existing urdf solution.

[sammy-tri]

I've started something like this while working on #16785 https://github.com/sammy-tri/drake/blob/sdformat_warn_unsupported/multibody/parsing/detail_sdf_diagnostic.cc

[azeey]

@EricCousineau-TRI , yes, we can queue this up, unless @sammy-tri's work resolves this.

[sammy-tri]

I certainly don't mind working on it while I'm in there adding the other diagnostic stuff. Feel free to reassign to me.

[EricCousineau-TRI]

@marcoag said he has this en queue, assigning @azeey as proxy. Thanks!

@marcoag Can you comment on this issue so we can assign you?

[marcoag]

Sorry missed the ping.

This issue might benefit on the completion of gazebosim/sdformat#820.

Adding some examples on the current status of the SDF side so we can further discuss:

• Some error/warnings produced at the detail_sdf_parser.cc level do not include filename/lineno. I.e. this python code. It throws:

error: An axis must be specified for joint 'slider'

• It seems like SDF library syntax related errors go through PropagateErrors adding filename + lineno to the diagnostic. The example here would throw:

ERROR:drake:SDFormat Error [Param.cc:404] Unknown error. Unable to set value ['' ] for key[xyz]
Traceback (most recent call last):
  File "filename_lineno_diagnostic_test.py", line 34, in <module>
    sys.exit(main())
  File "filename_lineno_diagnostic_test.py", line 31, in main
    parser.AddModelFromString(notknownelement, 'sdf');
RuntimeError: <data-string>:10: error: At XML path /sdf/model[@name="robot2"]/joint[@name="slider"]/axis/xyz: Error reading element <xyz>

• Other non syntax related errors in SDF library like the empty string example would produce:

ERROR:drake:SDFormat Error [parser.cc:834] Error parsing XML from string: Error=XML_ERROR_EMPTY_DOCUMENT ErrorID=15 (0xf) Line number=0
Traceback (most recent call last):
  File "filename_lineno_diagnostic_test.py", line 19, in <module>
    sys.exit(main())
  File "filename_lineno_diagnostic_test.py", line 16, in main
    parser.AddModelFromString(notknownelement, 'sdf');
RuntimeError: error: Unable to read SDF string: 

• This wrong joint type will also not use filename nor lineno:

RuntimeError: error: Joint type of nontknowntype is invalid. Refer to the SDF documentation for a list of valid joint types

I'm a bit unsure of the scope of the issue is it to ensure that if a filename/lineno is reported then it's included in the diagnostic mechanism? is it to make sure SDFormat always reports filename/lineno? do we also want it from the errors at the detail_sdf_parser.cc level?

[EricCousineau-TRI]

FTR, just reflecting VC:
Yeah, it would be nice to root-cause why the two XML-valid but SDFormat-invalid wrong things fail (bad joint axis, bad joint type), and see if we can fix those.

For the "Unable to read SDF string", it would be nice to somehow make it clear that it is a clear string. Suggestion was to surround in quotes if string is only whitespace or empty; otherwise, just print string in following line.

[EricCousineau-TRI]

Resolved by #18462