Skip to content

Commit 2fb34b5

Browse files
facontidavideclaude
facontidavide
and
claude
authored
Fix #953: getInput() now uses stored converter for plugin custom types (#1104)
When a node is loaded as a plugin, the convertFromString<T> specialization defined in the plugin is not visible to the main application. This caused getInput<T>() to fail when the custom type's converter was only in the plugin. The fix modifies TreeNode::getInputStamped() to use the StringConverter stored in PortInfo (captured when InputPort<T>() was called in the plugin) rather than directly calling convertFromString<T>(). Changes: - tree_node.h: Add parseStringWithConverter lambda that checks for stored converter before falling back to convertFromString<T>() - Add plugin test (plugin_issue953.so) with custom type and converter defined ONLY in the plugin - Add gtest_plugin_issue953.cpp with 3 test cases: - GetInputUsesStoredConverter: XML literal value - GetInputFromBlackboardString: Value from Script node - GetInputViaSubtreeRemapping: SubTree port remapping All 447 tests pass. Co-authored-by: Claude Opus 4.5 <[email protected]>
1 parent 061fd8e commit 2fb34b5

File tree

4 files changed

+272
-2
lines changed

4 files changed

+272
-2
lines changed

‎include/behaviortree_cpp/tree_node.h‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,27 @@ inline Expected<Timestamp> TreeNode::getInputStamped(const std::string& key,
526526
}
527527
}
528528

529+
// Helper lambda to parse string using the stored converter if available,
530+
// otherwise fall back to convertFromString<T>. This fixes the plugin issue
531+
// where convertFromString<T> specializations are not visible across shared
532+
// library boundaries (issue #953).
533+
auto parseStringWithConverter = [this, &key](const std::string& str) -> T {
534+
if(config().manifest)
535+
{
536+
auto port_it = config().manifest->ports.find(key);
537+
if(port_it != config().manifest->ports.end())
538+
{
539+
const auto& converter = port_it->second.converter();
540+
if(converter)
541+
{
542+
return converter(str).template cast<T>();
543+
}
544+
}
545+
}
546+
// Fall back to parseString which calls convertFromString
547+
return parseString<T>(str);
548+
};
549+
529550
auto blackboard_ptr = getRemappedKey(key, port_value_str);
530551
try
531552
{
@@ -534,7 +555,7 @@ inline Expected<Timestamp> TreeNode::getInputStamped(const std::string& key,
534555
{
535556
try
536557
{
537-
destination = parseString<T>(port_value_str);
558+
destination = parseStringWithConverter(port_value_str);
538559
}
539560
catch(std::exception& ex)
540561
{
@@ -566,7 +587,7 @@ inline Expected<Timestamp> TreeNode::getInputStamped(const std::string& key,
566587
{
567588
if(!std::is_same_v<T, std::string> && any_value.isString())
568589
{
569-
destination = parseString<T>(any_value.cast<std::string>());
590+
destination = parseStringWithConverter(any_value.cast<std::string>());
570591
}
571592
else
572593
{

‎tests/CMakeLists.txt‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,19 @@
11
######################################################
22
# TESTS
33

4+
######################################################
5+
# Plugin for Issue #953 test (must be built before tests)
6+
# This plugin has a custom type with convertFromString ONLY in the plugin
7+
add_library(plugin_issue953 SHARED plugin_issue953/plugin_issue953.cpp)
8+
target_compile_definitions(plugin_issue953 PRIVATE BT_PLUGIN_EXPORT)
9+
set_target_properties(plugin_issue953 PROPERTIES
10+
PREFIX ""
11+
LIBRARY_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
12+
)
13+
target_link_libraries(plugin_issue953 ${BTCPP_LIBRARY})
14+
15+
######################################################
16+
417
set(BT_TESTS
518
src/action_test_node.cpp
619
src/condition_test_node.cpp
@@ -38,6 +51,7 @@ set(BT_TESTS
3851
gtest_while_do_else.cpp
3952
gtest_interface.cpp
4053
gtest_simple_string.cpp
54+
gtest_plugin_issue953.cpp
4155

4256
script_parser_test.cpp
4357
test_helper.hpp
@@ -79,3 +93,9 @@ endif()
7993
target_include_directories(behaviortree_cpp_test PRIVATE include)
8094
target_link_libraries(behaviortree_cpp_test ${BTCPP_LIBRARY} bt_sample_nodes foonathan::lexy)
8195
target_compile_definitions(behaviortree_cpp_test PRIVATE BT_TEST_FOLDER="${CMAKE_CURRENT_SOURCE_DIR}")
96+
97+
# Ensure plugin is built before tests run, and tests can find it
98+
add_dependencies(behaviortree_cpp_test plugin_issue953)
99+
target_compile_definitions(behaviortree_cpp_test PRIVATE
100+
BT_PLUGIN_ISSUE953_PATH="$<TARGET_FILE:plugin_issue953>"
101+
)

‎tests/gtest_plugin_issue953.cpp‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
/**
2+
* Test for Issue #953: convertFromString specialization in plugins not visible
3+
* to main application.
4+
*
5+
* This test loads a plugin (plugin_issue953.so) that defines:
6+
* - A custom type (Issue953Type)
7+
* - The convertFromString<Issue953Type> specialization (ONLY in the plugin)
8+
* - An action node that uses getInput<Issue953Type>()
9+
*
10+
* The key point: this test file does NOT have access to the convertFromString
11+
* specialization. Before the fix, getInput() would fail. After the fix, it
12+
* works because the StringConverter is stored in PortInfo.
13+
*/
14+
15+
#include "behaviortree_cpp/bt_factory.h"
16+
17+
#include <filesystem>
18+
19+
#include <gtest/gtest.h>
20+
21+
using namespace BT;
22+
23+
// Plugin path is defined at compile time via CMake
24+
#ifndef BT_PLUGIN_ISSUE953_PATH
25+
#define BT_PLUGIN_ISSUE953_PATH "plugin_issue953.so"
26+
#endif
27+
28+
class PluginIssue953Test : public testing::Test
29+
{
30+
protected:
31+
void SetUp() override
32+
{
33+
plugin_path_ = BT_PLUGIN_ISSUE953_PATH;
34+
35+
if(!std::filesystem::exists(plugin_path_))
36+
{
37+
GTEST_SKIP() << "Plugin not found at: " << plugin_path_ << ". "
38+
<< "Make sure it's built before running this test.";
39+
}
40+
}
41+
42+
std::string plugin_path_;
43+
};
44+
45+
// Test that getInput works for a custom type defined only in the plugin
46+
TEST_F(PluginIssue953Test, GetInputUsesStoredConverter)
47+
{
48+
// This XML uses a literal string value for the input port
49+
const char* xml_text = R"(
50+
<root BTCPP_format="4">
51+
<BehaviorTree ID="MainTree">
52+
<Issue953Action input="42;hello_world;3.14159"/>
53+
</BehaviorTree>
54+
</root>
55+
)";
56+
57+
BehaviorTreeFactory factory;
58+
59+
// Load the plugin - this registers Issue953Action
60+
// The plugin has the convertFromString<Issue953Type> specialization,
61+
// but THIS file does not.
62+
factory.registerFromPlugin(plugin_path_);
63+
64+
auto tree = factory.createTreeFromText(xml_text);
65+
66+
// This should work because:
67+
// 1. InputPort<Issue953Type>() was called in the plugin
68+
// 2. GetAnyFromStringFunctor captured convertFromString at that point
69+
// 3. The fix makes getInput() use that stored converter
70+
auto status = tree.tickWhileRunning();
71+
72+
ASSERT_EQ(status, NodeStatus::SUCCESS);
73+
74+
// Verify the parsed values via output ports
75+
auto bb = tree.rootBlackboard();
76+
EXPECT_EQ(bb->get<int>("out_id"), 42);
77+
EXPECT_EQ(bb->get<std::string>("out_name"), "hello_world");
78+
EXPECT_DOUBLE_EQ(bb->get<double>("out_value"), 3.14159);
79+
}
80+
81+
// Test with blackboard - value stored as string, then parsed on read
82+
TEST_F(PluginIssue953Test, GetInputFromBlackboardString)
83+
{
84+
const char* xml_text = R"(
85+
<root BTCPP_format="4">
86+
<BehaviorTree ID="MainTree">
87+
<Sequence>
88+
<Script code="my_data := '99;from_script;2.718'" />
89+
<Issue953Action input="{my_data}"/>
90+
</Sequence>
91+
</BehaviorTree>
92+
</root>
93+
)";
94+
95+
BehaviorTreeFactory factory;
96+
factory.registerFromPlugin(plugin_path_);
97+
98+
auto tree = factory.createTreeFromText(xml_text);
99+
auto status = tree.tickWhileRunning();
100+
101+
ASSERT_EQ(status, NodeStatus::SUCCESS);
102+
103+
auto bb = tree.rootBlackboard();
104+
EXPECT_EQ(bb->get<int>("out_id"), 99);
105+
EXPECT_EQ(bb->get<std::string>("out_name"), "from_script");
106+
EXPECT_DOUBLE_EQ(bb->get<double>("out_value"), 2.718);
107+
}
108+
109+
// Test with SubTree port remapping
110+
TEST_F(PluginIssue953Test, GetInputViaSubtreeRemapping)
111+
{
112+
const char* xml_text = R"(
113+
<root BTCPP_format="4" main_tree_to_execute="MainTree">
114+
<BehaviorTree ID="MainTree">
115+
<SubTree ID="Issue953SubTree" data="123;subtree_test;1.5"/>
116+
</BehaviorTree>
117+
118+
<BehaviorTree ID="Issue953SubTree">
119+
<Issue953Action input="{data}"/>
120+
</BehaviorTree>
121+
</root>
122+
)";
123+
124+
BehaviorTreeFactory factory;
125+
factory.registerFromPlugin(plugin_path_);
126+
127+
auto tree = factory.createTreeFromText(xml_text);
128+
auto status = tree.tickWhileRunning();
129+
130+
ASSERT_EQ(status, NodeStatus::SUCCESS);
131+
132+
// Get the subtree's blackboard to check output
133+
auto subtree_bb = tree.subtrees[1]->blackboard;
134+
EXPECT_EQ(subtree_bb->get<int>("out_id"), 123);
135+
EXPECT_EQ(subtree_bb->get<std::string>("out_name"), "subtree_test");
136+
EXPECT_DOUBLE_EQ(subtree_bb->get<double>("out_value"), 1.5);
137+
}

‎tests/plugin_issue953/plugin_issue953.cpp‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
/**
2+
* Plugin for testing Issue #953: convertFromString in plugins
3+
*
4+
* This plugin defines a custom type (Issue953Type) with its convertFromString
5+
* specialization ONLY in this file. The test executable that loads this plugin
6+
* does NOT have access to the convertFromString specialization.
7+
*
8+
* Before the fix: getInput<Issue953Type>() would fail because the executor
9+
* couldn't find the convertFromString specialization.
10+
*
11+
* After the fix: getInput<Issue953Type>() works because the StringConverter
12+
* is captured in PortInfo when InputPort<Issue953Type>() is called (here in
13+
* the plugin), and getInput() uses that stored converter.
14+
*/
15+
16+
#include "behaviortree_cpp/bt_factory.h"
17+
18+
// Custom type defined ONLY in the plugin
19+
struct Issue953Type
20+
{
21+
int id;
22+
std::string name;
23+
double value;
24+
};
25+
26+
// convertFromString specialization ONLY in the plugin - not visible to executor
27+
namespace BT
28+
{
29+
template <>
30+
inline Issue953Type convertFromString(StringView str)
31+
{
32+
// Format: "id;name;value" e.g., "42;test;3.14"
33+
const auto parts = BT::splitString(str, ';');
34+
if(parts.size() != 3)
35+
{
36+
throw BT::RuntimeError("Invalid Issue953Type format. Expected: id;name;value");
37+
}
38+
39+
Issue953Type result;
40+
result.id = convertFromString<int>(parts[0]);
41+
result.name = std::string(parts[1]);
42+
result.value = convertFromString<double>(parts[2]);
43+
return result;
44+
}
45+
} // namespace BT
46+
47+
// Action node that uses Issue953Type
48+
class Issue953Action : public BT::SyncActionNode
49+
{
50+
public:
51+
Issue953Action(const std::string& name, const BT::NodeConfig& config)
52+
: BT::SyncActionNode(name, config)
53+
{}
54+
55+
BT::NodeStatus tick() override
56+
{
57+
// This getInput call relies on the stored StringConverter from PortInfo
58+
// because the executor doesn't have the convertFromString specialization
59+
auto result = getInput<Issue953Type>("input");
60+
if(!result)
61+
{
62+
std::cerr << "getInput failed: " << result.error() << std::endl;
63+
return BT::NodeStatus::FAILURE;
64+
}
65+
66+
const auto& data = result.value();
67+
68+
// Store results in output ports so the test can verify them
69+
setOutput("out_id", data.id);
70+
setOutput("out_name", data.name);
71+
setOutput("out_value", data.value);
72+
73+
return BT::NodeStatus::SUCCESS;
74+
}
75+
76+
static BT::PortsList providedPorts()
77+
{
78+
// When InputPort<Issue953Type>() is called here (in the plugin),
79+
// GetAnyFromStringFunctor<Issue953Type>() captures the convertFromString
80+
// specialization that IS visible in this compilation unit.
81+
return { BT::InputPort<Issue953Type>("input", "Input in format: id;name;value"),
82+
BT::OutputPort<int>("out_id", "{out_id}", "Parsed ID"),
83+
BT::OutputPort<std::string>("out_name", "{out_name}", "Parsed name"),
84+
BT::OutputPort<double>("out_value", "{out_value}", "Parsed value") };
85+
}
86+
};
87+
88+
// Register the node when plugin is loaded
89+
BT_REGISTER_NODES(factory)
90+
{
91+
factory.registerNodeType<Issue953Action>("Issue953Action");
92+
}

0 commit comments

Comments
 (0)