ros-navigation / navigation2

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

Feature: select controllers and planners from RViz #3269

Closed padhupradheep closed 9 months ago

padhupradheep commented 2 years ago

Feature request

Feature description

I had this idea of creating an RViz plugin for changing the controllers, planners, etc on the fly from the RViz. Like if someone is testing out a new controller, he or she might always have to use the controller_selector topic to publish the controller_id (provided they have a controller_selector node included in their BT plugin). Of course this can all be done from the terminal. This RViz plugin will just provide some additional cushion for users.

Some may want to even tryout different combinations of planners and controllers, in that case too, it would be easy to keep track of the planners and the controllers that are in use from the RViz plugin. Also it acts as a status indicator on which controller_id is being utilized for the process at hand.

Implementation considerations

Currently I just did a small prototype as shown below (yes, it comes with Nav2 logo as well :smile: , but not sure if I should keep it because of the space constraints )

select_planner_controller

So, when the robot is in the navigation state, you cannot do any changes to the planner id and the controller id. The robot has to reach the goal in order to change the planner or controller for the next process.

CC: @jwallace42

SteveMacenski commented 2 years ago

it comes with Nav2 logo as well smile , but not sure if I should keep it because of the space constraints

Could always put it on the left-hand side next to the controller/planner selector dropdowns instead of on top :wink: Maybe could have it added to the Nav2 panel on the right side aligned with the navigation / localization / feedback things (since that doesn't take the full width)

I think this makes sense as an independent plugin like you created. I thought about it being part of the default window, but I think this might be good for some users, but not something that's exposed to them right away by default if its not in the trees used by default. Just avoids the inevitable tickets about how XYZ doesn't work (which they didn't enable). We'd need some docs along with this to explain how to use it e.g. should add these selector nodes to your BT (or maybe even add all the selectors to our BTs so it works out of the box? Though, I like the idea better of people designing their BTs to pick algorithms for different portions of the tree vs setting it via 3rd party applications by default)

How does that list of potential options get populated? I'm not sure the rviz plugin would have a way of getting that list easily

padhupradheep commented 2 years ago

Could always put it on the left-hand side next to the controller/planner selector dropdowns instead of on top wink Maybe could have it added to the Nav2 panel on the right side aligned with the navigation / localization / feedback things (since that doesn't take the full width)

Sure, that is a nice idea. I also looked into the possibility of adding a GIF, something like the parachute taking off from the ground, and maybe just loop it once or twice. But, that seems to be a lot of work :-D .

We'd need some docs along with this to explain how to use it e.g. should add these selector nodes to your BT

Along with it, we could also make it impossible for the users to select the id's of whichever component they want and leave a status in the plugin stating that XYZ cannot be selected, because of the non inclusion of that particular component in the BT (and also if possible "please read the docs".. JK..).

How does that list of potential options get populated? I'm not sure the rviz plugin would have a way of getting that list easily

Currently, I query the parameter server to know about the options that are part of different component servers. The only challenge might occur when there are multi-robot namespace situation, which we might probably need to consider as well.


On a completely different note, while I was working on this RViz plugin, I felt that the names that we use for the different component id's such as "FollowPath" and "GridBased" didn't feel much intuitive, especially while deciding to change them during the execution. There might be some reason, which I might be unaware on following this particular naming style. A potential alternative could be the usage of the names of the planners/controllers/... I totally believe this part is completely subjective and it might be just me and few others in the community who could think like that.

SteveMacenski commented 2 years ago

something like the parachute taking off from the ground, and maybe just loop it once or twice.

Bro, don't give me any ideas. I might have to hire a designer now... I don't want to ship another 200 jackets though to fund it :cry:

Good idea! If we don't detect that there is a subscriber for that topic, then we know its not online and thus that feature can be greyed out.

The only challenge might occur when there are multi-robot namespace situation, which we might probably need to consider as well.

As long as you don't hardcode the global namespace of the topics,e.g. start with /, it should work as it remaps the topics. But yeah, if the nodes are in another namespace, or even the servers are renamed from controller_server to smontroller_server then grabbing parameters stops working immediately. Maybe we can have a service in the task servers for the valid plugins it contains. I can see querying that to be a useful feature in many places. That would move the dependency from a param to a topic name, which if remapped or thrown into a namespace, would continue to work as long as rviz was launched through that same environment. That service could return both the IDs they're mapped to (FollowPath / GridBased) and the literal plugin types they represent.

I don't love those names either and I'd be open to other suggestions. However, as a design philosophy, I want the names to be representative of what they do rather than literal names of what they are. So the algorithm designer (e.g me) knows what algorithm is good at what, but a system designer (e.g. the autonomy engineer building the BT) doesn't need to know much about the internal workings of the various algorithms. They just need to know "this is my crowd controller" or "this is my confined space controller" for how they want to use them.

SteveMacenski commented 1 year ago

Any progress here? I know you've been paneling recently :laughing: Just trying to flush out the issue tracker

padhupradheep commented 1 year ago

At the moment no. I'm bit swamped for the next few weeks. I'll try to get back to this feature development as early as end of Jan.

SteveMacenski commented 1 year ago

@padhupradheep any update on this? It looked like from your original ticket's image you had something working with it and it was just moving those buttons down into the main panel. I think also adding a service in the planner/controller servers to get the algorithms list, but that should be very straight forward.

Is this something you can potentially finish up? I'd love to have this off of our queue

padhupradheep commented 1 year ago

Yes, as said in the other issue. I’ll start working on it in soon as well. It’s in my ToDos as well.

SteveMacenski commented 1 year ago

OK! Personally, I'd prioritize this ticket since its more straight forward and would get immediate use. The WPF time logging ticket is a little more 'nice to have'.

I'm just going through old stale tickets and trying to see what to do with them this afternoon

padhupradheep commented 1 year ago

It looked like from your original ticket's image you had something working with it and it was just moving those buttons down into the main panel.

Can we please keep it as a separate plugin? as explained by you :smile: I really liked your points on why it needs to be a separate one.

I think this makes sense as an independent plugin like you created. I thought about it being part of the default window, but I think this might be good for some users, but not something that's exposed to them right away by default if its not in the trees used by default. Just avoids the inevitable tickets about how XYZ doesn't work (which they didn't enable). We'd need some docs along with this to explain how to use it e.g. should add these selector nodes to your BT (or maybe even add all the selectors to our BTs so it works out of the box? Though, I like the idea better of people designing their BTs to pick algorithms for different portions of the tree vs setting it via 3rd party applications by default)

Also I feel the code base for the main plugin seems to be already big. Having new plugins would help us segregate the basic and advanced features going forward. I've got some more ideas for RViz :wink: let us get this up and running first.

SteveMacenski commented 1 year ago

Sure thing on the separate plugin. I'm not sure how that separate plugin would actually work, e.g. how it updates the original plugin's goal requested algorithm ID (or publishes to the algorithm selector topic?) -- considering if you have multiple algorithms to choose from, you need to specify at least one to work at all since it doesn't guess. But perhaps you've worked that out already with some defaults in the BT selector node?

padhupradheep commented 11 months ago

Finally got some time to make a quick prototype of the feature:

https://github.com/ros-planning/navigation2/assets/20242192/6e9af9dc-4db2-4f2b-a61f-33e6ad509da9

I created 2 controller ids which are identical. In order to differentiate when the controller id changes, there is a difference in the max velocity. I hope it's visible in the video when I change the controller Id.

But perhaps you've worked that out already with some defaults in the BT selector node?

Exactly, by default I use FollowPath controller id. If we are going to have this extra plugin in our default bringup setup, then we'd probably need to add few lines in the default behavior trees with these additional behaviors for selecting the controllers, planners and so on.

What's your take?

Btw, really sorry for the slow progress.. unfortunately, both work and life got swamped :sweat_smile: for the past few months.

SteveMacenski commented 11 months ago

Neat! Lets make sure to update all the default Nav2 BT nodes with those selector nodes so that this works out of the box for all of them

How do you get the 2 versions? I think we discussed adding a service to the planner/controller servers to get the list of valid controller/planner IDs -- but there may be an easier way (and/or reading the server's parameters?)

This looks great! Thanks for eventually getting to it :-)

padhupradheep commented 11 months ago

How do you get the 2 versions?

As of now I'm using the parameter server. The reason why I had chose this option was because, later I thought, we could add a pop up button that brings all the parameters corresponding to the controllers/planners and changing that on the fly if that could help. Moving more towards the direction of making every single changes from the single window (and avoid opening rqt). I'm not sure if this feature would be something interesting, but I know I'm pitching it to someone who has a very good voice in the community.

However, if you think services would be good, then we can go for that as well..

This looks great! Thanks for eventually getting to it :-)

No problem.. I hope, I can also add a nice Nav2 logo somewhere..

SteveMacenski commented 11 months ago

Parameter works! What's the next step here? That looks like a pretty done-ish demo?

padhupradheep commented 11 months ago

next would be to add the other selectors

anyother selectors in the pipeline?

That looks like a pretty done-ish demo?

yes, that would be more or less the output.. if you want you can already take it to the community.

SteveMacenski commented 11 months ago

Also progress checkers if you're going to be changing the lot.

I think planner/controller is a good starting point though :-) Others are nice-to-haves but not required I don't think to fulfill the intent of this feature. I'd rather you finish up #3347 over spending more time on the checker plugins on this feature. But if you're open to both, I'd be glad for it!

padhupradheep commented 11 months ago

okay, then let me finish up the planner part and get this PR up. If there is someone who wants the other selectors, let's see about it then.

Once that's done, let me finish up #3347

I want to finish both these features by end of this year.

SteveMacenski commented 11 months ago

Thanks! I can't tell you how much of a help it is to have long-standing tickets off my mind :-)

padhupradheep commented 11 months ago

Now, I'm also able to read the parameters of the controller / planner id, that we have selected.

I'd be able to add a drop down list populated with all the params and change their values with a text box, added in parallel to it.

Would this additional component interest you?

(I know it's not in the scope of this ticket, I just got curious and dug all the way into it :sweat_smile: )

SteveMacenski commented 11 months ago

That feels like another rviz panel for another PR potentially. That also might be more sensible not to live in Nav2 if its not Nav2 related + mimicks RQT reconfigure. That might be better for your org's repository contributions to the community! :-)

padhupradheep commented 10 months ago

I'm back at it once again. It's almost done, just some final cleaning... we'll be good to go in few days.. Extremely sorry for the delay.

padhupradheep commented 9 months ago

At the moment, I don't see a way to populate the combo box with the selected defaults in the BT.

It could be confusing, however since the intent of this plugin is for development and testing, I think it could be fine.

What do you think?

SteveMacenski commented 9 months ago

I don't think it is likely possible to get the defaults statically set in the BT without some hacky magic or overcomplicating the selector BT nodes. I think a good solution is to have the controller/planner combo box default to just "default" (non-specific, whatever the set default happens to be)

padhupradheep commented 9 months ago

And adding a fun element, a GiF along with the plugin.. do you have any open navigation gif that we can place here? or anyother gif with the nav2 parachute?

sorry if it's deviating from the primary objective.

https://github.com/ros-planning/navigation2/assets/20242192/623460ec-d37a-45d0-90ef-0b52bed07491

SteveMacenski commented 9 months ago

flying

I have this flying away gif. Perhaps @bperseghetti wants to make a longer one or looping for this effort? :wink: Maybe a small one (to give some scale) so starts on the ground and slowly drifts upward to the top and goes out of screen? :heart:

SteveMacenski commented 9 months ago

Merge imminent