Implement transmission loading from URDF - Indigo #156
Conversation
- Allow to specify multiple hardware interfaces for joints and actuators. - Fix invalid xml_element tag. Contents are now stored as a string. - Unit test parser.
- Mirrors hardware_interface::RobotHW, but for transmissions.
- Only simple transmission type currently supported. - Can load forward map for act->jnt state and jnt->act pos,vel.eff commands. - Partial testing.
Mechanical reductions, offsets and roles are used by many transmission types. The TransmissionLoader base class exposes convenience methods for parsing these elements.
- std::vectors were being used to store raw joint data, and when new transmissions were added, push_back()s would (potentially) reallocate the vectors and invalidate already stored pointers in hardware_interfaces. We now use std::map. - Move plugin implementations to a separate library. - Export link libraries to the outside. - More complete tests.
- Only RobotHW and RobotTransmission instances are pointers as they are owned by the robot hardware abstraction. The rest are plain members whose lifetime is bound to the loader struct.
- cppcheck flagged a [passedByValue] warning. Using const references instead.
- As in the differential transmission, it's convenient to specify an additional mechanical reduction on the joint output. This is especially convenient for flipping the rotation direction of a joint (negative reduction value). - Update URDF loader. - Update documentation and tests.
- The result is the same, but this is more uniform with the rest of the code.
- Add missing libraries to catkin_package call. - Gate tests with CATKIN_ENABLE_TESTING. - Add missing files to install target.
| add_library(${PROJECT_NAME}_parser | ||
| src/transmission_parser.cpp | ||
| src/transmission_parser.cpp include/transmission_interface/transmission_parser.h |
There was a problem hiding this comment.
I don't think you need to include the header files for libraries.
There was a problem hiding this comment.
You don't need to, you're right. I did it for two reasons:
- When I started with
ros_control, the existing codebase already did this, so for consistency I followed suit. - There's actually an added benefit if you specify headers: certain IDEs like QtCreator and MS's Visual Studio don't show headers as part of the project unless you state them explicitly as belonging to a target. I've always wondered why these IDE's don't just parse the generated CMake artifacts, which do contain the depended-on headers.
|
I tried to review as much as I could, but this is a huge PR! |
|
I should mention that although we are parsing transmissions from URDF, there is no actual dependency on urdf-related packages because we're using a non-standard URDF extension (hooray!). Thanks @davetcoleman for your review. I'll wait until next Tuesday April 22 before proceeding to merge, in case somebody wants to further discuss the PR. |
|
Hopefully in the future this will be a standard URDF extension, though. We just need to settle on the format. |
|
No news, good news. Merging. |
Implement transmission loading from URDF - Indigo
|
Thanks for your continued efforts Adolfo! |
| { | ||
| ROS_ERROR_STREAM_NAMED("parser","No valid hardware interface element found in joint '" | ||
| << joint.name_ << "'."); | ||
| continue; |
There was a problem hiding this comment.
This line causes pre-Indigo style transmission definitions to fail, rather than be deprecated as stated in ros-simulation/gazebo_ros_pkgs#184 and as done (correctly) in gazebo_ros_control/src/default_robot_hw_sim.cpp line 136 in ros-simulation/gazebo_ros_pkgs#185.
This fails currently because it continues out of the parseJoints() function without ever pushing the new Joint element below. Until J Turtle, this should be a deprecation warning and not continue.
There was a problem hiding this comment.
You are correct. The new Indigo code should honor the deprecation cycle that gazebo_ros_control is going through. Ticketed as #164.
This PR supersedes #141. It's basically the same, but rebased on top of indigo-devel, hence with rosbuild artifacts removed.