Autoremap Feature of SubtreePlus not working for Ports accessed in Constructor of Node

[jakob-ludwiger]

If a port is accessed in the constructor of a node, the autoremapping feature of the SubtreePlus is not working.
Please consider my testcase below which implements a SaySomethingInConstructor which is basically the same node as the SaySomething Node except, that the message port is accessed in the constructor.
The value of this port is written to the blackboard before the tree is generated in the test body.

This feature would be quite useful since the BtActionNode of the Nav2 stack is doing exactly this with the node and the server_timeout port.

class SaySomethingInConstructor : public BT::SyncActionNode
{
 public:
  SaySomethingInConstructor(const std::string& name, const BT::NodeConfiguration& config)
      : BT::SyncActionNode(name, config)
  {
    auto msg = getInput<std::string>("message");
    if (!msg) {
      throw BT::RuntimeError("missing required input [message]: ", msg.error());
    }

    std::cout << "Robot says: " << msg.value() << std::endl;
  }

  // You must override the virtual function tick()
  BT::NodeStatus tick() override
  {
    std::cout << "tick! " << std::endl;
    return BT::NodeStatus::SUCCESS;
  }
  // It is mandatory to define this static method.
  static BT::PortsList providedPorts() { return {BT::InputPort<std::string>("message")}; }
};

TEST(SubTree, SubtreePlusD)
{
  BT::NodeConfiguration config;
  config.blackboard = BT::Blackboard::create();
  static const char* xml_text = R"(

<root main_tree_to_execute = "MainTree" >

    <BehaviorTree ID="MainTree">
        <Sequence>
            <SubTreePlus ID="mySubtree" __autoremap="1" message="{message}"/> <!-- This works -->
            <SubTreePlus ID="mySubtree" __autoremap="1"/> <!-- This does not work -->

        </Sequence>
    </BehaviorTree>
    <BehaviorTree ID="mySubtree">
            <SaySomethingInConstructor message="{message}" />
    </BehaviorTree>
</root> )";

  BT::BehaviorTreeFactory factory;
  factory.registerNodeType<SaySomethingInConstructor>("SaySomethingInConstructor");
  config.blackboard->set("message", "hello");
  BT::Tree tree = factory.createTreeFromText(xml_text, config.blackboard);
  auto ret = tree.tickRoot();
  ASSERT_EQ(ret, BT::NodeStatus::SUCCESS);
}

[facontidavide]

Thanks, I will have a look

[asasine]

I took a crack at this and I think I figured out why this happens, but I'm not sure how to fix it.

The recursivelyCreateTree function crawls the tree and creates an instance of SubTreePlus

BehaviorTree.CPP/src/xml_parsing.cpp

Line 622 in f54f6d8

auto node = createNodeFromXML(element, blackboard, parent);

then finds the __autoremap property to set a flag to autoremap

BehaviorTree.CPP/src/xml_parsing.cpp

Lines 674 to 681 in f54f6d8

	if( strcmp(attr->Name(), "__autoremap") == 0 )
	{
	if( convertFromString<bool>(attr->Value()) )
	{
	do_autoremap = true;
	}
	continue;
	}

but remapping is only done after the subtree's children are created recursively

BehaviorTree.CPP/src/xml_parsing.cpp

Lines 695 to 707 in f54f6d8

	recursivelyCreateTree( node->name(), output_tree, new_bb, node );
	if( do_autoremap )
	{
	auto keys = new_bb->getKeys();
	for( StringView key: keys)
	{
	if( mapped_keys.count(key) == 0)
	{
	new_bb->addSubtreeRemapping( key, key );
	}
	}
	}

I assume autoremapping is done after creating the subtree's children because you need the blackboard's keys to be populated to know which keys to remap. Since the autoremapping is done immediately after the subtree's children are recursively created, the constructor of SaySomethingInConstructor is called before its blackboard has its ports remapped to its parent blackboard.

The first example works with explicit port remapping because that type of remapping is done before children are recursively created, immediately when the XML attribute is read while parsing

BehaviorTree.CPP/src/xml_parsing.cpp

Lines 683 to 693 in f54f6d8

	StringView str = attr->Value();
	if( TreeNode::isBlackboardPointer(str))
	{
	StringView port_name = TreeNode::stripBlackboardPointer(str);
	new_bb->addSubtreeRemapping( attr->Name(), port_name);
	mapped_keys.insert(attr->Name());
	}
	else{
	new_bb->set(attr->Name(), static_cast<std::string>(str) );
	mapped_keys.insert(attr->Name());
	}

[facontidavide]

thanks a lot, @asasine , for investigating this. You are 100% right.

It is a nasty problem, I will try to figure something out

[asasine]

I've thought of a couple solutions since investigating, what do you think?

1. Recommend against using getInput() inside constructors. This is already mentioned in the second tutorial but could be made more explicit by throwing an error message if getInput() is called while crawling the tree and constructing the nodes

   It is always recommended to call the method getInput() inside the tick(), and not in the constructor of the class.

2. Crawl the tree once to set up the blackboards, set up autoremapping, then crawl again to construct nodes

3. Push the autoremap feature into the blackboard type itself, where retrieving/setting any port first checks the current blackboard, and then [recursively] calls the parent blackboard if autoremap is enabled for this blackboard instance.

[facontidavide]

1. People will keep doing it, because no one read documentation! I think we should find a solution or mitigation

I was thinking exactly about solution 3, but 2 is valid too

[facontidavide]

Fixed. Eventually I used solution 2. Thanks both of you @asasine and @jakob-ludwiger