[RQT-JTC] Find limits only in jtc joints | add robot_description combo

[delihus]

When there are joints what don't have implemented limits rqt-jtc throws a lot of errors.

I added looking for limits for joints what are controlled by actual JTC.

The main feature is that it is possible to change robot description topic to control specific robot. Unfortunately it is not possible to do this using namespace in my case. My namespace issue: https://github.com/ros-controls/ros2_control/issues/1506

Result video: Screencast from 10.05.2024 11:52:20.webm

[christophfroehlich]

When there are joints what don't have implemented limits rqt-jtc throws a lot of errors.

I see the need for the namespace issue, but what do you mean by that? A revolute/prismatic joint without limits is not a genuine URDF, and we throw an error since https://github.com/ros-controls/ros2_control/pull/1256, or ros2_control 4.9.0 respectively.

[delihus]

@christophfroehlich I'm wandering why RQT-JTC checks limits for all joints described in URDF if it has an access for joints controlled by JTC via jtc_info.

I'm asking about this because when the errors about unknown limits of mimic joints throw then there JTC doen't appear in controllers combo. Here is a video where I cannot select the controller.

Screencast from 15.05.2024 16:49:21.webm

[christophfroehlich]

Ok, so the problem comes from mimic joints, where no limits are given? That does not make a lot of sense to add it, but it is not explicitly given in the XML specification that it can be omitted. I'll come back later to check what we excpect in the component parser.

[delihus]

Take a look to robotiq_description. The controlled revolute joint ${prefix}robotiq_85_right_knuckle_joint has limits defined but its mimic joints does not. That is why the exceptions throw.

I don't really know if limits should be implemented in continuous mimic joints or not.

The issue with RQT-JTC it that it is not possible to select a controller if the RQT-JTC cannot read the limits of all joints defined in robot_description. I don't really understand why RQT-JTC reads the limits of all joints. I think RQT-JTC should focus on its controlled joints.

[christophfroehlich]

Thanks for the background information. I agree that it should check the to-be-controlled joints only, but I'm still curious what should be the correct XML in this case. We have the limit tag in our examples and the release notes, but this should not block this PR but related to: https://github.com/ros-controls/ros2_controllers/issues/891 This PR is also related to https://github.com/ros-controls/ros2_controllers/issues/894 (you fixed the URDF source, now where we subscribe always from topic).

[delihus]

@christophfroehlich I moved the reading of joints to the another PR https://github.com/ros-controls/ros2_controllers/pull/1146

After that I change the robot_description feature.

[mergify[bot]]

This pull request is in conflict. Could you fix it @delihus?

[bmagyar]

@delihus could you please rebase this one? Or just resolve the conflicts

[delihus]

@bmagyar Updated

[bmagyar]

Thank you!

[christophfroehlich]

If we need this kind of setup, does it make sense to have the controller_manager republish the robot_description it is currently using?. This way applications such as rqt_joint_trajectory_controller or others can make use of it. What's your opinion on this? @bmagyar @destogl @christophfroehlich

@delihus We discussed this in the ros2_control meeting tonight, we want to go forward with your proposal. Please address my points above (defaulting to /robot_description etc) and we can go ahead with this 😊

[github-actions[bot]]

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

[christophfroehlich]

@delihus Can you please address our comments, so we can merge this?

[codecov[bot]]

Codecov Report

:x: Patch coverage is 0% with 34 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 84.96%. Comparing base (9ed7cbe) to head (d7486b7). :warning: Report is 143 commits behind head on master.

Files with missing lines	Patch %	Lines
...ajectory_controller/joint_trajectory_controller.py	0.00%	21 Missing :warning:
...t_joint_trajectory_controller/joint_limits_urdf.py	0.00%	13 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1131      +/-   ##
==========================================
- Coverage   85.16%   84.96%   -0.21%     
==========================================
  Files         123      123              
  Lines       11755    11783      +28     
  Branches      996      999       +3     
==========================================
  Hits        10011    10011              
- Misses       1431     1461      +30     
+ Partials      313      311       -2     

Flag	Coverage Δ
unittests	84.96% <0.00%> (-0.21%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...t_joint_trajectory_controller/joint_limits_urdf.py	0.00% <0.00%> (ø)
...ajectory_controller/joint_trajectory_controller.py	0.00% <0.00%> (ø)

... and 2 files with indirect coverage changes

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions[bot]]

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

[github-actions[bot]]

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

[github-actions[bot]]

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.