Integrate planning tests #100
Conversation
…call to Dijkstra::execute to make a plan
… to executor" errors
…eems to be an issue with publishing from the dijkstra planner
mkhansenbot
requested review from
mkhansenbot,
crdelsey,
mhpanah and
bpwilcox
| @@ -34,7 +34,7 @@ template<class CommandMsg, class ResultMsg> | |||
| class TaskServer : public rclcpp::Node | |||
| { | |||
| public: | |||
| explicit TaskServer(const std::string & name) | |||
| TaskServer(const std::string & name, bool autoStart = true) | |||
There was a problem hiding this comment.
In the case where you are just using the TaskServer instead of inheriting from it, if you don't set autoStart to true, you can't start the worker thread because the startWorkerThread method is protected.
| { | ||
| RCLCPP_INFO(node_->get_logger(), "Costmap::Costmap"); | ||
| lethal_threshold_ = std::max(std::min(lethal_threshold_, 100), 0); |
There was a problem hiding this comment.
Why do we override the lethal_threshold here? Seems to me if the client set it to some value, we should honor that or throw/assert if it isn't valid.
There was a problem hiding this comment.
I'll enter an issue for Carlos for this item.
|
|
||
| set(TEST_LAUNCH_DIR ${CMAKE_CURRENT_SOURCE_DIR}/test_launch_files) | ||
|
|
||
| ament_add_gtest_executable(test_planner_node |
There was a problem hiding this comment.
All these cmake commands to build and add the tests should probably go in the BUILD_TESTING section. It means there won't be anything to build if that flag is not set, but I think that is correct behavior.
| TEST_MAP=${CMAKE_CURRENT_SOURCE_DIR}/maps/map.pgm | ||
| ) | ||
|
|
||
| install(TARGETS test_planner_node |
There was a problem hiding this comment.
I don't think we should install the test executable.
There was a problem hiding this comment.
Ok, updated to not install the test executable.
|
|
||
| auto costmap_service_callback = [this]( | ||
| const std::shared_ptr<rmw_request_id_t>/*request_header*/, | ||
| const std::shared_ptr<nav2_world_model_msgs::srv::GetCostmap::Request> request, |
There was a problem hiding this comment.
We should start using typedefs and using directives. This is really awful to read.
| } | ||
| } | ||
|
|
||
| TEST_F(PlannerTester, testWithOnFixedEndpoints) |
There was a problem hiding this comment.
Should this test be called testWithOneFixedEndpoint?
…on#100) * Add param to include closer zones * added behaviour to get out of blocked lidar zone * Fixing build errors * PR comments fixes
Integrate Carlos' planner tests.