Add "other ports" to NodeConfig (#910)

Author: facontidavide

include/behaviortree_cpp/basic_types.h

@@ -342,6 +342,8 @@ struct Timestamp
 
 [[nodiscard]] bool IsAllowedPortName(StringView str);
 
+[[nodiscard]] bool IsReservedAttribute(StringView str);
+
 class TypeInfo
 {
 public:

include/behaviortree_cpp/tree_node.h

@@ -41,6 +41,7 @@ struct TreeNodeManifest
 };
 
 using PortsRemapping = std::unordered_map<std::string, std::string>;
+using NonPortAttributes = std::unordered_map<std::string, std::string>;
 
 enum class PreCond
 {
@@ -52,6 +53,10 @@ enum class PreCond
   COUNT_
 };
 
+static const std::array<std::string, 4> PreCondNames = {  //
+  "_failureIf", "_successIf", "_skipIf", "_while"
+};
+
 enum class PostCond
 {
   // order of the enums also tell us the execution order
@@ -62,11 +67,15 @@ enum class PostCond
   COUNT_
 };
 
+static const std::array<std::string, 4> PostCondNames = {  //
+  "_onHalted", "_onFailure", "_onSuccess", "_post"
+};
+
 template <>
-[[nodiscard]] std::string toStr<BT::PostCond>(const BT::PostCond& status);
+[[nodiscard]] std::string toStr<BT::PostCond>(const BT::PostCond& cond);
 
 template <>
-[[nodiscard]] std::string toStr<BT::PreCond>(const BT::PreCond& status);
+[[nodiscard]] std::string toStr<BT::PreCond>(const BT::PreCond& cond);
 
 using ScriptingEnumsRegistry = std::unordered_map<std::string, int>;
 
@@ -84,6 +93,10 @@ struct NodeConfig
   // output ports
   PortsRemapping output_ports;
 
+  // Any other attributes found in the xml that are not parsed as ports
+  // or built-in identifier (e.g. anything with a leading '_')
+  NonPortAttributes other_attributes;
+
   const TreeNodeManifest* manifest = nullptr;
 
   // Numberic unique identifier

src/basic_types.cpp

@@ -1,4 +1,5 @@
 #include "behaviortree_cpp/basic_types.h"
+#include "behaviortree_cpp/tree_node.h"
 #include "behaviortree_cpp/json_export.h"
 
 #include <cstdlib>
@@ -420,10 +421,6 @@ const std::string& PortInfo::defaultValueString() const
 
 bool IsAllowedPortName(StringView str)
 {
-  if(str == "_autoremap")
-  {
-    return true;
-  }
   if(str.empty())
   {
     return false;
@@ -433,11 +430,26 @@ bool IsAllowedPortName(StringView str)
   {
     return false;
   }
-  if(str == "name" || str == "ID")
+  return !IsReservedAttribute(str);
+}
+
+bool IsReservedAttribute(StringView str)
+{
+  for(const auto& name : PreCondNames)
   {
-    return false;
+    if(name == str)
+    {
+      return true;
+    }
+  }
+  for(const auto& name : PostCondNames)
+  {
+    if(name == str)
+    {
+      return true;
+    }
   }
-  return true;
+  return str == "name" || str == "ID" || str == "_autoremap";
 }
 
 Any convertFromJSON(StringView json_text, std::type_index type)

src/tree_node.cpp

@@ -464,39 +464,23 @@ void TreeNode::modifyPortsRemapping(const PortsRemapping& new_remapping)
 }
 
 template <>
-std::string toStr<PreCond>(const PreCond& pre)
+std::string toStr<PreCond>(const PreCond& cond)
 {
-  switch(pre)
+  if(cond < PreCond::COUNT_)
   {
-    case PreCond::SUCCESS_IF:
-      return "_successIf";
-    case PreCond::FAILURE_IF:
-      return "_failureIf";
-    case PreCond::SKIP_IF:
-      return "_skipIf";
-    case PreCond::WHILE_TRUE:
-      return "_while";
-    default:
-      return "Undefined";
+    return BT::PreCondNames[static_cast<size_t>(cond)];
   }
+  return "Undefined";
 }
 
 template <>
-std::string toStr<PostCond>(const PostCond& pre)
+std::string toStr<PostCond>(const PostCond& cond)
 {
-  switch(pre)
+  if(cond < BT::PostCond::COUNT_)
   {
-    case PostCond::ON_SUCCESS:
-      return "_onSuccess";
-    case PostCond::ON_FAILURE:
-      return "_onFailure";
-    case PostCond::ALWAYS:
-      return "_post";
-    case PostCond::ON_HALTED:
-      return "_onHalted";
-    default:
-      return "Undefined";
+    return BT::PostCondNames[static_cast<size_t>(cond)];
   }
+  return "Undefined";
 }
 
 AnyPtrLocked BT::TreeNode::getLockedPortContent(const std::string& key)

src/xml_parsing.cpp

@@ -18,6 +18,7 @@
 #include <sstream>
 #include <string>
 #include <typeindex>
+#include "behaviortree_cpp/basic_types.h"
 
 #if defined(_MSVC_LANG) && !defined(__clang__)
 #define __bt_cplusplus (_MSC_VER == 1900 ? 201103L : _MSVC_LANG)
@@ -677,9 +678,13 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
   }
 
   PortsRemapping port_remap;
+  NonPortAttributes other_attributes;
+
   for(const XMLAttribute* att = element->FirstAttribute(); att; att = att->Next())
   {
-    if(IsAllowedPortName(att->Name()))
+    const std::string port_name = att->Name();
+    const std::string port_value = att->Value();
+    if(IsAllowedPortName(port_name))
     {
       const std::string port_name = att->Name();
       const std::string port_value = att->Value();
@@ -721,6 +726,10 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
 
       port_remap[port_name] = port_value;
     }
+    else if(!IsReservedAttribute(port_name))
+    {
+      other_attributes[port_name] = port_value;
+    }
   }
 
   NodeConfig config;
@@ -738,6 +747,7 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
     if(auto script = element->Attribute(attr_name))
     {
       conditions.insert({ ID, std::string(script) });
+      other_attributes.erase(attr_name);
     }
   };
 
@@ -752,6 +762,7 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
     AddCondition(config.post_conditions, toStr(post).c_str(), post);
   }
 
+  config.other_attributes = other_attributes;
   //---------------------------------------------
   TreeNode::Ptr new_node;
 

tests/gtest_ports.cpp

@@ -1,4 +1,5 @@
 #include <gtest/gtest.h>
+#include "behaviortree_cpp/basic_types.h"
 #include "behaviortree_cpp/bt_factory.h"
 #include "behaviortree_cpp/xml_parsing.h"
 #include "behaviortree_cpp/json_export.h"
@@ -129,6 +130,30 @@ TEST(PortTest, Descriptions)
   ASSERT_EQ(status, NodeStatus::FAILURE);  // failure because in_port_B="99"
 }
 
+TEST(PortsTest, NonPorts)
+{
+  std::string xml_txt =
+      R"(
+    <root BTCPP_format="4" >
+        <BehaviorTree ID="MainTree">
+            <Action ID="NodeWithPorts" name="NodeWithPortsName" in_port_B="66" _not_da_port="whateva" _skipIf="true" />
+        </BehaviorTree>
+    </root>)";
+
+  BehaviorTreeFactory factory;
+  factory.registerNodeType<NodeWithPorts>("NodeWithPorts");
+
+  auto tree = factory.createTreeFromText(xml_txt);
+
+  const TreeNode* root = tree.rootNode();
+  ASSERT_NE(root, nullptr);
+  ASSERT_EQ(root->type(), NodeType::ACTION);
+
+  EXPECT_EQ(root->config().other_attributes.size(), 1);
+  ASSERT_TRUE(root->config().other_attributes.contains("_not_da_port"));
+  EXPECT_EQ(root->config().other_attributes.at("_not_da_port"), "whateva");
+}
+
 struct MyType
 {
   std::string value;