Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[kuka_ros2_control_support] Fix ros2 control support test #29

Conversation

muritane
Copy link

@muritane muritane commented May 19, 2023

without pre-commit currently

@muritane muritane requested a review from gwalck May 19, 2023 14:51
Copy link

@gwalck gwalck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the revised access to the possible test choices, using python API instead of subprocess.

The tests pass, but the new joint_trajectory test should be slightly improved. I did not manage to make the test fail. I tried setting joints in URDF with lower=upper but the mock component does not respect joint limits, so not possible to see if a robot has bad limits that way.
What would be an example of bad URDF that would make the trajectory test fail ? (to see if the test can actually fail)
A bad topic name on the trajectory controller makes the wait function thrown an exception and not fail properly.

After some discussion with @destogl one should ensure that a broken setup between the kuka robot description and the ros2_controller additional description is captured by the test. Currently if a joint name is different than joint_a# in the robot URDF, the tests still pass but the robot_state_publisher does not receive correct joint_names and hence the robot is all white in RViz. Even if we set the command_names for the controllers to random names, the tests pass, because the mock interface uses those and the test relies on matching output/input of the controller_state, not on actual robot joint_states matching controller_state names.

At least wrong names in the additional ros2_control URDF and non-matching names in their param config fails.

@gwalck
Copy link

gwalck commented May 30, 2023

@muritane you wrote "without pre-commit" currently. Do you plan to fix the pre-commit issues ? Other than that, I am happy with the changes. Thanks.

@destogl
Copy link
Member

destogl commented May 30, 2023

If you are fixing pre-commit, please also update this option: StoglRobotics/ros_team_workspace#135

@gwalck
Copy link

gwalck commented May 30, 2023

@muritane , could you also make use_mock_hardware to true by default in the test_bringup.lauch.py

because current set of default values leads the manually started test_bringup to crash (RSI use_rsi_communication and EKI use_eki_communication are both to false as well leaving not hardware_interface instantiated at all) even with the minimum mandatory arguments given.

@muritane
Copy link
Author

for fixing precommit there is separate pr and still wip: #30

@muritane muritane merged commit 0e7f143 into StoglRobotics-forks:ros2_control_support_package_added May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants