renamings and fixes

Author: facontidavide

examples/ex04_waypoints.cpp

@@ -106,14 +106,14 @@ static const char* xml_tree = R"(
  <root BTCPP_format="4" >
      <BehaviorTree ID="TreeA">
         <Sequence>
-            <LoopPopDouble queue="1;2;3"  value="{number}">
+            <LoopDouble queue="1;2;3"  value="{number}">
               <PrintNumber value="{number}" />
-            </LoopPopDouble>
+            </LoopDouble>
 
             <GenerateWaypoints waypoints="{waypoints}" />
-            <LoopPopPose queue="{waypoints}"  value="{wp}">
+            <LoopPose queue="{waypoints}"  value="{wp}">
               <UseWaypoint waypoint="{wp}" />
-            </LoopPopPose>
+            </LoopPose>
         </Sequence>
      </BehaviorTree>
  </root>
@@ -124,8 +124,8 @@ static const char* xml_tree = R"(
 int main()
 {
   BehaviorTreeFactory factory;
-
-  factory.registerNodeType<LoopPopNode<Pose2D>>("LoopPopPose");
+  
+  factory.registerNodeType<LoopNode<Pose2D>>("LoopPose");
 
   factory.registerNodeType<UseWaypoint>("UseWaypoint");
   factory.registerNodeType<PrintNumber>("PrintNumber");

include/behaviortree_cpp/blackboard.h

@@ -16,9 +16,13 @@
 
 namespace BT
 {
+/// This type contains pointer to Any.
+/// Protected with a read-only lock as long as the object is in scope
+using AnyPtrReadLock = LockedPtrConst<Any>;
 
-using AnyReadRef = LockedConstRef<Any>;
-using AnyWriteRef = LockedRef<Any>;
+/// This type contains pointer to Any.
+/// Protected with a read-write lock as long as the object is in scope
+using AnyPtrWriteLock = LockedPtr<Any>;
 
 /**
  * @brief The Blackboard is the mechanism used by BehaviorTrees to exchange
@@ -61,7 +65,7 @@ class Blackboard
 
   virtual ~Blackboard() = default;
 
-  const Entry* getEntry(const std::string& key) const
+  [[nodiscard]] const Entry* getEntry(const std::string& key) const
   {
     std::unique_lock<std::mutex> lock(mutex_);
     // search first if this port was remapped
@@ -80,27 +84,27 @@ class Blackboard
     return (it == storage_.end()) ? nullptr : it->second.get();
   }
 
-  Entry* getEntry(const std::string& key)
+  [[nodiscard]] Entry* getEntry(const std::string& key)
   {
     // "Avoid Duplication in const and Non-const Member Function,"
     // on p. 23, in Item 3 "Use const whenever possible," in Effective C++, 3d ed
     return const_cast<Entry*>( static_cast<const Blackboard &>(*this).getEntry(key));
   }
 
-  AnyReadRef getAnyRead(const std::string& key) const
+  [[nodiscard]] AnyPtrReadLock getAnyRead(const std::string& key) const
   {
     if(auto entry = getEntry(key))
     {
-      return AnyReadRef(&entry->value, const_cast<std::shared_mutex*>(&entry->entry_mutex));
+      return AnyPtrReadLock(&entry->value, const_cast<std::shared_mutex*>(&entry->entry_mutex));
     }
     return {};
   }
 
-  AnyWriteRef getAnyWrite(const std::string& key)
+  [[nodiscard]] AnyPtrWriteLock getAnyWrite(const std::string& key)
   {
     if(auto entry = getEntry(key))
     {
-      return AnyWriteRef(&entry->value, &entry->entry_mutex);
+      return AnyPtrWriteLock(&entry->value, &entry->entry_mutex);
     }
     return {};
   }
@@ -120,7 +124,7 @@ class Blackboard
   /** Return true if the entry with the given key was found.
      *  Note that this method may throw an exception if the cast to T failed.
      */
-  template <typename T>
+  template <typename T> [[nodiscard]]
   bool get(const std::string& key, T& value) const
   {
     if (auto any_ref = getAnyRead(key))
@@ -134,7 +138,7 @@ class Blackboard
   /**
      * Version of get() that throws if it fails.
     */
-  template <typename T>
+  template <typename T> [[nodiscard]]
   T get(const std::string& key) const
   {
     if (auto any_ref = getAnyRead(key))
@@ -241,13 +245,13 @@ class Blackboard
 
   void setPortInfo(const std::string &key, const PortInfo& info);
 
-  const PortInfo* portInfo(const std::string& key);
+  [[nodiscard]] const PortInfo* portInfo(const std::string& key);
 
   void addSubtreeRemapping(StringView internal, StringView external);
 
   void debugMessage() const;
 
-  std::vector<StringView> getKeys(bool include_remapped = true) const;
+  [[nodiscard]] std::vector<StringView> getKeys(bool include_remapped = true) const;
 
   void clear()
   {

include/behaviortree_cpp/bt_factory.h

@@ -141,7 +141,7 @@ class Tree
     rootNode()->resetStatus();
   }
 
-  TreeNode* rootNode() const;
+  [[nodiscard]] TreeNode* rootNode() const;
 
   /// Sleep for a certain amount of time.
   /// This sleep could be interrupted by the method
@@ -167,22 +167,22 @@ class Tree
   NodeStatus
   tickWhileRunning(std::chrono::milliseconds sleep_time = std::chrono::milliseconds(10));
 
-  Blackboard::Ptr rootBlackboard();
+  [[nodiscard]] Blackboard::Ptr rootBlackboard();
 
   //Call the visitor for each node of the tree.
   void applyVisitor(const std::function<void(const TreeNode*)>& visitor);
 
   //Call the visitor for each node of the tree.
   void applyVisitor(const std::function<void(TreeNode*)>& visitor);
 
-  uint16_t getUID();
+  [[nodiscard]] uint16_t getUID();
 
   /// Get a list of nodes which fullPath() match a wildcard filter and
   /// a given path. Example:
   ///
   /// move_nodes = tree.getNodesByPath<MoveBaseNode>("move_*");
   ///
-  template <typename NodeType = BT::TreeNode>
+  template <typename NodeType = BT::TreeNode> [[nodiscard]]
   std::vector<const TreeNode*> getNodesByPath(StringView wildcard_filter) {
     std::vector<const TreeNode*> nodes;
     for (auto const& subtree : subtrees) {

include/behaviortree_cpp/decorators/loop_node.h

@@ -35,21 +35,20 @@ using SharedQueue = std::shared_ptr<std::deque<T>>;
  * the factory.
  */
 template <typename T = Any>
-class LoopPopNode : public DecoratorNode
+class LoopNode : public DecoratorNode
 {
   bool child_running_ = false;
   SharedQueue<T> static_queue_;
   SharedQueue<T> current_queue_;
-  std::string static_string_;
 
 public:
-  LoopPopNode(const std::string& name, const NodeConfig& config) :
+  LoopNode(const std::string& name, const NodeConfig& config) :
     DecoratorNode(name, config)
   {
     auto raw_port = getRawPortValue("queue");
     if(!isBlackboardPointer(raw_port))
     {
-      static_queue_ = convertFromString<SharedQueue<T>>(static_string_);
+      static_queue_ = convertFromString<SharedQueue<T>>(raw_port);
     }
   }
 
@@ -71,8 +70,8 @@ class LoopPopNode : public DecoratorNode
     {
       // if the port is static, any_ref is empty, otherwise it will keep access to
       // port locked for thread-safety
-      AnyWriteRef any_ref = static_queue_ ?
-                                AnyWriteRef() :
+      AnyPtrWriteLock any_ref = static_queue_ ?
+                                    AnyPtrWriteLock() :
                                 getLockedPortContent("queue");
       if(any_ref)
       {

include/behaviortree_cpp/tree_node.h

@@ -139,16 +139,16 @@ class TreeNode
 
   void haltNode();
 
-  bool isHalted() const;
+  [[nodiscard]] bool isHalted() const;
 
-  NodeStatus status() const;
+  [[nodiscard]] NodeStatus status() const;
 
   /// Name of the instance, not the type
-  const std::string& name() const;
+  [[nodiscard]] const std::string& name() const;
 
   /// Blocking function that will sleep until the setStatus() is called with
   /// either RUNNING, FAILURE or SUCCESS.
-  BT::NodeStatus waitValidStatus();
+  [[nodiscard]] BT::NodeStatus waitValidStatus();
 
   virtual NodeType type() const = 0;
 
@@ -170,7 +170,7 @@ class TreeNode
      *
      * @return the subscriber handle.
      */
-  StatusChangeSubscriber subscribeToStatusChange(StatusChangeCallback callback);
+  [[nodiscard]] StatusChangeSubscriber subscribeToStatusChange(StatusChangeCallback callback);
 
   /** This method attaches to the TreeNode a callback with signature:
      *
@@ -195,18 +195,18 @@ class TreeNode
 
   /// The unique identifier of this instance of treeNode.
   /// It is assigneld by the factory
-  uint16_t UID() const;
+  [[nodiscard]] uint16_t UID() const;
 
   /// Human readable identifier, that includes the hierarchy of Subtrees
   /// See tutorial 10 as an example.
-  const std::string& fullPath() const;
+  [[nodiscard]] const std::string& fullPath() const;
 
   /// registrationName is the ID used by BehaviorTreeFactory to create an instance.
-  const std::string& registrationName() const;
+  [[nodiscard]] const std::string& registrationName() const;
 
   /// Configuration passed at construction time. Can never change after the
   /// creation of the TreeNode instance.
-  const NodeConfig& config() const;
+  [[nodiscard]] const NodeConfig& config() const;
 
   /** Read an input port, which, in practice, is an entry in the blackboard.
    * If the blackboard contains a std::string and T is not a string,
@@ -223,7 +223,7 @@ class TreeNode
    *
    * @param key   the name of the port.
    */
-  template <typename T>
+  template <typename T> [[nodiscard]]
   Expected<T> getInput(const std::string& key) const
   {
     T out;
@@ -270,24 +270,28 @@ class TreeNode
    * @return     empty AnyWriteRef if the blackboard entry doesn't exist or the content
    *             of the port was a static string.
    */
-  AnyWriteRef getLockedPortContent(const std::string& key);
+  [[nodiscard]] AnyPtrWriteLock getLockedPortContent(const std::string& key);
 
   // function provided mostly for debugging purpose to see the raw value
   // in the port (no remapping and no conversion to a type)
-  StringView getRawPortValue(const std::string& key) const;
+  [[nodiscard]] StringView getRawPortValue(const std::string& key) const;
 
   /// Check a string and return true if it matches either one of these
   /// two patterns:  {...} or ${...}
+  [[nodiscard]]
   static bool isBlackboardPointer(StringView str);
 
+  [[nodiscard]]
   static StringView stripBlackboardPointer(StringView str);
 
+  [[nodiscard]]
   static Expected<StringView> getRemappedKey(StringView port_name,
                                              StringView remapped_port);
 
   /// Notify that the tree should be ticked again()
   void emitWakeUpSignal();
 
+  [[nodiscard]]
   bool requiresWakeUp() const;
 
   /** Used to inject config into a node, even if it doesn't have the proper

include/behaviortree_cpp/utils/locked_reference.hpp

@@ -6,18 +6,24 @@
 
 namespace BT
 {
+/**
+ * @brief The LockedPtr class is used to share a pointer to an object
+ * and a mutex that protects the write access to that object.
+ *
+ * As long as the object remains in scope, the mutex is locked
+ */
 template <typename T>
-class LockedRef {
+class LockedPtr {
   public:
 
-  LockedRef() = default;
+  LockedPtr() = default;
 
-  LockedRef(T* obj, std::shared_mutex* obj_mutex):
+      LockedPtr(T* obj, std::shared_mutex* obj_mutex):
         ref_(obj), mutex_(obj_mutex) {
     mutex_->lock();
   }
 
-  ~LockedRef() {
+      ~LockedPtr() {
     if(mutex_) {
       mutex_->unlock();
     }
@@ -56,22 +62,29 @@ class LockedRef {
   std::shared_mutex* mutex_ = nullptr;
 };
 
-
+/**
+ * @brief The LockedPtrConst class is used to share a pointer to an object
+ * and a mutex that protects the access to that object.
+ * It has a read-only interface, the object can not be modified.
+ * Multiple instances of LockedPtrConst can exist without a dead-lock.
+ *
+ * As long as the object remains in scope, the shared mutex is locked
+ */
 template <typename T>
-class LockedConstRef {
+class LockedPtrConst {
   public:
 
-  LockedConstRef() = default;
+  LockedPtrConst() = default;
 
-  LockedConstRef(LockedConstRef const&) = delete;
-  LockedConstRef& operator=(LockedConstRef const&) = delete;
+      LockedPtrConst(LockedPtrConst const&) = delete;
+  LockedPtrConst& operator=(LockedPtrConst const&) = delete;
 
-  LockedConstRef(const T* obj, std::shared_mutex* obj_mutex):
+      LockedPtrConst(const T* obj, std::shared_mutex* obj_mutex):
         ref_(obj), mutex_(obj_mutex) {
     mutex_->lock_shared();
   }
 
-  ~LockedConstRef() {
+      ~LockedPtrConst() {
     if(mutex_) {
       mutex_->unlock_shared();
     }
@@ -85,7 +98,7 @@ class LockedConstRef {
     return ref_ == nullptr;
   }
 
-  const T* get() const {
+  const T * get() const {
     return ref_;
   }
 

src/bt_factory.cpp

@@ -75,9 +75,9 @@ BehaviorTreeFactory::BehaviorTreeFactory()
   registerNodeType<SwitchNode<4>>("Switch4");
   registerNodeType<SwitchNode<5>>("Switch5");
   registerNodeType<SwitchNode<6>>("Switch6");
-
-  registerNodeType<LoopPopNode<double>>("LoopPopDouble");
-  registerNodeType<LoopPopNode<std::string>>("LoopPopString");
+  
+  registerNodeType<LoopNode<double>>("LoopDouble");
+  registerNodeType<LoopNode<std::string>>("LoopString");
 
   for (const auto& it : builders_)
   {

src/tree_node.cpp

@@ -411,7 +411,7 @@ std::string toStr<PostCond>(PostCond pre)
   }
 }
 
-AnyWriteRef BT::TreeNode::getLockedPortContent(const std::string &key)
+AnyPtrWriteLock BT::TreeNode::getLockedPortContent(const std::string &key)
 {
   if(auto remapped_key = getRemappedKey(key, getRawPortValue(key)))
   {