search

ros-controls / ros2_controllers

Generic robotic controllers to accompany ros2_control
https://control.ros.org
Apache License 2.0
347 stars 312 forks source link

test_load_controller tests always succeed #1258

Open christophfroehlich opened 3 weeks ago

christophfroehlich commented 3 weeks ago

@christophfroehlich it's probable that they are returning successful even without parameters. That's the only explanation. I could try fixing them tomorrow.

but it throws the exception, and CM::add_controller_impl should return nullptr..

Furthermore, I'm not sure why load_JTC succeeds. it should throw an error without paramaters, but I only see 

1: [INFO] [1724185010.487326696] [test_controller_manager]: Subscribing to '/robot_description' topic for robot description.
1: [INFO] [1724185010.487412910] [test_controller_manager]: Loading controller 'test_joint_trajectory_controller'
1: [ERROR] [1724185010.496710000] [test_joint_trajectory_controller]: Unable to start transition 5 from current state shuttingdown: Could not publish transition: publisher's context is invalid, at ./src/rcl/publisher.c:423, at ./src/rcl_lifecycle.c:368
1: [WARN] [1724185010.496722342] [rclcpp_lifecycle]: Shutdown error in destruction of LifecycleNode: final state(unconfigured)
1: [       OK ] TestLoadJointStateController.load_controller (40 ms)

This is not really linked to this PR here. I suggest applying the same changes to admittance_ctrl and range_sensor_broadcaster (the only controllers using a yaml file which are not already addressed) and create a follow-up issue for those false-positive tests.

_Originally posted by @christophfroehlich in https://github.com/ros-controls/ros2_controllers/issues/1256#issuecomment-2299717499_

saikishor commented 3 weeks ago

@christophfroehlich I checked the tests earlier and I saw that some of them are using ASSERT_EQ instead of ASSERT_NE to nullptr. I guess that's why they are passing