Converted SDF doesn't parse successfully.

Environment

OS Version: Debian bullseye
Source or binary build? Source: 17a4cf853a097b9adfa508876250044f367a4090

Description

Expected behavior: Either
1. Parses the SDF correctly (this SDF parses in libsdformat version 6.2).
2. Produces an error saying that it could not be converted properly.
Actual behavior: Produces an SDF that cannot be parsed correctly.

Note: using ::sdf::readString directly seems to work fine, but using the available API, this doesn't give access to an ::sdf::Root object.

Steps to reproduce

Run the following program:

#include <string>
#include <sstream>
#include <iostream>

#include "sdf/sdf.hh"

int main(int argc, char** argv) {
  const std::string sdf_text = R"(
    <?xml version="1.0"?>
    <sdf version="1.6">
      <model name="box">
        <pose>1 2 3 0 0 0</pose>
      </model>
    </sdf>)";

  ::sdf::SDFPtr sdf_ptr(new ::sdf::SDF);
  ::sdf::init(sdf_ptr);
  if (!::sdf::convertString(sdf_text, SDF_PROTOCOL_VERSION, sdf_ptr)) {
    std::cerr << "Error converting SDF";
    return 1;
  }

  std::cout << sdf_ptr->ToString();

  ::sdf::Root sdf_root;
  auto errors = sdf_root.Load(sdf_ptr);
  if (!errors.empty()) {
    std::stringstream status_string;
    for (const auto& error : errors) {
      status_string << error << "\n";
    }
    std::cerr << status_string.str();
    return 1;
  }

  std::cout << "Parsed sdf correctly!";
  return 0;
}

Output

<?xml version='1.0'?>
<sdf version='1.8'>
  <model name='box'>
    <pose>1 2 3 0 -0 0</pose>
  </model>
</sdf>
Error Code 16 Msg: A model must have at least one link.
Error Code 16 Msg: A model must have at least one link.
Error Code 24 Msg: FrameAttachedToGraph error: scope context name[] does not match __model__ or world.

The DOM classes sdf::Root, sdf::World, sdf::Model, etc add more semantic structure to the parsed SDFormat file than what is available through the sdf::Element API. An example of this structure is pose frame semantics, which was added in SDFormat 1.7 (http://sdformat.org/tutorials?tut=pose_frame_semantics&cat=specification&). This feature adds extra constraints, such as valid relative_to references and the requirement that non-static models have at least one link. However, the conversion that happens via sdf::convertString is more geared toward converting syntax changes between versions of the SDFormat spec.

It would be nice for the converter to know that the model is semantically invalid in SDFormat 1.8 and report that it can't be converted properly as you suggested, but I don't think that functionality should go in sdf::convertString. Perhaps we can update ign sdf -p to validate the converted string and report any errors. By the way, ign sdf -k does that already, but it doesn't print out the converted SDFormat.

gazebosim / sdformat