Nav2 Docking: The docking_server does not work if you do not define instances of docks using the DockDatabase about navigation2 HOT 4 CLOSED

I agree, I think also having the reloading service work in either case (https://github.com/ros-navigation/navigation2/blob/main/nav2_docking/opennav_docking/src/dock_database.cpp#L45) would be a positive improvement in this part of the code as well (I'm not 100% sure what I was thinking there). But, if that function always returns true when validly configured (i.e. no corrupted yaml files) and the dock plugins has to have at least 1 valid option, your change would virtually eliminate that issue.

I agree. I think the reloading service should be available as long as there is at least 1 valid dock plugin

How I get around this now is just having a dummy_dock that we ignore, but it would be great not to require such a thing. I'm of 2 minds if I should ask you to remove that from our YAML file. I think its useful to have an example nonetheless. Perhaps you could comment it out so its just there for illustration purposes in your PR?

I think it is important to keep it at least as an example to make the configuration easier for users. In the PR below, I have commented it out.

I have opened a PR-4442 with these changes. We can further discuss changes in it if that's okay with you.

from navigation2.

Seems like a good improvement to make it fully optional in case you want to not specify dock instances. You are required to load all the dock plugins required - since those must be known by the server for use in the database or via the action request. But, instances could be made fully optional. Note that specifying the full dock information at request-time is really meant more as a mechanism for testing and if you're setting up a facility with new docks live. If you have a predefined set of docks, you should really load them either via the Nav2 configuration file or linking the dock's list file as laid out in the tutorial https://docs.nav2.org/tutorials/docs/using_docking.html

Open to submitting a PR?

from navigation2.

@SteveMacenski Yes, I agree. This functionality would provide flexibility to set up a facility with new docks live, and even facilitate integration with other higher-level systems that manage the storage of robot data via database or other mechanisms.

Open to submitting a PR?

I am open to submitting it. Just to discuss the approach, I think the functionality could be achieved by logging a good warning message here and returning true. And also, adding some log messages elsewhere to be more informative to the user.

What do you think? Do you think there are any additional requirements or considerations in order to add this feature?

from navigation2.

I think the functionality could be achieved by logging a good warning message here and returning true

I agree, I think also having the reloading service work in either case (https://github.com/ros-navigation/navigation2/blob/main/nav2_docking/opennav_docking/src/dock_database.cpp#L45) would be a positive improvement in this part of the code as well (I'm not 100% sure what I was thinking there). But, if that function always returns true when validly configured (i.e. no corrupted yaml files) and the dock plugins has to have at least 1 valid option, your change would virtually eliminate that issue.

How I get around this now is just having a dummy_dock that we ignore, but it would be great not to require such a thing. I'm of 2 minds if I should ask you to remove that from our YAML file. I think its useful to have an example nonetheless. Perhaps you could comment it out so its just there for illustration purposes in your PR?

from navigation2.