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

Update gripper descriptions for use on HW #206

Merged
merged 7 commits into from
Feb 14, 2024

Conversation

dyackzan
Copy link
Collaborator

Pass though more hardware parameters for gripper control:

  • use_internal_bus_gripper_comm: specify if gripper comms will pass through the robot internally
  • (gripper_)include_ros2_control: specify if a ros2_control instance is needed for the gripper (will be needed if we are simulating or directly communicating with the gripper via USB from the PC)
  • (gripper_)com_port: specify the port the gripper can be exected on

Pass though more hardware parameters for gripper control:
* `use_internal_bus_gripper_comm`: specify if gripper comms will pass
  through the robot internally
* `(gripper_)include_ros2_control`: specify if a ros2_control instance
  is needed for the gripper (will be needed if we are simulating or
  directly communicating with the gripper via USB from the PC)
* `(gripper_)com_port`: specify the port the gripper can be exected on
@@ -29,7 +29,9 @@
use_external_cable:=false
initial_positions:=${dict(joint_1=0.0,joint_2=0.0,joint_3=0.0,joint_4=0.0,joint_5=0.0,joint_6=0.0,joint_7=0.0)}
gripper_max_velocity:=100.0
gripper_max_force:=100.0">
gripper_max_force:=100.0
gripper_include_ros2_control:=true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this default to true? Is that really the common use case?

Can we add some documentation to the main README to tell users about the options the have here and what they do?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I defaulted it to true since it should be true in simulation, but I can change that if you think the most common use case is on hardware when the gripper is plugged in to the kinova arm interface. We could also just remove the default and require it to be explicitly specified...I'm also thinking now, couldn't we just determine this one based on the use_internal_bus_gripper_comm parameter? In all cases where we're not using the internal kinova comms to the gripper (simulation or USB connection to the gripper) wouldn't we require a ros2_control instance for the gripper?
I added documentation on the parameters in the kortex_description repo.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm also thinking now, couldn't we just determine this one based on the use_internal_bus_gripper_comm parameter?

I was thinking the same thing and agree it would be a better approach.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok done. And just tested it, looks like it's working as expected.

@dyackzan dyackzan requested a review from MarqRazz February 13, 2024 01:18
`use_internal_bus_gripper_comm` was removed from the `load_gripper`
macro since it was unused.
Add "Default" column to table and add "Example Usage" section in place
of "Example Value" column in table
Add `use_internal_bus_gripper_comm` param back into `load_gripper` macro
and use it to determine whether or not to launch a ros2_control instance
for the gripper. We expect `use_internal_bus_gripper_comm` to be false
if we are running the gripper in simulation or the gripper is
connected to the PC via USB rather than through the internal kinova
bus. In those cases we need to launch a ros2_control instance to control
the gripper.

Remove `gripper_include_ros2_control` param from macros since we're
using the `use_internal_bus_gripper_comm` param and the simulation
params to determine if we need to launch a ros2_control instance for the
gripper.

Update the kortex_description readme to reflect these changes.
@dyackzan dyackzan force-pushed the update-gripper-descriptions-for-hw branch from 67ce71f to b870401 Compare February 13, 2024 17:37
@dyackzan dyackzan enabled auto-merge (squash) February 13, 2024 18:36
Copy link
Collaborator

@MarqRazz MarqRazz left a comment

Choose a reason for hiding this comment

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

These changes look good but because lots of the included launch files are in a broken state this is difficult to test.

I was able to run

ros2 launch kinova_gen3_7dof_robotiq_2f_85_moveit_config robot.launch.py   robot_ip:=yyy.yyy.yyy.yyy   use_fake_hardware:=true

and had to manually switch the planner to OMPL and then I was able to move the gripper and arm in sim. I don't have access to hardware right now to test so we will have to open up new PR's if this introduces issues.

@dyackzan dyackzan merged commit cbfb28e into main Feb 14, 2024
10 checks passed
@dyackzan dyackzan deleted the update-gripper-descriptions-for-hw branch February 14, 2024 17:32
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.

2 participants