Merge pull request #691 from galou/small_refactor_and_doc

Small code refactor, log- and doc changes

examples/t04_reactive_sequence.cpp

@@ -61,11 +61,11 @@ int main()
   factory.registerNodeType<SaySomething>("SaySomething");
 
   // Compare the state transitions and messages using either
-  // xml_text_sequence and xml_text_sequence_star
+  // xml_text_sequence and xml_text_reactive.
 
   // The main difference that you should notice is:
-  //  1) When Sequence is used, BatteryOK is executed at __each__ tick()
-  //  2) When SequenceStar is used, those ConditionNodes are executed only __once__.
+  //  1) When Sequence is used, the ConditionNode is executed only __once__ because it returns SUCCESS.
+  //  2) When ReaciveSequence is used, BatteryOK is executed at __each__ tick()
 
   for (auto& xml_text : {xml_text_sequence, xml_text_reactive})
   {

include/behaviortree_cpp/controls/parallel_node.h

@@ -47,10 +47,10 @@ class ParallelNode : public ControlNode
   static PortsList providedPorts()
   {
     return {InputPort<int>(THRESHOLD_SUCCESS, -1,
-                           "number of children which need to succeed to trigger a "
+                           "number of children that need to succeed to trigger a "
                            "SUCCESS"),
             InputPort<int>(THRESHOLD_FAILURE, 1,
-                           "number of children which need to fail to trigger a FAILURE")};
+                           "number of children that need to fail to trigger a FAILURE")};
   }
 
   ~ParallelNode() override = default;

src/xml_parsing.cpp

@@ -558,8 +558,8 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
                                                   const std::string& prefix_path,
                                                   Tree& output_tree)
 {
-  auto element_name = element->Name();
-  auto element_ID = element->Attribute("ID");
+  const auto element_name = element->Name();
+  const auto element_ID = element->Attribute("ID");
 
   auto node_type = convertFromString<NodeType>(element_name);
   // name used by the factory
@@ -593,7 +593,7 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
   // By default, the instance name is equal to ID, unless the
   // attribute [name] is present.
   const char* attr_name = element->Attribute("name");
-  std::string instance_name = attr_name ? attr_name : type_ID;
+  const std::string instance_name = (attr_name != nullptr) ? attr_name : type_ID;
 
   const TreeNodeManifest* manifest = nullptr;
 
@@ -663,13 +663,13 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
     }
 
     //Check that name in remapping can be found in the manifest
-    for (const auto& remap_it : port_remap)
+    for (const auto& [name_in_subtree, _] : port_remap)
     {
-      if (manifest->ports.count(remap_it.first) == 0)
+      if (manifest->ports.count(name_in_subtree) == 0)
       {
         throw RuntimeError("Possible typo? In the XML, you tried to remap port \"",
-                           remap_it.first, "\" in node [", type_ID, " / ", instance_name,
-                           "], but the manifest of this node does not contain a port "
+                           name_in_subtree, "\" in node [", config.path, "(type ", type_ID,
+                           ")], but the manifest of this node does not contain a port "
                            "with this name.");
       }
     }
@@ -760,13 +760,13 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
   }
 
   // add the pointer of this node to the parent
-  if (node_parent)
+  if (node_parent != nullptr)
   {
     if (auto control_parent = dynamic_cast<ControlNode*>(node_parent.get()))
     {
       control_parent->addChild(new_node.get());
     }
-    if (auto decorator_parent = dynamic_cast<DecoratorNode*>(node_parent.get()))
+    else if (auto decorator_parent = dynamic_cast<DecoratorNode*>(node_parent.get()))
     {
       decorator_parent->setChild(new_node.get());
     }