Remove empty //inertial/pose/@relative_to during 1_7->1.8 conversion #720
Conversation
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
The <remove_empty/> tag in a .convert file will remove any empty attributes or elemements with no child elements or text values that match the specified name. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
This attribute was removed from the 1.8 spec, but causes warnings during forward conversion. This removes empty attributes to silence trivial warnings. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Codecov Report
@@ Coverage Diff @@
## sdf11 #720 +/- ##
=======================================
Coverage 88.08% 88.09%
=======================================
Files 73 73
Lines 11023 11030 +7
=======================================
+ Hits 9710 9717 +7
Misses 1313 1313
Continue to review full report at Codecov.
|
src/Converter.hh
Outdated
| @@ -100,9 +100,11 @@ namespace sdf | |||
|
|
|||
| /// \brief Remove an element. | |||
| /// \param[in] _elem The element that has the _removeElem child. | |||
| /// \param[in] _removeElem The element to remove. | |||
| /// \param[in] _removeElem The metadata about what to remove. | |||
| /// \param[in] _removeEmpty If true, only remove empty nodes. | |||
There was a problem hiding this comment.
Can you document how this works for attributes and elements? From what I can understand, for attributes, it removes just the attribute if it's empty. For elements, it removes the element if it has an empty value and has no children, but it can have attributes.
There was a problem hiding this comment.
would it be better to change the behavior for removing elements if they also have no attributes? We aren't yet using that functionality, so I couldn't decide. For now, I'll just document it as you suggest
There was a problem hiding this comment.
updated documentation and renamed the parameter to _removeOnlyEmpty in e6eb293
There was a problem hiding this comment.
Is it possible to chain these? By that I mean, would it be possible to remove the attributes first and then remove the element? If so, then I'd say let's make it so that it removes elements only if they have no attributes. This would make it possible to remove just the deprecated attributes first and remove the element if it doesn't contain anything else. For example:
<test_elem attr1="" attr2="some_val"/>
<test_elem attr1="" />If we deprecate the use of //test_elem and know that //test_elem[@attr1=""] is the default, we can do
<remove_empty attribute='attr1'/>
<remove_empty element='test_elem'/>and it will only delete the second //test_elem and we would be able to issue a warning about the first //test_elem
There was a problem hiding this comment.
sure, I'll rework the function logic accordingly
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
|
This pull request has been mentioned on Gazebo Community. There might be relevant details there: https://community.gazebosim.org/t/new-ignition-releases-2022-03-25-fortress-edifice-citadel/1343/1 |
🦟 Bug fix
Fixes warnings when loading models from 1.7 or earlier with
//inertial/pose/@relative_to, part of gazebosim/gz-sim#1056Summary
The
//inertial/pose/@relative_toattribute was removed from the 1.8 spec in 93f8c56, which causes warnings when loading a file from 1.7 or earlier that contains this attribute. This PR removes this attribute during conversion from 1.7 -> 1.8 if the attribute is empty, which should silence many of the warnings, while retaining those warnings that may require action from the user.I added an empty attribute to a
test/sdffile in 34d9587, which causes a warning that causesUNIT_ign_TESTto fail. The failure is fixed by new functionality in theConverterclass, which allows removal of empty attributes or elements.Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge