Should Costmap2DROS ptrs be removed from nav2_core interfaces?

[mkhansenbot]

For discussion -

Both the local_planner and global_planner abstract interfaces include a Costmap2DROS ptr in the configure interface.

  /**
   * @param  parent pointer to user's node
   * @param  name The name of this planner
   * @param  tf A pointer to a TF buffer
   * @param  costmap_ros A pointer to the costmap
   */
  virtual void configure(
    rclcpp_lifecycle::LifecycleNode::SharedPtr parent,
    std::string name, tf2_ros::Buffer * tf,
    std::shared_ptr<nav2_costmap_2d::Costmap2DROS> costmap_ros) = 0;

I don't think the Costmap2DROS object is used in the planner_server or controller_server, is it?

I see it is created, then a thread is created to spin it:

 // Launch a thread to run the costmap node
  costmap_thread_ = std::make_unique<std::thread>(
    [&](rclcpp_lifecycle::LifecycleNode::SharedPtr node)
    {
      // TODO(mjeronimo): Once Brian pushes his change upstream to rlcpp executors, we'll
      costmap_executor_.add_node(node->get_node_base_interface());
      costmap_executor_.spin();
      costmap_executor_.remove_node(node->get_node_base_interface());
    }, costmap_ros_);

It seems to me that this could all be done in the plugin as needed. It also seems as though the costmap is an implementation detail of the plugin itself, not the planner_server. If it isn't really used by the planner_server, shouldn't we push it down to the internals of the plugin? That way if someone wanted to use a planner without a costmap, they could do that.

[SteveMacenski]

So costmaps are heavy, you're right to say that the server themselves don't use the costmap, but its the arbiter of the costmap to hand off to the plugins so they can share the costmaps between them and not have a costmap for pluginA and another for pluginB, or lets the plugins go wild and try to reinvent the wheel because you're now throwing them the task of a planning algorithm and also its own representation of the environment for that algorithm to run over. At that point, there's barely any isolation.

Technically the costmap is used in the servers, to get the TF buffer which is used to get the pose information, but that's not me making an argument that this is a reason to keep it, just a statement.

Down the line, assuming we do a bunch of benchmarking and we find this acceptable, I'd like to replace the costmap ptr with a env_model ptr which is an interface to the server running the environmental representation and the wrapper contains all the conversion / rapid access / sampling methods needed for the costmap if it comes to a point where we have the cycles to redesign the costmap models.

In this case, the server hosting the env. representation would be the thing that could decide what implementation to use, but either way we want 1 interface for all the plugins to share, and still would not want to instantiate them in the plugins themselves.

[bpwilcox]

Down the line, assuming we do a bunch of benchmarking and we find this acceptable, I'd like to replace the costmap ptr with a env_model ptr which is an interface to the server running the environmental representation and the wrapper contains all the conversion / rapid access / sampling methods needed for the costmap if it comes to a point where we have the cycles to redesign the costmap models.

In this case, the server hosting the env. representation would be the thing that could decide what implementation to use, but either way we want 1 interface for all the plugins to share, and still would not want to instantiate them in the plugins themselves.

I think this environmental representation wrapper is an important next step for the navigation2 stack. A while ago, we were considering to move to grid_map (ANYbotics/grid_map#178) so I'd like to see a wrapper to support different map representations.

[SteveMacenski]

I think grid_maps is an example of a total replacement for costmap_2d for a variety of reasons rather than an option, but it would be good to have an example of supporting multiple

[orduno]

Another option is to create an abstract interface for a costmap, as this.

Planner still requires a costmap, as in here.

Then we create adaptors for different representations to a costmap.

[SteveMacenski]

@orduno that's certainly a valid direction we could go, but I thought alot of the design you all liked was to have everything as separate servers? I would have thought creating a CostmapInterface class that contains the items necessary to subscribe / request / operate over some costmap stream coming from another environmental representation server would have been more your style.

Then the server does whatever it does as long as it provides a model on a specific topic and exposes specific servers. My ideal version of this interface would contain all the information needed for a height/gradient map that could also be used for a costmap.

[SteveMacenski]

@mkhansen-intel does this ticket have any action items or was the discussion we had enough?

[mkhansenbot]

I'm good with closing this now. It's not a priority until someone has the need for grid maps.