[RQT-JTC] 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: ros-controls/ros2_control#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 ros-controls/ros2_control#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: #891
This PR is also related to #894 (you fixed the URDF source, now where we subscribe always from topic).

[christophfroehlich]

I was able to find some time to test it now.
Maybe you should split the joint-parsing from the the combo-box PR. The first one is great and works well, but I'm not yet convinced from the combo-box:

• Maybe I don't see it, but couldn't you just run
  ros2 run rqt_joint_trajectory_controller rqt_joint_trajectory_controller --ros-args -r /robot_description:=/my_robot_descr instead of changing the gui? Yes, it is more comfortable with the combo box but at least we should add robot_description as default.
• The console output is rather noisy now (>40x Waiting for the robot_description!) after selecting the topic (if the controller_manager was selected before). I don't see what has changed in the logic, please have a look.
• The three combo boxes side by side are not very userfriendly in the default window size. should we change it into several lines?
  

btw: I found a bug, but this also happens with the head version #1136

[delihus]

@christophfroehlich I moved the reading of joints to the another PR #1146

After that I change the robot_description feature.

[saikishor]

I agree with @christophfroehlich on this. Having 3 dropdowns is not very user-friendly. Moreover, Now it is up to the user to choose the proper combination of controller_manager and the robot description. This is very error prone right?

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

[mergify]

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!

[destogl]

@christophfroehlich I have updated this as follows:

1. Do not crash if the joint is missing limits. If a joint on a robot is missing limits, gray it out. This was also crashing if we had wheels on the robot that are not used with JTC, but still wouldn't be an error with joint limits.
2. Reduced output for waiting robot description. And added new output for clarity when waiting for the CM and JTC.
3. Setting default values for the CM and controller, as usually we have one, improves usability. Otherwise, we can choose a different one.
4. @saikishor I would not republish "/robot_description" for now. I don't see a good, sensible example for it.

[destogl]

for stats 👍

[christophfroehlich]

I wanted to test this with example_1, but the sliders are deactivated for the continuous joints. In the rolling version, they were added with -3.14/3.14 limits. Don't you think this makes sense? Or should we change our famous rrbot to have revolute joints instead?

[christophfroehlich]

I tested the robot_description_topic now by remapping it in example_1:
I can't select a JTC with this console log:
[INFO] [1774725639.484563238] [rqt_joint_trajectory_controller]: Waiting for a joint trajectory controller with 'JointTrajectoryController' in the type...
unless I select the topic first. This is not very intuitive

[christophfroehlich]

@lakhmanisahil do you want to take this over? you can open a new PR and fix the conflicts first, it seems that @delihus is not active anymore.

[lakhmanisahil]

@lakhmanisahil do you want to take this over? you can open a new PR and fix the conflicts first, it seems that @delihus is not active anymore.

Hey @christophfroehlich,
Let me take a look at this issue, and I will update you accordingly.

[github-actions]

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.

[mergify]

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

[christophfroehlich]

To make this possible for review, I removed the limit handling and created a separate PR #2404 for that.