ros-navigation / navigation2

ROS 2 Navigation Framework and System
https://nav2.org/
Other
2.6k stars 1.3k forks source link

Documentation on BT nodes, design, and provided XMLs #1700

Closed SteveMacenski closed 3 years ago

SteveMacenski commented 4 years ago

We now have several BT XMLs, many nodes, and questions like #1698 asking about best practices and designing BT nodes / BT trees.

We should add documentation on best practices, available nodes, explanations, and notes for behavior tree designers and behavior tree node designers to help.

I'm also slightly hoping that through this effort to document and explain these topics we may find that there are ways to simplify our nav2_behavior_tree code or take more advantage of smaller behavior tree primitives rather than complex control flow nodes or logic in the factory / navigator.

SteveMacenski commented 4 years ago

It would be good to have visual representations of our provided BTs and also migrate any content from the bt navigator and nav2 BT packages readmes to the website and remove there. We want to have a single centralized place for all the documentation to keep up to date. General package info is OK but detailed explanations and walk throughs do not belong there.

Sarath18 commented 4 years ago

This will be a great addition to the navigation docs, I would like to help with this.

It would be good to have visual representations of our provided BTs

I was wondering why don't we use BehaviorTree.CPP's companion Groot for this task. Currently, I see a few scripts under the tools folder to generate BT visualization from the Behavior Nodes.

Adding the Groot TreeNodesModel will help in visualizing, creating and editing the BT nodes very easily. For starters, I have created the TreeNodesModel for currently used BT Nodes, not all. You can save this as an XML file and then import the models to start creating your own BT or you can add this to the existing BT description inside the <root>...</root> tag.

  <TreeNodesModel>
    <!-- Action Nodes -->
    <Action ID="ComputePathToPose">
      <input_port name="goal"/>
      <output_port name="path"/>
      <input_port name="planner_id" default="GridBased"/>
    </Action>

    <Action ID="FollowPath">
      <input_port name="controller_id" default="FollowPath"/>
      <input_port name="path"/>
    </Action>

    <Action ID="Spin">
      <input_port name="spin_dist"/>
    </Action>

    <Action ID="Wait">
      <input_port name="wait_duration"/>
    </Action>

    <Action ID="ClearEntireCostmap">
      <input_port name="service_name"/>
    </Action>

    <!-- Control Nodes -->
    <Control ID="PipelineSequence"/>

    <Control ID="RecoveryNode">
      <input_port name="number_of_retries"/>
    </Control>

    <Control ID="RoundRobin"/>

    <!-- Decorators -->
    <Decorator ID="RateController">
      <input_port name="hz"/>
    </Decorator>

  </TreeNodesModel>

The tree visualization screenshots:

Navigate with replanning

navigate_w_replanning

Navigate with replanning and recovery

navigate_w_replanning_and_recovery

Navigate with replanning and round robin recovery

navigate_w_replanning_and_round_robin_recovery

SteveMacenski commented 4 years ago

I was wondering why don't we use BehaviorTree.CPP's companion Groot for this task

Something about not supporting custom nodes. It looks like you found how to get around it though, we can add that to tooling or to the BT package and add that support!

SteveMacenski commented 4 years ago

We're in the middle of a big change with the Time / distance / new goal / speed BT updates. I think you should reconsider a documentation push on this front until we sort that out. Hopefully we can get everything in before middle of next week. However, if you're interested in the Groot stuff, none of that should really change so you're welcome to PR files and instructions needed to run that. Also starting the migration from READMEs to the website. There will obviously be changes but not massive.

On the docs list for a BT section on the site:

I'm sure there's more, please let me know what else you think we need

SteveMacenski commented 4 years ago

@Sarath18 a PR to enable groot would be great!

Sarath18 commented 4 years ago

The trick I suggested above will only work in visualizing the trees and creating new nodes (except ConditionNode. Need to do that manually) in Groot. Saving the tree after editing through Groot leads to a different file structure (not the tree). This newly saved tree structure cannot be used with the stack.

One thing I can do right now is to create a TreeNodesModel file for all the existing nodes in the repository which can be imported to visualize all the trees as shown above. This will also work with the stack. More nodes can be created and added to the TreeNodesModel and exported/saved for visualizing trees built in future with these BT Nodes.

Something about not supporting custom nodes.

@SteveMacenski You are right. All the above seems hacky. To get a better integration of Groot with navigation2 we need support on Groot side. I even tried using ZMQ publisher to enable realtime monitoring of BT but again Groot support for custom nodes are not available at the moment.

SteveMacenski commented 4 years ago

One thing I can do right now is to create a TreeNodesModel file for all the existing nodes in the repository which can be imported to visualize all the trees as shown above.

Still a step in the right direction.

What's wrong with export of the trees? Groot is from the same family as BT.CPP, I'm surprised to hear they don't play well.

Sarath18 commented 4 years ago

I'll do that then. I'll even add a README.md on instructions to use Groot

What's wrong with export of the trees? Groot is from the same family as BT.CPP, I'm surprised to hear they don't play well.

Take a look here. You can see the difference the two files. I've tried to load the Groot exported file to the bot in simulation, but I didn't succeed.

Sarath18 commented 4 years ago

What's wrong with export of the trees? Groot is from the same family as BT.CPP, I'm surprised to hear they don't play well.

I checked again, there is nothing wrong with the export of trees as you can visualize the exported trees later in Groot without any issues.

Something about not supporting custom nodes.

To be specific the problem might be because of the custom ControlNodes that are used in navigation2. Neither Groot nor BehaviorTree.CPP has support for creating custom control nodes.

[bt_navigator-10] [ERROR] []: Original error: Error at line 5: -> Node not recognized: Control

If somehow this issue is solved, I think we can use Groot seamlessly to create, edit, modify and even monitor/log the tree status.

SteveMacenski commented 4 years ago

May want to file a ticket for that on groot

gramss commented 4 years ago

Are you aware of these two links: https://github.com/BehaviorTree/BehaviorTree.ROS/issues/1 And this repo here: https://github.com/mjeronimo/ros2_behavior_tree @mjeronimo did some work of stripping the nav2 related code and implemented some easier examples. Could probably be useful for this :) Maybe we can get the interested folks together @SteveMacenski ?

Besides that also BehaviorTree could use some help with documentation. Some cool new "functions" were added, like a switch-case, manual-node, parallelism and do_while. But those were not (fully) listed in the documentation of BT. @facontidavide might be also interested in this and could point out some new non-documented functions that could be integrated while doing some documentation for ROS2.

I got some pretty interesting parts from the behaviorTreev3 implementation while consuming docs and code. Seems very similar/related to SMACH to me, with its internal key/value "blackboard" and input/output keys, but being far more advanced. Having a GUI that can do the linking (and not only introspection) helps enormously.. After even a slightly complex architecture with SMACH you can easily go nuts... (I was trying to implement sort of an BT with it, while not knowing that the concept BT existed.. fun times.. 😄 )

Side note: just saw that it got an update to Python3 9 days ago from Open Robotics for Noetic. Not bad.. https://github.com/ros/executive_smach/commit/31cc55743431f48e37ee71d26acd5833ebd3753e Even a community ROS2 port is on its way.. Never the less, I think I'll stick with BT. Did some intense hands on with SMACH around New Year. Out of my learning perspective it was lacking important functionalities that BT seems to have already.

SteveMacenski commented 4 years ago

@gramss on your 2 links: Those are predecessors of this work. Mike was working on Nav2 in the earlier days and the work that that repo represents is a older and less capable version than the nav2_behavior_tree package.

The rest of your comment I'm not sure what to respond to - it doesn't seem on topic for the ticket. This is a ticket to discuss documentation around the navigation2 behavior trees for application designer and new to BT folks. Also Smach is a really, really old state machine framework. We use behavior trees which as far as I'm aware from academia and industry are far superior. We would not be interested in moving Nav2 over to Smach.

gramss commented 4 years ago

@gramss on your 2 links: Those are predecessors of this work.

Ah cool, good to know :) I wasn't aware of this. Currently exploring mostly the current Behavior.Tree V3 release. Maybe your comment here is something that can be added in like a really short history chapter about NAV2? To give more people like me, who are new to NAV2, more context. But no prio on that. Or at least put a big indicator ontop of a readme in those repos where to find the latest code / doc for BT + ROS2. (That's what I know from other opensource project/repos that were handed over)

Offtopic: > The rest of your comment I'm not sure what to respond to Nevertheless, you totally got my point/situation! I'm coming from academia and want to transition into a more industrial grade software, that BT seems to offer. I just wanted to add the info about SMACH as I was surprised that it got upgraded, while being as you said really old. I'm not at all trying to move NAV2 to SMACH. Just trying to explain where I'm coming from. Besides that, I want to offer my help on this as well. As said, I'm currently investigating the BT code stack. Learning from old issues. Currently investigating what is already there with BT and learning from existing nodes in NAV2.

Regarding docu:

My idea was that improving the BT docu could help improve the docu for NAV2 as well. NAV2 docu about BT should only include ROS2 related interaction with BT and point to BT docu for the rest..? Or Maybe this should even be extended to what is possible with dedicated nodes and BT and what is possible with BT combined with NAV2. From #1698 it seems a bit to me that NAV2 stack might break the modularity BT could offer, while mentioning I have not looked into NAV2 impl of BT interaction. More XMLs would provide a good kickstart for ROS related programming. Becoming creative with your tools would include not only making your ROS-Node BT compatible, but also requiring mastering all the features BT has to offer.

Therefor, I want to propose these points to you:

Why is Parallelism important: Robot is constantly querrying a condition (while sunny) parallel start NAV2 to navigate robot to paris. When sun is stoped, robot should go back/charge/whatever.

More Robot like: Scan Environment during Nav, do some work with robot arms during nav, etc etc.

SteveMacenski commented 4 years ago

I think what you say is to add documentation about BTs, that's what this ticket is for adding. If you read above, you'll see a big check list I made of the things we need to add to that documentation which I think meets your intent.

To your 2 specific boxes

We'd certainly love more help with documentation around the behavior trees. I'd suggest to look at the running list of themes / ideas to chat about above and see if there's a subsect of them that makes sense for you to potentially work on. Even having a good page about "introduction to BT concepts"; a one-pager for the basics you "need to know" before getting into design best practices / considerations could be helpful. I think some of the other WG folks were interested in writing up something on best practices so that would segway well for them to add that. It would be a good start.

gramss commented 4 years ago

I now reflect that my example might have been a bad pick.

Parallism example - take 2:

As seen in some presentations from some major AGV OEMs etc during last years fairs.

BT.V3 includes a decorator for parallism. And it is not spinning separate threads for this. Ticks are still being used but the parallel nodes are ticked sequentially. The ASync nodes should be implemented through action servers. I think you are already doing that. BT now has a way to turn it into something more advanced by shifting the context for tick().

Now comes the new stuff /detailed code explanation: Example: You want to have 2 BT nodes running in parallel with two action clients implemented. First round: The first gets ticked and should return BT::NodeStatus::RUNNING. You are already setting it here: https://github.com/ros-planning/navigation2/blob/09a4878d7b4d58446c2069fae2de952ba83f4a08/nav2_behavior_tree/include/nav2_behavior_tree/bt_action_node.hpp#L130

Because in this parallel decorator here: https://github.com/BehaviorTree/BehaviorTree.CPP/blob/561f4fa86b45c64c9bb2fd3273f81363e6fd222f/src/controls/parallel_node.cpp#L107 Running nodes are simplified "ignored". Like you currently know it. The BT waits for the next tick() if your node now has failed or finished. The Parallel Node is now ready for the next tick(). Now it ticks the second node, that then gets its first tick that gets it from IDLE to RUNNING. After this init round they are continuously checked (could also be a condition at this point) how many of the nodes have already Finished. Here it is important that a tick() on the BT site should exit fast after setting the current BT::NodeStatus. After a user defined threshold of successfully exited nodes, the parallel decorator node finishes and gives the overall output to the next BT "step"/decorator/condition etc.

The execute tick on the BT side is not creating a dedicated thread for an ASync node. So the tick() functions in ASync Nodes should return the context really fast. https://github.com/BehaviorTree/BehaviorTree.CPP/blob/561f4fa86b45c64c9bb2fd3273f81363e6fd222f/src/tree_node.cpp#L33

How to get NAV2 to use that..? I think it probably already can do this..? I was misinterpretating https://github.com/ros-planning/navigation2/blob/09a4878d7b4d58446c2069fae2de952ba83f4a08/nav2_behavior_tree/include/nav2_behavior_tree/bt_action_node.hpp#L141 as "wait_for_result". I think in the current BT impl without Parallelism nodes are already being ticked with a high frequency, as you described it in your comment. So nav2_action_BT should already be able to handle it. The only thing different now from the BT side now would be that multiple nodes are ticked sequentially. This would then decrease the polling level you anticipate on your tick() normally by the other parallel nodes.

I'm very interested what you have to say about this. I hope that you got my point now a bit more clearer. Yes it is breaking traditional BT desings maybe. But there exists also a paper about this parallel behavior. As the title says "improving" I believe that parallism in BT is not totally new in general. You can find a pre-print here: https://www.researchgate.net/publication/330595547_Improving_the_Parallel_Execution_of_Behavior_Trees The first author is also working on the BT impl we are using. I just don't know to what extend and was not able to fully read the paper. The pseudo code in it looked good and relating to the real code, so I'm confident in sharing it with you 🙂

Side note on my example - take 1: the paper uses a example that nav should run while they are seeking for something. So not too far of.. 😄

Maybe reactive Sequence can be seen as a predecessor of a condition<->node relationship towards a node<->node<->condition<->....N relationship ?

Offtopic: I reedited this whole comment twice before commenting.. So some things might sound clearer in upper lines, while they appear more as question in the detailed code explanation section a bit lower.. Again 3am here.
SteveMacenski commented 4 years ago

That's a long topic... but it sounds like that's reasonable and doable. I don't know that there's anything in navigation2 that can specifically take advantage of that capability but good to know its around.

SteveMacenski commented 4 years ago

@gramss @Sarath18 any updates? I would really love to have this worked on :-)

gramss commented 4 years ago

100% commited to do this. Deadline for having a documentation about BT for me would be end of July. Is this okay for you?

SteveMacenski commented 4 years ago

Yeah, that works! I think where we need to start though is just with an outline so that in 1.5 months you don't resurface with something complete but maybe not what we were looking for. Can you give me a rough bulleted outline of the pages and content to discuss on those pages?

We obviously have a non-exhaustive but a good starting point list of topics in this thread (in my literal list and in discussion).

Are you planning on working on something with BTs too, or just the documentation?

SteveMacenski commented 3 years ago

A bunch of this has been added to the nav2 docs in recent months -- as well as other more specific tickets in the docs repo regarding specific things to add.

Closing since it is partially done and partially covered by other tickets located in the docs repo