Merge branch 'sdf11' into fix-canonical-link

gazebosim · Nov 8, 2021 · 9debfb6 · 9debfb6
2 parents 750d5dc + 8b12338
commit 9debfb6
Show file tree

Hide file tree

Showing 9 changed files with 156 additions and 72 deletions.
diff --git a/Changelog.md b/Changelog.md
@@ -290,6 +290,77 @@
 
 ## libsdformat 10.X
 
+### libsdformat 10.6.0 (2021-09-08)
+
+1. Parse URDF continuous joint effort/velocity limits
+    * [Pull request #684](https://github.com/ignitionrobotics/sdformat/pull/684)
+
+1. Add enable_orientation SDF element to imu
+    * [Pull request #651](https://github.com/ignitionrobotics/sdformat/pull/651)
+
+1. Add a codecheck make target
+    * [Pull request #682](https://github.com/ignitionrobotics/sdformat/pull/682)
+
+1. Refactor sdf::readXml
+    * [Pull request #681](https://github.com/ignitionrobotics/sdformat/pull/681)
+
+1. Upgrade cpplint and fix new errors
+    * [Pull request #680](https://github.com/ignitionrobotics/sdformat/pull/680)
+
+1. BUG: add missing plugin element to include
+    * [Pull request #675](https://github.com/ignitionrobotics/sdformat/pull/675)
+
+1. Added comment reminder to update functions
+    * [Pull request #677](https://github.com/ignitionrobotics/sdformat/pull/677)
+
+1. Adds enable_metrics flag to Sensor.
+    * [Pull request #665](https://github.com/ignitionrobotics/sdformat/pull/665)
+
+1. Add GPS / NavSat sensor to sdf9
+    * [Pull request #453](https://github.com/ignitionrobotics/sdformat/pull/453)
+
+1. Support parsing elements that are not part of the schema
+    * [Pull request #638](https://github.com/ignitionrobotics/sdformat/pull/638)
+
+1. Add lightmap to 1.7 spec and PBR material DOM
+    * [Pull request #429](https://github.com/ignitionrobotics/sdformat/pull/429)
+
+1. Fix urdf link extension tags
+    * [Pull request #628](https://github.com/ignitionrobotics/sdformat/pull/628)
+
+1. Updated material spec
+    * [Pull request #644](https://github.com/ignitionrobotics/sdformat/pull/644)
+
+1. Minor fix to Migration guide
+    * [Pull request #630](https://github.com/ignitionrobotics/sdformat/pull/630)
+
+1. Error: move << operator from .hh to .cc file
+    * [Pull request #625](https://github.com/ignitionrobotics/sdformat/pull/625)
+
+1. Update build system to allow overriding CXX flags and using clang on Linux
+    * [Pull request #621](https://github.com/ignitionrobotics/sdformat/pull/621)
+
+1. Add Element::FindElement as an alternative to Element::GetElement
+    * [Pull request #620](https://github.com/ignitionrobotics/sdformat/pull/620)
+
+1. Add ValidateGraphs methods to Model/World (sdf9)
+    * [Pull request #602](https://github.com/ignitionrobotics/sdformat/pull/602)
+
+1. Fix ABI break
+    * [Pull request #605](https://github.com/ignitionrobotics/sdformat/pull/605)
+
+1. Parameter passing prototype
+    * [Pull request #413](https://github.com/ignitionrobotics/sdformat/pull/413)
+
+1. Port particle scatter ratio param to sdf 1.6
+    * [Pull request #595](https://github.com/ignitionrobotics/sdformat/pull/595)
+
+1. Making PrintValues() and ToString() able to not print default elements
+    * [Pull request #575](https://github.com/ignitionrobotics/sdformat/pull/575)
+
+1. Add API for determining if an element was set by the user
+    * [Pull request #542](https://github.com/ignitionrobotics/sdformat/pull/542)
+
 ### libsdformat 10.5.0 (2021-05-17)
 
 1. Add scatter ratio parameter to Particle Emitter DOM.
@@ -433,7 +504,7 @@
 1. Adds `enable_metrics` flag to Sensor.
     * [Pull request #665](https://github.com/ignitionrobotics/sdformat/pull/665)
 
-1. Add GPS sensor DOM to sdf9
+1. Add GPS / NavSat sensor DOM to sdf9
     * [Pull request #453](https://github.com/ignitionrobotics/sdformat/pull/453)
 
 1. Support parsing elements that are not part of the schema

diff --git a/include/sdf/parser.hh b/include/sdf/parser.hh
@@ -388,6 +388,16 @@ namespace sdf
   SDFORMAT_VISIBLE
   bool checkJointParentChildLinkNames(const sdf::Root *_root);
 
+  /// \brief Check that all joints in contained models specify parent
+  /// and child names that match the names of sibling links, joints,
+  /// models, or frames.
+  /// This checks recursively and should check the files exhaustively
+  /// rather than terminating early when the first error is found.
+  /// \param[in] _root SDF Root object to check recursively.
+  /// \param[out] _errors Detected errors will be appended to this variable.
+  SDFORMAT_VISIBLE
+  void checkJointParentChildNames(const sdf::Root *_root, Errors &_errors);
+
   /// \brief For the world and each model, check that the attached_to graphs
   /// build without errors and have no cycles.
   /// Confirm that following directed edges from each vertex in the graph

diff --git a/src/Joint.cc b/src/Joint.cc
@@ -159,7 +159,7 @@ Errors Joint::Load(ElementPtr _sdf)
   {
     errors.push_back({ErrorCode::JOINT_PARENT_SAME_AS_CHILD,
         "Joint with name[" + this->dataPtr->name +
-        "] must specify different link names for "
+        "] must specify different frame names for "
         "parent and child, while [" + this->dataPtr->childLinkName +
         "] was specified for both."});
   }

diff --git a/src/Root.cc b/src/Root.cc
@@ -325,6 +325,10 @@ Errors Root::Load(SDFPtr _sdf, const ParserConfig &_config)
     }
   }
 
+  // Check that Joint parent and child names resolve to valid and
+  // different frames.
+  checkJointParentChildNames(this, errors);
+
   return errors;
 }
 

diff --git a/src/ign_TEST.cc b/src/ign_TEST.cc
@@ -257,7 +257,7 @@ TEST(check, IGN_UTILS_TEST_DISABLED_ON_WIN32(SDF))
     std::string output =
       custom_exec_str(IgnCommand() + " sdf -k " + path + SdfVersion());
     EXPECT_NE(output.find("Joint with name[joint] must "
-                          "specify different link names for parent and child, "
+                          "specify different frame names for parent and child, "
                           "while [link] was specified for both."),
               std::string::npos) << output;
   }
@@ -322,6 +322,17 @@ TEST(check, IGN_UTILS_TEST_DISABLED_ON_WIN32(SDF))
     EXPECT_EQ("Valid.\n", output) << output;
   }
 
+  // Check an SDF file with the infinite values for joint axis limits.
+  // This is a valid file.
+  {
+    std::string path = pathBase +"/joint_axis_infinite_limits.sdf";
+
+    // Check joint_axis_infinite_limits.sdf
+    std::string output =
+      custom_exec_str(IgnCommand() + " sdf -k " + path + SdfVersion());
+    EXPECT_EQ("Valid.\n", output) << output;
+  }
+
   // Check an SDF file with the second link specified as the canonical link.
   // This is a valid file.
   {

diff --git a/src/parser.cc b/src/parser.cc
@@ -2232,12 +2232,24 @@ bool checkPoseRelativeToGraph(const sdf::Root *_root)
 //////////////////////////////////////////////////
 bool checkJointParentChildLinkNames(const sdf::Root *_root)
 {
-  bool result = true;
+  Errors errors;
+  checkJointParentChildNames(_root, errors);
+  if (!errors.empty())
+  {
+    std::cerr << "Error when attempting to resolve child link name:"
+              << std::endl
+              << errors;
+    return false;
+  }
+  return true;
+}
 
+//////////////////////////////////////////////////
+void checkJointParentChildNames(const sdf::Root *_root, Errors &_errors)
+{
   auto checkModelJointParentChildNames = [](
-      const sdf::Model *_model) -> bool
+      const sdf::Model *_model, Errors &errors) -> void
   {
-    bool modelResult = true;
     for (uint64_t j = 0; j < _model->JointCount(); ++j)
     {
       auto joint = _model->JointByIndex(j);
@@ -2247,99 +2259,73 @@ bool checkJointParentChildLinkNames(const sdf::Root *_root)
           !_model->JointNameExists(parentName) &&
           !_model->FrameNameExists(parentName))
       {
-        std::cerr << "Error: parent frame with name[" << parentName
-                  << "] specified by joint with name[" << joint->Name()
-                  << "] not found in model with name[" << _model->Name()
-                  << "]."
-                  << std::endl;
-        modelResult = false;
+        errors.push_back({ErrorCode::JOINT_PARENT_LINK_INVALID,
+          "parent frame with name[" + parentName +
+          "] specified by joint with name[" + joint->Name() +
+          "] not found in model with name[" + _model->Name() + "]."});
       }
 
       const std::string &childName = joint->ChildLinkName();
       if (childName == "world")
       {
-        std::cerr << "Error: invalid child name[world"
-                  << "] specified by joint with name[" << joint->Name()
-                  << "] in model with name[" << _model->Name()
-                  << "]."
-                  << std::endl;
-        modelResult = false;
+        errors.push_back({ErrorCode::JOINT_CHILD_LINK_INVALID,
+          "invalid child name[world] specified by joint with name[" +
+          joint->Name() + "] in model with name[" + _model->Name() + "]."});
       }
 
       if (!_model->LinkNameExists(childName) &&
           !_model->JointNameExists(childName) &&
           !_model->FrameNameExists(childName) &&
           !_model->ModelNameExists(childName))
       {
-        std::cerr << "Error: child frame with name[" << childName
-                  << "] specified by joint with name[" << joint->Name()
-                  << "] not found in model with name[" << _model->Name()
-                  << "]."
-                  << std::endl;
-        modelResult = false;
+        errors.push_back({ErrorCode::JOINT_CHILD_LINK_INVALID,
+          "child frame with name[" + childName +
+          "] specified by joint with name[" + joint->Name() +
+          "] not found in model with name[" + _model->Name() + "]."});
       }
 
       if (childName == joint->Name())
       {
-        std::cerr << "Error: joint with name[" << joint->Name()
-                  << "] in model with name[" << _model->Name()
-                  << "] must not specify its own name as the child frame."
-                  << std::endl;
-        modelResult = false;
+        errors.push_back({ErrorCode::JOINT_CHILD_LINK_INVALID,
+          "joint with name[" + joint->Name() +
+          "] in model with name[" + _model->Name() +
+          "] must not specify its own name as the child frame."});
       }
 
       if (parentName == joint->Name())
       {
-        std::cerr << "Error: joint with name[" << joint->Name()
-                  << "] in model with name[" << _model->Name()
-                  << "] must not specify its own name as the parent frame."
-                  << std::endl;
-        modelResult = false;
+        errors.push_back({ErrorCode::JOINT_PARENT_LINK_INVALID,
+          "joint with name[" + joint->Name() +
+          "] in model with name[" + _model->Name() +
+          "] must not specify its own name as the parent frame."});
       }
 
       // Check that parent and child frames resolve to different links
       std::string resolvedChildName;
       std::string resolvedParentName;
-      auto errors = joint->ResolveChildLink(resolvedChildName);
-      if (!errors.empty())
-      {
-        std::cerr << "Error when attempting to resolve child link name:"
-                  << std::endl;
-        for (auto error : errors)
-        {
-          std::cerr << error.Message() << std::endl;
-        }
-        modelResult = false;
-      }
-      errors = joint->ResolveParentLink(resolvedParentName);
-      if (!errors.empty())
-      {
-        std::cerr << "Error when attempting to resolve parent link name:"
-                  << std::endl;
-        for (auto error : errors)
-        {
-          std::cerr << error.Message() << std::endl;
-        }
-        modelResult = false;
-      }
+
+      auto resolveErrors = joint->ResolveChildLink(resolvedChildName);
+      errors.insert(errors.end(), resolveErrors.begin(), resolveErrors.end());
+
+      resolveErrors = joint->ResolveParentLink(resolvedParentName);
+      errors.insert(errors.end(), resolveErrors.begin(), resolveErrors.end());
+
       if (resolvedChildName == resolvedParentName)
       {
-        std::cerr << "Error: joint with name[" << joint->Name()
-                  << "] in model with name[" << _model->Name()
-                  << "] specified parent frame [" << parentName
-                  << "] and child frame [" << childName
-                  << "] that both resolve to [" << resolvedChildName
-                  << "], but they should resolve to different values."
-                  << std::endl;
-        modelResult = false;
+        errors.push_back({ErrorCode::JOINT_PARENT_SAME_AS_CHILD,
+          "joint with name[" + joint->Name() +
+          "] in model with name[" + _model->Name() +
+          "] specified parent frame [" + parentName +
+          "] and child frame [" + childName +
+          "] that both resolve to [" + resolvedChildName +
+          "], but they should resolve to different values."});
       }
     }
-    return modelResult;
   };
 
   if (_root->Model())
   {
-    result = checkModelJointParentChildNames(_root->Model()) && result;
+    checkModelJointParentChildNames(_root->Model(), _errors);
   }
 
   for (uint64_t w = 0; w < _root->WorldCount(); ++w)
@@ -2348,11 +2334,9 @@ bool checkJointParentChildLinkNames(const sdf::Root *_root)
     for (uint64_t m = 0; m < world->ModelCount(); ++m)
     {
       auto model = world->ModelByIndex(m);
-      result = checkModelJointParentChildNames(model) && result;
+      checkModelJointParentChildNames(model, _errors);
     }
   }
-
-  return result;
 }
 
 //////////////////////////////////////////////////

diff --git a/test/integration/joint_dom.cc b/test/integration/joint_dom.cc
@@ -218,7 +218,7 @@ TEST(DOMJoint, LoadInvalidJointChildWorld)
   auto errors = root.Load(testFile);
   for (auto e : errors)
     std::cout << e << std::endl;
-  ASSERT_EQ(7u, errors.size());
+  ASSERT_EQ(10u, errors.size());
   EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::JOINT_CHILD_LINK_INVALID);
   EXPECT_NE(std::string::npos,
     errors[0].Message().find(
@@ -549,7 +549,7 @@ TEST(DOMJoint, LoadInvalidChild)
   for (auto e : errors)
     std::cout << e << std::endl;
   EXPECT_FALSE(errors.empty());
-  EXPECT_EQ(6u, errors.size());
+  EXPECT_EQ(8u, errors.size());
   EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::JOINT_CHILD_LINK_INVALID);
   EXPECT_NE(std::string::npos,
     errors[0].Message().find(
@@ -564,6 +564,8 @@ TEST(DOMJoint, LoadInvalidChild)
   // errors[3]
   // errors[4]
   // errors[5]
+  // errors[6]
+  // errors[7]
 }
 
 /////////////////////////////////////////////////
@@ -584,7 +586,7 @@ TEST(DOMJoint, LoadLinkJointSameName17Invalid)
   for (auto e : errors)
     std::cout << e << std::endl;
   EXPECT_FALSE(errors.empty());
-  EXPECT_EQ(7u, errors.size());
+  EXPECT_EQ(9u, errors.size());
   EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::DUPLICATE_NAME);
   EXPECT_NE(std::string::npos,
     errors[0].Message().find(

diff --git a/test/sdf/joint_axis_infinite_limits.sdf b/test/sdf/joint_axis_infinite_limits.sdf
@@ -6,6 +6,7 @@
     <link name="link3"/>
     <link name="link4"/>
     <link name="link5"/>
+    <link name="link6"/>
 
     <joint name="default_joint_limits" type="revolute">
       <child>link1</child>

diff --git a/test/sdf/joint_axis_xyz_normalization.sdf b/test/sdf/joint_axis_xyz_normalization.sdf
@@ -6,6 +6,7 @@
     <link name="link3"/>
     <link name="link4"/>
     <link name="link5"/>
+    <link name="link6"/>
 
     <joint name="joint1" type="revolute">
       <child>link1</child>