Rework base robot classes and add Kuka iiwa Med support #60
Conversation
…rate victor. Victor basic motion working in gazebo.
val_basic_motion demo working
arm_robots/launch/med.launch
Outdated
| </include> | ||
|
|
||
| <!-- ROS control for real victor, does nothing at the moment --> | ||
| <!-- <include file="$(find victor_control)/launch/victor_control.launch"> --> |
There was a problem hiding this comment.
Change or remove.
We'll never want to launch victor_control.launch from this med.launch file
| @@ -0,0 +1,152 @@ | |||
| #! /usr/bin/env python | |||
There was a problem hiding this comment.
We decided to move manual motion to the "Kuka Hardware Interface" package.
#61
UM-ARM-Lab/kuka_iiwa_interface#130
We could revisit that decision. Regardless, I like manual motion better from a launch file
arm_robots/launch/med.launch
Outdated
| <group if="$(arg sim)"> | ||
| <!-- Start gazebo --> | ||
| <group> <!-- this empty group tag creates a scope for the "remap" command --> | ||
| <!-- because we use ros_control only for the joints of the arms, not the grippers, |
There was a problem hiding this comment.
arms -> arm
grippers -> gripper
There was a problem hiding this comment.
For now, we don't need this remapping, so I've removed the group/remap entirely.
arm_robots/launch/med.launch
Outdated
| args="-urdf -model med -param robot_description"/> | ||
|
|
||
| <!-- start the ROS controllers, only used in gazebo. This merges gripper and arm joint states --> | ||
| <include file="$(find victor_control)/launch/med_control_gazebo.launch"> |
There was a problem hiding this comment.
why is this under victor_control ?
There was a problem hiding this comment.
This is because for now I've been throwing the med configs into the kuka_iiwa_interface existing packages. On my todo list today is to pull all these med configs/launches out into their own packages that rely on your kuka_iiwa_interface packages.
| </include> | ||
| </group> | ||
| <group unless="$(arg sim)"> | ||
| <!-- Start gazebo --> |
There was a problem hiding this comment.
what's with this being commented out? should we just delete?
There was a problem hiding this comment.
This is b/c I'm not sure yet how we will update the planning scene, and I may want to use Gazebo to do it, the way you guys do. I want to keep it for now and will remove it if we decide to go a different route.
| @@ -0,0 +1,20 @@ | |||
| import numpy as np | |||
There was a problem hiding this comment.
since you're not starting from existing code like we were, I'd encourage you to get rid of this file if possible. Most of this stuff is already available on the parameter server, so looking it up would make things simpler and reduce duplication of hard-coded values. Look at the yaml/config files, or srdf file for where this stuff might belong
There was a problem hiding this comment.
Do you guys have a plan to shift away from this setup? I can imagine adding a get_joint_limits() function to the MoveItEnabledRobot class to do this.
There was a problem hiding this comment.
Then each subclass would need to implement it according to their arms.
There was a problem hiding this comment.
ARM_JOINT_NAMES is the most problematic part. That should just be based on a move_group., which you can get via the robot_commander. But that breaks the assumption that BaseVictor (for instance) doesn't depend on MoveIt. So calling it MoveitEnabledRobot doesn't make sense, not sure what the distinction between Base and Movet robot should be, if indeed there should be one at all?
as for the vel/accel limits, again those can be retreived from the parameter server, and there's probably API on RobotCommander or somewhere else in moveit to get that info
Ideally our get_joint_limits would be a wrapper to some existing moveit function call? not sure
There was a problem hiding this comment.
Ok, well for vel/accel limits, it seems to make sense to have that in MoveItEnabledRobot and use the RobotCommander or similar to get access. Maybe that can be a separate PR?
It's not immediately clear to me how to address the ARM_JOINT_NAMES issue, but it also seems like something both Victor/Med suffer from. Maybe enough for now to open an issue and explore options later?
There was a problem hiding this comment.
Already is an issue:
#14
I agree this change is tangential to your refactor, and should be solved in a separate PR
|
This requires the wsg50 packages to run, where should we get those? |
We have a public fork on the MMintlab github: https://github.com/MMintLab/wsg50-ros-pkg. We only use the services published by these packages, no code dependencies. |
|
@mvandermerwe I would like to drop the |
Updated robot base classes to be more flexible for single arm configurations and for different gripper APIs. All dual arm and gripper helper functions have been moved to the Base[Robot] or [Robot] classes. The API for Victor/Val should be unchanged. Also introduces the BaseMed/Med robot abstractions.
victor_basic_motionandval_basic_motionperform as expected in Gazebo.