Skip to content

Commit fa1003e

Browse files
mergify[bot]mergify[bot]
authored
fix(hardware_interface): include component name in parsing error messages (#3144) (#3161)
1 parent d51c95d commit fa1003e

2 files changed

Lines changed: 133 additions & 17 deletions

File tree

‎hardware_interface/src/component_parser.cpp‎

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,16 @@ std::string get_attribute_value(
117117
attr = element_it->FindAttribute(attribute_name);
118118
if (!attr)
119119
{
120+
const char * name = element_it->Attribute(kNameAttribute);
121+
if (name && std::strlen(name) > 0)
122+
{
123+
throw std::runtime_error(
124+
fmt::format(
125+
FMT_COMPILE("no attribute '{}' in '{}' tag with name '{}'"), attribute_name, tag_name,
126+
name));
127+
}
120128
throw std::runtime_error(
121-
fmt::format(FMT_COMPILE("no attribute {} in {} tag"), attribute_name, tag_name));
129+
fmt::format(FMT_COMPILE("no attribute '{}' in '{}' tag"), attribute_name, tag_name));
122130
}
123131
return ros2_control::strip(element_it->Attribute(attribute_name));
124132
}
@@ -337,7 +345,7 @@ int parse_thread_priority_attribute(const tinyxml2::XMLElement * elem)
337345
* \throws std::runtime_error if a component attribute or tag is not found
338346
*/
339347
std::unordered_map<std::string, std::string> parse_parameters_from_xml(
340-
const tinyxml2::XMLElement * params_it)
348+
const tinyxml2::XMLElement * params_it, const std::string & context_name)
341349
{
342350
std::unordered_map<std::string, std::string> parameters;
343351
const tinyxml2::XMLAttribute * attr;
@@ -348,7 +356,10 @@ std::unordered_map<std::string, std::string> parse_parameters_from_xml(
348356
attr = params_it->FindAttribute(kNameAttribute);
349357
if (!attr)
350358
{
351-
throw std::runtime_error("no parameter name attribute set in param tag");
359+
throw std::runtime_error(
360+
fmt::format(
361+
FMT_COMPILE("no parameter name attribute set in param tag{}"),
362+
context_name.empty() ? "" : " for '" + context_name + "'"));
352363
}
353364
const std::string parameter_name = ros2_control::strip(params_it->Attribute(kNameAttribute));
354365
const std::string parameter_value =
@@ -368,7 +379,7 @@ std::unordered_map<std::string, std::string> parse_parameters_from_xml(
368379
* \throws std::runtime_error if the interfaceType text not set in a tag
369380
*/
370381
hardware_interface::InterfaceInfo parse_interfaces_from_xml(
371-
const tinyxml2::XMLElement * interfaces_it)
382+
const tinyxml2::XMLElement * interfaces_it, const std::string & context_name)
372383
{
373384
hardware_interface::InterfaceInfo interface;
374385

@@ -378,7 +389,7 @@ hardware_interface::InterfaceInfo parse_interfaces_from_xml(
378389

379390
// Optional min/max attributes
380391
std::unordered_map<std::string, std::string> interface_params =
381-
parse_parameters_from_xml(interfaces_it->FirstChildElement(kParamTag));
392+
parse_parameters_from_xml(interfaces_it->FirstChildElement(kParamTag), context_name);
382393
auto interface_param = interface_params.find(kMinTag);
383394
if (interface_param != interface_params.end())
384395
{
@@ -414,7 +425,7 @@ hardware_interface::InterfaceInfo parse_interfaces_from_xml(
414425
const auto * params_it = interfaces_it->FirstChildElement(kParamTag);
415426
if (params_it)
416427
{
417-
interface.parameters = parse_parameters_from_xml(params_it);
428+
interface.parameters = parse_parameters_from_xml(params_it, context_name);
418429
}
419430

420431
interface.data_type = parse_data_type_attribute(interfaces_it);
@@ -466,7 +477,7 @@ ComponentInfo parse_component_from_xml(const tinyxml2::XMLElement * component_it
466477
const auto * command_interfaces_it = component_it->FirstChildElement(kCommandInterfaceTag);
467478
while (command_interfaces_it)
468479
{
469-
InterfaceInfo cmd_info = parse_interfaces_from_xml(command_interfaces_it);
480+
InterfaceInfo cmd_info = parse_interfaces_from_xml(command_interfaces_it, component.name);
470481
cmd_info.enable_limits &= component.enable_limits;
471482
component.command_interfaces.push_back(cmd_info);
472483
command_interfaces_it = command_interfaces_it->NextSiblingElement(kCommandInterfaceTag);
@@ -476,7 +487,7 @@ ComponentInfo parse_component_from_xml(const tinyxml2::XMLElement * component_it
476487
const auto * state_interfaces_it = component_it->FirstChildElement(kStateInterfaceTag);
477488
while (state_interfaces_it)
478489
{
479-
InterfaceInfo state_info = parse_interfaces_from_xml(state_interfaces_it);
490+
InterfaceInfo state_info = parse_interfaces_from_xml(state_interfaces_it, component.name);
480491
state_info.enable_limits &= component.enable_limits;
481492
component.state_interfaces.push_back(state_info);
482493
state_interfaces_it = state_interfaces_it->NextSiblingElement(kStateInterfaceTag);
@@ -486,7 +497,7 @@ ComponentInfo parse_component_from_xml(const tinyxml2::XMLElement * component_it
486497
const auto * params_it = component_it->FirstChildElement(kParamTag);
487498
if (params_it)
488499
{
489-
component.parameters = parse_parameters_from_xml(params_it);
500+
component.parameters = parse_parameters_from_xml(params_it, component.name);
490501
}
491502

492503
return component;
@@ -513,23 +524,25 @@ ComponentInfo parse_complex_component_from_xml(const tinyxml2::XMLElement * comp
513524
const auto * command_interfaces_it = component_it->FirstChildElement(kCommandInterfaceTag);
514525
while (command_interfaces_it)
515526
{
516-
component.command_interfaces.push_back(parse_interfaces_from_xml(command_interfaces_it));
527+
component.command_interfaces.push_back(
528+
parse_interfaces_from_xml(command_interfaces_it, component.name));
517529
command_interfaces_it = command_interfaces_it->NextSiblingElement(kCommandInterfaceTag);
518530
}
519531

520532
// Parse state interfaces
521533
const auto * state_interfaces_it = component_it->FirstChildElement(kStateInterfaceTag);
522534
while (state_interfaces_it)
523535
{
524-
component.state_interfaces.push_back(parse_interfaces_from_xml(state_interfaces_it));
536+
component.state_interfaces.push_back(
537+
parse_interfaces_from_xml(state_interfaces_it, component.name));
525538
state_interfaces_it = state_interfaces_it->NextSiblingElement(kStateInterfaceTag);
526539
}
527540

528541
// Parse parameters
529542
const auto * params_it = component_it->FirstChildElement(kParamTag);
530543
if (params_it)
531544
{
532-
component.parameters = parse_parameters_from_xml(params_it);
545+
component.parameters = parse_parameters_from_xml(params_it, component.name);
533546
}
534547

535548
return component;
@@ -574,7 +587,10 @@ TransmissionInfo parse_transmission_from_xml(const tinyxml2::XMLElement * transm
574587
const auto * type_it = transmission_it->FirstChildElement(kPluginNameTag);
575588
if (!type_it)
576589
{
577-
throw std::runtime_error("Missing <plugin> tag of <transmission> element in your URDF.");
590+
throw std::runtime_error(
591+
fmt::format(
592+
FMT_COMPILE("Missing <plugin> tag of <transmission> element for '{}' in your URDF."),
593+
transmission.name));
578594
}
579595
transmission.type = get_text_for_element(type_it, kPluginNameTag);
580596

@@ -598,7 +614,7 @@ TransmissionInfo parse_transmission_from_xml(const tinyxml2::XMLElement * transm
598614
const auto * params_it = transmission_it->FirstChildElement(kParamTag);
599615
if (params_it)
600616
{
601-
transmission.parameters = parse_parameters_from_xml(params_it);
617+
transmission.parameters = parse_parameters_from_xml(params_it, transmission.name);
602618
}
603619

604620
return transmission;
@@ -692,7 +708,10 @@ HardwareInfo parse_resource_from_xml(
692708
const auto * type_it = ros2_control_child_it->FirstChildElement(kPluginNameTag);
693709
if (!type_it)
694710
{
695-
throw std::runtime_error("Missing <plugin> tag of <hardware> element in your URDF.");
711+
throw std::runtime_error(
712+
fmt::format(
713+
FMT_COMPILE("Missing <plugin> tag of <hardware> element for '{}' in your URDF."),
714+
hardware.name));
696715
}
697716
hardware.hardware_plugin_name =
698717
get_text_for_element(type_it, std::string("hardware ") + kPluginNameTag);
@@ -704,7 +723,7 @@ HardwareInfo parse_resource_from_xml(
704723
const auto * params_it = ros2_control_child_it->FirstChildElement(kParamTag);
705724
if (params_it)
706725
{
707-
hardware.hardware_parameters = parse_parameters_from_xml(params_it);
726+
hardware.hardware_parameters = parse_parameters_from_xml(params_it, hardware.name);
708727
}
709728
}
710729
else if (std::string(kPropertiesTag) == ros2_control_child_it->Name())
@@ -987,7 +1006,8 @@ std::vector<HardwareInfo> parse_control_resources_from_urdf(const std::string &
9871006

9881007
if (!ros2_control_it)
9891008
{
990-
throw std::runtime_error(fmt::format(FMT_COMPILE("no {} tag"), kROS2ControlTag));
1009+
throw std::runtime_error(
1010+
fmt::format(FMT_COMPILE("no '{}' tag found in the URDF"), kROS2ControlTag));
9911011
}
9921012

9931013
std::vector<HardwareInfo> hardware_info;

‎hardware_interface/test/test_component_parser.cpp‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,3 +2032,99 @@ TEST_F(TestComponentParser, successfully_parse_valid_sdf)
20322032
EXPECT_EQ(hardware_info.joints[1].state_interfaces[0].name, HW_IF_VELOCITY);
20332033
EXPECT_EQ(hardware_info.joints[1].state_interfaces[1].name, HW_IF_POSITION);
20342034
}
2035+
2036+
TEST_F(TestComponentParser, missing_hardware_plugin_tag_includes_component_name)
2037+
{
2038+
const std::string broken_urdf =
2039+
R"(
2040+
<?xml version="1.0"?>
2041+
<robot name="robot">
2042+
<ros2_control name="MySpecificRobot" type="system">
2043+
<hardware>
2044+
<!-- plugin tag intentionally missing -->
2045+
<param name="some_param">1.0</param>
2046+
</hardware>
2047+
</ros2_control>
2048+
</robot>
2049+
)";
2050+
2051+
try
2052+
{
2053+
parse_control_resources_from_urdf(broken_urdf);
2054+
FAIL() << "Should have thrown std::runtime_error";
2055+
}
2056+
catch (const std::runtime_error & e)
2057+
{
2058+
EXPECT_THAT(std::string(e.what()), HasSubstr("MySpecificRobot"));
2059+
EXPECT_THAT(std::string(e.what()), HasSubstr("<plugin>"));
2060+
EXPECT_THAT(std::string(e.what()), HasSubstr("<hardware>"));
2061+
}
2062+
}
2063+
2064+
TEST_F(TestComponentParser, missing_joint_attribute_includes_joint_name)
2065+
{
2066+
const std::string broken_urdf =
2067+
R"(
2068+
<?xml version="1.0"?>
2069+
<robot name="robot">
2070+
<ros2_control name="RRBot" type="system">
2071+
<hardware>
2072+
<plugin>some_plugin</plugin>
2073+
</hardware>
2074+
<joint name="joint1">
2075+
<command_interface name="position"/>
2076+
</joint>
2077+
<joint name="joint2">
2078+
<!-- missing name attribute here is tricky since we parse it first,
2079+
let's test missing attribute in command_interface instead -->
2080+
<command_interface>
2081+
<param name="min">-1</param>
2082+
</command_interface>
2083+
</joint>
2084+
</ros2_control>
2085+
</robot>
2086+
)";
2087+
2088+
try
2089+
{
2090+
parse_control_resources_from_urdf(broken_urdf);
2091+
FAIL() << "Should have thrown std::runtime_error";
2092+
}
2093+
catch (const std::runtime_error & e)
2094+
{
2095+
EXPECT_THAT(std::string(e.what()), HasSubstr("name"));
2096+
EXPECT_THAT(std::string(e.what()), HasSubstr("command_interface"));
2097+
// Ideally it should say it failed for joint2, but tag_name is "command_interface"
2098+
}
2099+
}
2100+
2101+
TEST_F(TestComponentParser, parameter_missing_name_includes_parent_context)
2102+
{
2103+
const std::string broken_urdf =
2104+
R"(
2105+
<?xml version="1.0"?>
2106+
<robot name="robot">
2107+
<ros2_control name="RRBot" type="system">
2108+
<hardware>
2109+
<plugin>some_plugin</plugin>
2110+
</hardware>
2111+
<joint name="joint1">
2112+
<command_interface name="position">
2113+
<param>1.0</param> <!-- Missing name attribute -->
2114+
</command_interface>
2115+
</joint>
2116+
</ros2_control>
2117+
</robot>
2118+
)";
2119+
2120+
try
2121+
{
2122+
parse_control_resources_from_urdf(broken_urdf);
2123+
FAIL() << "Should have thrown std::runtime_error";
2124+
}
2125+
catch (const std::runtime_error & e)
2126+
{
2127+
EXPECT_THAT(std::string(e.what()), HasSubstr("joint1"));
2128+
EXPECT_THAT(std::string(e.what()), HasSubstr("parameter name"));
2129+
}
2130+
}

0 commit comments

Comments
 (0)