diff --git a/src/xml_parsing.cpp b/src/xml_parsing.cpp
index d15fd0a26..7c6c91e68 100644
--- a/src/xml_parsing.cpp
+++ b/src/xml_parsing.cpp
@@ -900,6 +900,29 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
}
else if(!IsReservedAttribute(port_name))
{
+ // A name that IsAllowedPortName refuses can still be a declared port.
+ // validatePortName only rejects a leading digit, so a SubTree model may
+ // declare ``, which no C++ InputPort() can
+ // ever bind. Falling straight through to other_attributes drops
+ // `_myPort="{value}"` without a word and leaves the node reading its
+ // default. Report it rather than ignoring the remapping in silence.
+ bool declared_port = manifest != nullptr && manifest->ports.count(port_name) > 0;
+ if(!declared_port && node_type == NodeType::SUBTREE)
+ {
+ auto model_it = subtree_models.find(type_ID);
+ declared_port = model_it != subtree_models.end() &&
+ model_it->second.ports.count(port_name) > 0;
+ }
+ if(declared_port)
+ {
+ const std::string reason = " and is declared, but the name cannot be used"
+ " as a port. A port name must begin with an"
+ " alphabetic character. Rename the port, for"
+ " example [_my_port] to [my_port].";
+ throw RuntimeError(StrCat("A port with name [", port_name,
+ "] is found in the XML (", type_ID, ", line ",
+ std::to_string(element->GetLineNum()), ")", reason));
+ }
other_attributes[port_name] = port_value;
}
}
diff --git a/tests/gtest_name_validation.cpp b/tests/gtest_name_validation.cpp
index e09987f2a..c2535508f 100644
--- a/tests/gtest_name_validation.cpp
+++ b/tests/gtest_name_validation.cpp
@@ -414,3 +414,110 @@ TEST_F(NameValidationXMLTest, InvalidSubTreePortName_StartsWithDigit)
)";
EXPECT_THROW(factory.createTreeFromText(xml), RuntimeError);
}
+
+// A SubTree model may declare a port whose name IsAllowedPortName refuses.
+// validatePortName only rejects a leading digit, so `_myPort` is accepted as a
+// declaration, but the instance attribute that would remap it is diverted to
+// other_attributes and the node reads its default instead. Silently.
+TEST_F(NameValidationXMLTest, RemappingADeclaredUnusablePortNameThrows)
+{
+ const char* xml = R"(
+
+
+
+
+
+
+
+
+
+
+
+
+ )";
+ try
+ {
+ factory.createTreeFromText(xml);
+ FAIL() << "expected a RuntimeError";
+ }
+ catch(const RuntimeError& err)
+ {
+ const std::string msg = err.what();
+ EXPECT_NE(msg.find("_myPort"), std::string::npos) << msg;
+ EXPECT_NE(msg.find("my_port"), std::string::npos) << msg;
+ }
+}
+
+// The same underscore attribute on a SubTree that does not declare it is UI
+// state, not a port. moveit_pro_example_ws carries 148 of these. Keep loading.
+TEST_F(NameValidationXMLTest, UndeclaredUnderscoreAttributeIsStillIgnored)
+{
+ const char* xml = R"(
+
+
+
+
+
+
+
+
+
+
+
+
+ )";
+ EXPECT_NO_THROW(factory.createTreeFromText(xml));
+}
+
+// A declared-but-unusable port that nobody remaps keeps its default and keeps
+// loading. This shape ships in moveit_pro_example_ws.
+TEST_F(NameValidationXMLTest, DeclaredUnusablePortNameLoadsWhenNotRemapped)
+{
+ const char* xml = R"(
+
+
+
+
+
+
+
+
+
+
+
+
+ )";
+ EXPECT_NO_THROW(factory.createTreeFromText(xml));
+}
+
+namespace
+{
+struct UnusablePortAction : public SyncActionNode
+{
+ UnusablePortAction(const std::string& name, const NodeConfig& config)
+ : SyncActionNode(name, config)
+ {}
+ NodeStatus tick() override
+ {
+ return NodeStatus::SUCCESS;
+ }
+};
+} // namespace
+
+// The same trap reaches a registered node, not only a SubTree. `CreatePort`
+// screens names through IsAllowedPortName, but the PortsList overload of
+// registerNodeType takes the map as given, so a hand-built list can carry a key
+// no instance attribute will ever bind.
+TEST_F(NameValidationXMLTest, RemappingADeclaredUnusablePortOnARegisteredNodeThrows)
+{
+ factory.registerNodeType(
+ "UnusablePortAction", PortsList{ { "_foo", PortInfo(PortDirection::INPUT) } });
+
+ const char* xml = R"(
+
+
+
+
+ )";
+ EXPECT_THROW(factory.createTreeFromText(xml), RuntimeError);
+}