Adding the reserved word "_description" (#394)

Author: facontidavide

include/behaviortree_cpp_v3/basic_types.h

@@ -209,6 +209,12 @@ template <typename T> using Optional = nonstd::expected<T, std::string>;
  * */
 using Result = Optional<void>;
 
+
+const std::unordered_set<std::string> ReservedPortNames =
+{
+    "ID", "name", "_description"
+};
+
 class PortInfo
 {
 
@@ -261,15 +267,20 @@ std::pair<std::string,PortInfo> CreatePort(PortDirection direction,
                                            StringView name,
                                            StringView description = {})
 {
+    auto sname = static_cast<std::string>(name);
+    if( ReservedPortNames.count(sname) != 0 )
+    {
+        throw std::runtime_error("A port can not use a reserved name. See ReservedPortNames");
+    }
+
     std::pair<std::string,PortInfo> out;
 
     if( std::is_same<T, void>::value)
     {
-        out = {static_cast<std::string>(name), PortInfo(direction) };
+        out = {sname, PortInfo(direction) };
     }
     else{
-        out =  {static_cast<std::string>(name), PortInfo(direction, typeid(T),
-                                          GetAnyFromStringFunctor<T>() ) };
+        out =  {sname, PortInfo(direction, typeid(T), GetAnyFromStringFunctor<T>() ) };
     }
     if( !description.empty() )
     {

include/behaviortree_cpp_v3/bt_factory.h

@@ -425,6 +425,10 @@ class BehaviorTreeFactory
     Tree createTree(const std::string& tree_name,
                      Blackboard::Ptr blackboard = Blackboard::create());
 
+    /// Add a description to a specific manifest. This description will be added
+    /// to <TreeNodesModel> with the function writeTreeNodesModelXML()
+    void addDescriptionToManifest(const std::string& node_id, const std::string& description );
+
 private:
     std::unordered_map<std::string, NodeBuilder> builders_;
     std::unordered_map<std::string, TreeNodeManifest> manifests_;

include/behaviortree_cpp_v3/tree_node.h

@@ -35,6 +35,7 @@ struct TreeNodeManifest
     NodeType type;
     std::string registration_ID;
     PortsList ports;
+    std::string description;
 };
 
 typedef std::unordered_map<std::string, std::string> PortsRemapping;

src/bt_factory.cpp

@@ -294,6 +294,16 @@ Tree BehaviorTreeFactory::createTree(const std::string &tree_name, Blackboard::P
     return tree;
 }
 
+void BehaviorTreeFactory::addDescriptionToManifest(const std::string &node_id, const std::string &description)
+{
+    auto it = manifests_.find(node_id);
+    if( it == manifests_.end() )
+    {
+        throw std::runtime_error("addDescriptionToManifest: wrong ID");
+    }
+    it->second.description = description;
+}
+
 void Tree::sleep(std::chrono::system_clock::duration timeout)
 {
     wake_up_->waitFor(timeout);

src/xml_parsing.cpp

@@ -516,7 +516,7 @@ TreeNode::Ptr XMLParser::Pimpl::createNodeFromXML(const XMLElement *element,
         for (const XMLAttribute* att = element->FirstAttribute(); att; att = att->Next())
         {
             const std::string attribute_name = att->Name();
-            if (attribute_name != "ID" && attribute_name != "name")
+            if ( ReservedPortNames.count(attribute_name) == 0 )
             {
                 port_remap[attribute_name] = att->Value();
             }
@@ -661,10 +661,10 @@ void BT::XMLParser::Pimpl::recursivelyCreateTree(const std::string& tree_ID,
 
                 for (const XMLAttribute* attr = element->FirstAttribute(); attr != nullptr; attr = attr->Next())
                 {
-                    if( strcmp(attr->Name(), "__shared_blackboard") == 0  &&
-                        convertFromString<bool>(attr->Value()) == true )
+                    if( StrEqual(attr->Name(), "__shared_blackboard") )
                     {
-                        is_isolated = false;
+                        is_isolated = !convertFromString<bool>(attr->Value());
+                        break;
                     }
                 }
 
@@ -678,11 +678,10 @@ void BT::XMLParser::Pimpl::recursivelyCreateTree(const std::string& tree_ID,
 
                     for (const XMLAttribute* attr = element->FirstAttribute(); attr != nullptr; attr = attr->Next())
                     {
-                        if( strcmp(attr->Name(), "ID") == 0 )
+                        if( ReservedPortNames.count(attr->Name()) == 0 )
                         {
-                            continue;
+                            new_bb->addSubtreeRemapping( attr->Name(), attr->Value() );
                         }
-                        new_bb->addSubtreeRemapping( attr->Name(), attr->Value() );
                     }
                     output_tree.blackboard_stack.emplace_back(new_bb);
                     recursivelyCreateTree( node->name(), output_tree, new_bb, node );
@@ -701,7 +700,7 @@ void BT::XMLParser::Pimpl::recursivelyCreateTree(const std::string& tree_ID,
                     const char* attr_name = attr->Name();
                     const char* attr_value = attr->Value();
 
-                    if( StrEqual(attr_name, "ID") )
+                    if( ReservedPortNames.count(attr->Name()) != 0 )
                     {
                         continue;
                     }
@@ -772,9 +771,8 @@ void XMLParser::Pimpl::getPortsRecursively(const XMLElement *element,
     {
         const char* attr_name = attr->Name();
         const char* attr_value = attr->Value();
-        if( !StrEqual(attr_name, "ID") &&
-            !StrEqual(attr_name, "name") &&
-             TreeNode::isBlackboardPointer(attr_value) )
+        if( ReservedPortNames.count(attr_name) == 0 &&
+            TreeNode::isBlackboardPointer(attr_value) )
         {
             auto port_name = TreeNode::stripBlackboardPointer(attr_value);
             output_ports.push_back( static_cast<std::string>(port_name) );
@@ -870,10 +868,14 @@ std::string writeTreeNodesModelXML(const BehaviorTreeFactory& factory,
             {
                 port_element->SetText( port_info.description().c_str() );
             }
-
             element->InsertEndChild(port_element);
         }
 
+        if(!model.description.empty())
+        {
+            element->SetAttribute("description", model.registration_ID.c_str());
+        }
+
         model_root->InsertEndChild(element);
     }
 

tests/gtest_ports.cpp

@@ -50,6 +50,32 @@ TEST(PortTest, DefaultPorts)
     ASSERT_EQ( status, NodeStatus::SUCCESS );
 }
 
+TEST(PortTest, Descriptions)
+{
+    std::string xml_txt = R"(
+    <root main_tree_to_execute = "MainTree" >
+        <BehaviorTree ID="MainTree" _description="this is my tree" >
+            <Sequence>
+                <NodeWithPorts name="first"  in_port_B="66" _description="this is my action" />
+                <SubTree ID="SubTree" name="second" _description="this is a subtree"/>
+            </Sequence>
+        </BehaviorTree>
+
+        <BehaviorTree ID="SubTree" _description="this is a subtre" >
+            <NodeWithPorts name="third" in_port_B="99" />
+        </BehaviorTree>
+
+    </root>)";
+
+    BehaviorTreeFactory factory;
+    factory.registerNodeType<NodeWithPorts>("NodeWithPorts");
+
+    auto tree = factory.createTreeFromText(xml_txt);
+
+    NodeStatus status = tree.tickRoot();
+    ASSERT_EQ( status, NodeStatus::FAILURE ); // failure because in_port_B="99"
+}
+
 struct MyType
 {
   std::string value;
@@ -126,3 +152,25 @@ TEST(PortTest, EmptyPort)
   ASSERT_EQ( status, NodeStatus::FAILURE );
 }
 
+
+class IllegalPorts: public SyncActionNode
+{
+  public:
+    IllegalPorts(const std::string & name, const NodeConfiguration & config)
+    : SyncActionNode(name, config)
+    { }
+
+    NodeStatus tick() { return NodeStatus::SUCCESS; }
+
+    static PortsList providedPorts()
+    {
+        return { BT::InputPort<std::string>("name") };
+    }
+};
+
+TEST(PortTest, IllegalPorts)
+{
+  BehaviorTreeFactory factory;
+  ASSERT_ANY_THROW(factory.registerNodeType<IllegalPorts>("nope"));
+}
+