Joint: rename parent/child *LinkName APIs

Migration.md

@@ -25,6 +25,20 @@ but with improved human-readability..
 - **sdf/Types.hh**: The `Inertia` class has been deprecated. Please use the
    `Inertial` class in the `gz-math` library.
 
+- **sdf/Joint.hh**:
+   + ***Deprecation:*** const std::string &ChildLinkName() const
+   + ***Replacement:*** const std::string &ChildName() const
+   + ***Deprecation:*** const std::string &ParentLinkName() const
+   + ***Replacement:*** const std::string &ParentName() const
+   + ***Deprecation:*** void SetChildLinkName(const std::string &)
+   + ***Replacement:*** void SetChildName(const std::string &)
+   + ***Deprecation:*** void SetParentLinkName(const std::string &)
+   + ***Replacement:*** void SetParentName(const std::string &)
+
+- **sdf/parser.hh**:
+   + ***Deprecation:*** bool checkJointParentChildLinkNames(const sdf::Root \*)
+   + ***Replacement:*** bool checkJointParentChildNames(const sdf::Root \*)
+
 ## libsdformat 11.x to 12.0
 
 An error is now emitted instead of a warning for a file containing more than

include/sdf/Joint.hh

@@ -113,21 +113,41 @@ namespace sdf
     /// \param[in] _jointType The type of joint.
     public: void SetType(const JointType _jointType);
 
+    /// \brief Get the name of this joint's parent frame.
+    /// \return The name of the parent frame.
+    public: const std::string &ParentName() const;
+
+    /// \brief Set the name of the parent frame.
+    /// \param[in] _name Name of the parent frame.
+    public: void SetParentName(const std::string &_name);
+
+    /// \brief Get the name of this joint's child frame.
+    /// \return The name of the child frame.
+    public: const std::string &ChildName() const;
+
+    /// \brief Set the name of the child frame.
+    /// \param[in] _name Name of the child frame.
+    public: void SetChildName(const std::string &_name);
+
     /// \brief Get the name of this joint's parent link.
     /// \return The name of the parent link.
-    public: const std::string &ParentLinkName() const;
+    /// \deprecated Use ParentName.
+    public: GZ_DEPRECATED(13) const std::string &ParentLinkName() const;
 
     /// \brief Set the name of the parent link.
     /// \param[in] _name Name of the parent link.
-    public: void SetParentLinkName(const std::string &_name);
+    /// \deprecated Use SetParentName.
+    public: GZ_DEPRECATED(13) void SetParentLinkName(const std::string &_name);
 
     /// \brief Get the name of this joint's child link.
     /// \return The name of the child link.
-    public: const std::string &ChildLinkName() const;
+    /// \deprecated Use ChildName.
+    public: GZ_DEPRECATED(13) const std::string &ChildLinkName() const;
 
     /// \brief Set the name of the child link.
     /// \param[in] _name Name of the child link.
-    public: void SetChildLinkName(const std::string &_name);
+    /// \deprecated Use SetChildName.
+    public: GZ_DEPRECATED(13) void SetChildLinkName(const std::string &_name);
 
     /// \brief Resolve the name of the child link from the
     /// FrameAttachedToGraph.

include/sdf/parser.hh

@@ -393,6 +393,17 @@ namespace sdf
   /// \return True if all models have joints with valid parent and child
   /// link names.
   SDFORMAT_VISIBLE
+  bool checkJointParentChildNames(const sdf::Root *_root);
+
+  /// \brief Check that all joints in contained models specify parent
+  /// and child link names that match the names of sibling links.
+  /// 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.
+  /// \return True if all models have joints with valid parent and child
+  /// link names.
+  /// \deprecated Use checkJointParentChildNames.
+  SDFORMAT_VISIBLE GZ_DEPRECATED(13)
   bool checkJointParentChildLinkNames(const sdf::Root *_root);
 
   /// \brief Check that all joints in contained models specify parent

python/src/sdf/pyJoint.cc

@@ -45,14 +45,14 @@ void defineJoint(pybind11::object module)
          "Get the joint type")
     .def("set_type", &sdf::Joint::SetType,
          "Set the joint type.")
-    .def("parent_link_name", &sdf::Joint::ParentLinkName,
-         "Get the name of this joint's parent link.")
-    .def("set_parent_link_name", &sdf::Joint::SetParentLinkName,
-         "Set the name of the parent link.")
-    .def("child_link_name", &sdf::Joint::ChildLinkName,
-         "Get the name of this joint's child link.")
-    .def("set_child_link_name", &sdf::Joint::SetChildLinkName,
-         "Set the name of the child link")
+    .def("parent_name", &sdf::Joint::ParentName,
+         "Get the name of this joint's parent frame.")
+    .def("set_parent_name", &sdf::Joint::SetParentName,
+         "Set the name of the parent frame.")
+    .def("child_name", &sdf::Joint::ChildName,
+         "Get the name of this joint's child frame.")
+    .def("set_child_name", &sdf::Joint::SetChildName,
+         "Set the name of the child frame.")
     .def("resolve_child_link",
          [](const sdf::Joint &self)
          {

python/test/pyJoint_TEST.py

@@ -26,8 +26,8 @@ def test_default_construction(self):
         joint = Joint()
         self.assertFalse(joint.name())
         self.assertEqual(sdf.JointType.INVALID, joint.type())
-        self.assertFalse(joint.parent_link_name())
-        self.assertFalse(joint.child_link_name())
+        self.assertFalse(joint.parent_name())
+        self.assertFalse(joint.child_name())
         self.assertEqual(Pose3d.ZERO, joint.raw_pose())
         self.assertFalse(joint.pose_relative_to())
 
@@ -55,11 +55,11 @@ def test_default_construction(self):
         joint.set_name("test_joint")
         self.assertEqual("test_joint", joint.name())
 
-        joint.set_parent_link_name("parent")
-        self.assertEqual("parent", joint.parent_link_name())
+        joint.set_parent_name("parent")
+        self.assertEqual("parent", joint.parent_name())
 
-        joint.set_child_link_name("child")
-        self.assertEqual("child", joint.child_link_name())
+        joint.set_child_name("child")
+        self.assertEqual("child", joint.child_name())
 
         errors, resolveChildLink = joint.resolve_child_link()
         self.assertEqual(1, len(errors))

src/FrameSemantics.cc

@@ -383,7 +383,7 @@ struct JointWrapper : public WrapperBase
       : WrapperBase{_joint.Name(), "Joint", FrameType::JOINT},
         rawPose(_joint.RawPose()),
         rawRelativeTo(_joint.PoseRelativeTo()),
-        childName(_joint.ChildLinkName()),
+        childName(_joint.ChildName()),
         relativeTo(rawRelativeTo.empty() ? childName : rawRelativeTo)
   {
   }

src/Joint.cc

@@ -37,11 +37,11 @@ class sdf::Joint::Implementation
   /// \brief Name of the joint.
   public: std::string name = "";
 
-  /// \brief Name of the parent link.
-  public: std::string parentLinkName = "";
+  /// \brief Name of the parent frame.
+  public: std::string parentName = "";
 
-  /// \brief Name of the child link.
-  public: std::string childLinkName = "";
+  /// \brief Name of the child frame.
+  public: std::string childName = "";
 
   /// \brief the joint type.
   public: JointType type = JointType::INVALID;
@@ -117,11 +117,11 @@ Errors Joint::Load(ElementPtr _sdf)
     _sdf->Get<std::string>("parent", "");
   if (parentPair.second)
   {
-    this->dataPtr->parentLinkName = parentPair.first;
-    if (!isValidFrameReference(this->dataPtr->parentLinkName))
+    this->dataPtr->parentName = parentPair.first;
+    if (!isValidFrameReference(this->dataPtr->parentName))
     {
       errors.push_back({ErrorCode::RESERVED_NAME,
-          "The supplied joint parent name [" + this->dataPtr->parentLinkName +
+          "The supplied joint parent name [" + this->dataPtr->parentName +
               "] is not valid."});
     }
   }
@@ -135,11 +135,11 @@ Errors Joint::Load(ElementPtr _sdf)
   std::pair<std::string, bool> childPair = _sdf->Get<std::string>("child", "");
   if (childPair.second)
   {
-    this->dataPtr->childLinkName = childPair.first;
-    if (!isValidFrameReference(this->dataPtr->childLinkName))
+    this->dataPtr->childName = childPair.first;
+    if (!isValidFrameReference(this->dataPtr->childName))
     {
       errors.push_back({ErrorCode::RESERVED_NAME,
-          "The supplied joint child name [" + this->dataPtr->childLinkName +
+          "The supplied joint child name [" + this->dataPtr->childName +
               "] is not valid."});
     }
   }
@@ -149,19 +149,19 @@ Errors Joint::Load(ElementPtr _sdf)
         "The child element is missing."});
   }
 
-  if (this->dataPtr->childLinkName == "world")
+  if (this->dataPtr->childName == "world")
   {
     errors.push_back({ErrorCode::JOINT_CHILD_LINK_INVALID,
         "Joint with name[" + this->dataPtr->name +
         "] specified invalid child link [world]."});
   }
 
-  if (this->dataPtr->childLinkName == this->dataPtr->parentLinkName)
+  if (this->dataPtr->childName == this->dataPtr->parentName)
   {
     errors.push_back({ErrorCode::JOINT_PARENT_SAME_AS_CHILD,
         "Joint with name[" + this->dataPtr->name +
         "] must specify different frame names for "
-        "parent and child, while [" + this->dataPtr->childLinkName +
+        "parent and child, while [" + this->dataPtr->childName +
         "] was specified for both."});
   }
 
@@ -250,28 +250,52 @@ void Joint::SetType(const JointType _jointType)
   this->dataPtr->type = _jointType;
 }
 
+/////////////////////////////////////////////////
+const std::string &Joint::ParentName() const
+{
+  return this->dataPtr->parentName;
+}
+
+/////////////////////////////////////////////////
+void Joint::SetParentName(const std::string &_name)
+{
+  this->dataPtr->parentName = _name;
+}
+
+/////////////////////////////////////////////////
+const std::string &Joint::ChildName() const
+{
+  return this->dataPtr->childName;
+}
+
+/////////////////////////////////////////////////
+void Joint::SetChildName(const std::string &_name)
+{
+  this->dataPtr->childName = _name;
+}
+
 /////////////////////////////////////////////////
 const std::string &Joint::ParentLinkName() const
 {
-  return this->dataPtr->parentLinkName;
+  return this->ParentName();
 }
 
 /////////////////////////////////////////////////
 void Joint::SetParentLinkName(const std::string &_name)
 {
-  this->dataPtr->parentLinkName = _name;
+  this->SetParentName(_name);
 }
 
 /////////////////////////////////////////////////
 const std::string &Joint::ChildLinkName() const
 {
-  return this->dataPtr->childLinkName;
+  return this->ChildName();
 }
 
 /////////////////////////////////////////////////
 void Joint::SetChildLinkName(const std::string &_name)
 {
-  this->dataPtr->childLinkName = _name;
+  this->SetChildName(_name);
 }
 
 /////////////////////////////////////////////////
@@ -352,7 +376,7 @@ Errors Joint::ResolveChildLink(std::string &_link) const
   }
 
   std::string link;
-  errors = resolveFrameAttachedToBody(link, graph, this->ChildLinkName());
+  errors = resolveFrameAttachedToBody(link, graph, this->ChildName());
   if (errors.empty())
   {
     _link = link;
@@ -367,7 +391,7 @@ Errors Joint::ResolveParentLink(std::string &_link) const
 
   // special case for world, return without resolving since it's not in a
   // model's FrameAttachedToGraph
-  if ("world" == this->ParentLinkName())
+  if ("world" == this->ParentName())
   {
     _link = "world";
     return errors;
@@ -382,7 +406,7 @@ Errors Joint::ResolveParentLink(std::string &_link) const
   }
 
   std::string link;
-  errors = resolveFrameAttachedToBody(link, graph, this->ParentLinkName());
+  errors = resolveFrameAttachedToBody(link, graph, this->ParentName());
   if (errors.empty())
   {
     _link = link;
@@ -397,7 +421,7 @@ sdf::SemanticPose Joint::SemanticPose() const
       this->dataPtr->name,
       this->dataPtr->pose,
       this->dataPtr->poseRelativeTo,
-      this->ChildLinkName(),
+      this->ChildName(),
       this->dataPtr->poseRelativeToGraph);
 }
 
@@ -516,8 +540,8 @@ sdf::ElementPtr Joint::ToElement() const
   }
 
   elem->GetAttribute("type")->Set<std::string>(jointType);
-  elem->GetElement("parent")->Set<std::string>(this->ParentLinkName());
-  elem->GetElement("child")->Set<std::string>(this->ChildLinkName());
+  elem->GetElement("parent")->Set<std::string>(this->ParentName());
+  elem->GetElement("child")->Set<std::string>(this->ChildName());
   for (unsigned int i = 0u; i < 2u; ++i)
   {
     const JointAxis *axis =  this->Axis(i);

src/Joint_TEST.cc

@@ -27,8 +27,14 @@ TEST(DOMJoint, Construction)
   sdf::Joint joint;
   EXPECT_TRUE(joint.Name().empty());
   EXPECT_EQ(sdf::JointType::INVALID, joint.Type());
+
+  EXPECT_TRUE(joint.ParentName().empty());
+  EXPECT_TRUE(joint.ChildName().empty());
+  GZ_UTILS_WARN_IGNORE__DEPRECATED_DECLARATION
   EXPECT_TRUE(joint.ParentLinkName().empty());
   EXPECT_TRUE(joint.ChildLinkName().empty());
+  GZ_UTILS_WARN_RESUME__DEPRECATED_DECLARATION
+
   EXPECT_EQ(gz::math::Pose3d::Zero, joint.RawPose());
   EXPECT_TRUE(joint.PoseRelativeTo().empty());
   EXPECT_EQ(nullptr, joint.Element());
@@ -59,11 +65,23 @@ TEST(DOMJoint, Construction)
   joint.SetName("test_joint");
   EXPECT_EQ("test_joint", joint.Name());
 
+  GZ_UTILS_WARN_IGNORE__DEPRECATED_DECLARATION
   joint.SetParentLinkName("parent");
   EXPECT_EQ("parent", joint.ParentLinkName());
+  GZ_UTILS_WARN_RESUME__DEPRECATED_DECLARATION
+  EXPECT_EQ("parent", joint.ParentName());
 
+  GZ_UTILS_WARN_IGNORE__DEPRECATED_DECLARATION
   joint.SetChildLinkName("child");
   EXPECT_EQ("child", joint.ChildLinkName());
+  GZ_UTILS_WARN_RESUME__DEPRECATED_DECLARATION
+  EXPECT_EQ("child", joint.ChildName());
+
+  joint.SetParentName("parent2");
+  EXPECT_EQ("parent2", joint.ParentName());
+
+  joint.SetChildName("child2");
+  EXPECT_EQ("child2", joint.ChildName());
 
   std::string body;
   EXPECT_FALSE(joint.ResolveChildLink(body).empty());
@@ -260,8 +278,8 @@ TEST(DOMJoint, ToElement)
   joint.SetRawPose({-1, -2, -3, 0, GZ_PI, 0});
   joint.SetPoseRelativeTo("link");
   joint.SetName("test_joint");
-  joint.SetParentLinkName("parent");
-  joint.SetChildLinkName("child");
+  joint.SetParentName("parent");
+  joint.SetChildName("child");
   joint.SetType(sdf::JointType::BALL);
   sdf::JointAxis axis;
   EXPECT_TRUE(axis.SetXyz(gz::math::Vector3d(1, 0, 0)).empty());
@@ -295,8 +313,8 @@ TEST(DOMJoint, ToElement)
             joint2.RawPose());
   EXPECT_EQ("link", joint2.PoseRelativeTo());
   EXPECT_EQ("test_joint", joint2.Name());
-  EXPECT_EQ("parent", joint2.ParentLinkName());
-  EXPECT_EQ("child", joint2.ChildLinkName());
+  EXPECT_EQ("parent", joint2.ParentName());
+  EXPECT_EQ("child", joint2.ChildName());
   EXPECT_EQ(sdf::JointType::BALL, joint2.Type());
   ASSERT_TRUE(nullptr != joint2.Axis(0));
   ASSERT_TRUE(nullptr != joint2.Axis(1));
@@ -308,12 +326,12 @@ TEST(DOMJoint, ToElement)
     EXPECT_NE(nullptr, joint.SensorByIndex(i));
 
   // make changes to DOM and verify ToElement produces updated values
-  joint2.SetParentLinkName("new_parent");
+  joint2.SetParentName("new_parent");
   sdf::ElementPtr joint2Elem = joint2.ToElement();
   EXPECT_NE(nullptr, joint2Elem);
   sdf::Joint joint3;
   joint3.Load(joint2Elem);
-  EXPECT_EQ("new_parent", joint3.ParentLinkName());
+  EXPECT_EQ("new_parent", joint3.ParentName());
 }
 
 /////////////////////////////////////////////////