Add a method to get results from the bt service node

[MZLover]

Feature request

Feature description

Implementing service bt using bt_service_node.hpp. I want to do something with the result of the service from the corresponding bt, but unlike the bt action node, there seems to be no method or member variable to get the result value from the bt service node. I don't know if it's the intended configuration or if I misunderstood the code.

Implementation considerations

I just wish I could get the result of ros service.

[padhupradheep]

We already have a variable for response..

You can already accessed them from within your BT..

You can see in the clearcostmap BT on how the request is being already access in the class.. There should not be much difference for the response as well I guess.

[SteveMacenski]

This is true, you can future_result_.get() to get a pointer to the message that you can access the response fields from in the on_completion() method.

It might be more natural for users if we changed the signature of on_completion(msg_ptr) so they could access more naturally after the service is done. This line https://github.com/ros-planning/navigation2/blob/806029d0652c2618400f13fe5822f8ad671e1371/nav2_behavior_tree/include/nav2_behavior_tree/bt_service_node.hpp#L171 could be changed to on_completion(future_result_.get()); and then change the signature of the on_completion() function appropriately.

@MZLover would you be open to submitting that as a PR? I'd love your contribution! It's very similar to that of #2958 but for services, good timing

[MZLover]

@SteveMacenski @padhupradheep It was my mistake. I worked on the foxy_devel branch (because my ROS version is foxy). I was embarrassed because the code you mentioned was different from mine. I will change my codes to the main branch and proceed again. Maybe it can be solved with the code of the main code of the main branch. Is there a problem with the code of the main branch and ros foxy?

[SteveMacenski]

No worries at all, it takes time to get familiar with the codebase :-)

I took a look and there is not a way to resolve this in Foxy without API breaking changes (either adding a new function or changing the signature of an existing function). This is the role of the main branch that we can make whatever breaking changes we want to be 'bleeding edge' but then we promise stability in already released distributions. However, I don't believe anything in BT navigator would not work in Foxy, so if you wanted to grab just that package and rebuild on Foxy, it would probably work for you with only minor tweaks. We just couldn't make that change in the foxy branch itself.

[SteveMacenski]

If you could get that PR in sooner than later, that would be fantastic, I'd like this in before we branch off for Humble so that the next LTS distribution (Humble) after Foxy will have this change so you can use it on a stable distribution

[padhupradheep]

Just in case, if @MZLover is not up for it.. I can take this!

[SteveMacenski]

Feel free @padhupradheep ! I just like to give folks the opportunity to contribute if they wish 😄

[SteveMacenski]

Sooner is better - I'm hoping to get a Humble sync by week's end

[SteveMacenski]

If you could do so in the next day, that would be appreciated, we're going to release to Humble this week and I'd like this in before that fork is made.

[MZLover]

@padhupradheep I've been modifying the code so far, but it's not working well. I want you to do it if you can.

[padhupradheep]

I've been modifying the code so far, but it's not working well. I want you to do it if you can.

No worries @MZLover we will cover you!

I just like to give folks the opportunity to contribute if they wish

@SteveMacenski a big +1 for that!

On a similar topic, is there something else that I could help with before we kickout an humble fork. New documentations, fixing faulty node (I know there will be none) or anything?

[MZLover]

@padhupradheep Thank you. I misunderstood and thought I wanted to modify it in foxy development, so I modified the code so that it can be used in foxy.
@SteveMacenski Do you not update the ros foxy version anymore? Modified to use this function in foxy version. It was a simple revision, but I would appreciate it if you could tell me if you need it

[padhupradheep]

Do you not update the ros foxy version anymore? Modified to use this function in foxy version. It was a simple revision, but I would appreciate it if you could tell me if you need it

I cannot answer you the question about foxy updates. But one thing for sure is that, we usually do the updates to other distributions from the main branch..

[SteveMacenski]

It would not be able to be backported to Foxy, it breaks API - but it'll be waiting for you in Humble! Sorry about that :/ we have to promise stability for commercial and professional users so we can't willy-nilly change API mid-distribution unless its a red-alarm fire