Skip to content

Commit

Permalink
fixes and checks in default values, based on PR #773
Browse files Browse the repository at this point in the history
  • Loading branch information
facontidavide committed Feb 14, 2024
1 parent 21ab495 commit 6c9929c
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 8 deletions.
2 changes: 1 addition & 1 deletion include/behaviortree_cpp/actions/script_node.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class ScriptNode : public SyncActionNode

static PortsList providedPorts()
{
return {InputPort("code", "Piece of code that can be parsed")};
return {InputPort<std::string>("code", "Piece of code that can be parsed")};
}

private:
Expand Down
13 changes: 10 additions & 3 deletions include/behaviortree_cpp/basic_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ using StringView = std::string_view;
// vector of key/value pairs
using KeyValueVector = std::vector<std::pair<std::string, std::string>>;


struct AnyTypeAllowed
{};


/**
* convertFromString is used to convert a string into a custom type.
*
Expand Down Expand Up @@ -142,6 +147,11 @@ inline StringConverter GetAnyFromStringFunctor()
{
return [](StringView str) { return Any(str); };
}
else if constexpr(std::is_same_v<BT::AnyTypeAllowed, T> ||
std::is_enum_v<T>)
{
return {};
}
else {
return [](StringView str) { return Any(convertFromString<T>(str)); };
}
Expand Down Expand Up @@ -265,9 +275,6 @@ using Result = Expected<std::monostate>;
[[nodiscard]]
bool IsAllowedPortName(StringView str);

struct AnyTypeAllowed
{};

class TypeInfo
{
public:
Expand Down
38 changes: 35 additions & 3 deletions src/xml_parsing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,

const TreeNodeManifest* manifest = nullptr;

auto manifest_it = factory.manifests().find(type_ID);
auto manifest_it = factory.manifests().find(type_ID);
if(manifest_it != factory.manifests().end())
{
manifest = &manifest_it->second;
Expand All @@ -647,8 +647,40 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
{
if (IsAllowedPortName(att->Name()))
{
const std::string attribute_name = att->Name();
port_remap[attribute_name] = att->Value();
const std::string port_name = att->Name();
const std::string port_value = att->Value();

if(manifest)
{
auto port_model_it = manifest->ports.find(port_name);
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()"));
}
else {
const auto& port_model = port_model_it->second;
bool is_blacbkboard = port_value.size() >= 3 &&
port_value.front() == '{' &&
port_value.back() == '}';
// let's test already if conversion is possible
if(!is_blacbkboard && port_model.converter() && port_model.isStronglyTyped())
{
// This may throw
try {
port_model.converter()(port_value);
}
catch(std::exception& ex)
{
auto msg = StrCat("The port with name \"", port_name, "\" and value \"", port_value,
"\" can not be converted to ", port_model.typeName());
throw LogicError(msg);
}
}
}
}

port_remap[port_name] = port_value;
}
}

Expand Down
51 changes: 50 additions & 1 deletion tests/gtest_ports.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -533,8 +533,32 @@ TEST(PortTest, DefaultInputStrings)
ASSERT_EQ(status, NodeStatus::SUCCESS);
}

struct TestStruct
{
int a;
double b;
std::string c;
};

TEST(PortTest, Default_Issues_767_768)
class NodeWithDefaultNullptr : public SyncActionNode
{
public:
NodeWithDefaultNullptr(const std::string& name, const NodeConfig& config) :
SyncActionNode(name, config) {}

NodeStatus tick() override
{
return NodeStatus::SUCCESS;
}

static PortsList providedPorts()
{
return {BT::InputPort< std::shared_ptr<TestStruct> >("input", nullptr, "default value is nullptr")};
}
};


TEST(PortTest, Default_Issues_767)
{
using namespace BT;

Expand All @@ -545,4 +569,29 @@ TEST(PortTest, Default_Issues_767_768)
ASSERT_NO_THROW(auto p = InputPort<std::shared_ptr<std::string>>("ptr_B", nullptr, "default nullptr"));
}

TEST(PortTest, DefaultWronglyOverriden)
{
BT::BehaviorTreeFactory factory;
factory.registerNodeType<NodeWithDefaultNullptr>("NodeWithDefaultNullptr");

std::string xml_txt_wrong = R"(
<root BTCPP_format="4" >
<BehaviorTree>
<NodeWithDefaultNullptr input=""/>
</BehaviorTree>
</root>)";

std::string xml_txt_correct = R"(
<root BTCPP_format="4" >
<BehaviorTree>
<NodeWithDefaultNullptr/>
</BehaviorTree>
</root>)";

// this should throw, because we are NOT using the default,
// but overriding it with an empty string instead.
// See issue 768 for reference
ASSERT_ANY_THROW( auto tree = factory.createTreeFromText(xml_txt_wrong));
// This is correct
ASSERT_NO_THROW( auto tree = factory.createTreeFromText(xml_txt_correct));
}

0 comments on commit 6c9929c

Please sign in to comment.