ros-navigation / navigation2

ROS 2 Navigation Framework and System
https://nav2.org/
Other
2.3k stars 1.2k forks source link

[Docking] Feature: support for not needing to set dock instances #4442

Closed jcarlosgm30 closed 2 weeks ago

jcarlosgm30 commented 2 weeks ago

Basic Info

Info Please fill out this column
Ticket(s) this addresses #4415
Primary OS tested on Ubuntu 22.04
Robotic platform tested on Gazebo Simulation
Does this PR contain AI generated software? No

Description of contribution in a few bullet points

The changes made are as follows:

These changes enable the system to work in the following use cases:

Also, the reload database service is available in any case where at least one dock plugin has been defined.

Description of documentation updates required from your changes


Future work that may be required in bullet points

For Maintainers:

SteveMacenski commented 2 weeks ago

test_dock_database.gtest.missing_result Your changes still are making a test fail, I think you need to look into that, but otherwise this looks good to me! My guess is the change in error handling has made a test fail and then since we pass successfully for no-docks created, something tries to access an empty vector and thus crashes.

But you should look into why that test is failing to make sure its not something in the logic you changed either!

jcarlosgm30 commented 2 weeks ago

Yes, that test fails because there is no dock plugin initialized so the /reload_database service will not be created.

The old logic was that the /reload_database service was only created when getDockPlugins and getDockInstances returned false. As we discussed, the new logic creates the /reload_database service whenever at least a dock plugin is initialized.

So I think it will be enough to initialize a dock plugin in the test.

SteveMacenski commented 2 weeks ago

Fabulous, thank you!