preparing release 3.2.0

Author: facontidavide

CHANGELOG.rst

@@ -2,8 +2,8 @@
 Changelog for package behaviortree_cpp
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-Forthcoming
------------
+3.2.0 (2020-03-22)
+------------------
 * root_node removed in favour of a method. tickRoot() added
 * Moving to c++14
 * fixed compilation on ROS2 and ubuntu 18.94

CMakeLists.txt

@@ -5,7 +5,7 @@ project(behaviortree_cpp_v3)
 set(CMAKE_CONFIG_PATH ${CMAKE_MODULE_PATH}  "${CMAKE_CURRENT_LIST_DIR}/cmake")
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_CONFIG_PATH}")
 
-#---- Enable C++11 ----
+#---- Enable C++14 ----
 set(CMAKE_CXX_STANDARD 14)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 

conan/test_package/test_package.cpp

@@ -62,7 +62,7 @@ int main()
         std::this_thread::sleep_for(std::chrono::milliseconds(100));
     }
 
-    haltAllActions(&root);
+    
 
     return 0;
 }

examples/broken_sequence.cpp

@@ -63,7 +63,7 @@ int main()
         std::this_thread::sleep_for(std::chrono::milliseconds(100));
     }
 
-    haltAllActions(&root);
+    
 
     return 0;
 }

include/behaviortree_cpp_v3/action_node.h

@@ -16,6 +16,7 @@
 
 #include <atomic>
 #include <thread>
+#include <future>
 #include "leaf_node.h"
 
 namespace BT
@@ -96,48 +97,44 @@ class SimpleActionNode : public SyncActionNode
 };
 
 /**
- * @brief The AsyncActionNode uses a different thread where the action will be
+ * @brief The AsyncActionNode uses a different thread, where the action will be
  * executed.
  *
- * The user must implement the methods tick() and halt().
+ * IMPORTANT: this action is quite hard to implement correctly. Please be sure that you know what you are doing.
  *
- * WARNING: this should probably be deprecated. It is too easy to use incorrectly
- * and there is not a good way to halt it in a thread safe way.
+ * - In your overriden tick() method, you must check periodically
+ *   the result of the method isHaltRequested() and stop your execution accordingly.
  *
- * Use it at your own risk.
+ * - in the overriden halt() method, you can do some cleanup, but do not forget to
+ *   invoke the base class method AsyncActionNode::halt();
+ *
+ * - remember, with few exceptions, a halted AsyncAction must return NodeStatus::IDLE.
+ *
+ * For a complete example, look at __AsyncActionTest__ in action_test_node.h in the folder test.
  */
 class AsyncActionNode : public ActionNodeBase
 {
   public:
 
-    AsyncActionNode(const std::string& name, const NodeConfiguration& config);
-    virtual ~AsyncActionNode() override;
+    AsyncActionNode(const std::string& name, const NodeConfiguration& config):ActionNodeBase(name, config)
+    {
+    }
+
+    bool isHaltRequested() const
+    {
+        return halt_requested_.load();
+    }
 
-    // This method triggers the TickEngine. Do NOT remove the "final" keyword.
+    // This method spawn a new thread. Do NOT remove the "final" keyword.
     virtual NodeStatus executeTick() override final;
 
-    void stopAndJoinThread();
+    virtual void halt() override;
 
   private:
 
-    // The method that will be executed by the thread
-    void asyncThreadLoop();
-
-    void waitStart();
-
-    void notifyStart();
-
-    std::atomic<bool> keep_thread_alive_;
-
-    bool start_action_;
-
-    std::mutex start_mutex_;
-
-    std::condition_variable start_signal_;
-
     std::exception_ptr exptr_;
-
-    std::thread thread_;
+    std::atomic_bool halt_requested_;
+    std::future<NodeStatus> thread_handle_;
 };
 
 /**

include/behaviortree_cpp_v3/behavior_tree.h

@@ -54,11 +54,6 @@ void applyRecursiveVisitor(TreeNode* root_node, const std::function<void(TreeNod
  */
 void printTreeRecursively(const TreeNode* root_node);
 
-/// Invoke AsyncActionNode::stopAndJoinThread() to the entire tree,
-/// when needed.
-void haltAllActions(TreeNode* root_node);
-
-
 typedef std::vector<std::pair<uint16_t, uint8_t>> SerializedTreeStatus;
 
 /**

include/behaviortree_cpp_v3/bt_factory.h

@@ -156,6 +156,10 @@ class Tree
 
     void haltTree()
     {
+        if(!rootNode())
+        {
+            return;
+        }
         // the halt should propagate to all the node if the nodes
         // have been implemented correctly
         rootNode()->halt();

package.xml

@@ -1,7 +1,7 @@
 <?xml version="1.0"?>
 <package format="3">
   <name>behaviortree_cpp_v3</name>
-  <version>3.1.1</version>
+  <version>3.2.0</version>
   <description>
   This package provides the Behavior Trees core library.
   </description>

src/action_node.cpp

@@ -50,102 +50,6 @@ NodeStatus SimpleActionNode::tick()
 
 //-------------------------------------------------------
 
-AsyncActionNode::AsyncActionNode(const std::string& name, const NodeConfiguration& config)
-  : ActionNodeBase(name, config)
-{
-
-}
-
-AsyncActionNode::~AsyncActionNode()
-{
-    if (thread_.joinable())
-    {
-        stopAndJoinThread();
-    }
-}
-
-void AsyncActionNode::waitStart()
-{
-    std::unique_lock<std::mutex> lock(start_mutex_);
-    while (!start_action_)
-    {
-        start_signal_.wait(lock);
-    }
-    start_action_ = false;
-}
-
-void AsyncActionNode::notifyStart()
-{
-    std::unique_lock<std::mutex> lock(start_mutex_);
-    start_action_ = true;
-    start_signal_.notify_all();
-}
-
-void AsyncActionNode::asyncThreadLoop()
-{
-    while (keep_thread_alive_.load())
-    {
-        waitStart();
-
-        // check keep_thread_alive_ again because the tick_engine_ could be
-        // notified from the method stopAndJoinThread
-        if (keep_thread_alive_)
-        {
-            // this will execute the blocking code.
-            try {
-                setStatus(tick());
-            }
-            catch (std::exception&)
-            {
-                std::cerr << "\nUncaught exception from the method tick() of an AsyncActionNode: ["
-                          << registrationName() << "/" << name() << "]\n" << std::endl;
-                exptr_ = std::current_exception();
-                keep_thread_alive_ = false;
-            }
-        }
-    }
-}
-
-NodeStatus AsyncActionNode::executeTick()
-{
-    //send signal to other thread.
-    // The other thread is in charge for changing the status
-    if (status() == NodeStatus::IDLE)
-    {
-        if( thread_.joinable() == false) {
-            keep_thread_alive_ = true;
-            thread_ = std::thread(&AsyncActionNode::asyncThreadLoop, this);
-        }
-        setStatus( NodeStatus::RUNNING );
-        notifyStart();
-    }
-
-    if( exptr_ )
-    {
-        std::rethrow_exception(exptr_);
-    }
-    return status();
-}
-
-void AsyncActionNode::stopAndJoinThread()
-{
-    keep_thread_alive_.store(false);
-    if( status() == NodeStatus::RUNNING )
-    {
-        halt();
-    }
-    else{
-        // loop in asyncThreadLoop() is blocked at waitStart(). Unblock it.
-        notifyStart();
-    }
-
-    if (thread_.joinable())
-    {
-        thread_.join();
-    }
-}
-
-
 SyncActionNode::SyncActionNode(const std::string &name, const NodeConfiguration& config):
   ActionNodeBase(name, config)
 {}
@@ -260,3 +164,44 @@ void StatefulActionNode::halt()
   }
   setStatus(NodeStatus::IDLE);
 }
+
+NodeStatus BT::AsyncActionNode::executeTick()
+{
+    //send signal to other thread.
+    // The other thread is in charge for changing the status
+    if (status() == NodeStatus::IDLE)
+    {
+        setStatus( NodeStatus::RUNNING );
+        halt_requested_ = false;
+        thread_handle_ = std::async(std::launch::async, [this]() {
+
+            try {
+                setStatus(tick());
+            }
+            catch (std::exception&)
+            {
+                std::cerr << "\nUncaught exception from the method tick(): ["
+                          << registrationName() << "/" << name() << "]\n" << std::endl;
+                exptr_ = std::current_exception();
+                thread_handle_.wait();
+            }
+            return status();
+        });
+    }
+
+    if( exptr_ )
+    {
+        std::rethrow_exception(exptr_);
+    }
+    return status();
+}
+
+void AsyncActionNode::halt()
+{
+    halt_requested_.store(true);
+
+    if( thread_handle_.valid() ){
+        thread_handle_.wait();
+    }
+    thread_handle_ = {};
+}

src/behavior_tree.cpp

@@ -107,15 +107,4 @@ void buildSerializedStatusSnapshot(TreeNode* root_node, SerializedTreeStatus& se
     applyRecursiveVisitor(root_node, visitor);
 }
 
-void haltAllActions(TreeNode* root_node)
-{
-    auto visitor = [](TreeNode* node) {
-        if (auto action = dynamic_cast<AsyncActionNode*>(node))
-        {
-            action->stopAndJoinThread();
-        }
-    };
-    applyRecursiveVisitor(root_node, visitor);
-}
-
 } // end namespace

src/bt_factory.cpp

@@ -261,9 +261,7 @@ Tree BehaviorTreeFactory::createTreeFromFile(const std::string &file_path,
 
 Tree::~Tree()
 {
-    if (rootNode()) {
-        haltAllActions(rootNode());
-    }
+    haltTree();
 }
 
 Blackboard::Ptr Tree::rootBlackboard()

tests/gtest_decorator.cpp

@@ -30,7 +30,7 @@ struct DeadlineTest : testing::Test
     }
     ~DeadlineTest()
     {
-        haltAllActions(&root);
+        
     }
 };
 
@@ -45,7 +45,7 @@ struct RepeatTest : testing::Test
     }
     ~RepeatTest()
     {
-        haltAllActions(&root);
+        
     }
 };
 
@@ -60,7 +60,7 @@ struct RetryTest : testing::Test
     }
     ~RetryTest()
     {
-        haltAllActions(&root);
+        
     }
 };
 
@@ -78,10 +78,6 @@ struct TimeoutAndRetry : testing::Test
         timeout_root.setChild(&retry);
         retry.setChild(&action);
     }
-    ~TimeoutAndRetry()
-    {
-        haltAllActions(&timeout_root);
-    }
 };
 
 /****************TESTS START HERE***************************/

tests/gtest_fallback.cpp

@@ -34,7 +34,7 @@ struct SimpleFallbackTest : testing::Test
     }
     ~SimpleFallbackTest()
     {
-        haltAllActions(&root);
+        
     }
 };
 
@@ -57,7 +57,7 @@ struct ReactiveFallbackTest : testing::Test
     }
     ~ReactiveFallbackTest()
     {
-        haltAllActions(&root);
+        
     }
 };
 
@@ -77,7 +77,7 @@ struct SimpleFallbackWithMemoryTest : testing::Test
     }
     ~SimpleFallbackWithMemoryTest()
     {
-        haltAllActions(&root);
+        
     }
 };
 
@@ -116,7 +116,7 @@ struct ComplexFallbackWithMemoryTest : testing::Test
     }
     ~ComplexFallbackWithMemoryTest()
     {
-        haltAllActions(&root);
+        
     }
 };