gazebosim / sdformat

Simulation Description Format (SDFormat) parser and description files.
http://sdformat.org
Apache License 2.0
155 stars 90 forks source link

Add schema file name accessor to each DOM type C++ class #1376

Open qingyouzhao opened 4 months ago

qingyouzhao commented 4 months ago

Desired behavior

Getting the schema file for each SDF DOM type element should be possible. This should make initializing an element for loading standalone DOM types easier.

Without this format. I am doing something like this in my code to initialize a DOM type element so that I can get type checking

// Schema file type
template <typename T>
struct SdfTypeToSchemaFile {
  static const std::string kSchemaFile;
};

template <>
const std::string SdfTypeToSchemaFile<sdf::Surface>::kSchemaFile = "surface.sdf";
// Other definitions with template specialization.

template <typename T>
sdf::ElementPtr init_element_for_type(){
  auto element = std::make_shared<::sdf::Element>();
  sdf::initFile(SdfTypeToSchemaFile<T>::kSchemaFile, element);
  return element;
}

Ideally, I hope to do something like this for simplicity.

template <typename T>
sdf::ElementPtr init_element_for_type(){
  auto element = std::make_shared<::sdf::Element>();
  sdf::initFile(T::SchemaFile(), element);
  return element;
}

Alternatives considered

Implementation suggestion

Implement a static function for each DOM type. For example for Surface:

// sdformat/include/sdf/Surface.hh
class Surface{
  // Omitted details
  public: static const std::string& SchemaFile();
};

// sdformat/src/Surface.cc

static const std::string& Surface::SchemaFile(){
  static const std::string* kSchemaFile = new std::string("surface.sdf");
  return *kSchemaFile;
}

Additional context

Currently the schema file is hard coded in it's T::ToElement() for example: https://github.com/gazebosim/sdformat/blob/c4bfe35bdaca8e667089bf9adf99b7930192ccaf/src/Surface.cc#L412

aagrawal05 commented 3 months ago

Hi, could you please assign this to me? Just to confirm, what would be the list of all DOM elements which would need this feature? Also would it be advisable to add a test for this in the default construction or is this unnecessary?

Thanks.

qingyouzhao commented 3 months ago

Hi, could you please assign this to me? Just to confirm, what would be the list of all DOM elements which would need this feature? Also would it be advisable to add a test for this in the default construction or is this unnecessary?

Thanks.

The schemas are in sdformat/sdf/1.11 for the current version. The DOM elements corresponding are mostly located in sdformat/include. The names roughly matches each other regardless of the casing(snake_case for sdf and CamelCase for class).

aagrawal05 commented 3 months ago

Hi @qingyouzhao, I have implemented the required changes on my personal fork of the repository aagrawal05/sdformat on the schema_accessor branch.

Brief summary of the work I've done:

  1. I've created a script format_sdf.py which reads from the /sdf/1.11 folder and generates/updates the source for all the relevant classes in the src/ and include/ directories as per the suggested implementation.
  2. There were DOM elements navsat.sdf and forcetorque.sdf for which the script does not work and I had to manually edit the script because their naming convention did not match the camel-case conversion (they would need to benav_sat.sdf and force_torque.sdf) This may be undesirable from a code style perspective but may be a breaking change—this is a minor point but I just wanted to point it out.
  3. The code builds successfully, I have not thoroughly tested it but it does seem to work.
  4. I can add tests for the SchemaFile in all the DOM element tests but I thought it might be overkill because the names are hardcoded in the method anyways. I can add it easily with the script if needed.

I can delete the format_sdf.py from the commit as well, if it's no longer useful. Or I could place it in the sdf 1.11 folder similar to the embedSdf.py—not sure if it's relevant.

Could you please take a look and let me know if anything needs to be changed—I'd be happy to make any edits and open the pull request.