add payloads to Action

[facontidavide]

The main goal of this PR is to support users that want to send and receive more comple payloads, when executing a Tree (arguments and return values).

A single string is used for both, since the majority of the users can use JSON to store any argitrary set of arguments.

The return value of onTreeExecutionCompleted has been changed to support a return value.

Also, about this code :

  // return success or aborted for the action result
  if(status == BT::NodeStatus::SUCCESS)
  {
    RCLCPP_INFO(kLogger, "BT finished with status: %s", BT::toStr(status).c_str());
    goal_handle->succeed(action_result);
  }
  else
  {
    action_result->error_message = std::string("Behavior Tree failed during execution "
                                               "with status: ") +
                                   BT::toStr(status);
    RCLCPP_ERROR(kLogger, action_result->error_message.c_str());
    goal_handle->abort(action_result);
  }

I don't think that a FAILURe shoult trigger a goal_handle->abort since, from a technical point of view, the tree has been executed.

[MarqRazz]

I really like the idea of a payload, my original thought was to make it a file path to json files and this easily extends to that and more. I would like to include some tooling to make working with json encoded strings and files easy because I see that as a common use case! But that can come in future PRs.

As far as the tree failing and the result of the action returning aborted, I think it makes sense. Let's say I make a behavior that asks the user if the tree should continue and they say no, or we need sensor data and it's not available. If the tree finishes early and is not able to finish wouldn't that be considered aborting?

[facontidavide]

@MarqRazz check my latest changes

[MarqRazz]

Great work simplifying 🥇

[MarqRazz]

Should we override the return message or add a payload for this additional information in the result? What if the tree errors/fails and there is useful information in the result_message but we also want a payload? It looks like the message is not overwritten when we encounter an exception and that is the only place I can think of where we need the additional info so maybe 1 string is enough.

[facontidavide]

we already have many hints about the fact that an error occurred:

• the Action client returned a gola aborted or cancelled
• NodeStatus will not be SUCCESS

Also, the payload may contain both human-readable and machine-readable data (not my problem 😝 )

[MarqRazz]

Is there any chance of the user accessing the tree and it being outdated or empty? It doesn't seem like there is.

[facontidavide]

Technically an empty Tree can be recognized.
But in general I don't believe this can happen, this is the reason why I made the change