Skip serializing nested model with //pose/@relative_to attribute

[iche033]

Signed-off-by: Ian Chen [email protected]

🦟 Bug fix

Not a bug but suppresses misleading error messages.
Related issue: #1071

Summary

Details are described in #1071. This PR prevents serialization of a model component if it has the //pose/@relative_to attribute to avoid flooding the console with error messages when there are multiple models with nested models using this attribute. Since model deserialization with //pose/@relative_to never worked, this should not affect existing behavior. I believe the model serialization / deserialization functionality is mainly used by the component editor.

More info:

Problem is that during ign-gazebo serialization, a nested model is treated as an individual (top level) model (wrapped by <sdf> tag) and serialized to string. However, when ign-gazebo tries to deserialize it, sdformat sees it a top level model so the //pose/@relative_to attribute causes root.LoadSdfString to fail. It then produces these error messages:

Error:   Could not find the 'robot' element in the xml file
         at line 80 in /build/urdfdom-YMMa9X/urdfdom-1.0.0/urdf_parser/src/model.cpp
Error [parser_urdf.cc:3255] Unable to call parseURDF on robot model
Error [parser.cc:820] parse as old deprecated model file failed.
[GUI] [Err] [Model.hh:73] Unable to unserialize sdf::Model

I think sdformat is doing its job correctly when it fails to parse the sdf string. Maybe we need a way for ign-gazebo to tell sdformat to skip this check, or have a better serialization / deserialization method for [nested] models.

I also changed the verbosity level for serialization / serialization issue from error to warning.

See #1071 on how to reproduce the issue.

Checklist

• Signed all commits for DCO
• Added tests
• Updated documentation (as needed)
• Updated migration guide (as needed)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

[codecov]

Codecov Report

Merging #1454 (1ef1c55) into ign-gazebo6 (d57d9de) will not change coverage.
The diff coverage is n/a.

❗ Current head 1ef1c55 differs from pull request most recent head 716512d. Consider uploading reports for the commit 716512d to get more accurate results

@@             Coverage Diff              @@
##           ign-gazebo6    #1454   +/-   ##
============================================
  Coverage        33.58%   33.58%           
============================================
  Files               44       44           
  Lines             2260     2260           
============================================
  Hits               759      759           
  Misses            1501     1501           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d57d9de...716512d. Read the comment docs.

[scpeters]

once gazebosim/sdformat#820 is fully resolved, there should be an API that returns sdf::Errors objects instead of printing console messages, so it would be easier to ignore specific errors.

[scpeters]

once ignitionrobotics/sdformat#820 is fully resolved, there should be an API that returns sdf::Errors objects instead of printing console messages, so it would be easier to ignore specific errors.

this was just an FYI, I think this is fine to merge

[iche033]

once gazebosim/sdformat#820 is fully resolved, there should be an API that returns sdf::Errors objects instead of printing console messages, so it would be easier to ignore specific errors.

I added this comment to the code as a reminder. 91d77a5