Set groot monitoring parameters individually for bt_navigator's pose …

[jonatanolofsson]

Set groot monitoring parameters individually for bt_navigator's pose and poses navigators

---

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	#2406, #2408
Primary OS tested on	Ubuntu 20.04 (docker container)
Robotic platform tested on	gazebo simulation

---

Description of contribution in a few bullet points

• Groot monitoring now set up for poses and pose bt_navigator action nodes separately
• Interface added to allow setting of relevant parameters

Description of documentation updates required from your changes

• Added six new parameters, so need to add that to default configs and documentation page

---

Future work that may be required in bullet points

• Regression tests could be added

For Maintainers:

• Check that any new parameters added are updated in navigation.ros.org
• Check that any significant change is added to the migration guide
• Check that any new functions have Doxygen added
• Check that any new features have test coverage
• Check that any new plugins is added to the plugins page
• If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

[SteveMacenski]

All PRs must also be targeted to main and they will be cherry picked back down to released distributions if ABI/API compatible. This helps us make sure that any changes are reflected in all new distributions as well. This PR changes parameter names / defaults so this would not be possible to backport to Galactic. It could though be merged into main and you'd get this capability available to you on rolling or master builds (maybe also galactic, I haven't checked their compatibility recently)

This definitely does solve a real issue #2386 and #2406 so I'd like to get this merged in to main at the very least, but I'm also thinking about how we can migrate this to Galactic that wouldn't break ABI/API.

[SteveMacenski]

Overall looks good, just some clean up so that we don't have some parallel APIs people are confused by

[SteveMacenski]

Hey! It looks like your force push here also caused some havoc, see above

[SteveMacenski]

Note @jonatanolofsson #2417 (the default for enable should be false now) please make sure when you resolve your conflicts you also make sure this is false.

[jonatanolofsson]

I fixed the target branch and the defaulting to false.

However, I still can't seem to connect to the poses tree. All printouts I have added (locally) indicate that the monitoring is instantiated correctly and in the same was as the pose monitor, so I don't believe the error is in this code.

[SteveMacenski]

Keep in mind that in this situation, even though you've set different ports, only 1 Groot Monitor object is active at a time. This has nothing to do with ports anymore and just how ZMQ works (it crashes if you try to make 2 instance in the same process). So at instantiation, we make the Pose BT monitored, so if you launch groot, you'd see that.

If you call on the Nav Through Poses, I think you'd probably have to fully exit the Groot system, and then relaunch it with the new ports since its now a completely different object that's been switched to. You may also want to verify that we do indeed switch correctly (but sounds like you verified that).

[jonatanolofsson]

Keep in mind that in this situation, even though you've set different ports, only 1 Groot Monitor object is active at a time. This has nothing to do with ports anymore and just how ZMQ works (it crashes if you try to make 2 instance in the same process). So at instantiation, we make the Pose BT monitored, so if you launch groot, you'd see that.

@SteveMacenski Is this why I could only connect to the "pose" tree? If it's impossible to have an active monitor on both simultaneously, then perhaps we should only start one? Where is it that you "make the Pose BT monitored"?

Sorry for the late reply, last week was busy!

[SteveMacenski]

I had asked you to test if you started the Nav through poses action and then launched groot to see if you see it then, since only 1 instance is active at any give time, and they have different ports. If you can see it then, then things are working properly, its just that groot needs a restart when a new object / tree are formed and that's not really an "us" problem, that's a problem you'd need to field with them. But you need to do some testing to see how to get a groot session opened with the nav through poses tree. If you're saying that its running and now from this PR its on its own port, there's no reason it shouldn't be accessible to groot. My only guess is that maybe try launching groot after its already on this tree or maybe you didn't change the ports to the other one in your existing groot session (or the groot sessions don't handle changing ports correctly). I can't tell you, I don't use groot.

Frankly, I'm at half a mind to remove groot entirely. Its caused a bunch of headaches and if you're reporting that its not working in a way that makes it useful to you, I'm not sure that the usefulness of it is in general. If you can get it to work and have some instructions to do so, then we can keep it, but I'm not totally off the camp that this was a 'nice to have' feature that's causing more problems than its worth.

[SteveMacenski]

@jonatanolofsson any update here? Did this make things work eventually? Else, I'm planning on removing groot support before the next release. Happy to keep it if there's a workaround for you to view the active BT.

[jonatanolofsson]

Thanks for the heads up! I just got back from vacation, so I will check with my team tomorrow to see if we can help!

[SteveMacenski]

Awesome! Definitely would love to keep this and I think this PR is pretty much good to go as long as you tell me that it works (e.g. you can visualize both the trees -- probably by closing the groot application and reopening again between switches. A groot issue but if that works, then we can punt the ticket over to groot to enable changing of trees). There were a few comments above on naming and such but those are small and easy to fix.

If we can't find a way to visualize both trees though at all, I wonder if we should just remove groot visualization entirely from the BT navigator.

[albuckley]

A lot of people in the ROS community are not going to be very vocal. I would like to make the claim that dropping support for Groot would be gutting and it shouldn't be a nice-to-have. Developing behaviour with a viewer isn't strictly critical but it is a night and day difference without one. With the other behaviour frameworks seemingly orphaned, it leaves us with Groot as the only option that can potentially work.

I would think there are many people that would be grateful to see this integrated.

[mergify]

This pull request is in conflict. Could you fix it @jonatanolofsson?

[SteveMacenski]

@jonatanolofsson I'm taking over this PR just to get it moving again. I'll be patching it up and then resubmitting a new PR with these changes. I think its 90% there. I'll link to that PR here for visibility.

@albuckley it wouldn't be removing support for using Groot to develop / adjust your behavior tree. It would only be removing support for live visualization of the currently executing behavior tree during live navigation. Is that what you mean, or just using groot to create your behavior tree and export the XML? Right now, the live visualization is quirky because of how BT.CPP is with zero-MQ creating an issue where it might not show the currently executing BT because we have 2 parallel navigators for nav through poses and nav to pose.

[SteveMacenski]

#2627

[jonatanolofsson]

@SteveMacenski Thanks. I fear I never got the time to debug this. IIRC, I still failed to contact the "poses" server when both were started, though other's experience may vary. The new (now merged) interface is to no detriment though, and any issues with e.g. zmq can be resolved separately.

Good work, @SteveMacenski