Precondition Node should run children to completion before checking it's if statement again

CMakeLists.txt

@@ -165,7 +165,7 @@ target_link_libraries(${BTCPP_LIBRARY}
         Threads::Threads
         ${CMAKE_DL_LIBS}
         $<BUILD_INTERFACE:foonathan::lexy>
-        minitrace
+        $<BUILD_INTERFACE:minitrace>
     PUBLIC
         ${BTCPP_EXTRA_LIBRARIES}
 )

examples/t09_scripting.cpp

@@ -10,7 +10,7 @@ static const char* xml_text = R"(
         <Sequence>
             <Script code=" msg:='hello world' " />
             <Script code=" A:=THE_ANSWER; B:=3.14; color:=RED " />
-            <Precondition if="A>B && color != BLUE" else="FAILURE">
+            <Precondition if="A>-B && color != BLUE" else="FAILURE">
                 <Sequence>
                   <SaySomething message="{A}"/>
                   <SaySomething message="{B}"/>

examples/t11_groot_howto.cpp

@@ -120,7 +120,7 @@ int main()
   // Both formats are compatible with Groot2
 
   // Logging with lightweight serialization
-  BT::FileLogger2 logger2(tree, "t12_logger2.btlog");
+  BT::FileLogger2 logger2(tree, "t11_groot_howto.btlog");
   BT::MinitraceLogger minilog(tree, "minitrace.json");
 
   while(1)

include/behaviortree_cpp/bt_factory.h

@@ -118,10 +118,14 @@ class Tree
 
   [[nodiscard]] TreeNode* rootNode() const;
 
-  /// Sleep for a certain amount of time.
-  /// This sleep could be interrupted by the method
-  /// TreeNode::emitWakeUpSignal()
-  void sleep(std::chrono::system_clock::duration timeout);
+  /**
+    * @brief Sleep for a certain amount of time. This sleep could be interrupted by the method TreeNode::emitWakeUpSignal()
+    *
+    * @param timeout  duration of the sleep
+    * @return         true if the timeout was NOT reached and the signal was received.
+    *
+    * */
+  bool sleep(std::chrono::system_clock::duration timeout);
 
   ~Tree();
 

include/behaviortree_cpp/decorators/script_precondition.h

@@ -48,20 +48,27 @@ class PreconditionNode : public DecoratorNode
       throw RuntimeError("Missing parameter [else] in Precondition");
     }
 
-    Ast::Environment env = { config().blackboard, config().enums };
-    if(_executor(env).cast<bool>())
+    // Only check the 'if' script if we haven't started ticking the children yet.
+    bool tick_children = _children_running;
+    if(!_children_running)
     {
-      auto const child_status = child_node_->executeTick();
-      if(isStatusCompleted(child_status))
-      {
-        resetChild();
-      }
-      return child_status;
+      Ast::Environment env = { config().blackboard, config().enums };
+      tick_children = _executor(env).cast<bool>();
+      _children_running = tick_children;
     }
-    else
+
+    if(!tick_children)
     {
       return else_return;
     }
+
+    auto const child_status = child_node_->executeTick();
+    if(isStatusCompleted(child_status))
+    {
+      resetChild();
+      _children_running = false;
+    }
+    return child_status;
   }
 
   void loadExecutor()
@@ -89,6 +96,7 @@ class PreconditionNode : public DecoratorNode
 
   std::string _script;
   ScriptFunction _executor;
+  bool _children_running = false;
 };
 
 }  // namespace BT

include/behaviortree_cpp/scripting/operators.hpp

@@ -796,9 +796,9 @@ struct Expression : lexy::expression_production
                                dsl::op<Ast::ExprComparison::greater_equal>(LEXY_LIT(">"
                                                                                     "="));
 
-    // The use of dsl::groups ensures that an expression can either contain math or bit
+    // The use of dsl::groups ensures that an expression can either contain math or bit or string
     // operators. Mixing requires parenthesis.
-    using operand = dsl::groups<math_sum, bit_or>;
+    using operand = dsl::groups<math_sum, bit_or, string_concat>;
   };
 
   // Logical operators,  || and &&
@@ -808,7 +808,7 @@ struct Expression : lexy::expression_production
         dsl::op<Ast::ExprBinaryArithmetic::logic_or>(LEXY_LIT("||")) /
         dsl::op<Ast::ExprBinaryArithmetic::logic_and>(LEXY_LIT("&&"));
 
-    using operand = dsl::groups<string_concat, comparison>;
+    using operand = comparison;
   };
 
   // x ? y : z

src/bt_factory.cpp

@@ -586,9 +586,10 @@ TreeNode* Tree::rootNode() const
   return subtree_nodes.empty() ? nullptr : subtree_nodes.front().get();
 }
 
-void Tree::sleep(std::chrono::system_clock::duration timeout)
+bool Tree::sleep(std::chrono::system_clock::duration timeout)
 {
-  wake_up_->waitFor(std::chrono::duration_cast<std::chrono::milliseconds>(timeout));
+  return wake_up_->waitFor(
+      std::chrono::duration_cast<std::chrono::milliseconds>(timeout));
 }
 
 Tree::~Tree()

src/xml_parsing.cpp

@@ -541,6 +541,29 @@ void VerifyXML(const std::string& xml_text,
           ThrowError(line_number,
                      std::string("The node <") + name + "> must have 1 or more children");
         }
+        if(name == "ReactiveSequence")
+        {
+          size_t async_count = 0;
+          for(auto child = node->FirstChildElement(); child != nullptr;
+              child = child->NextSiblingElement())
+          {
+            const std::string child_name = node->FirstChildElement()->Name();
+            const auto child_search = registered_nodes.find(child_name);
+            const auto child_type = child_search->second;
+            if(child_type == NodeType::CONTROL &&
+               ((child_name == "ThreadedAction") ||
+                (child_name == "StatefulActionNode") ||
+                (child_name == "CoroActionNode") || (child_name == "AsyncSequence")))
+            {
+              ++async_count;
+              if(async_count > 1)
+              {
+                ThrowError(line_number, std::string("A ReactiveSequence cannot have more "
+                                                    "than one async child."));
+              }
+            }
+          }
+        }
       }
     }
     //recursion
@@ -667,8 +690,10 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
         if(port_model_it == manifest->ports.end())
         {
           throw RuntimeError(StrCat("a port with name [", port_name,
-                                    "] is found in the XML, but not in the "
-                                    "providedPorts()"));
+                                    "] is found in the XML (<", element->Name(),
+                                    ">, line ", std::to_string(att->GetLineNum()),
+                                    ") but not in the providedPorts() of its "
+                                    "registered node type."));
         }
         else
         {

tests/gtest_preconditions.cpp

@@ -107,6 +107,111 @@ TEST(PreconditionsDecorator, StringEquals)
   ASSERT_EQ(counters[1], 1);
 }
 
+class KeepRunning : public BT::StatefulActionNode
+{
+public:
+  KeepRunning(const std::string& name, const BT::NodeConfig& config)
+    : BT::StatefulActionNode(name, config)
+  {}
+
+  static BT::PortsList providedPorts()
+  {
+    return {};
+  }
+
+  BT::NodeStatus onStart() override
+  {
+    return BT::NodeStatus::RUNNING;
+  }
+
+  BT::NodeStatus onRunning() override
+  {
+    return BT::NodeStatus::RUNNING;
+  }
+
+  void onHalted() override
+  {
+    std::cout << "Node halted\n";
+  }
+};
+
+TEST(PreconditionsDecorator, ChecksConditionOnce)
+{
+  BehaviorTreeFactory factory;
+  factory.registerNodeType<KeepRunning>("KeepRunning");
+
+  const std::string xml_text = R"(
+
+    <root BTCPP_format="4" >
+        <BehaviorTree ID="MainTree">
+            <Sequence>
+                <Script code = "A:=0" />
+                <Script code = "B:=0" />
+                <Precondition if=" A==0 " else="FAILURE">
+                    <KeepRunning _while="B==0" />
+                </Precondition>
+            </Sequence>
+        </BehaviorTree>
+    </root>)";
+
+  auto tree = factory.createTreeFromText(xml_text);
+
+  EXPECT_EQ(tree.tickOnce(), NodeStatus::RUNNING);
+  // While the child is still running, attempt to fail the precondition.
+  tree.rootBlackboard()->set("A", 1);
+  EXPECT_EQ(tree.tickOnce(), NodeStatus::RUNNING);
+  // Finish running the tree, the else condition should not be hit.
+  tree.rootBlackboard()->set("B", 1);
+  EXPECT_EQ(tree.tickOnce(), NodeStatus::SUCCESS);
+}
+
+TEST(PreconditionsDecorator, CanRunChildrenMultipleTimes)
+{
+  BehaviorTreeFactory factory;
+  factory.registerNodeType<KeepRunning>("KeepRunning");
+  std::array<int, 1> counters;
+  RegisterTestTick(factory, "Test", counters);
+
+  const std::string xml_text = R"(
+
+    <root BTCPP_format="4" >
+        <BehaviorTree ID="MainTree">
+            <Sequence>
+                <Script code = "A:=0" />
+                <Script code = "B:=0" />
+                <Script code = "C:=1" />
+                <Repeat num_cycles="3">
+                    <Sequence>
+                        <Precondition if=" A==0 " else="SUCCESS">
+                            <TestA/>
+                        </Precondition>
+                        <KeepRunning _while="C==0" />
+                        <KeepRunning _while="B==0" />
+                    </Sequence>
+                </Repeat>
+            </Sequence>
+        </BehaviorTree>
+    </root>)";
+
+  auto tree = factory.createTreeFromText(xml_text);
+
+  EXPECT_EQ(tree.tickOnce(), NodeStatus::RUNNING);
+  EXPECT_EQ(counters[0], 1);  // Precondition hit once;
+
+  // In the second repeat, fail the precondition
+  tree.rootBlackboard()->set("A", 1);
+  tree.rootBlackboard()->set("B", 1);
+  tree.rootBlackboard()->set("C", 0);
+  EXPECT_EQ(tree.tickOnce(), NodeStatus::RUNNING);
+  EXPECT_EQ(counters[0], 1);  // Precondition still only hit once.
+
+  // Finally in the last repeat, hit the condition again.
+  tree.rootBlackboard()->set("A", 0);
+  tree.rootBlackboard()->set("C", 1);
+  EXPECT_EQ(tree.tickOnce(), NodeStatus::SUCCESS);
+  EXPECT_EQ(counters[0], 2);  // Precondition hit twice now.
+}
+
 TEST(Preconditions, Basic)
 {
   BehaviorTreeFactory factory;
@@ -246,34 +351,6 @@ TEST(Preconditions, Issue615_NoSkipWhenRunning_A)
   ASSERT_EQ(tree.tickOnce(), NodeStatus::RUNNING);
 }
 
-class KeepRunning : public BT::StatefulActionNode
-{
-public:
-  KeepRunning(const std::string& name, const BT::NodeConfig& config)
-    : BT::StatefulActionNode(name, config)
-  {}
-
-  static BT::PortsList providedPorts()
-  {
-    return {};
-  }
-
-  BT::NodeStatus onStart() override
-  {
-    return BT::NodeStatus::RUNNING;
-  }
-
-  BT::NodeStatus onRunning() override
-  {
-    return BT::NodeStatus::RUNNING;
-  }
-
-  void onHalted() override
-  {
-    std::cout << "Node halted\n";
-  }
-};
-
 TEST(Preconditions, Issue615_NoSkipWhenRunning_B)
 {
   static constexpr auto xml_text = R"(

tests/gtest_reactive.cpp

@@ -156,3 +156,31 @@ TEST(Reactive, TestLogging)
   ASSERT_EQ(observer.getStatistics("testA").success_count, num_ticks);
   ASSERT_EQ(observer.getStatistics("success").success_count, num_ticks);
 }
+
+TEST(Reactive, TwoAsyncNodesInReactiveSequence)
+{
+  static const char* reactive_xml_text = R"(
+<root BTCPP_format="4" >
+  <BehaviorTree ID="MainTree">
+    <ReactiveSequence>
+      <AsyncSequence name="first">
+        <TestA/>
+        <TestB/>
+        <TestC/>
+      </AsyncSequence>
+      <AsyncSequence name="second">
+        <TestD/>
+        <TestE/>
+        <TestF/>
+      </AsyncSequence>
+    </ReactiveSequence>
+  </BehaviorTree>
+</root>
+)";
+
+  BT::BehaviorTreeFactory factory;
+  std::array<int, 6> counters;
+  RegisterTestTick(factory, "Test", counters);
+
+  EXPECT_ANY_THROW(auto tree = factory.createTreeFromText(reactive_xml_text));
+}