Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion include/behaviortree_cpp/scripting/operators.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,11 @@ struct ExprBinaryArithmetic : ExprBase
throw RuntimeError(ErrorNotInit("right", opStr()));
}

if(rhs_v.isNumber() && lhs_v.isNumber())
// If just one value is strongly typed as a number, and the other can be casted to one, do arithmetic. If both are not strongly typed, continue. In the case of `+`, non-strongly typed values will result in concatenation.
bool are_numbers = (rhs_v.isNumber() && lhs_v.isNumber()) ||
((rhs_v.tryCast<double>().has_value() && lhs_v.isNumber()) !=
(rhs_v.isNumber() && lhs_v.tryCast<double>().has_value()));
if(are_numbers)
{
auto lv = lhs_v.cast<double>();
auto rv = rhs_v.cast<double>();
Expand Down
55 changes: 2 additions & 53 deletions src/xml_parsing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,10 @@

#include <algorithm>
#include <cctype>
#include <charconv>
#include <cstdio>
#include <cstring>
#include <functional>
#include <iostream>
#include <limits>
#include <list>
#include <sstream>
#include <string>
Expand Down Expand Up @@ -1008,59 +1006,10 @@ void BT::XMLParser::PImpl::recursivelyCreateSubtree(const std::string& tree_ID,
}
else
{
// constant value: set it into the BB with appropriate type
// constant string: just set that constant value into the BB
// IMPORTANT: this must not be autoremapped!!!
new_bb->enableAutoRemapping(false);
const std::string str_value(port_value);

// Try to preserve numeric types so that Script expressions
// can perform arithmetic without type-mismatch errors.
// Use std::from_chars with strict full-string validation to avoid
// false positives on compound strings like "1;2;3" or "2.2;2.4".
bool stored = false;
if(!str_value.empty())
{
const char* begin = str_value.data();
const char* end = begin + str_value.size();
// Try integer first (no decimal point, no exponent notation).
// Use int when the value fits, to match the most common port
// declarations. Fall back to int64_t for larger values.
if(str_value.find('.') == std::string::npos &&
str_value.find('e') == std::string::npos &&
str_value.find('E') == std::string::npos)
{
int64_t int_val = 0;
auto [ptr, ec] = std::from_chars(begin, end, int_val);
if(ec == std::errc() && ptr == end)
{
if(int_val >= std::numeric_limits<int>::min() &&
int_val <= std::numeric_limits<int>::max())
{
new_bb->set(port_name, static_cast<int>(int_val));
}
else
{
new_bb->set(port_name, int_val);
}
stored = true;
}
}
// Try double
if(!stored)
{
double dbl_val = 0;
auto [ptr, ec] = std::from_chars(begin, end, dbl_val);
if(ec == std::errc() && ptr == end)
{
new_bb->set(port_name, dbl_val);
stored = true;
}
}
}
if(!stored)
{
new_bb->set(port_name, str_value);
}
new_bb->set(port_name, static_cast<std::string>(port_value));
new_bb->enableAutoRemapping(do_autoremap);
}
}
Expand Down
108 changes: 70 additions & 38 deletions tests/gtest_subtree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,76 @@ TEST(SubTree, StringConversions_Issue530)
tree.tickOnce();
}

// Regression test for https://github.com/PickNikRobotics/moveit_pro/issues/18587.
TEST(SubTree, ScriptingTypingIssue)
{
static const char* xml_text = R"(

<root BTCPP_format="4" >
<BehaviorTree ID="MainTree">
<Sequence>
<SubTree ID="mySubtree" in_value="1.0" out_value="{y}" />
</Sequence>
</BehaviorTree>

<BehaviorTree ID="mySubtree">
<Script code = "out_value := in_value + 1" />
</BehaviorTree>
<TreeNodesModel>
<SubTree ID="mySubtree">
<input_port name="in_value" />
<output_port name="out_value" />
</SubTree>
</TreeNodesModel>
</root> )";

BehaviorTreeFactory factory;
factory.registerBehaviorTreeFromText(xml_text);

auto blackboard = BT::Blackboard::create();
Tree tree = factory.createTree("MainTree", blackboard);

auto status = tree.tickWhileRunning();
ASSERT_EQ(status, NodeStatus::SUCCESS);

EXPECT_EQ(blackboard->get<int>("y"), 2);
}

// Test to make sure that the fix for the addressed bug above doesn't cause another regression: string concatenation with `+` should still work... for now.
TEST(SubTree, ScriptingTypingIssue_StringConcatWorks)
{
static const char* xml_text = R"(

<root BTCPP_format="4" >
<BehaviorTree ID="MainTree">
<Sequence>
<SubTree ID="mySubtree" in_value="12" out_value="{y}" />
</Sequence>
</BehaviorTree>

<BehaviorTree ID="mySubtree">
<Script code = "out_value := in_value + '34'" />
</BehaviorTree>
<TreeNodesModel>
<SubTree ID="mySubtree">
<input_port name="in_value" />
<output_port name="out_value" />
</SubTree>
</TreeNodesModel>
</root> )";

BehaviorTreeFactory factory;
factory.registerBehaviorTreeFromText(xml_text);

auto blackboard = BT::Blackboard::create();
Tree tree = factory.createTree("MainTree", blackboard);

auto status = tree.tickWhileRunning();
ASSERT_EQ(status, NodeStatus::SUCCESS);

EXPECT_EQ(blackboard->get<std::string>("y"), "1234");
}

class NaughtyNav2Node : public BT::SyncActionNode
{
public:
Expand Down Expand Up @@ -799,41 +869,3 @@ TEST(SubTree, SubtreeNameNotRegistered)
ASSERT_ANY_THROW(auto tree = factory.createTreeFromText(xml_text));
ASSERT_ANY_THROW(factory.registerBehaviorTreeFromText(xml_text));
}

// Regression test: literal numeric values passed to subtrees should preserve
// their numeric type so that Script expressions can do arithmetic.
TEST(SubTree, LiteralNumericPortsPreserveType)
{
// clang-format off
static const char* xml_text = R"(
<root BTCPP_format="4" main_tree_to_execute="MainTree">

<BehaviorTree ID="MainTree">
<Sequence>
<SubTree ID="DoMath" int_val="42" dbl_val="3.14" str_val="hello"
remapped_val="{from_parent}" />
</Sequence>
</BehaviorTree>

<BehaviorTree ID="DoMath">
<Sequence>
<ScriptCondition code=" int_val + 1 == 43 " />
<ScriptCondition code=" dbl_val > 3.0 " />
<ScriptCondition code=" remapped_val + 1 == 101 " />
</Sequence>
</BehaviorTree>

</root>
)";
// clang-format on

BehaviorTreeFactory factory;

auto tree = factory.createTreeFromText(xml_text);

// Set the remapped parent value as an integer
tree.rootBlackboard()->set("from_parent", 100);

const auto status = tree.tickWhileRunning();
ASSERT_EQ(status, NodeStatus::SUCCESS);
}
Loading