Merge branch 'master' of github.com:BehaviorTree/BehaviorTree.CPP

Author: facontidavide

.github/workflows/cmake_windows.yml

@@ -43,5 +43,5 @@ jobs:
 
     - name: run test (Windows)
       working-directory: ${{github.workspace}}/build
-      run: ./tests/Release/behaviortree_cpp_test
+      run: ctest -C $BUILD_TYPE
 

include/behaviortree_cpp/action_node.h

@@ -102,9 +102,9 @@ class SimpleActionNode : public SyncActionNode
  *   the result of the method isHaltRequested() and stop your execution accordingly.
  *
  * - in the overriden halt() method, you can do some cleanup, but do not forget to
- *   invoke the base class method AsyncActionNode::halt();
+ *   invoke the base class method ThreadedAction::halt();
  *
- * - remember, with few exceptions, a halted AsyncAction must return NodeStatus::IDLE.
+ * - remember, with few exceptions, a halted ThreadedAction must return NodeStatus::IDLE.
  *
  * For a complete example, look at __AsyncActionTest__ in action_test_node.h in the folder test.
  *
@@ -137,12 +137,12 @@ class ThreadedAction : public ActionNodeBase
 };
 
 #ifdef USE_BTCPP3_OLD_NAMES
-using AsyncActionNode = ThreadedActionNode;
+using AsyncActionNode = ThreadedAction;
 #endif
 
 /**
- * @brief The StatefulAsyncAction is the preferred way to implement asynchronous Actions.
- * It is actually easier to use correctly, when compared with AsyncAction
+ * @brief The StatefulActionNode is the preferred way to implement asynchronous Actions.
+ * It is actually easier to use correctly, when compared with ThreadedAction
  *
  * It is particularly useful when your code contains a request-reply pattern,
  * i.e. when the actions sends an asynchronous request, then checks periodically

include/behaviortree_cpp/basic_types.h

@@ -201,13 +201,13 @@ using enable_if_not = typename std::enable_if<!Predicate::value>::type*;
  *
  * */
 template <typename T>
+using Expected = nonstd::expected<T, std::string>;
 #ifdef USE_BTCPP3_OLD_NAMES
+// note: we also use the name Optional instead of expected because it is more intuitive
+// for users that are not up to date with "modern" C++
+template <typename T>
 using Optional = nonstd::expected<T, std::string>;
-#else
-using Expected = nonstd::expected<T, std::string>;
 #endif
-// note: we use the name Optional instead of expected because it is more intuitive
-// for users that are not up to date with "modern" C++
 
 /** Usage: given a function/method like:
  *
@@ -292,7 +292,8 @@ std::pair<std::string, PortInfo> CreatePort(PortDirection direction, StringView
   auto sname = static_cast<std::string>(name);
   if (!IsAllowedPortName(sname))
   {
-    throw RuntimeError("The name of a port must start with an alphabetic character. "
+    throw RuntimeError("The name of a port must not be `name` or `ID` "
+                       "and must start with an alphabetic character. "
                        "Underscore is reserved.");
   }
 

include/behaviortree_cpp/tree_node.h

@@ -14,6 +14,7 @@
 #pragma once
 
 #include <condition_variable>
+#include <exception>
 #include <mutex>
 #include <map>
 
@@ -79,7 +80,7 @@ struct NodeConfig
 
 #ifdef USE_BTCPP3_OLD_NAMES
 // back compatibility
-using NodeConfig = NodeConfig;
+using NodeConfiguration = NodeConfig;
 #endif
 
 template <typename T>
@@ -343,9 +344,9 @@ inline Result TreeNode::getInput(const std::string& key, T& destination) const
 
     std::unique_lock<std::mutex> entry_lock(config_.blackboard->entryMutex());
     const Any* val = config_.blackboard->getAny(static_cast<std::string>(remapped_key));
-    if (val && val->empty() == false)
+    if (val && !val->empty())
     {
-      if (std::is_same<T, std::string>::value == false &&
+      if (!std::is_same_v<T, std::string> &&
           val->type() == typeid(std::string))
       {
         destination = convertFromString<T>(val->cast<std::string>());

src/basic_types.cpp

@@ -253,7 +253,10 @@ PortDirection convertFromString<PortDirection>(StringView str)
     return PortDirection::INPUT;
   if (str == "Output" || str == "OUTPUT")
     return PortDirection::OUTPUT;
-  return PortDirection::INOUT;
+  if (str == "InOut" || str == "INOUT")
+    return PortDirection::INOUT;
+  throw RuntimeError(std::string("Cannot convert this to PortDirection: ") +
+                     static_cast<std::string>(str));
 }
 
 std::ostream& operator<<(std::ostream& os, const NodeType& type)
@@ -287,7 +290,7 @@ std::vector<StringView> splitString(const StringView& strToSplit, char delimeter
     {
       new_pos = strToSplit.size();
     }
-    StringView sv = {&strToSplit.data()[pos], new_pos - pos};
+    const auto sv = StringView{&strToSplit.data()[pos], new_pos - pos};
     splitted_strings.push_back(sv);
     pos = new_pos + 1;
   }

src/controls/fallback_node.cpp

@@ -12,7 +12,7 @@
 */
 
 #include "behaviortree_cpp/controls/fallback_node.h"
-#include "behaviortree_cpp/action_node.h"
+
 namespace BT
 {
 FallbackNode::FallbackNode(const std::string& name) :

src/decorators/repeat_node.cpp

@@ -15,7 +15,6 @@
 
 namespace BT
 {
-constexpr const char* RepeatNode::NUM_CYCLES;
 
 RepeatNode::RepeatNode(const std::string& name, int NTries) :
   DecoratorNode(name, {}),

tests/CMakeLists.txt

@@ -28,12 +28,6 @@ if( BTCPP_ENABLE_COROUTINES AND BT_COROUTINES_FOUND )
     LIST( APPEND BT_TESTS gtest_coroutines.cpp)
 endif()
 
-#if(ament_cmake_FOUND OR catkin_FOUND)
-#    # This test requires gmock. Since we don't have a uniform way to include
-#    # gmock for non-users, it is turned of when build without ros.
-#    list(APPEND BT_TESTS gtest_async_action_node.cpp)
-#endif()
-
 
 if(ament_cmake_FOUND AND BUILD_TESTING)
 
@@ -81,4 +75,14 @@ elseif(BTCPP_UNIT_TESTS)
 
 endif()
 
+# if(BUILD_TESTING AND (ament_cmake_FOUND OR catkin_FOUND))
+#    # This test requires gmock. Since we don't have a uniform way to include
+#    # gmock for non-users, it is turned of when build without ros.
+#    list(APPEND BT_TESTS gtest_async_action_node.cpp)
+#    target_link_libraries(${BEHAVIOR_TREE_LIBRARY}_test
+#         ${BEHAVIOR_TREE_LIBRARY}
+#         gmock
+#     )
+# endif()
+
 target_compile_definitions(${BEHAVIOR_TREE_LIBRARY}_test PRIVATE BT_TEST_FOLDER="${CMAKE_CURRENT_SOURCE_DIR}")

tests/gtest_async_action_node.cpp

@@ -13,9 +13,9 @@
 #include <gmock/gmock.h>
 
 // The mocked version of the base.
-struct MockedAsyncActionNode : public BT::AsyncActionNode
+struct MockedThreadedAction : public BT::ThreadedAction
 {
-  using BT::AsyncActionNode::AsyncActionNode;
+  using BT::ThreadedAction::ThreadedAction;
   MOCK_METHOD0(tick, BT::NodeStatus());
 
   // Tick while the node is running.
@@ -29,25 +29,25 @@ struct MockedAsyncActionNode : public BT::AsyncActionNode
   }
 
   // Expose the setStatus method.
-  using BT::AsyncActionNode::setStatus;
+  using BT::ThreadedAction::setStatus;
 };
 
 // The fixture taking care of the node-setup.
-struct MockedAsyncActionFixture : public testing::Test
+struct MockedThreadedActionFixture : public testing::Test
 {
   BT::NodeConfig config;
-  MockedAsyncActionNode sn;
-  MockedAsyncActionFixture() : sn("node", config)
+  MockedThreadedAction sn;
+  MockedThreadedActionFixture() : sn("node", config)
   {}
 };
 
 // Parameters for the terminal node states.
 struct NodeStatusFixture : public testing::WithParamInterface<BT::NodeStatus>,
-                           public MockedAsyncActionFixture
+                           public MockedThreadedActionFixture
 {
 };
 
-INSTANTIATE_TEST_CASE_P(/**/, NodeStatusFixture,
+INSTANTIATE_TEST_SUITE_P(/**/, NodeStatusFixture,
                         testing::Values(BT::NodeStatus::SUCCESS,
                                         BT::NodeStatus::FAILURE));
 
@@ -67,7 +67,7 @@ TEST_P(NodeStatusFixture, normal_routine)
   ASSERT_EQ(sn.spinUntilDone(), state);
 }
 
-TEST_F(MockedAsyncActionFixture, no_halt)
+TEST_F(MockedThreadedActionFixture, no_halt)
 {
   // Test verifies that halt returns immediately, if the node is idle. It
   // further checks if the halt-flag is resetted correctly.
@@ -83,7 +83,7 @@ TEST_F(MockedAsyncActionFixture, no_halt)
   ASSERT_FALSE(sn.isHaltRequested());
 }
 
-TEST_F(MockedAsyncActionFixture, halt)
+TEST_F(MockedThreadedActionFixture, halt)
 {
   // Test verifies that calling halt() is blocking.
   bool release = false;
@@ -120,7 +120,7 @@ TEST_F(MockedAsyncActionFixture, halt)
   ASSERT_EQ(sn.status(), state);
 }
 
-TEST_F(MockedAsyncActionFixture, exception)
+TEST_F(MockedThreadedActionFixture, exception)
 {
   // Verifies that we can recover from the exceptions in the tick method:
   // 1) catch the exception, 2) re-raise it in the caller thread.