Allow to set server_name/service_name in RegisterRosAction/RegisterRosService

[panwauu]

Referring to Issue #14

Open points:

• Current version is compiling but throws a runtime error. Make the changes work
• Throw error if neither port nor input is non-empty
• Make input optional by parameter overloading (leads to code duplication?)
• Add changes to documentation

[facontidavide]

what I notice is that you did not remove the input port with server_name, you just add a default value.
I am not sure if this reduces bloat much.

To obtain the same result, there is potentially a better "trick"

template <class DerivedT> static
  void RegisterRosAction(BT::BehaviorTreeFactory& factory,
                         const std::string& registration_ID,
                         ros::NodeHandle& node_handle,
                         const std::string& default_server_name = {} )
{
  NodeBuilder builder = [&node_handle, default_server_name](const std::string& name, const NodeConfiguration& config)
  {
    auto name_it = config.input_ports.find("server_name");
    // The server_name passed as port has priority over the default_server_name
    if( !default_server_name.empty() && name_it->second.empty() )
    {
      auto new_config = config;
      new_config.input_ports["server_name"] = default_server_name;
      return std::make_unique<DerivedT>(node_handle, name, new_config );
    }
    else{
      return std::make_unique<DerivedT>(node_handle, name, config );
    }

  };

  TreeNodeManifest manifest;
  manifest.type = getType<DerivedT>();
  manifest.ports = DerivedT::providedPorts();
  manifest.registration_ID = registration_ID;
  const auto& basic_ports = RosActionNode< typename DerivedT::ActionType>::providedPorts();
  manifest.ports.insert( basic_ports.begin(), basic_ports.end() );
  factory.registerBuilder( manifest, builder );
}

[facontidavide]

note as my code obtain the very same result and does not require any modification of RosAction

[panwauu]

Your solution seems very reasonable. It now does still work with server_name in the tree but I get an error when I remove the port from the tree:

terminate called after throwing an instance of 'nonstd::expected_lite::bad_expected_access<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >'
  what():  bad_expected_access
Aborted (core dumped)

[facontidavide]

Let me investigate

[panwauu]

@facontidavide: I removed the bug and refactored the code a little bit. I tested it for all cases (with/without port, with/without default in RegisterRosX). The port is prioritized and an error is thrown if no port/default is provided. The solution is backwards compatible.

From my side this is ready to merge the include files. Should we document this feature in the test_bt.cpp file?

[panwauu]

Hi @facontidavide,
Do you think we need to document it (for example in test_bt.cpp) ?
It should be ready to merge otherwise.
Thanks and Best