From 0277a3660f61e1c358f91544f5374d61152f0483 Mon Sep 17 00:00:00 2001 From: Colin MacKenzie Date: Fri, 2 Oct 2020 00:48:20 -0400 Subject: [PATCH 01/14] merged code from ddengster that loads joint and actuator sections. reverted TransmissionInfo name/type fields to those in current ros2_controls master. Enabled and fixed up unit tests. Added joint, actuator type and mechanicalReduction to transmission test URDF. --- transmission_interface/CMakeLists.txt | 17 +- .../transmission_info.hpp | 38 ++- .../transmission_parser.hpp | 56 +++- .../src/transmission_parser.cpp | 271 +++++++++++++++--- .../test/test_transmission_parser.cpp | 62 +++- 5 files changed, 374 insertions(+), 70 deletions(-) diff --git a/transmission_interface/CMakeLists.txt b/transmission_interface/CMakeLists.txt index 6d6add84f3..16193f66c1 100644 --- a/transmission_interface/CMakeLists.txt +++ b/transmission_interface/CMakeLists.txt @@ -12,23 +12,22 @@ endif() # find dependencies find_package(ament_cmake REQUIRED) -find_package(hardware_interface REQUIRED) +#find_package(hardware_interface REQUIRED) # todo: we should be using this for hardware interface constants find_package(tinyxml2_vendor REQUIRED) -find_package(TinyXML2 REQUIRED) +find_package(rclcpp REQUIRED) + +# todo: we should be using the URDF project to load our tranmissions from URDF +#find_package(urdf REQUIRED) add_library(transmission_parser SHARED src/transmission_parser.cpp) target_include_directories(transmission_parser PUBLIC include) -ament_target_dependencies(transmission_parser - hardware_interface - TinyXML2 -) target_compile_definitions(transmission_parser PRIVATE "TRANSMISSION_INTERFACE_BUILDING_DLL") +ament_target_dependencies(transmission_parser rclcpp tinyxml2_vendor) install( DIRECTORY include/ DESTINATION include ) - install( TARGETS transmission_parser RUNTIME DESTINATION bin @@ -45,7 +44,9 @@ if(BUILD_TESTING) test_transmission_parser test/test_transmission_parser.cpp ) - target_link_libraries(test_transmission_parser transmission_parser) + target_include_directories(test_transmission_parser PUBLIC include) + target_link_libraries(test_transmission_parser transmission_parser ${TinyXML_LIBRARY}) + ament_target_dependencies(test_transmission_parser rclcpp tinyxml2_vendor) endif() ament_export_include_directories( diff --git a/transmission_interface/include/transmission_interface/transmission_info.hpp b/transmission_interface/include/transmission_interface/transmission_info.hpp index 60c4e0c2c9..5e1b606f9e 100644 --- a/transmission_interface/include/transmission_interface/transmission_info.hpp +++ b/transmission_interface/include/transmission_interface/transmission_info.hpp @@ -15,18 +15,50 @@ #ifndef TRANSMISSION_INTERFACE__TRANSMISSION_INFO_HPP_ #define TRANSMISSION_INTERFACE__TRANSMISSION_INFO_HPP_ +// C++ standard +#include #include -#include "hardware_interface/control_type.hpp" +// TinyXML +#include namespace transmission_interface { +/** + * \brief Contains semantic info about a given joint loaded from XML (URDF) + */ +struct JointInfo +{ + std::string name; + std::vector hardware_interfaces; + std::string role_; + std::string xml_element_; +}; + +/** + * \brief Contains semantic info about a given actuator loaded from XML (URDF) + */ +struct ActuatorInfo +{ + std::string name; + std::vector hardware_interfaces; + double mechanical_reduction; + std::string xml_element_; +}; + +/** + * \brief Contains semantic info about a given transmission loaded from XML (URDF) + */ struct TransmissionInfo { std::string joint_name; std::string joint_control_type; + std::vector joints; + std::vector actuators; }; -} // namespace transmission_interface -#endif // TRANSMISSION_INTERFACE__TRANSMISSION_INFO_HPP_ +} // namespace + + +#endif \ No newline at end of file diff --git a/transmission_interface/include/transmission_interface/transmission_parser.hpp b/transmission_interface/include/transmission_interface/transmission_parser.hpp index 98bd200f07..18e6bb4024 100644 --- a/transmission_interface/include/transmission_interface/transmission_parser.hpp +++ b/transmission_interface/include/transmission_interface/transmission_parser.hpp @@ -12,24 +12,64 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef TRANSMISSION_INTERFACE__TRANSMISSION_PARSER_HPP_ -#define TRANSMISSION_INTERFACE__TRANSMISSION_PARSER_HPP_ +/** + * \file + * \brief Parses \ elements into corresponding structs from XML (URDF). + * \author Dave Coleman + */ + +#pragma once + -#include -#include +#include +#include -#include "transmission_interface/transmission_info.hpp" -#include "transmission_interface/visibility_control.h" +// XML +#include namespace transmission_interface { + +/** \brief Parse all transmissions specified in a URDF. */ +class TransmissionParser +{ +public: + + /** + * \brief Parses the transmission elements of a URDF + * \param[in] urdf_string XML string of a valid URDF file that contains \ elements + * \param[out] transmissions vector of loaded transmission meta data + * \return true if parsing was successful + */ + static bool parse(const std::string& urdf_string, std::vector& transmissions); + +protected: + /** + * \brief Parses the joint elements within transmission elements of a URDF + * \param[in] trans_it pointer to the current XML element being parsed + * \param[out] joints resulting list of joints in the transmission + * \return true if successful + */ + static bool parseJoints(TiXmlElement *trans_it, std::vector& joints); + + /** + * \brief Parses the actuator elements within transmission elements of a URDF + * \param[in] trans_it pointer to the current XML element being parsed + * \param[out] actuators resulting list of actuators in the transmission + * \return true if successful + */ + static bool parseActuators(TiXmlElement *trans_it, std::vector& actuators); + +}; // class + + /** - * \brief Parse transmission information from a URDF + * \brief Parse transmission information from a URDF. * \param urdf A string containing the URDF xml * \return parsed transmission information * \throws std::runtime_error on malformed or empty xml */ TRANSMISSION_INTERFACE_PUBLIC std::vector parse_transmissions_from_urdf(const std::string & urdf); + } // namespace transmission_interface -#endif // TRANSMISSION_INTERFACE__TRANSMISSION_PARSER_HPP_ diff --git a/transmission_interface/src/transmission_parser.cpp b/transmission_interface/src/transmission_parser.cpp index 4d1076ae8b..79f88014ea 100644 --- a/transmission_interface/src/transmission_parser.cpp +++ b/transmission_interface/src/transmission_parser.cpp @@ -12,68 +12,257 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "transmission_interface/transmission_parser.hpp" -#include "transmission_interface/transmission_info.hpp" +#include +#include -#include -#include -#include -#include - -namespace -{ -constexpr const auto kTransmissionTag = "transmission"; -constexpr const auto kJointTag = "joint"; -constexpr const auto kHardwareInterfaceTag = "hardwareInterface"; -} // namespace +// ROS +#include namespace transmission_interface { +static const rclcpp::Logger g_logger(rclcpp::get_logger("transmission_parser")); + +std::vector parse_transmissions_from_urdf(const std::string & urdf) { + TransmissionParser parser; + std::vector transmissions; + if(!parser.parse(urdf, transmissions)) + throw std::runtime_error("failed to parse transmissions in URDF"); + return transmissions; +} -std::vector parse_transmissions_from_urdf(const std::string & urdf) + +bool TransmissionParser::parse(const std::string& urdf, std::vector& transmissions) { - if (urdf.empty()) { - throw std::runtime_error("empty URDF passed in to transmission parser"); + // initialize TiXmlDocument doc with a string + TiXmlDocument doc; + if (!doc.Parse(urdf.c_str()) && doc.Error()) + { + RCLCPP_ERROR(g_logger, "Can't parse transmissions. Invalid robot description."); + return false; } - tinyxml2::XMLDocument doc; - if (!doc.Parse(urdf.c_str()) && doc.Error()) { - throw std::runtime_error("invalid URDF passed in to transmission parser"); + // Find joints in transmission tags + TiXmlElement *root = doc.RootElement(); + + // Constructs the transmissions by parsing custom xml. + TiXmlElement *trans_it = nullptr; + for (trans_it = root->FirstChildElement("transmission"); trans_it; + trans_it = trans_it->NextSiblingElement("transmission")) + { + transmission_interface::TransmissionInfo transmission; + + // Transmission name + if(trans_it->Attribute("name")) + { + transmission.joint_name = trans_it->Attribute("name"); + if (transmission.joint_name.empty()) + { + RCLCPP_ERROR_STREAM(g_logger, "Empty name attribute specified for transmission."); + throw std::runtime_error("Empty name attribute specified for transmission"); + } + } + else + { + RCLCPP_ERROR_STREAM(g_logger, "No name attribute specified for transmission."); + throw std::runtime_error("No name attribute specified for transmission"); + } + + // Transmission type + TiXmlElement *type_child = trans_it->FirstChildElement("type"); + if(!type_child) + { + RCLCPP_ERROR_STREAM(g_logger, "No type element found in transmission '" + << transmission.joint_name << "'."); + throw std::runtime_error("No type element found in transmission"); + } + if (!type_child->GetText()) + { + RCLCPP_ERROR_STREAM(g_logger, "Skipping empty type element in transmission '" + << transmission.joint_name << "'."); + throw std::runtime_error("empty type element in transmission"); + } + transmission.joint_control_type = type_child->GetText(); + + // Load joints + if(!parseJoints(trans_it, transmission.joints)) + { + RCLCPP_ERROR_STREAM(g_logger, "Failed to load joints for transmission '" + << transmission.joint_name << "'."); + throw std::runtime_error("Failed to load joints for transmission"); + } + + // Load actuators + if(!parseActuators(trans_it, transmission.actuators)) + { + RCLCPP_ERROR_STREAM(g_logger, "Failed to load actuators for transmission '" + << transmission.joint_name << "'."); + throw std::runtime_error("Failed to load actuators for transmission"); + } + + // Save loaded transmission + transmissions.push_back(transmission); + + } // end for + + if( transmissions.empty() ) + { + RCLCPP_DEBUG_STREAM(g_logger, "No valid transmissions found."); } - std::vector transmissions; - // Find joints in transmission tags - const tinyxml2::XMLElement * root_it = doc.RootElement(); - const tinyxml2::XMLElement * trans_it = root_it->FirstChildElement(kTransmissionTag); - while (trans_it) { + return true; +} + +bool TransmissionParser::parseJoints(TiXmlElement *trans_it, std::vector& joints) +{ + // Loop through each available joint + TiXmlElement *joint_it = nullptr; + for (joint_it = trans_it->FirstChildElement("joint"); joint_it; + joint_it = joint_it->NextSiblingElement("joint")) + { + // Create new joint + transmission_interface::JointInfo joint; + // Joint name - auto joint_it = trans_it->FirstChildElement(kJointTag); - if (!joint_it) { - throw std::runtime_error("no joint child element found"); + if(joint_it->Attribute("name")) + { + joint.name = joint_it->Attribute("name"); + if (joint.name.empty()) + { + RCLCPP_ERROR_STREAM(g_logger, "Empty name attribute specified for joint."); + continue; + } + } + else + { + RCLCPP_ERROR_STREAM(g_logger, "No name attribute specified for joint."); + return false; + } + + TiXmlElement *role_it = joint_it->FirstChildElement("role"); + if(role_it) + { + joint.role_ = role_it->GetText() ? role_it->GetText() : std::string(); + } + + // Hardware interfaces (required) + TiXmlElement *hw_iface_it = nullptr; + for (hw_iface_it = joint_it->FirstChildElement("hardwareInterface"); hw_iface_it; + hw_iface_it = hw_iface_it->NextSiblingElement("hardwareInterface")) + { + if(!hw_iface_it) {continue;} + if (!hw_iface_it->GetText()) + { + RCLCPP_DEBUG_STREAM(g_logger, "Skipping empty hardware interface element in joint '" + << joint.name << "'."); + throw std::runtime_error("empty hardware interface element in joint"); + } + const std::string hw_iface_name = hw_iface_it->GetText(); + joint.hardware_interfaces.push_back(hw_iface_name); } + if (joint.hardware_interfaces.empty()) + { + RCLCPP_ERROR_STREAM(g_logger, "No valid hardware interface element found in joint '" + << joint.name << "'."); + throw std::runtime_error("No valid hardware interface element found in joint"); + } + + // Joint xml element + std::stringstream ss; + ss << *joint_it; + joint.xml_element_ = ss.str(); + + // Add joint to vector + joints.push_back(joint); + } + + if(joints.empty()) + { + RCLCPP_DEBUG(g_logger, "No valid joint element found."); + return false; + } + + return true; +} + +bool TransmissionParser::parseActuators(TiXmlElement *trans_it, std::vector& actuators) +{ + // Loop through each available actuator + TiXmlElement *actuator_it = nullptr; + for (actuator_it = trans_it->FirstChildElement("actuator"); actuator_it; + actuator_it = actuator_it->NextSiblingElement("actuator")) + { + // Create new actuator + transmission_interface::ActuatorInfo actuator; - const std::string joint_name = joint_it->Attribute("name"); - if (joint_name.empty()) { - throw std::runtime_error("no joint name attribute set"); + // Actuator name + if(actuator_it->Attribute("name")) + { + actuator.name = actuator_it->Attribute("name"); + if (actuator.name.empty()) + { + RCLCPP_ERROR_STREAM(g_logger, "Empty name attribute specified for actuator."); + throw std::runtime_error("Empty name attribute specified for actuator"); + } + } + else + { + RCLCPP_ERROR_STREAM(g_logger, "No name attribute specified for actuator."); + throw std::runtime_error("No name attribute specified for actuator"); } - const auto hardware_interface_it = joint_it->FirstChildElement(kHardwareInterfaceTag); - if (!hardware_interface_it) { - throw std::runtime_error( - "no hardware interface tag found under transmission joint" + joint_name); + // Hardware interfaces (optional) + TiXmlElement *hw_iface_it = nullptr; + for (hw_iface_it = actuator_it->FirstChildElement("hardwareInterface"); hw_iface_it; + hw_iface_it = hw_iface_it->NextSiblingElement("hardwareInterface")) + { + if (!hw_iface_it->GetText()) + { + RCLCPP_DEBUG_STREAM(g_logger, "Skipping empty hardware interface element in actuator '" + << actuator.name << "'."); + continue; + } + const std::string hw_iface_name = hw_iface_it->GetText(); + actuator.hardware_interfaces.push_back(hw_iface_name); + } + if (actuator.hardware_interfaces.empty()) + { + RCLCPP_DEBUG_STREAM(g_logger, "No valid hardware interface element found in actuator '" + << actuator.name << "'."); + // continue; // NOTE: Hardware interface is optional, so we keep on going } - const std::string interface_name = hardware_interface_it->GetText(); - if (interface_name.empty()) { - throw std::runtime_error("no hardware interface specified in joint " + joint_name); + // mechanical reduction (optional) + actuator.mechanical_reduction = 1; + TiXmlElement *mechred_it = nullptr; + for (mechred_it = actuator_it->FirstChildElement("mechanicalReduction"); mechred_it; + mechred_it = mechred_it->NextSiblingElement("mechanicalReduction")) + { + if (!mechred_it->GetText()) + { + RCLCPP_DEBUG_STREAM(g_logger, "Skipping empty mechanicalReduction element in actuator '" + << actuator.name << "'."); + continue; + } + const auto value = mechred_it->GetText(); + actuator.mechanical_reduction = atoi(value); } - transmissions.push_back({joint_name, interface_name}); + // Actuator xml element + std::stringstream ss; + ss << *actuator_it; + actuator.xml_element_ = ss.str(); - trans_it = trans_it->NextSiblingElement(kTransmissionTag); + // Add actuator to vector + actuators.push_back(actuator); } - return transmissions; + if(actuators.empty()) + { + RCLCPP_DEBUG(g_logger, "No valid actuator element found."); + return false; + } + + return true; } -} // namespace transmission_interface +} // namespace diff --git a/transmission_interface/test/test_transmission_parser.cpp b/transmission_interface/test/test_transmission_parser.cpp index 08e0f0f24b..88829e381c 100644 --- a/transmission_interface/test/test_transmission_parser.cpp +++ b/transmission_interface/test/test_transmission_parser.cpp @@ -16,6 +16,8 @@ #include #include +#include "hardware_interface/robot_hardware.hpp" +#include "hardware_interface/control_type.hpp" #include "transmission_interface/transmission_parser.hpp" using namespace ::testing; // NOLINT @@ -175,6 +177,7 @@ class TestTransmissionParser : public Test 1 + PositionJointInterface @@ -183,7 +186,8 @@ class TestTransmissionParser : public Test VelocityJointInterface - 1 + 60 + VelocityJointInterface @@ -380,33 +384,71 @@ class TestTransmissionParser : public Test std::string wrong_urdf_xml_; }; -using transmission_interface::parse_transmissions_from_urdf; +using transmission_interface::TransmissionParser; +using transmission_interface::TransmissionInfo; TEST_F(TestTransmissionParser, successfully_parse_valid_urdf) { - const auto transmissions = parse_transmissions_from_urdf(valid_urdf_xml_); + TransmissionParser parser; + std::vector transmissions; + EXPECT_TRUE(parser.parse(valid_urdf_xml_, transmissions)); ASSERT_THAT(transmissions, SizeIs(2)); - EXPECT_EQ("rrbot_joint1", transmissions[0].joint_name); - EXPECT_EQ(hardware_interface::joint_control_type::POSITION, transmissions[0].joint_control_type); - EXPECT_EQ("rrbot_joint2", transmissions[1].joint_name); - EXPECT_EQ(hardware_interface::joint_control_type::VELOCITY, transmissions[1].joint_control_type); + + // first transmission + EXPECT_EQ("rrbot_tran1", transmissions[0].joint_name); + EXPECT_EQ("transmission_interface/SimpleTransmission", transmissions[0].joint_control_type); + + ASSERT_THAT(transmissions[0].joints, SizeIs(1)); + ASSERT_THAT(transmissions[0].joints[0].hardware_interfaces, SizeIs(1)); + EXPECT_EQ("rrbot_joint1", transmissions[0].joints[0].name); + EXPECT_EQ("PositionJointInterface", transmissions[0].joints[0].hardware_interfaces[0]); + + ASSERT_THAT(transmissions[0].actuators, SizeIs(1)); + ASSERT_THAT(transmissions[0].actuators[0].hardware_interfaces, SizeIs(1)); + EXPECT_EQ("rrbot_motor1", transmissions[0].actuators[0].name); + EXPECT_EQ("PositionJointInterface", transmissions[0].actuators[0].hardware_interfaces[0]); + EXPECT_EQ(1, transmissions[0].actuators[0].mechanical_reduction); + + // second transmission + EXPECT_EQ("rrbot_tran2", transmissions[1].joint_name); + EXPECT_EQ("transmission_interface/SimpleTransmission", transmissions[1].joint_control_type); + + ASSERT_THAT(transmissions[1].joints, SizeIs(1)); + ASSERT_THAT(transmissions[1].joints[0].hardware_interfaces, SizeIs(1)); + EXPECT_EQ("rrbot_joint2", transmissions[1].joints[0].name); + EXPECT_EQ("VelocityJointInterface", transmissions[1].joints[0].hardware_interfaces[0]); + + ASSERT_THAT(transmissions[1].actuators, SizeIs(1)); + ASSERT_THAT(transmissions[1].actuators[0].hardware_interfaces, SizeIs(1)); + EXPECT_EQ("rrbot_motor2", transmissions[1].actuators[0].name); + EXPECT_EQ("VelocityJointInterface", transmissions[1].actuators[0].hardware_interfaces[0]); + EXPECT_EQ(60, transmissions[1].actuators[0].mechanical_reduction); } TEST_F(TestTransmissionParser, empty_string_throws_error) { - ASSERT_THROW(parse_transmissions_from_urdf(""), std::runtime_error); + //ASSERT_THROW(parse_transmissions_from_urdf(""), std::runtime_error); } TEST_F(TestTransmissionParser, empty_urdf_returns_empty) { const std::string empty_urdf = ""; - const auto transmissions = parse_transmissions_from_urdf(empty_urdf); + TransmissionParser parser; + std::vector transmissions; + EXPECT_TRUE(parser.parse(empty_urdf, transmissions)); ASSERT_THAT(transmissions, IsEmpty()); } +TEST_F(TestTransmissionParser, simpler_parse_transmissions_from_urdf_returns_list) +{ + const auto transmissions = transmission_interface::parse_transmissions_from_urdf(valid_urdf_xml_); + ASSERT_THAT(transmissions, SizeIs(2)); +} + TEST_F(TestTransmissionParser, wrong_urdf_throws_error) { - ASSERT_THROW(parse_transmissions_from_urdf(wrong_urdf_xml_), std::runtime_error); + EXPECT_THROW(transmission_interface::parse_transmissions_from_urdf(wrong_urdf_xml_), std::runtime_error); + //ASSERT_THROW(parse_transmissions_from_urdf(wrong_urdf_xml_), std::runtime_error); } From 12ef5b8afa78c62c04bfb8bfcc49b4cbaff4ef42 Mon Sep 17 00:00:00 2001 From: Colin MacKenzie Date: Fri, 2 Oct 2020 01:24:09 -0400 Subject: [PATCH 02/14] fixed up tinyxml lib issues and other dependancies in CMakefile --- transmission_interface/CMakeLists.txt | 12 +++++++----- .../transmission_interface/transmission_info.hpp | 2 -- .../test/test_transmission_parser.cpp | 1 - 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/transmission_interface/CMakeLists.txt b/transmission_interface/CMakeLists.txt index 16193f66c1..b1fa08cca4 100644 --- a/transmission_interface/CMakeLists.txt +++ b/transmission_interface/CMakeLists.txt @@ -12,17 +12,19 @@ endif() # find dependencies find_package(ament_cmake REQUIRED) -#find_package(hardware_interface REQUIRED) # todo: we should be using this for hardware interface constants -find_package(tinyxml2_vendor REQUIRED) find_package(rclcpp REQUIRED) +find_package(hardware_interface REQUIRED) # todo: we should be using this for hardware interface constants +find_package(tinyxml_vendor REQUIRED) +find_package(TinyXML REQUIRED) # provided by tinyxml2 upstream, or tinyxml2_vendor # todo: we should be using the URDF project to load our tranmissions from URDF #find_package(urdf REQUIRED) add_library(transmission_parser SHARED src/transmission_parser.cpp) target_include_directories(transmission_parser PUBLIC include) +ament_target_dependencies(transmission_parser rclcpp hardware_interface tinyxml_vendor TinyXML) +ament_export_dependencies(hardware_interface tinyxml2_vendor TinyXML) target_compile_definitions(transmission_parser PRIVATE "TRANSMISSION_INTERFACE_BUILDING_DLL") -ament_target_dependencies(transmission_parser rclcpp tinyxml2_vendor) install( DIRECTORY include/ @@ -45,8 +47,8 @@ if(BUILD_TESTING) test/test_transmission_parser.cpp ) target_include_directories(test_transmission_parser PUBLIC include) - target_link_libraries(test_transmission_parser transmission_parser ${TinyXML_LIBRARY}) - ament_target_dependencies(test_transmission_parser rclcpp tinyxml2_vendor) + message(STATUS "tiny xml in ${TinyXML_LIBRARIES}") + target_link_libraries(test_transmission_parser transmission_parser) endif() ament_export_include_directories( diff --git a/transmission_interface/include/transmission_interface/transmission_info.hpp b/transmission_interface/include/transmission_interface/transmission_info.hpp index 5e1b606f9e..bdb51fb42e 100644 --- a/transmission_interface/include/transmission_interface/transmission_info.hpp +++ b/transmission_interface/include/transmission_interface/transmission_info.hpp @@ -19,8 +19,6 @@ #include #include -// TinyXML -#include namespace transmission_interface { diff --git a/transmission_interface/test/test_transmission_parser.cpp b/transmission_interface/test/test_transmission_parser.cpp index 88829e381c..14dc58958e 100644 --- a/transmission_interface/test/test_transmission_parser.cpp +++ b/transmission_interface/test/test_transmission_parser.cpp @@ -16,7 +16,6 @@ #include #include -#include "hardware_interface/robot_hardware.hpp" #include "hardware_interface/control_type.hpp" #include "transmission_interface/transmission_parser.hpp" From 887d50a6cbfe6683851231a1600cf052f5c02ea3 Mon Sep 17 00:00:00 2001 From: Colin MacKenzie Date: Fri, 2 Oct 2020 10:11:13 -0400 Subject: [PATCH 03/14] doh. changed joint_name back to name, since its the transmission name not the joint. Joint name is in the joints[] array elements --- .../transmission_interface/transmission_info.hpp | 4 ++-- transmission_interface/src/transmission_parser.cpp | 14 +++++++------- .../test/test_transmission_parser.cpp | 8 ++++---- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/transmission_interface/include/transmission_interface/transmission_info.hpp b/transmission_interface/include/transmission_interface/transmission_info.hpp index bdb51fb42e..2cd2a6bd65 100644 --- a/transmission_interface/include/transmission_interface/transmission_info.hpp +++ b/transmission_interface/include/transmission_interface/transmission_info.hpp @@ -50,8 +50,8 @@ struct ActuatorInfo */ struct TransmissionInfo { - std::string joint_name; - std::string joint_control_type; + std::string name; + std::string control_type; std::vector joints; std::vector actuators; }; diff --git a/transmission_interface/src/transmission_parser.cpp b/transmission_interface/src/transmission_parser.cpp index 79f88014ea..2981be712c 100644 --- a/transmission_interface/src/transmission_parser.cpp +++ b/transmission_interface/src/transmission_parser.cpp @@ -54,8 +54,8 @@ bool TransmissionParser::parse(const std::string& urdf, std::vectorAttribute("name")) { - transmission.joint_name = trans_it->Attribute("name"); - if (transmission.joint_name.empty()) + transmission.name = trans_it->Attribute("name"); + if (transmission.name.empty()) { RCLCPP_ERROR_STREAM(g_logger, "Empty name attribute specified for transmission."); throw std::runtime_error("Empty name attribute specified for transmission"); @@ -72,22 +72,22 @@ bool TransmissionParser::parse(const std::string& urdf, std::vectorGetText()) { RCLCPP_ERROR_STREAM(g_logger, "Skipping empty type element in transmission '" - << transmission.joint_name << "'."); + << transmission.name << "'."); throw std::runtime_error("empty type element in transmission"); } - transmission.joint_control_type = type_child->GetText(); + transmission.control_type = type_child->GetText(); // Load joints if(!parseJoints(trans_it, transmission.joints)) { RCLCPP_ERROR_STREAM(g_logger, "Failed to load joints for transmission '" - << transmission.joint_name << "'."); + << transmission.name << "'."); throw std::runtime_error("Failed to load joints for transmission"); } @@ -95,7 +95,7 @@ bool TransmissionParser::parse(const std::string& urdf, std::vector Date: Fri, 2 Oct 2020 10:25:28 -0400 Subject: [PATCH 04/14] removed debugging message from cmake file --- transmission_interface/CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/transmission_interface/CMakeLists.txt b/transmission_interface/CMakeLists.txt index b1fa08cca4..866d92e844 100644 --- a/transmission_interface/CMakeLists.txt +++ b/transmission_interface/CMakeLists.txt @@ -47,7 +47,6 @@ if(BUILD_TESTING) test/test_transmission_parser.cpp ) target_include_directories(test_transmission_parser PUBLIC include) - message(STATUS "tiny xml in ${TinyXML_LIBRARIES}") target_link_libraries(test_transmission_parser transmission_parser) endif() From ee5f5a8efec03de218fc97094e8b85912fe31db4 Mon Sep 17 00:00:00 2001 From: Colin MacKenzie Date: Sun, 4 Oct 2020 00:34:23 -0400 Subject: [PATCH 05/14] uncrustify fixes, enabled one more unit test for testing blank URDFs --- .../transmission_info.hpp | 8 ++------ .../transmission_parser.hpp | 5 +++-- .../src/transmission_parser.cpp | 18 ++++++++++-------- .../test/test_transmission_parser.cpp | 6 ++++-- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/transmission_interface/include/transmission_interface/transmission_info.hpp b/transmission_interface/include/transmission_interface/transmission_info.hpp index 2cd2a6bd65..ac22db1ce2 100644 --- a/transmission_interface/include/transmission_interface/transmission_info.hpp +++ b/transmission_interface/include/transmission_interface/transmission_info.hpp @@ -12,8 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef TRANSMISSION_INTERFACE__TRANSMISSION_INFO_HPP_ -#define TRANSMISSION_INTERFACE__TRANSMISSION_INFO_HPP_ +#pragma once // C++ standard #include @@ -56,7 +55,4 @@ struct TransmissionInfo std::vector actuators; }; -} // namespace - - -#endif \ No newline at end of file +} // namespace transmission_interface diff --git a/transmission_interface/include/transmission_interface/transmission_parser.hpp b/transmission_interface/include/transmission_interface/transmission_parser.hpp index 18e6bb4024..61cbdb2f90 100644 --- a/transmission_interface/include/transmission_interface/transmission_parser.hpp +++ b/transmission_interface/include/transmission_interface/transmission_parser.hpp @@ -27,6 +27,9 @@ // XML #include +#include +#include + namespace transmission_interface { @@ -34,7 +37,6 @@ namespace transmission_interface class TransmissionParser { public: - /** * \brief Parses the transmission elements of a URDF * \param[in] urdf_string XML string of a valid URDF file that contains \ elements @@ -59,7 +61,6 @@ class TransmissionParser * \return true if successful */ static bool parseActuators(TiXmlElement *trans_it, std::vector& actuators); - }; // class diff --git a/transmission_interface/src/transmission_parser.cpp b/transmission_interface/src/transmission_parser.cpp index 2981be712c..416c455130 100644 --- a/transmission_interface/src/transmission_parser.cpp +++ b/transmission_interface/src/transmission_parser.cpp @@ -12,12 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include #include // ROS #include +#include + + namespace transmission_interface { static const rclcpp::Logger g_logger(rclcpp::get_logger("transmission_parser")); @@ -31,7 +33,8 @@ std::vector parse_transmissions_from_urdf(const std::string & } -bool TransmissionParser::parse(const std::string& urdf, std::vector& transmissions) +bool TransmissionParser::parse + (const std::string& urdf, std::vector& transmissions) { // initialize TiXmlDocument doc with a string TiXmlDocument doc; @@ -60,8 +63,7 @@ bool TransmissionParser::parse(const std::string& urdf, std::vector& actuators) +bool TransmissionParser::parseActuators + (TiXmlElement *trans_it, std::vector& actuators) { // Loop through each available actuator TiXmlElement *actuator_it = nullptr; @@ -203,8 +206,7 @@ bool TransmissionParser::parseActuators(TiXmlElement *trans_it, std::vector Date: Mon, 5 Oct 2020 01:01:00 -0400 Subject: [PATCH 06/14] uncrustify fixes --- .../transmission_parser.hpp | 6 +- .../src/transmission_parser.cpp | 166 ++++++++---------- .../test/test_transmission_parser.cpp | 10 +- 3 files changed, 87 insertions(+), 95 deletions(-) diff --git a/transmission_interface/include/transmission_interface/transmission_parser.hpp b/transmission_interface/include/transmission_interface/transmission_parser.hpp index 61cbdb2f90..9ded56d0ea 100644 --- a/transmission_interface/include/transmission_interface/transmission_parser.hpp +++ b/transmission_interface/include/transmission_interface/transmission_parser.hpp @@ -43,7 +43,7 @@ class TransmissionParser * \param[out] transmissions vector of loaded transmission meta data * \return true if parsing was successful */ - static bool parse(const std::string& urdf_string, std::vector& transmissions); + static bool parse(const std::string & urdf_string, std::vector & transmissions); protected: /** @@ -52,7 +52,7 @@ class TransmissionParser * \param[out] joints resulting list of joints in the transmission * \return true if successful */ - static bool parseJoints(TiXmlElement *trans_it, std::vector& joints); + static bool parseJoints(TiXmlElement * trans_it, std::vector & joints); /** * \brief Parses the actuator elements within transmission elements of a URDF @@ -60,7 +60,7 @@ class TransmissionParser * \param[out] actuators resulting list of actuators in the transmission * \return true if successful */ - static bool parseActuators(TiXmlElement *trans_it, std::vector& actuators); + static bool parseActuators(TiXmlElement * trans_it, std::vector & actuators); }; // class diff --git a/transmission_interface/src/transmission_parser.cpp b/transmission_interface/src/transmission_parser.cpp index 416c455130..229e7192a5 100644 --- a/transmission_interface/src/transmission_parser.cpp +++ b/transmission_interface/src/transmission_parser.cpp @@ -24,80 +24,79 @@ namespace transmission_interface { static const rclcpp::Logger g_logger(rclcpp::get_logger("transmission_parser")); -std::vector parse_transmissions_from_urdf(const std::string & urdf) { +std::vector parse_transmissions_from_urdf(const std::string & urdf) +{ TransmissionParser parser; std::vector transmissions; - if(!parser.parse(urdf, transmissions)) + if (!parser.parse(urdf, transmissions)) { throw std::runtime_error("failed to parse transmissions in URDF"); + } return transmissions; } -bool TransmissionParser::parse - (const std::string& urdf, std::vector& transmissions) +bool TransmissionParser::parse( + const std::string & urdf, + std::vector & transmissions) { // initialize TiXmlDocument doc with a string TiXmlDocument doc; - if (!doc.Parse(urdf.c_str()) && doc.Error()) - { + if (!doc.Parse(urdf.c_str()) && doc.Error()) { RCLCPP_ERROR(g_logger, "Can't parse transmissions. Invalid robot description."); return false; } // Find joints in transmission tags - TiXmlElement *root = doc.RootElement(); + TiXmlElement * root = doc.RootElement(); // Constructs the transmissions by parsing custom xml. - TiXmlElement *trans_it = nullptr; + TiXmlElement * trans_it = nullptr; for (trans_it = root->FirstChildElement("transmission"); trans_it; - trans_it = trans_it->NextSiblingElement("transmission")) + trans_it = trans_it->NextSiblingElement("transmission")) { transmission_interface::TransmissionInfo transmission; // Transmission name - if(trans_it->Attribute("name")) - { + if (trans_it->Attribute("name")) { transmission.name = trans_it->Attribute("name"); - if (transmission.name.empty()) - { + if (transmission.name.empty()) { RCLCPP_ERROR_STREAM(g_logger, "Empty name attribute specified for transmission."); throw std::runtime_error("Empty name attribute specified for transmission"); } - } else - { + } else { RCLCPP_ERROR_STREAM(g_logger, "No name attribute specified for transmission."); throw std::runtime_error("No name attribute specified for transmission"); } // Transmission type - TiXmlElement *type_child = trans_it->FirstChildElement("type"); - if(!type_child) - { - RCLCPP_ERROR_STREAM(g_logger, "No type element found in transmission '" - << transmission.name << "'."); + TiXmlElement * type_child = trans_it->FirstChildElement("type"); + if (!type_child) { + RCLCPP_ERROR_STREAM( + g_logger, "No type element found in transmission '" << + transmission.name << "'."); throw std::runtime_error("No type element found in transmission"); } - if (!type_child->GetText()) - { - RCLCPP_ERROR_STREAM(g_logger, "Skipping empty type element in transmission '" - << transmission.name << "'."); + if (!type_child->GetText()) { + RCLCPP_ERROR_STREAM( + g_logger, "Skipping empty type element in transmission '" << + transmission.name << "'."); throw std::runtime_error("empty type element in transmission"); } transmission.control_type = type_child->GetText(); // Load joints - if(!parseJoints(trans_it, transmission.joints)) - { - RCLCPP_ERROR_STREAM(g_logger, "Failed to load joints for transmission '" - << transmission.name << "'."); + if (!parseJoints(trans_it, transmission.joints)) { + RCLCPP_ERROR_STREAM( + g_logger, "Failed to load joints for transmission '" << + transmission.name << "'."); throw std::runtime_error("Failed to load joints for transmission"); } // Load actuators - if(!parseActuators(trans_it, transmission.actuators)) - { - RCLCPP_ERROR_STREAM(g_logger, "Failed to load actuators for transmission '" - << transmission.name << "'."); + if (!parseActuators(trans_it, transmission.actuators)) { + RCLCPP_ERROR_STREAM( + g_logger, "Failed to load actuators for transmission '" << + transmission.name << "'."); throw std::runtime_error("Failed to load actuators for transmission"); } @@ -106,65 +105,60 @@ bool TransmissionParser::parse } // end for - if( transmissions.empty() ) - { + if (transmissions.empty()) { RCLCPP_DEBUG_STREAM(g_logger, "No valid transmissions found."); } return true; } -bool TransmissionParser::parseJoints(TiXmlElement *trans_it, std::vector& joints) +bool TransmissionParser::parseJoints(TiXmlElement * trans_it, std::vector & joints) { // Loop through each available joint - TiXmlElement *joint_it = nullptr; + TiXmlElement * joint_it = nullptr; for (joint_it = trans_it->FirstChildElement("joint"); joint_it; - joint_it = joint_it->NextSiblingElement("joint")) + joint_it = joint_it->NextSiblingElement("joint")) { // Create new joint transmission_interface::JointInfo joint; // Joint name - if(joint_it->Attribute("name")) - { + if (joint_it->Attribute("name")) { joint.name = joint_it->Attribute("name"); - if (joint.name.empty()) - { + if (joint.name.empty()) { RCLCPP_ERROR_STREAM(g_logger, "Empty name attribute specified for joint."); continue; } - } - else - { + } else { RCLCPP_ERROR_STREAM(g_logger, "No name attribute specified for joint."); return false; } - TiXmlElement *role_it = joint_it->FirstChildElement("role"); - if(role_it) - { + TiXmlElement * role_it = joint_it->FirstChildElement("role"); + if (role_it) { joint.role_ = role_it->GetText() ? role_it->GetText() : std::string(); } // Hardware interfaces (required) - TiXmlElement *hw_iface_it = nullptr; + TiXmlElement * hw_iface_it = nullptr; for (hw_iface_it = joint_it->FirstChildElement("hardwareInterface"); hw_iface_it; - hw_iface_it = hw_iface_it->NextSiblingElement("hardwareInterface")) + hw_iface_it = hw_iface_it->NextSiblingElement("hardwareInterface")) { - if(!hw_iface_it) {continue;} - if (!hw_iface_it->GetText()) - { - RCLCPP_DEBUG_STREAM(g_logger, "Skipping empty hardware interface element in joint '" - << joint.name << "'."); + if (!hw_iface_it) {continue;} + if (!hw_iface_it->GetText()) { + RCLCPP_DEBUG_STREAM( + g_logger, "Skipping empty hardware interface element in joint '" << + joint.name << "'."); throw std::runtime_error("empty hardware interface element in joint"); } const std::string hw_iface_name = hw_iface_it->GetText(); joint.hardware_interfaces.push_back(hw_iface_name); } - if (joint.hardware_interfaces.empty()) - { - RCLCPP_ERROR_STREAM(g_logger, "No valid hardware interface element found in joint '" - << joint.name << "'."); + + if (joint.hardware_interfaces.empty()) { + RCLCPP_ERROR_STREAM( + g_logger, "No valid hardware interface element found in joint '" << + joint.name << "'."); throw std::runtime_error("No valid hardware interface element found in joint"); } @@ -177,8 +171,7 @@ bool TransmissionParser::parseJoints(TiXmlElement *trans_it, std::vector& actuators) +bool TransmissionParser::parseActuators( + TiXmlElement * trans_it, + std::vector & actuators) { // Loop through each available actuator - TiXmlElement *actuator_it = nullptr; + TiXmlElement * actuator_it = nullptr; for (actuator_it = trans_it->FirstChildElement("actuator"); actuator_it; - actuator_it = actuator_it->NextSiblingElement("actuator")) + actuator_it = actuator_it->NextSiblingElement("actuator")) { // Create new actuator transmission_interface::ActuatorInfo actuator; // Actuator name - if(actuator_it->Attribute("name")) - { + if (actuator_it->Attribute("name")) { actuator.name = actuator_it->Attribute("name"); - if (actuator.name.empty()) - { + if (actuator.name.empty()) { RCLCPP_ERROR_STREAM(g_logger, "Empty name attribute specified for actuator."); throw std::runtime_error("Empty name attribute specified for actuator"); } - } else - { + } else { RCLCPP_ERROR_STREAM(g_logger, "No name attribute specified for actuator."); throw std::runtime_error("No name attribute specified for actuator"); } // Hardware interfaces (optional) - TiXmlElement *hw_iface_it = nullptr; + TiXmlElement * hw_iface_it = nullptr; for (hw_iface_it = actuator_it->FirstChildElement("hardwareInterface"); hw_iface_it; - hw_iface_it = hw_iface_it->NextSiblingElement("hardwareInterface")) + hw_iface_it = hw_iface_it->NextSiblingElement("hardwareInterface")) { - if (!hw_iface_it->GetText()) - { - RCLCPP_DEBUG_STREAM(g_logger, "Skipping empty hardware interface element in actuator '" - << actuator.name << "'."); + if (!hw_iface_it->GetText()) { + RCLCPP_DEBUG_STREAM( + g_logger, "Skipping empty hardware interface element in actuator '" << + actuator.name << "'."); continue; } const std::string hw_iface_name = hw_iface_it->GetText(); actuator.hardware_interfaces.push_back(hw_iface_name); } - if (actuator.hardware_interfaces.empty()) - { - RCLCPP_DEBUG_STREAM(g_logger, "No valid hardware interface element found in actuator '" - << actuator.name << "'."); + if (actuator.hardware_interfaces.empty()) { + RCLCPP_DEBUG_STREAM( + g_logger, "No valid hardware interface element found in actuator '" << + actuator.name << "'."); // continue; // NOTE: Hardware interface is optional, so we keep on going } // mechanical reduction (optional) actuator.mechanical_reduction = 1; - TiXmlElement *mechred_it = nullptr; + TiXmlElement * mechred_it = nullptr; for (mechred_it = actuator_it->FirstChildElement("mechanicalReduction"); mechred_it; - mechred_it = mechred_it->NextSiblingElement("mechanicalReduction")) + mechred_it = mechred_it->NextSiblingElement("mechanicalReduction")) { - if (!mechred_it->GetText()) - { - RCLCPP_DEBUG_STREAM(g_logger, "Skipping empty mechanicalReduction element in actuator '" - << actuator.name << "'."); + if (!mechred_it->GetText()) { + RCLCPP_DEBUG_STREAM( + g_logger, "Skipping empty mechanicalReduction element in actuator '" << + actuator.name << "'."); continue; } const auto value = mechred_it->GetText(); @@ -258,8 +249,7 @@ bool TransmissionParser::parseActuators actuators.push_back(actuator); } - if(actuators.empty()) - { + if (actuators.empty()) { RCLCPP_DEBUG(g_logger, "No valid actuator element found."); return false; } diff --git a/transmission_interface/test/test_transmission_parser.cpp b/transmission_interface/test/test_transmission_parser.cpp index 64625985e2..638d3e3d18 100644 --- a/transmission_interface/test/test_transmission_parser.cpp +++ b/transmission_interface/test/test_transmission_parser.cpp @@ -427,8 +427,9 @@ TEST_F(TestTransmissionParser, successfully_parse_valid_urdf) TEST_F(TestTransmissionParser, empty_string_throws_error) { - ASSERT_THROW(transmission_interface::parse_transmissions_from_urdf(""), - std::runtime_error); + ASSERT_THROW( + transmission_interface::parse_transmissions_from_urdf(""), + std::runtime_error); } TEST_F(TestTransmissionParser, empty_urdf_returns_empty) @@ -449,7 +450,8 @@ TEST_F(TestTransmissionParser, simpler_parse_transmissions_from_urdf_returns_lis TEST_F(TestTransmissionParser, wrong_urdf_throws_error) { - EXPECT_THROW(transmission_interface::parse_transmissions_from_urdf(wrong_urdf_xml_), - std::runtime_error); + EXPECT_THROW( + transmission_interface::parse_transmissions_from_urdf(wrong_urdf_xml_), + std::runtime_error); //ASSERT_THROW(parse_transmissions_from_urdf(wrong_urdf_xml_), std::runtime_error); } From 8d4bebbdf85c2c611fc7d44f7bbcf1e18b1bd972 Mon Sep 17 00:00:00 2001 From: Colin MacKenzie Date: Mon, 5 Oct 2020 01:20:54 -0400 Subject: [PATCH 07/14] cpplint fixes --- .../include/transmission_interface/transmission_info.hpp | 7 +++++-- .../include/transmission_interface/transmission_parser.hpp | 7 +++++-- transmission_interface/src/transmission_parser.cpp | 5 +++-- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/transmission_interface/include/transmission_interface/transmission_info.hpp b/transmission_interface/include/transmission_interface/transmission_info.hpp index ac22db1ce2..a2b79ee447 100644 --- a/transmission_interface/include/transmission_interface/transmission_info.hpp +++ b/transmission_interface/include/transmission_interface/transmission_info.hpp @@ -12,7 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -#pragma once +#ifndef TRANSMISSION_INTERFACE__TRANSMISSION_INFO_HPP_ +#define TRANSMISSION_INTERFACE__TRANSMISSION_INFO_HPP_ // C++ standard #include @@ -55,4 +56,6 @@ struct TransmissionInfo std::vector actuators; }; -} // namespace transmission_interface +} // namespace transmission_interface + +#endif // TRANSMISSION_INTERFACE__TRANSMISSION_INFO_HPP_ diff --git a/transmission_interface/include/transmission_interface/transmission_parser.hpp b/transmission_interface/include/transmission_interface/transmission_parser.hpp index 9ded56d0ea..aeac21ff66 100644 --- a/transmission_interface/include/transmission_interface/transmission_parser.hpp +++ b/transmission_interface/include/transmission_interface/transmission_parser.hpp @@ -18,7 +18,8 @@ * \author Dave Coleman */ -#pragma once +#ifndef TRANSMISSION_INTERFACE__TRANSMISSION_PARSER_HPP_ +#define TRANSMISSION_INTERFACE__TRANSMISSION_PARSER_HPP_ #include @@ -61,7 +62,7 @@ class TransmissionParser * \return true if successful */ static bool parseActuators(TiXmlElement * trans_it, std::vector & actuators); -}; // class +}; // class /** @@ -74,3 +75,5 @@ TRANSMISSION_INTERFACE_PUBLIC std::vector parse_transmissions_from_urdf(const std::string & urdf); } // namespace transmission_interface + +#endif // TRANSMISSION_INTERFACE__TRANSMISSION_PARSER_HPP_ diff --git a/transmission_interface/src/transmission_parser.cpp b/transmission_interface/src/transmission_parser.cpp index 229e7192a5..4dace774a5 100644 --- a/transmission_interface/src/transmission_parser.cpp +++ b/transmission_interface/src/transmission_parser.cpp @@ -18,6 +18,8 @@ #include #include +#include +#include namespace transmission_interface @@ -102,8 +104,7 @@ bool TransmissionParser::parse( // Save loaded transmission transmissions.push_back(transmission); - - } // end for + } // end for if (transmissions.empty()) { RCLCPP_DEBUG_STREAM(g_logger, "No valid transmissions found."); From bb78f684d535601a46622c4631a69dac0bbf5435 Mon Sep 17 00:00:00 2001 From: Colin MacKenzie Date: Mon, 5 Oct 2020 01:31:38 -0400 Subject: [PATCH 08/14] removed commented code to satisfy cpplinter --- transmission_interface/test/test_transmission_parser.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/transmission_interface/test/test_transmission_parser.cpp b/transmission_interface/test/test_transmission_parser.cpp index 638d3e3d18..b584dadf97 100644 --- a/transmission_interface/test/test_transmission_parser.cpp +++ b/transmission_interface/test/test_transmission_parser.cpp @@ -453,5 +453,4 @@ TEST_F(TestTransmissionParser, wrong_urdf_throws_error) EXPECT_THROW( transmission_interface::parse_transmissions_from_urdf(wrong_urdf_xml_), std::runtime_error); - //ASSERT_THROW(parse_transmissions_from_urdf(wrong_urdf_xml_), std::runtime_error); } From ae53454f48f712751fd625f7f098a2d767aacc39 Mon Sep 17 00:00:00 2001 From: Colin MacKenzie Date: Thu, 22 Oct 2020 01:09:10 -0400 Subject: [PATCH 09/14] No code changes. Various linter/format fixes and conversion to snake case --- transmission_interface/CMakeLists.txt | 7 ++----- .../transmission_info.hpp | 1 - .../transmission_parser.hpp | 5 ++--- .../src/transmission_parser.cpp | 17 ++++++++--------- 4 files changed, 12 insertions(+), 18 deletions(-) diff --git a/transmission_interface/CMakeLists.txt b/transmission_interface/CMakeLists.txt index 866d92e844..bd127d388e 100644 --- a/transmission_interface/CMakeLists.txt +++ b/transmission_interface/CMakeLists.txt @@ -13,12 +13,9 @@ endif() # find dependencies find_package(ament_cmake REQUIRED) find_package(rclcpp REQUIRED) -find_package(hardware_interface REQUIRED) # todo: we should be using this for hardware interface constants +find_package(hardware_interface REQUIRED) find_package(tinyxml_vendor REQUIRED) -find_package(TinyXML REQUIRED) # provided by tinyxml2 upstream, or tinyxml2_vendor - -# todo: we should be using the URDF project to load our tranmissions from URDF -#find_package(urdf REQUIRED) +find_package(TinyXML REQUIRED) add_library(transmission_parser SHARED src/transmission_parser.cpp) target_include_directories(transmission_parser PUBLIC include) diff --git a/transmission_interface/include/transmission_interface/transmission_info.hpp b/transmission_interface/include/transmission_interface/transmission_info.hpp index a2b79ee447..2e05e8ee13 100644 --- a/transmission_interface/include/transmission_interface/transmission_info.hpp +++ b/transmission_interface/include/transmission_interface/transmission_info.hpp @@ -15,7 +15,6 @@ #ifndef TRANSMISSION_INTERFACE__TRANSMISSION_INFO_HPP_ #define TRANSMISSION_INTERFACE__TRANSMISSION_INFO_HPP_ -// C++ standard #include #include diff --git a/transmission_interface/include/transmission_interface/transmission_parser.hpp b/transmission_interface/include/transmission_interface/transmission_parser.hpp index aeac21ff66..b7ca5da628 100644 --- a/transmission_interface/include/transmission_interface/transmission_parser.hpp +++ b/transmission_interface/include/transmission_interface/transmission_parser.hpp @@ -53,7 +53,7 @@ class TransmissionParser * \param[out] joints resulting list of joints in the transmission * \return true if successful */ - static bool parseJoints(TiXmlElement * trans_it, std::vector & joints); + static bool parse_joints(TiXmlElement * trans_it, std::vector & joints); /** * \brief Parses the actuator elements within transmission elements of a URDF @@ -61,7 +61,7 @@ class TransmissionParser * \param[out] actuators resulting list of actuators in the transmission * \return true if successful */ - static bool parseActuators(TiXmlElement * trans_it, std::vector & actuators); + static bool parse_actuators(TiXmlElement * trans_it, std::vector & actuators); }; // class @@ -73,7 +73,6 @@ class TransmissionParser */ TRANSMISSION_INTERFACE_PUBLIC std::vector parse_transmissions_from_urdf(const std::string & urdf); - } // namespace transmission_interface #endif // TRANSMISSION_INTERFACE__TRANSMISSION_PARSER_HPP_ diff --git a/transmission_interface/src/transmission_parser.cpp b/transmission_interface/src/transmission_parser.cpp index 4dace774a5..88d21c166c 100644 --- a/transmission_interface/src/transmission_parser.cpp +++ b/transmission_interface/src/transmission_parser.cpp @@ -14,13 +14,12 @@ #include -// ROS -#include - #include #include #include +#include "rclcpp/rclcpp.hpp" + namespace transmission_interface { @@ -74,8 +73,8 @@ bool TransmissionParser::parse( TiXmlElement * type_child = trans_it->FirstChildElement("type"); if (!type_child) { RCLCPP_ERROR_STREAM( - g_logger, "No type element found in transmission '" << - transmission.name << "'."); + g_logger, + "No type element found in transmission '" << transmission.name << "'."); throw std::runtime_error("No type element found in transmission"); } if (!type_child->GetText()) { @@ -87,7 +86,7 @@ bool TransmissionParser::parse( transmission.control_type = type_child->GetText(); // Load joints - if (!parseJoints(trans_it, transmission.joints)) { + if (!parse_joints(trans_it, transmission.joints)) { RCLCPP_ERROR_STREAM( g_logger, "Failed to load joints for transmission '" << transmission.name << "'."); @@ -95,7 +94,7 @@ bool TransmissionParser::parse( } // Load actuators - if (!parseActuators(trans_it, transmission.actuators)) { + if (!parse_actuators(trans_it, transmission.actuators)) { RCLCPP_ERROR_STREAM( g_logger, "Failed to load actuators for transmission '" << transmission.name << "'."); @@ -113,7 +112,7 @@ bool TransmissionParser::parse( return true; } -bool TransmissionParser::parseJoints(TiXmlElement * trans_it, std::vector & joints) +bool TransmissionParser::parse_joints(TiXmlElement * trans_it, std::vector & joints) { // Loop through each available joint TiXmlElement * joint_it = nullptr; @@ -180,7 +179,7 @@ bool TransmissionParser::parseJoints(TiXmlElement * trans_it, std::vector & actuators) { From 65cee027d5910e3da10265277b058675555e0bc1 Mon Sep 17 00:00:00 2001 From: Colin MacKenzie Date: Thu, 22 Oct 2020 02:13:37 -0400 Subject: [PATCH 10/14] removed rclcpp logging in parsing for throwing runtime_error for now --- .../transmission_parser.hpp | 10 +- .../src/transmission_parser.cpp | 141 +++++++----------- .../test/test_transmission_parser.cpp | 7 +- 3 files changed, 67 insertions(+), 91 deletions(-) diff --git a/transmission_interface/include/transmission_interface/transmission_parser.hpp b/transmission_interface/include/transmission_interface/transmission_parser.hpp index b7ca5da628..cf348a0c77 100644 --- a/transmission_interface/include/transmission_interface/transmission_parser.hpp +++ b/transmission_interface/include/transmission_interface/transmission_parser.hpp @@ -40,26 +40,30 @@ class TransmissionParser public: /** * \brief Parses the transmission elements of a URDF + * If parse errors occur a std::runtime_error will be thrown with a description of the problem. * \param[in] urdf_string XML string of a valid URDF file that contains \ elements * \param[out] transmissions vector of loaded transmission meta data - * \return true if parsing was successful + * \return true if one or more transmissions were successfully parsed. False if the URDF contained no + * transmission elements. */ static bool parse(const std::string & urdf_string, std::vector & transmissions); protected: /** * \brief Parses the joint elements within transmission elements of a URDF + * If parse errors occur a std::runtime_error will be thrown with a description of the problem. * \param[in] trans_it pointer to the current XML element being parsed * \param[out] joints resulting list of joints in the transmission - * \return true if successful + * \return true if joint information for a transmission was successfully parsed. */ static bool parse_joints(TiXmlElement * trans_it, std::vector & joints); /** * \brief Parses the actuator elements within transmission elements of a URDF + * If parse errors occur a std::runtime_error will be thrown with a description of the problem. * \param[in] trans_it pointer to the current XML element being parsed * \param[out] actuators resulting list of actuators in the transmission - * \return true if successful + * \return true if actuator information for a transmission was successfully parsed. */ static bool parse_actuators(TiXmlElement * trans_it, std::vector & actuators); }; // class diff --git a/transmission_interface/src/transmission_parser.cpp b/transmission_interface/src/transmission_parser.cpp index 88d21c166c..1c43b71648 100644 --- a/transmission_interface/src/transmission_parser.cpp +++ b/transmission_interface/src/transmission_parser.cpp @@ -20,22 +20,30 @@ #include "rclcpp/rclcpp.hpp" +namespace +{ +constexpr const auto kTransmissionParserLoggerName = "transmission_parser"; +} namespace transmission_interface { -static const rclcpp::Logger g_logger(rclcpp::get_logger("transmission_parser")); std::vector parse_transmissions_from_urdf(const std::string & urdf) { TransmissionParser parser; std::vector transmissions; - if (!parser.parse(urdf, transmissions)) { - throw std::runtime_error("failed to parse transmissions in URDF"); + try { + parser.parse(urdf, transmissions); + } catch (const std::runtime_error & err) { + RCLCPP_ERROR( + rclcpp::get_logger(kTransmissionParserLoggerName), + err.what() + ); + throw err; } return transmissions; } - bool TransmissionParser::parse( const std::string & urdf, std::vector & transmissions) @@ -43,7 +51,7 @@ bool TransmissionParser::parse( // initialize TiXmlDocument doc with a string TiXmlDocument doc; if (!doc.Parse(urdf.c_str()) && doc.Error()) { - RCLCPP_ERROR(g_logger, "Can't parse transmissions. Invalid robot description."); + throw std::runtime_error("Can't parse transmissions. Invalid robot description."); return false; } @@ -61,55 +69,44 @@ bool TransmissionParser::parse( if (trans_it->Attribute("name")) { transmission.name = trans_it->Attribute("name"); if (transmission.name.empty()) { - RCLCPP_ERROR_STREAM(g_logger, "Empty name attribute specified for transmission."); - throw std::runtime_error("Empty name attribute specified for transmission"); + throw std::runtime_error("empty name attribute specified for transmission"); } } else { - RCLCPP_ERROR_STREAM(g_logger, "No name attribute specified for transmission."); - throw std::runtime_error("No name attribute specified for transmission"); + throw std::runtime_error("no name attribute specified for transmission"); } // Transmission type TiXmlElement * type_child = trans_it->FirstChildElement("type"); if (!type_child) { - RCLCPP_ERROR_STREAM( - g_logger, - "No type element found in transmission '" << transmission.name << "'."); - throw std::runtime_error("No type element found in transmission"); + throw std::runtime_error( + "no type element found in transmission '" + transmission.name + "'."); } if (!type_child->GetText()) { - RCLCPP_ERROR_STREAM( - g_logger, "Skipping empty type element in transmission '" << - transmission.name << "'."); - throw std::runtime_error("empty type element in transmission"); + throw std::runtime_error( + "expected non-empty type element in transmission '" + transmission.name + "'."); } transmission.control_type = type_child->GetText(); - // Load joints - if (!parse_joints(trans_it, transmission.joints)) { - RCLCPP_ERROR_STREAM( - g_logger, "Failed to load joints for transmission '" << - transmission.name << "'."); - throw std::runtime_error("Failed to load joints for transmission"); - } + try { + // Load joints + if (!parse_joints(trans_it, transmission.joints)) { + throw std::runtime_error("requires one joint element."); + } - // Load actuators - if (!parse_actuators(trans_it, transmission.actuators)) { - RCLCPP_ERROR_STREAM( - g_logger, "Failed to load actuators for transmission '" << - transmission.name << "'."); - throw std::runtime_error("Failed to load actuators for transmission"); + // Load actuators + if (!parse_actuators(trans_it, transmission.actuators)) { + throw std::runtime_error("requires one actuator element."); + } + } catch (const std::runtime_error & ex) { + // add the transmission name and rethrow + throw std::runtime_error("transmission '" + transmission.name + "' " + ex.what()); } // Save loaded transmission transmissions.push_back(transmission); } // end for - if (transmissions.empty()) { - RCLCPP_DEBUG_STREAM(g_logger, "No valid transmissions found."); - } - - return true; + return !transmissions.empty(); } bool TransmissionParser::parse_joints(TiXmlElement * trans_it, std::vector & joints) @@ -126,11 +123,11 @@ bool TransmissionParser::parse_joints(TiXmlElement * trans_it, std::vectorAttribute("name")) { joint.name = joint_it->Attribute("name"); if (joint.name.empty()) { - RCLCPP_ERROR_STREAM(g_logger, "Empty name attribute specified for joint."); + throw std::runtime_error("expected valid joint name attribute."); continue; } } else { - RCLCPP_ERROR_STREAM(g_logger, "No name attribute specified for joint."); + throw std::runtime_error("expected name attribute for joint."); return false; } @@ -144,22 +141,17 @@ bool TransmissionParser::parse_joints(TiXmlElement * trans_it, std::vectorFirstChildElement("hardwareInterface"); hw_iface_it; hw_iface_it = hw_iface_it->NextSiblingElement("hardwareInterface")) { - if (!hw_iface_it) {continue;} - if (!hw_iface_it->GetText()) { - RCLCPP_DEBUG_STREAM( - g_logger, "Skipping empty hardware interface element in joint '" << - joint.name << "'."); - throw std::runtime_error("empty hardware interface element in joint"); + if (hw_iface_it->GetText()) { + const std::string hw_iface_name = hw_iface_it->GetText(); + joint.hardware_interfaces.push_back(hw_iface_name); + } else { + throw std::runtime_error("expected hardware interface name for joint " + joint.name + '.'); } - const std::string hw_iface_name = hw_iface_it->GetText(); - joint.hardware_interfaces.push_back(hw_iface_name); } if (joint.hardware_interfaces.empty()) { - RCLCPP_ERROR_STREAM( - g_logger, "No valid hardware interface element found in joint '" << - joint.name << "'."); - throw std::runtime_error("No valid hardware interface element found in joint"); + throw std::runtime_error( + "joint " + joint.name + " has no valid hardware interface."); } // Joint xml element @@ -171,12 +163,7 @@ bool TransmissionParser::parse_joints(TiXmlElement * trans_it, std::vectorAttribute("name")) { actuator.name = actuator_it->Attribute("name"); if (actuator.name.empty()) { - RCLCPP_ERROR_STREAM(g_logger, "Empty name attribute specified for actuator."); - throw std::runtime_error("Empty name attribute specified for actuator"); + throw std::runtime_error("expected valid actuator name attribute."); } } else { - RCLCPP_ERROR_STREAM(g_logger, "No name attribute specified for actuator."); - throw std::runtime_error("No name attribute specified for actuator"); + throw std::runtime_error("expected name attribute for actuator."); } // Hardware interfaces (optional) @@ -208,20 +193,18 @@ bool TransmissionParser::parse_actuators( for (hw_iface_it = actuator_it->FirstChildElement("hardwareInterface"); hw_iface_it; hw_iface_it = hw_iface_it->NextSiblingElement("hardwareInterface")) { - if (!hw_iface_it->GetText()) { - RCLCPP_DEBUG_STREAM( - g_logger, "Skipping empty hardware interface element in actuator '" << - actuator.name << "'."); - continue; + // Skipping empty hardware interface element in actuator + if (hw_iface_it->GetText()) { + const std::string hw_iface_name = hw_iface_it->GetText(); + actuator.hardware_interfaces.push_back(hw_iface_name); + } else { + throw std::runtime_error( + "expected hardware interface name for actuator " + actuator.name + "."); } - const std::string hw_iface_name = hw_iface_it->GetText(); - actuator.hardware_interfaces.push_back(hw_iface_name); } if (actuator.hardware_interfaces.empty()) { - RCLCPP_DEBUG_STREAM( - g_logger, "No valid hardware interface element found in actuator '" << - actuator.name << "'."); - // continue; // NOTE: Hardware interface is optional, so we keep on going + throw std::runtime_error( + "actuator " + actuator.name + " has no valid hardware interface."); } // mechanical reduction (optional) @@ -230,14 +213,11 @@ bool TransmissionParser::parse_actuators( for (mechred_it = actuator_it->FirstChildElement("mechanicalReduction"); mechred_it; mechred_it = mechred_it->NextSiblingElement("mechanicalReduction")) { - if (!mechred_it->GetText()) { - RCLCPP_DEBUG_STREAM( - g_logger, "Skipping empty mechanicalReduction element in actuator '" << - actuator.name << "'."); - continue; + // optional tag, so no error if element is empty + if (mechred_it->GetText()) { + const auto value = mechred_it->GetText(); + actuator.mechanical_reduction = atoi(value); } - const auto value = mechred_it->GetText(); - actuator.mechanical_reduction = atoi(value); } // Actuator xml element @@ -249,12 +229,7 @@ bool TransmissionParser::parse_actuators( actuators.push_back(actuator); } - if (actuators.empty()) { - RCLCPP_DEBUG(g_logger, "No valid actuator element found."); - return false; - } - - return true; + return !actuators.empty(); } } // namespace transmission_interface diff --git a/transmission_interface/test/test_transmission_parser.cpp b/transmission_interface/test/test_transmission_parser.cpp index b584dadf97..78e72b82b6 100644 --- a/transmission_interface/test/test_transmission_parser.cpp +++ b/transmission_interface/test/test_transmission_parser.cpp @@ -434,11 +434,8 @@ TEST_F(TestTransmissionParser, empty_string_throws_error) TEST_F(TestTransmissionParser, empty_urdf_returns_empty) { - const std::string empty_urdf = - ""; - TransmissionParser parser; - std::vector transmissions; - EXPECT_TRUE(parser.parse(empty_urdf, transmissions)); + auto transmissions = transmission_interface::parse_transmissions_from_urdf( + ""); ASSERT_THAT(transmissions, IsEmpty()); } From 268e71996dbf27244597cd422f19ad9fe0447173 Mon Sep 17 00:00:00 2001 From: Colin MacKenzie Date: Thu, 22 Oct 2020 02:34:55 -0400 Subject: [PATCH 11/14] migrated to TinyXML2 from 1 --- transmission_interface/CMakeLists.txt | 8 ++-- .../transmission_info.hpp | 4 +- .../transmission_parser.hpp | 15 ++++-- .../src/transmission_parser.cpp | 48 +++++++++---------- 4 files changed, 37 insertions(+), 38 deletions(-) diff --git a/transmission_interface/CMakeLists.txt b/transmission_interface/CMakeLists.txt index bd127d388e..f5be015e99 100644 --- a/transmission_interface/CMakeLists.txt +++ b/transmission_interface/CMakeLists.txt @@ -14,13 +14,13 @@ endif() find_package(ament_cmake REQUIRED) find_package(rclcpp REQUIRED) find_package(hardware_interface REQUIRED) -find_package(tinyxml_vendor REQUIRED) -find_package(TinyXML REQUIRED) +find_package(tinyxml2_vendor REQUIRED) +find_package(TinyXML2 REQUIRED) add_library(transmission_parser SHARED src/transmission_parser.cpp) target_include_directories(transmission_parser PUBLIC include) -ament_target_dependencies(transmission_parser rclcpp hardware_interface tinyxml_vendor TinyXML) -ament_export_dependencies(hardware_interface tinyxml2_vendor TinyXML) +ament_target_dependencies(transmission_parser rclcpp hardware_interface tinyxml2_vendor TinyXML2) +ament_export_dependencies(hardware_interface tinyxml2_vendor TinyXML2) target_compile_definitions(transmission_parser PRIVATE "TRANSMISSION_INTERFACE_BUILDING_DLL") install( diff --git a/transmission_interface/include/transmission_interface/transmission_info.hpp b/transmission_interface/include/transmission_interface/transmission_info.hpp index 2e05e8ee13..3a764084b4 100644 --- a/transmission_interface/include/transmission_interface/transmission_info.hpp +++ b/transmission_interface/include/transmission_interface/transmission_info.hpp @@ -29,8 +29,7 @@ struct JointInfo { std::string name; std::vector hardware_interfaces; - std::string role_; - std::string xml_element_; + std::string role; }; /** @@ -41,7 +40,6 @@ struct ActuatorInfo std::string name; std::vector hardware_interfaces; double mechanical_reduction; - std::string xml_element_; }; /** diff --git a/transmission_interface/include/transmission_interface/transmission_parser.hpp b/transmission_interface/include/transmission_interface/transmission_parser.hpp index cf348a0c77..89f76a667a 100644 --- a/transmission_interface/include/transmission_interface/transmission_parser.hpp +++ b/transmission_interface/include/transmission_interface/transmission_parser.hpp @@ -25,8 +25,7 @@ #include #include -// XML -#include +#include #include #include @@ -46,7 +45,9 @@ class TransmissionParser * \return true if one or more transmissions were successfully parsed. False if the URDF contained no * transmission elements. */ - static bool parse(const std::string & urdf_string, std::vector & transmissions); + static bool parse( + const std::string & urdf_string, + std::vector & transmissions); protected: /** @@ -56,7 +57,9 @@ class TransmissionParser * \param[out] joints resulting list of joints in the transmission * \return true if joint information for a transmission was successfully parsed. */ - static bool parse_joints(TiXmlElement * trans_it, std::vector & joints); + static bool parse_joints( + tinyxml2::XMLElement * trans_it, + std::vector & joints); /** * \brief Parses the actuator elements within transmission elements of a URDF @@ -65,7 +68,9 @@ class TransmissionParser * \param[out] actuators resulting list of actuators in the transmission * \return true if actuator information for a transmission was successfully parsed. */ - static bool parse_actuators(TiXmlElement * trans_it, std::vector & actuators); + static bool parse_actuators( + tinyxml2::XMLElement * trans_it, + std::vector & actuators); }; // class diff --git a/transmission_interface/src/transmission_parser.cpp b/transmission_interface/src/transmission_parser.cpp index 1c43b71648..a60efc4d08 100644 --- a/transmission_interface/src/transmission_parser.cpp +++ b/transmission_interface/src/transmission_parser.cpp @@ -48,18 +48,24 @@ bool TransmissionParser::parse( const std::string & urdf, std::vector & transmissions) { + if (urdf.empty()) { + throw std::runtime_error("empty URDF passed to robot"); + } + // initialize TiXmlDocument doc with a string - TiXmlDocument doc; + tinyxml2::XMLDocument doc; if (!doc.Parse(urdf.c_str()) && doc.Error()) { throw std::runtime_error("Can't parse transmissions. Invalid robot description."); - return false; } // Find joints in transmission tags - TiXmlElement * root = doc.RootElement(); + tinyxml2::XMLElement * root = doc.RootElement(); + if (root == nullptr) { + throw std::runtime_error("Can't parse transmissions. Invalid robot description."); + } // Constructs the transmissions by parsing custom xml. - TiXmlElement * trans_it = nullptr; + tinyxml2::XMLElement * trans_it = nullptr; for (trans_it = root->FirstChildElement("transmission"); trans_it; trans_it = trans_it->NextSiblingElement("transmission")) { @@ -76,7 +82,7 @@ bool TransmissionParser::parse( } // Transmission type - TiXmlElement * type_child = trans_it->FirstChildElement("type"); + tinyxml2::XMLElement * type_child = trans_it->FirstChildElement("type"); if (!type_child) { throw std::runtime_error( "no type element found in transmission '" + transmission.name + "'."); @@ -109,10 +115,12 @@ bool TransmissionParser::parse( return !transmissions.empty(); } -bool TransmissionParser::parse_joints(TiXmlElement * trans_it, std::vector & joints) +bool TransmissionParser::parse_joints( + tinyxml2::XMLElement * trans_it, + std::vector & joints) { // Loop through each available joint - TiXmlElement * joint_it = nullptr; + tinyxml2::XMLElement * joint_it = nullptr; for (joint_it = trans_it->FirstChildElement("joint"); joint_it; joint_it = joint_it->NextSiblingElement("joint")) { @@ -124,20 +132,18 @@ bool TransmissionParser::parse_joints(TiXmlElement * trans_it, std::vectorAttribute("name"); if (joint.name.empty()) { throw std::runtime_error("expected valid joint name attribute."); - continue; } } else { throw std::runtime_error("expected name attribute for joint."); - return false; } - TiXmlElement * role_it = joint_it->FirstChildElement("role"); + tinyxml2::XMLElement * role_it = joint_it->FirstChildElement("role"); if (role_it) { - joint.role_ = role_it->GetText() ? role_it->GetText() : std::string(); + joint.role = role_it->GetText() ? role_it->GetText() : std::string(); } // Hardware interfaces (required) - TiXmlElement * hw_iface_it = nullptr; + tinyxml2::XMLElement * hw_iface_it = nullptr; for (hw_iface_it = joint_it->FirstChildElement("hardwareInterface"); hw_iface_it; hw_iface_it = hw_iface_it->NextSiblingElement("hardwareInterface")) { @@ -154,11 +160,6 @@ bool TransmissionParser::parse_joints(TiXmlElement * trans_it, std::vector & actuators) { // Loop through each available actuator - TiXmlElement * actuator_it = nullptr; + tinyxml2::XMLElement * actuator_it = nullptr; for (actuator_it = trans_it->FirstChildElement("actuator"); actuator_it; actuator_it = actuator_it->NextSiblingElement("actuator")) { @@ -189,7 +190,7 @@ bool TransmissionParser::parse_actuators( } // Hardware interfaces (optional) - TiXmlElement * hw_iface_it = nullptr; + tinyxml2::XMLElement * hw_iface_it = nullptr; for (hw_iface_it = actuator_it->FirstChildElement("hardwareInterface"); hw_iface_it; hw_iface_it = hw_iface_it->NextSiblingElement("hardwareInterface")) { @@ -209,7 +210,7 @@ bool TransmissionParser::parse_actuators( // mechanical reduction (optional) actuator.mechanical_reduction = 1; - TiXmlElement * mechred_it = nullptr; + tinyxml2::XMLElement * mechred_it = nullptr; for (mechred_it = actuator_it->FirstChildElement("mechanicalReduction"); mechred_it; mechred_it = mechred_it->NextSiblingElement("mechanicalReduction")) { @@ -220,11 +221,6 @@ bool TransmissionParser::parse_actuators( } } - // Actuator xml element - std::stringstream ss; - ss << *actuator_it; - actuator.xml_element_ = ss.str(); - // Add actuator to vector actuators.push_back(actuator); } From 6edd316b5ae4d8f440d1de3a71cdf5558babe5f9 Mon Sep 17 00:00:00 2001 From: Colin MacKenzie Date: Thu, 22 Oct 2020 02:43:14 -0400 Subject: [PATCH 12/14] using constexpr tags for XML tags instead of string literals --- .../src/transmission_parser.cpp | 38 +++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/transmission_interface/src/transmission_parser.cpp b/transmission_interface/src/transmission_parser.cpp index a60efc4d08..009c62f515 100644 --- a/transmission_interface/src/transmission_parser.cpp +++ b/transmission_interface/src/transmission_parser.cpp @@ -23,7 +23,15 @@ namespace { constexpr const auto kTransmissionParserLoggerName = "transmission_parser"; -} + +constexpr const auto kTransmissionTag = "transmission"; +constexpr const auto kJointTag = "joint"; +constexpr const auto kActuatorTag = "actuator"; +constexpr const auto kTypeTag = "type"; +constexpr const auto kRoleTag = "role"; +constexpr const auto kHardwareInterfaceTag = "hardwareInterface"; +constexpr const auto kMechanicalReductionTag = "mechanicalReduction"; +} // namespace namespace transmission_interface { @@ -66,8 +74,8 @@ bool TransmissionParser::parse( // Constructs the transmissions by parsing custom xml. tinyxml2::XMLElement * trans_it = nullptr; - for (trans_it = root->FirstChildElement("transmission"); trans_it; - trans_it = trans_it->NextSiblingElement("transmission")) + for (trans_it = root->FirstChildElement(kTransmissionTag); trans_it; + trans_it = trans_it->NextSiblingElement(kTransmissionTag)) { transmission_interface::TransmissionInfo transmission; @@ -82,7 +90,7 @@ bool TransmissionParser::parse( } // Transmission type - tinyxml2::XMLElement * type_child = trans_it->FirstChildElement("type"); + tinyxml2::XMLElement * type_child = trans_it->FirstChildElement(kTypeTag); if (!type_child) { throw std::runtime_error( "no type element found in transmission '" + transmission.name + "'."); @@ -121,8 +129,8 @@ bool TransmissionParser::parse_joints( { // Loop through each available joint tinyxml2::XMLElement * joint_it = nullptr; - for (joint_it = trans_it->FirstChildElement("joint"); joint_it; - joint_it = joint_it->NextSiblingElement("joint")) + for (joint_it = trans_it->FirstChildElement(kJointTag); joint_it; + joint_it = joint_it->NextSiblingElement(kJointTag)) { // Create new joint transmission_interface::JointInfo joint; @@ -137,15 +145,15 @@ bool TransmissionParser::parse_joints( throw std::runtime_error("expected name attribute for joint."); } - tinyxml2::XMLElement * role_it = joint_it->FirstChildElement("role"); + tinyxml2::XMLElement * role_it = joint_it->FirstChildElement(kRoleTag); if (role_it) { joint.role = role_it->GetText() ? role_it->GetText() : std::string(); } // Hardware interfaces (required) tinyxml2::XMLElement * hw_iface_it = nullptr; - for (hw_iface_it = joint_it->FirstChildElement("hardwareInterface"); hw_iface_it; - hw_iface_it = hw_iface_it->NextSiblingElement("hardwareInterface")) + for (hw_iface_it = joint_it->FirstChildElement(kHardwareInterfaceTag); hw_iface_it; + hw_iface_it = hw_iface_it->NextSiblingElement(kHardwareInterfaceTag)) { if (hw_iface_it->GetText()) { const std::string hw_iface_name = hw_iface_it->GetText(); @@ -173,8 +181,8 @@ bool TransmissionParser::parse_actuators( { // Loop through each available actuator tinyxml2::XMLElement * actuator_it = nullptr; - for (actuator_it = trans_it->FirstChildElement("actuator"); actuator_it; - actuator_it = actuator_it->NextSiblingElement("actuator")) + for (actuator_it = trans_it->FirstChildElement(kActuatorTag); actuator_it; + actuator_it = actuator_it->NextSiblingElement(kActuatorTag)) { // Create new actuator transmission_interface::ActuatorInfo actuator; @@ -191,8 +199,8 @@ bool TransmissionParser::parse_actuators( // Hardware interfaces (optional) tinyxml2::XMLElement * hw_iface_it = nullptr; - for (hw_iface_it = actuator_it->FirstChildElement("hardwareInterface"); hw_iface_it; - hw_iface_it = hw_iface_it->NextSiblingElement("hardwareInterface")) + for (hw_iface_it = actuator_it->FirstChildElement(kHardwareInterfaceTag); hw_iface_it; + hw_iface_it = hw_iface_it->NextSiblingElement(kHardwareInterfaceTag)) { // Skipping empty hardware interface element in actuator if (hw_iface_it->GetText()) { @@ -211,8 +219,8 @@ bool TransmissionParser::parse_actuators( // mechanical reduction (optional) actuator.mechanical_reduction = 1; tinyxml2::XMLElement * mechred_it = nullptr; - for (mechred_it = actuator_it->FirstChildElement("mechanicalReduction"); mechred_it; - mechred_it = mechred_it->NextSiblingElement("mechanicalReduction")) + for (mechred_it = actuator_it->FirstChildElement(kMechanicalReductionTag); mechred_it; + mechred_it = mechred_it->NextSiblingElement(kMechanicalReductionTag)) { // optional tag, so no error if element is empty if (mechred_it->GetText()) { From 310796372e51cb425478b1135be4e39f71c5589d Mon Sep 17 00:00:00 2001 From: Bence Magyar Date: Sat, 24 Oct 2020 16:12:57 +0100 Subject: [PATCH 13/14] No TransmissionParser class --- .../transmission_info.hpp | 6 +- .../transmission_parser.hpp | 56 ++---- .../src/transmission_parser.cpp | 184 +++++++----------- .../test/test_transmission_parser.cpp | 35 ++-- 4 files changed, 105 insertions(+), 176 deletions(-) diff --git a/transmission_interface/include/transmission_interface/transmission_info.hpp b/transmission_interface/include/transmission_interface/transmission_info.hpp index 3a764084b4..3e21c6bdf3 100644 --- a/transmission_interface/include/transmission_interface/transmission_info.hpp +++ b/transmission_interface/include/transmission_interface/transmission_info.hpp @@ -28,7 +28,7 @@ namespace transmission_interface struct JointInfo { std::string name; - std::vector hardware_interfaces; + std::vector interfaces; std::string role; }; @@ -38,7 +38,7 @@ struct JointInfo struct ActuatorInfo { std::string name; - std::vector hardware_interfaces; + std::vector interfaces; double mechanical_reduction; }; @@ -48,7 +48,7 @@ struct ActuatorInfo struct TransmissionInfo { std::string name; - std::string control_type; + std::string type; std::vector joints; std::vector actuators; }; diff --git a/transmission_interface/include/transmission_interface/transmission_parser.hpp b/transmission_interface/include/transmission_interface/transmission_parser.hpp index 89f76a667a..55f439a414 100644 --- a/transmission_interface/include/transmission_interface/transmission_parser.hpp +++ b/transmission_interface/include/transmission_interface/transmission_parser.hpp @@ -33,45 +33,25 @@ namespace transmission_interface { -/** \brief Parse all transmissions specified in a URDF. */ -class TransmissionParser -{ -public: - /** - * \brief Parses the transmission elements of a URDF - * If parse errors occur a std::runtime_error will be thrown with a description of the problem. - * \param[in] urdf_string XML string of a valid URDF file that contains \ elements - * \param[out] transmissions vector of loaded transmission meta data - * \return true if one or more transmissions were successfully parsed. False if the URDF contained no - * transmission elements. - */ - static bool parse( - const std::string & urdf_string, - std::vector & transmissions); - -protected: - /** - * \brief Parses the joint elements within transmission elements of a URDF - * If parse errors occur a std::runtime_error will be thrown with a description of the problem. - * \param[in] trans_it pointer to the current XML element being parsed - * \param[out] joints resulting list of joints in the transmission - * \return true if joint information for a transmission was successfully parsed. - */ - static bool parse_joints( - tinyxml2::XMLElement * trans_it, - std::vector & joints); +/** + * \brief Parses the joint elements within transmission elements of a URDF + * If parse errors occur a std::runtime_error will be thrown with a description of the problem. + * \param[in] trans_it pointer to the current XML element being parsed + * \param[out] joints resulting list of joints in the transmission + * \return true if joint information for a transmission was successfully parsed. + */ +TRANSMISSION_INTERFACE_PUBLIC +std::vector parse_joints(tinyxml2::XMLElement * trans_it); - /** - * \brief Parses the actuator elements within transmission elements of a URDF - * If parse errors occur a std::runtime_error will be thrown with a description of the problem. - * \param[in] trans_it pointer to the current XML element being parsed - * \param[out] actuators resulting list of actuators in the transmission - * \return true if actuator information for a transmission was successfully parsed. - */ - static bool parse_actuators( - tinyxml2::XMLElement * trans_it, - std::vector & actuators); -}; // class +/** + * \brief Parses the actuator elements within transmission elements of a URDF + * If parse errors occur a std::runtime_error will be thrown with a description of the problem. + * \param[in] trans_it pointer to the current XML element being parsed + * \param[out] actuators resulting list of actuators in the transmission + * \return true if actuator information for a transmission was successfully parsed. + */ +TRANSMISSION_INTERFACE_PUBLIC +std::vector parse_actuators(tinyxml2::XMLElement * trans_it); /** diff --git a/transmission_interface/src/transmission_parser.cpp b/transmission_interface/src/transmission_parser.cpp index 009c62f515..d324da30fd 100644 --- a/transmission_interface/src/transmission_parser.cpp +++ b/transmission_interface/src/transmission_parser.cpp @@ -18,13 +18,12 @@ #include #include -#include "rclcpp/rclcpp.hpp" - namespace { constexpr const auto kTransmissionParserLoggerName = "transmission_parser"; constexpr const auto kTransmissionTag = "transmission"; +constexpr const auto kNameTag = "name"; constexpr const auto kJointTag = "joint"; constexpr const auto kActuatorTag = "actuator"; constexpr const auto kTypeTag = "type"; @@ -37,51 +36,26 @@ namespace transmission_interface { std::vector parse_transmissions_from_urdf(const std::string & urdf) -{ - TransmissionParser parser; - std::vector transmissions; - try { - parser.parse(urdf, transmissions); - } catch (const std::runtime_error & err) { - RCLCPP_ERROR( - rclcpp::get_logger(kTransmissionParserLoggerName), - err.what() - ); - throw err; - } - return transmissions; -} - -bool TransmissionParser::parse( - const std::string & urdf, - std::vector & transmissions) { if (urdf.empty()) { - throw std::runtime_error("empty URDF passed to robot"); + throw std::runtime_error("empty URDF passed in to transmission parser"); } - // initialize TiXmlDocument doc with a string tinyxml2::XMLDocument doc; if (!doc.Parse(urdf.c_str()) && doc.Error()) { - throw std::runtime_error("Can't parse transmissions. Invalid robot description."); - } - - // Find joints in transmission tags - tinyxml2::XMLElement * root = doc.RootElement(); - if (root == nullptr) { - throw std::runtime_error("Can't parse transmissions. Invalid robot description."); + throw std::runtime_error("invalid URDF passed in to transmission parser"); } - // Constructs the transmissions by parsing custom xml. + std::vector transmissions; + tinyxml2::XMLElement * root_it = doc.RootElement(); tinyxml2::XMLElement * trans_it = nullptr; - for (trans_it = root->FirstChildElement(kTransmissionTag); trans_it; + for (trans_it = root_it->FirstChildElement(kTransmissionTag); trans_it; trans_it = trans_it->NextSiblingElement(kTransmissionTag)) { transmission_interface::TransmissionInfo transmission; - // Transmission name - if (trans_it->Attribute("name")) { - transmission.name = trans_it->Attribute("name"); + if (trans_it->Attribute(kNameTag)) { + transmission.name = trans_it->Attribute(kNameTag); if (transmission.name.empty()) { throw std::runtime_error("empty name attribute specified for transmission"); } @@ -99,141 +73,125 @@ bool TransmissionParser::parse( throw std::runtime_error( "expected non-empty type element in transmission '" + transmission.name + "'."); } - transmission.control_type = type_child->GetText(); + transmission.type = type_child->GetText(); try { // Load joints - if (!parse_joints(trans_it, transmission.joints)) { - throw std::runtime_error("requires one joint element."); - } - + transmission.joints = parse_joints(trans_it); // Load actuators - if (!parse_actuators(trans_it, transmission.actuators)) { - throw std::runtime_error("requires one actuator element."); - } + transmission.actuators = parse_actuators(trans_it); } catch (const std::runtime_error & ex) { // add the transmission name and rethrow throw std::runtime_error("transmission '" + transmission.name + "' " + ex.what()); } - // Save loaded transmission transmissions.push_back(transmission); - } // end for + } - return !transmissions.empty(); + return transmissions; } -bool TransmissionParser::parse_joints( - tinyxml2::XMLElement * trans_it, - std::vector & joints) +std::vector parse_joints(tinyxml2::XMLElement * trans_it) { + std::vector joints; // Loop through each available joint - tinyxml2::XMLElement * joint_it = nullptr; - for (joint_it = trans_it->FirstChildElement(kJointTag); joint_it; - joint_it = joint_it->NextSiblingElement(kJointTag)) - { - // Create new joint + auto joint_it = trans_it->FirstChildElement(kJointTag); + if (!joint_it) { + throw std::runtime_error("no joint child element found"); + } + for (; joint_it; joint_it = joint_it->NextSiblingElement(kJointTag)) { transmission_interface::JointInfo joint; - // Joint name - if (joint_it->Attribute("name")) { - joint.name = joint_it->Attribute("name"); - if (joint.name.empty()) { - throw std::runtime_error("expected valid joint name attribute."); - } - } else { - throw std::runtime_error("expected name attribute for joint."); + joint.name = joint_it->Attribute(kNameTag); + if (joint.name.empty()) { + throw std::runtime_error("no joint name attribute set"); } - tinyxml2::XMLElement * role_it = joint_it->FirstChildElement(kRoleTag); + const tinyxml2::XMLElement * role_it = joint_it->FirstChildElement(kRoleTag); if (role_it) { joint.role = role_it->GetText() ? role_it->GetText() : std::string(); } - // Hardware interfaces (required) - tinyxml2::XMLElement * hw_iface_it = nullptr; - for (hw_iface_it = joint_it->FirstChildElement(kHardwareInterfaceTag); hw_iface_it; - hw_iface_it = hw_iface_it->NextSiblingElement(kHardwareInterfaceTag)) - { - if (hw_iface_it->GetText()) { - const std::string hw_iface_name = hw_iface_it->GetText(); - joint.hardware_interfaces.push_back(hw_iface_name); - } else { - throw std::runtime_error("expected hardware interface name for joint " + joint.name + '.'); + // Interfaces (required) + auto interface_it = joint_it->FirstChildElement(kHardwareInterfaceTag); + if (!interface_it) { + throw std::runtime_error( + "no hardware interface tag found under transmission joint" + joint.name); + } + + for (; interface_it; interface_it = interface_it->NextSiblingElement(kHardwareInterfaceTag)) { + const std::string interface_name = interface_it->GetText(); + if (interface_name.empty()) { + throw std::runtime_error("no hardware interface specified in joint " + joint.name); } + joint.interfaces.push_back(interface_name); } - if (joint.hardware_interfaces.empty()) { + if (joint.interfaces.empty()) { throw std::runtime_error( "joint " + joint.name + " has no valid hardware interface."); } - // Add joint to vector joints.push_back(joint); } - return !joints.empty(); + return joints; } -bool TransmissionParser::parse_actuators( - tinyxml2::XMLElement * trans_it, - std::vector & actuators) +std::vector parse_actuators(tinyxml2::XMLElement * trans_it) { + std::vector actuators; // Loop through each available actuator - tinyxml2::XMLElement * actuator_it = nullptr; - for (actuator_it = trans_it->FirstChildElement(kActuatorTag); actuator_it; - actuator_it = actuator_it->NextSiblingElement(kActuatorTag)) - { - // Create new actuator + auto actuator_it = trans_it->FirstChildElement(kActuatorTag); + if (!actuator_it) { + throw std::runtime_error("no actuator child element found"); + } + + for (; actuator_it; actuator_it = actuator_it->NextSiblingElement(kActuatorTag)) { transmission_interface::ActuatorInfo actuator; - // Actuator name - if (actuator_it->Attribute("name")) { - actuator.name = actuator_it->Attribute("name"); - if (actuator.name.empty()) { - throw std::runtime_error("expected valid actuator name attribute."); - } - } else { - throw std::runtime_error("expected name attribute for actuator."); + actuator.name = actuator_it->Attribute(kNameTag); + if (actuator.name.empty()) { + throw std::runtime_error("no actuator name attribute set"); } // Hardware interfaces (optional) - tinyxml2::XMLElement * hw_iface_it = nullptr; - for (hw_iface_it = actuator_it->FirstChildElement(kHardwareInterfaceTag); hw_iface_it; - hw_iface_it = hw_iface_it->NextSiblingElement(kHardwareInterfaceTag)) - { - // Skipping empty hardware interface element in actuator - if (hw_iface_it->GetText()) { - const std::string hw_iface_name = hw_iface_it->GetText(); - actuator.hardware_interfaces.push_back(hw_iface_name); - } else { - throw std::runtime_error( - "expected hardware interface name for actuator " + actuator.name + "."); + auto interface_it = actuator_it->FirstChildElement(kHardwareInterfaceTag); + if (!interface_it) { + throw std::runtime_error( + "no hardware interface tag found under transmission actuator" + actuator.name); + } + + for (; interface_it; interface_it = interface_it->NextSiblingElement(kHardwareInterfaceTag)) { + const std::string interface_name = interface_it->GetText(); + if (interface_name.empty()) { + throw std::runtime_error("no hardware interface specified in actuator " + actuator.name); } + actuator.interfaces.push_back(interface_name); } - if (actuator.hardware_interfaces.empty()) { + + if (actuator.interfaces.empty()) { throw std::runtime_error( "actuator " + actuator.name + " has no valid hardware interface."); } // mechanical reduction (optional) actuator.mechanical_reduction = 1; - tinyxml2::XMLElement * mechred_it = nullptr; - for (mechred_it = actuator_it->FirstChildElement(kMechanicalReductionTag); mechred_it; - mechred_it = mechred_it->NextSiblingElement(kMechanicalReductionTag)) - { - // optional tag, so no error if element is empty - if (mechred_it->GetText()) { - const auto value = mechred_it->GetText(); - actuator.mechanical_reduction = atoi(value); + auto mechred_it = actuator_it->FirstChildElement(kMechanicalReductionTag); + for (; mechred_it; mechred_it = mechred_it->NextSiblingElement(kMechanicalReductionTag)) { + // optional tag but if specified, it should not be empty! + const std::string mech_red_str = mechred_it->GetText(); + if (mech_red_str.empty()) { + throw std::runtime_error("mechanical reduction tag was specified without value"); + } else { + actuator.mechanical_reduction = atoi(mech_red_str.c_str()); } } - // Add actuator to vector actuators.push_back(actuator); } - return !actuators.empty(); + return actuators; } } // namespace transmission_interface diff --git a/transmission_interface/test/test_transmission_parser.cpp b/transmission_interface/test/test_transmission_parser.cpp index 78e72b82b6..0506e310da 100644 --- a/transmission_interface/test/test_transmission_parser.cpp +++ b/transmission_interface/test/test_transmission_parser.cpp @@ -16,7 +16,6 @@ #include #include -#include "hardware_interface/control_type.hpp" #include "transmission_interface/transmission_parser.hpp" using namespace ::testing; // NOLINT @@ -383,45 +382,43 @@ class TestTransmissionParser : public Test std::string wrong_urdf_xml_; }; -using transmission_interface::TransmissionParser; +using transmission_interface::parse_transmissions_from_urdf; using transmission_interface::TransmissionInfo; TEST_F(TestTransmissionParser, successfully_parse_valid_urdf) { - TransmissionParser parser; - std::vector transmissions; - EXPECT_TRUE(parser.parse(valid_urdf_xml_, transmissions)); + const auto transmissions = parse_transmissions_from_urdf(valid_urdf_xml_); ASSERT_THAT(transmissions, SizeIs(2)); // first transmission EXPECT_EQ("rrbot_tran1", transmissions[0].name); - EXPECT_EQ("transmission_interface/SimpleTransmission", transmissions[0].control_type); + EXPECT_EQ("transmission_interface/SimpleTransmission", transmissions[0].type); ASSERT_THAT(transmissions[0].joints, SizeIs(1)); - ASSERT_THAT(transmissions[0].joints[0].hardware_interfaces, SizeIs(1)); + ASSERT_THAT(transmissions[0].joints[0].interfaces, SizeIs(1)); EXPECT_EQ("rrbot_joint1", transmissions[0].joints[0].name); - EXPECT_EQ("PositionJointInterface", transmissions[0].joints[0].hardware_interfaces[0]); + EXPECT_EQ("PositionJointInterface", transmissions[0].joints[0].interfaces[0]); ASSERT_THAT(transmissions[0].actuators, SizeIs(1)); - ASSERT_THAT(transmissions[0].actuators[0].hardware_interfaces, SizeIs(1)); + ASSERT_THAT(transmissions[0].actuators[0].interfaces, SizeIs(1)); EXPECT_EQ("rrbot_motor1", transmissions[0].actuators[0].name); - EXPECT_EQ("PositionJointInterface", transmissions[0].actuators[0].hardware_interfaces[0]); + EXPECT_EQ("PositionJointInterface", transmissions[0].actuators[0].interfaces[0]); EXPECT_EQ(1, transmissions[0].actuators[0].mechanical_reduction); // second transmission EXPECT_EQ("rrbot_tran2", transmissions[1].name); - EXPECT_EQ("transmission_interface/SimpleTransmission", transmissions[1].control_type); + EXPECT_EQ("transmission_interface/SimpleTransmission", transmissions[1].type); ASSERT_THAT(transmissions[1].joints, SizeIs(1)); - ASSERT_THAT(transmissions[1].joints[0].hardware_interfaces, SizeIs(1)); + ASSERT_THAT(transmissions[1].joints[0].interfaces, SizeIs(1)); EXPECT_EQ("rrbot_joint2", transmissions[1].joints[0].name); - EXPECT_EQ("VelocityJointInterface", transmissions[1].joints[0].hardware_interfaces[0]); + EXPECT_EQ("VelocityJointInterface", transmissions[1].joints[0].interfaces[0]); ASSERT_THAT(transmissions[1].actuators, SizeIs(1)); - ASSERT_THAT(transmissions[1].actuators[0].hardware_interfaces, SizeIs(1)); + ASSERT_THAT(transmissions[1].actuators[0].interfaces, SizeIs(1)); EXPECT_EQ("rrbot_motor2", transmissions[1].actuators[0].name); - EXPECT_EQ("VelocityJointInterface", transmissions[1].actuators[0].hardware_interfaces[0]); + EXPECT_EQ("VelocityJointInterface", transmissions[1].actuators[0].interfaces[0]); EXPECT_EQ(60, transmissions[1].actuators[0].mechanical_reduction); } @@ -434,17 +431,11 @@ TEST_F(TestTransmissionParser, empty_string_throws_error) TEST_F(TestTransmissionParser, empty_urdf_returns_empty) { - auto transmissions = transmission_interface::parse_transmissions_from_urdf( + const auto transmissions = transmission_interface::parse_transmissions_from_urdf( ""); ASSERT_THAT(transmissions, IsEmpty()); } -TEST_F(TestTransmissionParser, simpler_parse_transmissions_from_urdf_returns_list) -{ - const auto transmissions = transmission_interface::parse_transmissions_from_urdf(valid_urdf_xml_); - ASSERT_THAT(transmissions, SizeIs(2)); -} - TEST_F(TestTransmissionParser, wrong_urdf_throws_error) { EXPECT_THROW( From f2ab949088270fb0a369bf691e09a0869603d1b3 Mon Sep 17 00:00:00 2001 From: Bence Magyar Date: Sat, 24 Oct 2020 16:24:38 +0100 Subject: [PATCH 14/14] No rclcpp --- transmission_interface/CMakeLists.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/transmission_interface/CMakeLists.txt b/transmission_interface/CMakeLists.txt index f5be015e99..8339e99f3f 100644 --- a/transmission_interface/CMakeLists.txt +++ b/transmission_interface/CMakeLists.txt @@ -12,14 +12,13 @@ endif() # find dependencies find_package(ament_cmake REQUIRED) -find_package(rclcpp REQUIRED) find_package(hardware_interface REQUIRED) find_package(tinyxml2_vendor REQUIRED) find_package(TinyXML2 REQUIRED) add_library(transmission_parser SHARED src/transmission_parser.cpp) target_include_directories(transmission_parser PUBLIC include) -ament_target_dependencies(transmission_parser rclcpp hardware_interface tinyxml2_vendor TinyXML2) +ament_target_dependencies(transmission_parser hardware_interface tinyxml2_vendor TinyXML2) ament_export_dependencies(hardware_interface tinyxml2_vendor TinyXML2) target_compile_definitions(transmission_parser PRIVATE "TRANSMISSION_INTERFACE_BUILDING_DLL")