Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add component for costmap_filter_info_server #3589

Closed
FiIipe opened this issue May 31, 2023 · 15 comments
Closed

Add component for costmap_filter_info_server #3589

FiIipe opened this issue May 31, 2023 · 15 comments
Assignees

Comments

@FiIipe
Copy link
Contributor

FiIipe commented May 31, 2023

Feature request

Feature description

Currently, there is no possibility to use the costmap_filter_info_server as a component.

Upon reviewing the recent commit, I noticed that the map_saver and map_server components were added, but the costmap_filter_info_server was not included as a component.

Therefore, I would like to request the addition of the costmap_filter_info_server as a component in the Navigation2 package, if possible.

Implementation considerations

No special considerations for the implementation.

Thank you for considering my feature request. I appreciate your dedication and efforts in continually enhancing the Nav2 package.

@FiIipe FiIipe changed the title Add lib for costmap_filter_info_server Add component for costmap_filter_info_server May 31, 2023
@SteveMacenski
Copy link
Member

SteveMacenski commented May 31, 2023

That seems reasonable enough to me - I'm surprised we overlooked that. @AlexeyMerzlyakov I think that's in your maintainer bin - can you make it a component + update tutorial to use in that way? Unless @FiIipe wanted to contribute it!

@FiIipe
Copy link
Contributor Author

FiIipe commented May 31, 2023

Sure, I would love to contribute to the Nav2 project!
I will start by reading the development guidelines for the Nav2 project and then I will implement the costmap_filter_info_server as a component.

Thank you!

@SteveMacenski
Copy link
Member

SteveMacenski commented May 31, 2023

I think broadly you should only need to register it as a component + cmake to support as lib / registered and it should be good to go with documentation updates. @AlexeyMerzlyakov, anything else you think @FiIipe would need to do?

@AlexeyMerzlyakov
Copy link
Collaborator

AlexeyMerzlyakov commented Jun 1, 2023

@FiIipe, thank you for the participation to contribute it!
It should be done the following:

  1. Register nav2_map_server::CostmapFilterInfoServer class from the MapServer's core library as a component node in the CMakeLists.txt
  2. Add standard registration include/comment/macros for this class at the end of costmap_filter_info_server.cpp file
  3. Support nav2_map_server::CostmapFilterInfoServer class to use ROS node-options to be able to operate in a composition with another nodes (for more details, please refer to the Add argument node options #2522 PR)

Since costmap_filter_info_server is not a part of standard Nav2 bringup, we do not need to do anything else in nav2_bringup, so the above action items should be enough.

@SteveMacenski
Copy link
Member

^ though tutorials changing to use as a component would be of real value

@FiIipe
Copy link
Contributor Author

FiIipe commented Jun 3, 2023

@SteveMacenski @AlexeyMerzlyakov I created two PR. One is already mentioned in this ticket and the other is for the navigation_tutorials package. The PR can be found here: ros-navigation/navigation2_tutorials#67

If my changes are approved, then it would be important to update also the documentation here: https://navigation.ros.org/tutorials/docs/navigation2_with_keepout_filter.html#configure-costmap-filter-info-publisher-server

Thank you 🤖

@SteveMacenski
Copy link
Member

Documentation + tutorials repo changes still required!

@alejama
Copy link

alejama commented Jun 9, 2023

Hi!
I'm trying to use nav2_costmap_filters_demo, but when I'm running shows me next problem:
launch.substitutions.substitution_failure.SubstitutionFailure: executable 'costmap_filter_info_server' not found on the libexec directory '/opt/ros/foxy/lib/nav2_map_server'

@SteveMacenski
Copy link
Member

SteveMacenski commented Jun 9, 2023

Foxy Is EOL and is no longer supported. It does not contain the costmap filter features. Upgrade to Humble :-)

@Filipe @AlexeyMerzlyakov how are the docs / tutorial changes coming along? It would be good to close this out next week

@FiIipe
Copy link
Contributor Author

FiIipe commented Jun 9, 2023

@SteveMacenski @AlexeyMerzlyakov I will look into it now, hopefully it will be merged soon :)

Thank you

@SteveMacenski
Copy link
Member

SteveMacenski commented Jun 27, 2023

@FiIipe any update? I'd like this squared away ASAP so we can get this off the issue tracker before everyone disappears for summer vacations

@FiIipe
Copy link
Contributor Author

FiIipe commented Jun 27, 2023

@SteveMacenski I'm waiting Alexey's review, he was on vacation. I'm not sure he is back

@SteveMacenski
Copy link
Member

Got it, thanks!

@AlexeyMerzlyakov
Copy link
Collaborator

AlexeyMerzlyakov commented Jul 5, 2023

Apologies for late review: hot season after vacation today. Tutorials PR is almost done. Please submit a new PR to documentation updates describing the switching of CostmapFilterInfoServer to using Composable mode.

@SteveMacenski
Copy link
Member

SteveMacenski commented Aug 30, 2023

@FiIipe @AlexeyMerzlyakov can we try to get this done shortly? The actual code change is done and its a bad spot to be in without the docs

Edit: I guess if its just a tutorial its not that big of a deal. I'll just add a remark inline that you can also use composition nodes and be done with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants