Interbotix / interbotix_ros_arms

ROS packages for the InterbotiX X-series family of robotic arms and turrets
BSD 2-Clause "Simplified" License
49 stars 27 forks source link

added trac_ik_kinematics_plugin dependency to package.xml #9

Closed bjsowa closed 4 years ago

bjsowa commented 4 years ago

The TRAC-IK kinematics plugin which is used as a default kinematics solver was not listed as a dependency and so it was not being installed by rosdep.

swinterbotix commented 4 years ago

Hi @bjsowa,

The only dependencies that I placed in the package.xml file are those that are resolvable by rosdep. However, the trac-ik package is not in the base.yaml file that rosdep references when installing packages. So I did not include it. That's why I specifically mention in the Quickstart guide that you should install the package using apt get.

bjsowa commented 4 years ago

Hi @swinterbotix, I tested whether the package is resolvable before making a commit by running:

rosdep resolve trac_ik_kinematics_plugin

and it successfully resolved the key to:

#apt
ros-melodic-trac-ik-kinematics-plugin

I also tested the kinetic distribution:

rosdep resolve --rosdistro kinetic --os=ubuntu:xenial trac_ik_kinematics_plugin 
#apt
ros-kinetic-trac-ik-kinematics-plugin

The command:

rosdep where-defined trac_ik_kinematics_plugin

returns:

https://raw.githubusercontent.com/ros/rosdistro/master/melodic/distribution.yaml
swinterbotix commented 4 years ago

@bjsowa,

Interesting! Right before I released the packages back in October of last year, I had the trac_ik package listed as a dependency, but took it out since I kept getting an error message that rosdep could not find it. Perhaps rosdep was updated since then... Anyway, I'll verify tomorrow that this works on a clean install of Linux and get back to you. Thanks for pointing this out!

swinterbotix commented 4 years ago

Hi @bjsowa,

I did a bit more digging and realized that the reason rosdep couldn't resolve the dependency originally was because I put 'trac-ik' in the package.xml file instead of 'trac_ik'. In any event, I tested the build with your added dependency and it works great!

swinterbotix commented 4 years ago

For anyone else reading this, I plan to remove the directions in the QuickStart specifying to manually install the trac_ik package in the next update.