Enable a BtActionNode child class to stop a goal from being sent if it is invalid

[mrmitchadams]

Feature request

Feature description

It would be nice if a child class of BtActionNode could have more control over whether a goal is sent. In the current implementation, the first time a BtActionNode is ticked in the behavior tree, it calls user defined on_tick() function, giving the user a chance to populate the goal, etc. Immediately after, the unlerlying tick() function calls the send_new_goal() function and the goal is sent. However, if for some reason the goal should not be sent (invalid blackboard parameters, the robot not in an appropriate state for executing the goal, etc.) there ought to be a way for the user to indicate that from the user defined function.

In short, there is no way to prevent a goal from being sent once a BtActionNode is ticked.

Implementation considerations

A simple solution could be to have on_tick() return a boolean (it is currently void). If the user returns true, the tick() function can proceed to send the goal. If the user returns false, the tick() function would not send the new goal, but would immediately return BT::NodeStatus::FAILURE

[SteveMacenski]

Seems like a solid idea! Seems like on_wait_for_result should too considering it can modify the goal and flip the flag goal_updated_ for sending a new goal too.

We have a couple of options: a return value, throwing an exception, or having another class member set to represent if a new goal should be sent that can be modified anywhere in the pipeline.

My preference would not be for exceptions. I kind of like the new class member since that's pretty flexible for anyone to set so that would cover both on_tick / on_wait_for_result, and anywhere else in the future we might want to do such a thing. We'd need to make sure its properly reset between iterations / goals, so a return type would probably be simpler to think about implementation wise.

Do you have a preference?

[mrmitchadams]

If we went the class member route, we would not need to change the function declaration. And this change (and any related changes in the future) would be backwards compatible, right? That benefit alone sways me in that direction.

[SteveMacenski]

Correct. I could backport this to Humble if we did not change the function signatures

[SteveMacenski]

Any update?

[mrmitchadams]

I got busy, placed this on the back burner, and have not looped back around to it. Thanks for the reminder. I am still working all on Galactic / 20.04, so as soon as I catch time to update part of my machine I'll get this done.

[mrmitchadams]

Thanks @owen7900 for taking this on!

[tonynajjar]

FYI opened #3431 to do similar in a BTServiceNode. Thanks for the original contribution