improved support for default values

Author: facontidavide

include/behaviortree_cpp/basic_types.h

@@ -130,9 +130,9 @@ NodeType convertFromString<NodeType>(StringView str);
 template <> [[nodiscard]]
 PortDirection convertFromString<PortDirection>(StringView str);
 
-typedef std::function<Any(StringView)> StringConverter;
+using StringConverter = std::function<Any(StringView)>;
 
-typedef std::unordered_map<const std::type_info*, StringConverter> StringConvertersMap;
+using StringConvertersMap = std::unordered_map<const std::type_info*, StringConverter>;
 
 // helper function
 template <typename T> [[nodiscard]]
@@ -152,10 +152,17 @@ inline StringConverter GetAnyFromStringFunctor<void>()
 template<typename T> [[nodiscard]]
 std::string toStr(const T& value)
 {
-  static_assert(std::is_arithmetic_v<T>,
-                "You need a template specialization of BT::toStr() and "
-                "it must be consistent with the implementation of BT::convertFromString");
-  return std::to_string(value);
+  if constexpr(!std::is_arithmetic_v<T>)
+  {
+    throw LogicError(
+        StrCat("Function BT::toStr<T>() not specialized for type [",
+               BT::demangle(typeid(T)), "],",
+               "Implement it consistently with BT::convertFromString<T>(), "
+               "or provide at dummy version that returns an empty string.")
+      );
+  } else {
+    return std::to_string(value);
+  }
 }
 
 template <> [[nodiscard]]
@@ -250,11 +257,11 @@ class PortInfo
   };
 
   PortInfo(PortDirection direction = PortDirection::INOUT) :
-    _type(direction), _type_info(typeid(AnyTypeAllowed))
+    type_(direction), type_info_(typeid(AnyTypeAllowed))
   {}
 
   PortInfo(PortDirection direction, std::type_index type_info, StringConverter conv) :
-    _type(direction), _type_info(type_info), _converter(conv)
+    type_(direction), type_info_(type_info), converter_(conv)
   {}
 
   [[nodiscard]] PortDirection direction() const;
@@ -274,28 +281,38 @@ class PortInfo
 
   void setDescription(StringView description);
 
-  void setDefaultValue(StringView default_value_as_string);
+  template <typename T>
+  void setDefaultValue(const T& default_value) {
+    default_value_ = Any(default_value);
+    try{
+      default_value_str_ = BT::toStr(default_value);
+    }
+    catch(LogicError&) {}
+  }
 
   [[nodiscard]] const std::string& description() const;
 
-  [[nodiscard]] std::optional<std::string> defaultValue() const;
+  [[nodiscard]] const Any& defaultValue() const;
+
+  [[nodiscard]] const std::string& defaultValueString() const;
 
   [[nodiscard]] bool isStronglyTyped() const
   {
-    return _type_info != typeid(AnyTypeAllowed);
+    return type_info_ != typeid(AnyTypeAllowed);
   }
 
   [[nodiscard]] const StringConverter& converter() const
   {
-    return _converter;
+    return converter_;
   }
 
 private:
-  PortDirection _type;
-  std::type_index _type_info;
-  StringConverter _converter;
+  PortDirection type_;
+  std::type_index type_info_;
+  StringConverter converter_;
   std::string description_;
-  std::optional<std::string> default_value_;
+  Any default_value_;
+  std::string default_value_str_;
 };
 
 template <typename T = PortInfo::AnyTypeAllowed> [[nodiscard]]
@@ -355,7 +372,7 @@ inline std::pair<std::string, PortInfo> InputPort(StringView name, const T& defa
                                                   StringView description)
 {
   auto out = CreatePort<T>(PortDirection::INPUT, name, description);
-  out.second.setDefaultValue(BT::toStr(default_value));
+  out.second.setDefaultValue(default_value);
   return out;
 }
 
@@ -365,12 +382,12 @@ inline std::pair<std::string, PortInfo> BidirectionalPort(StringView name,
                                                           StringView description)
 {
   auto out = CreatePort<T>(PortDirection::INOUT, name, description);
-  out.second.setDefaultValue(BT::toStr(default_value));
+  out.second.setDefaultValue(default_value);
   return out;
 }
 //----------
 
-typedef std::unordered_map<std::string, PortInfo> PortsList;
+using PortsList = std::unordered_map<std::string, PortInfo>;
 
 template <typename T, typename = void>
 struct has_static_method_providedPorts : std::false_type
@@ -398,8 +415,8 @@ inline PortsList
   return {};
 }
 
-typedef std::chrono::high_resolution_clock::time_point TimePoint;
-typedef std::chrono::high_resolution_clock::duration Duration;
+using TimePoint = std::chrono::high_resolution_clock::time_point;
+using Duration = std::chrono::high_resolution_clock::duration;
 
 }   // namespace BT
 

include/behaviortree_cpp/tree_node.h

@@ -41,7 +41,7 @@ struct TreeNodeManifest
   std::string description;
 };
 
-typedef std::unordered_map<std::string, std::string> PortsRemapping;
+using PortsRemapping = std::unordered_map<std::string, std::string>;
 
 enum class PreCond
 {
@@ -85,6 +85,8 @@ struct NodeConfig
   // output ports
   PortsRemapping output_ports;
 
+  const TreeNodeManifest* manifest = nullptr;
+
   // Numberic unique identifier
   uint16_t uid = 0;
   // Unique human-readable name, that encapsulate the subtree
@@ -339,7 +341,7 @@ class TreeNode
   /**
      * @brief setStatus changes the status of the node.
      * it will throw if you try to change the status to IDLE, because
-     * your parent node should do that, not the user!.
+     * your parent node should do that, not the user!
      */
   void setStatus(NodeStatus new_status);
 
@@ -410,13 +412,30 @@ inline Result TreeNode::getInput(const std::string& key, T& destination) const
                                           "does not contain the key: [",
                                           key, "]"));
   }
-  auto remapped_res = getRemappedKey(key, remap_it->second);
+
+  // special case. Empty port value, we should use the default value,
+  // if available in the model.
+  // BUT, it the port type is a string, then an empty string might be
+  // a valid value
+  const std::string& port_value_str = remap_it->second;
+  if(port_value_str.empty() && config_.manifest)
+  {
+    const auto& port_manifest = config_.manifest->ports.at(key);
+    const auto& default_value = port_manifest.defaultValue();
+    if(!default_value.empty() && !default_value.isString())
+    {
+      destination = default_value.cast<T>();
+      return {};
+    }
+  }
+
+  auto remapped_res = getRemappedKey(key, port_value_str);
   try
   {
     // pure string, not a blackboard key
     if (!remapped_res)
     {
-      destination = ParseString(remap_it->second);
+      destination = ParseString(port_value_str);
       return {};
     }
     const auto& remapped_key = remapped_res.value();

src/basic_types.cpp

@@ -324,28 +324,28 @@ std::vector<StringView> splitString(const StringView& strToSplit, char delimeter
 
 PortDirection PortInfo::direction() const
 {
-  return _type;
+  return type_;
 }
 
 const std::type_index& PortInfo::type() const
 {
-  return _type_info;
+  return type_info_;
 }
 
 Any PortInfo::parseString(const char* str) const
 {
-  if (_converter)
+  if (converter_)
   {
-    return _converter(str);
+    return converter_(str);
   }
   return {};
 }
 
 Any PortInfo::parseString(const std::string& str) const
 {
-  if (_converter)
+  if (converter_)
   {
-    return _converter(str);
+    return converter_(str);
   }
   return {};
 }
@@ -355,21 +355,21 @@ void PortInfo::setDescription(StringView description)
   description_ = static_cast<std::string>(description);
 }
 
-void PortInfo::setDefaultValue(StringView default_value_as_string)
-{
-  default_value_ = static_cast<std::string>(default_value_as_string);
-}
-
 const std::string& PortInfo::description() const
 {
   return description_;
 }
 
-std::optional<std::string> PortInfo::defaultValue() const
+const Any& PortInfo::defaultValue() const
 {
   return default_value_;
 }
 
+const std::string& PortInfo::defaultValueString() const
+{
+  return default_value_str_;
+}
+
 bool IsAllowedPortName(StringView str)
 {
   if( str == "_autoremap")

src/decorators/subtree_node.cpp

@@ -11,7 +11,7 @@ BT::PortsList BT::SubTreeNode::providedPorts()
 {
   auto port = PortInfo(PortDirection::INPUT, typeid(bool),
                        GetAnyFromStringFunctor<bool>());
-  port.setDefaultValue("false");
+  port.setDefaultValue(false);
   port.setDescription("If true, all the ports with the same name "
                       "will be remapped");
 

src/xml_parsing.cpp

@@ -529,6 +529,14 @@ TreeNode::Ptr XMLParser::Pimpl::createNodeFromXML(const XMLElement* element,
   const char* attr_name = element->Attribute("name");
   std::string instance_name = attr_name ? attr_name : type_ID;
 
+  const TreeNodeManifest* manifest = nullptr;
+
+  auto manifest_it =  factory.manifests().find(type_ID);
+  if(manifest_it != factory.manifests().end())
+  {
+    manifest = &manifest_it->second;
+  }
+
   PortsRemapping port_remap;
   for (const XMLAttribute* att = element->FirstAttribute(); att; att = att->Next())
   {
@@ -543,6 +551,7 @@ TreeNode::Ptr XMLParser::Pimpl::createNodeFromXML(const XMLElement* element,
   config.blackboard = blackboard;
   config.path = prefix_path + instance_name;
   config.uid = output_tree.getUID();
+  config.manifest = manifest;
 
   if(type_ID == instance_name)
   {
@@ -581,12 +590,15 @@ TreeNode::Ptr XMLParser::Pimpl::createNodeFromXML(const XMLElement* element,
   }
   else
   {
-    const auto& manifest = factory.manifests().at(type_ID);
+    if(!manifest)
+    {
+      throw RuntimeError("Missing manifest. It shouldn't happen. Please report this issue");
+    }
 
     //Check that name in remapping can be found in the manifest
     for (const auto& remap_it : port_remap)
     {
-      if (manifest.ports.count(remap_it.first) == 0)
+      if (manifest->ports.count(remap_it.first) == 0)
       {
         throw RuntimeError("Possible typo? In the XML, you tried to remap port \"",
                            remap_it.first, "\" in node [", type_ID, " / ", instance_name,
@@ -596,7 +608,7 @@ TreeNode::Ptr XMLParser::Pimpl::createNodeFromXML(const XMLElement* element,
     }
 
     // Initialize the ports in the BB to set the type
-    for (const auto& [port_name, port_info] : manifest.ports)
+    for (const auto& [port_name, port_info] : manifest->ports)
     {
       auto remap_it = port_remap.find(port_name);
       if (remap_it == port_remap.end())
@@ -643,8 +655,8 @@ TreeNode::Ptr XMLParser::Pimpl::createNodeFromXML(const XMLElement* element,
     for (const auto& remap_it : port_remap)
     {
       const auto& port_name = remap_it.first;
-      auto port_it = manifest.ports.find(port_name);
-      if (port_it != manifest.ports.end())
+      auto port_it = manifest->ports.find(port_name);
+      if (port_it != manifest->ports.end())
       {
         auto direction = port_it->second.direction();
         if (direction != PortDirection::OUTPUT)
@@ -659,17 +671,21 @@ TreeNode::Ptr XMLParser::Pimpl::createNodeFromXML(const XMLElement* element,
     }
 
     // use default value if available for empty ports. Only inputs
-    for (const auto& port_it : manifest.ports)
+    for (const auto& port_it : manifest->ports)
     {
       const std::string& port_name = port_it.first;
       const PortInfo& port_info = port_it.second;
 
       auto direction = port_info.direction();
+
       if (direction != PortDirection::OUTPUT &&
           config.input_ports.count(port_name) == 0 &&
-          port_info.defaultValue())
+          !port_info.defaultValue().empty())
       {
-        config.input_ports.insert({port_name, *port_info.defaultValue()});
+        try {
+          config.input_ports.insert({port_name, port_info.defaultValueString()});
+        }
+        catch(LogicError&) {}
       }
     }
 
@@ -886,9 +902,9 @@ void addNodeModelToXML(const TreeNodeManifest& model,
     {
       port_element->SetAttribute("type", BT::demangle(port_info.type()).c_str());
     }
-    if (port_info.defaultValue().has_value())
+    if (!port_info.defaultValue().empty())
     {
-      port_element->SetAttribute("default", port_info.defaultValue()->c_str());
+      port_element->SetAttribute("default", port_info.defaultValueString().c_str());
     }
 
     if (!port_info.description().empty())