API review
Proposer: E. Gil Jones
Present at review:
- List reviewers
Explanation
I made this an API review as that seems like the most general category. The purpose of this review is to discuss the way forward for the pr2_teleop_general package. This package was originally pr2_teleop_booth - designed for robot demos - but has proved pretty useful, and I'd like to see released for a general audience. It currently lives in pr2_apps_trunk - if you'd like to check out you can use this overlay text:
- svn: uri: https://code.ros.org/svn/wg-ros-pkg/stacks/pr2_apps/trunk local-name: stacks/pr2_apps
There's also a description page at: http://www.ros.org/wiki/pr2_teleop_general
Note that I also wrote a pretty full-featured keyboard implementation that I've already used pretty extensively in simulation.
More than an API review this is really a review of what this package should do, what it should be called, and where it should live. I'd like feedback on a number of issues that I detail below.
Specific issues
Should this be merged with/gradually replace pr2_teleop?
One issue Wim raised in keeping this pr2_teleop_general and having it live alongside pr2_teleop is that it essentially already provides a superset of the functionality of pr2_teleop. One path of integration is to actually check this code into the pr2_teleop package, make sure that it provides a proper subset of the functionality, and deprecate the existing pr2_teleop code. But given the widespread use of pr2_teleop I want to be very careful about moving forward.
Design for interoperability
The existence of the mux in pr2_teleop associated with navigation already attests that getting teleop programs to interact with running launch files. This is functionality I don't currently have in pr2_teleop_general, though you can already disable all arm behavior easily, for instance. This leads to several questions:
- What is the full set of functionality these nodes should implement to support application-specific launch file creation?
- Should the mux system be extended in lieu of a full resource management system?
- Do programs need to enable or disable specific buttons or change what buttons do?
Fidelity in naming
A final issue I'd like to discuss is naming. Should everything that seeks to teleop the pr2 live in pr2_teleop? Another possibility is to make a pr2_teleop stack with a pr2_teleop_common stack for shared code, and particular implementations for methods of control based on interface - pr2_teleop_joystick, pr2_teleop_keyboard, potentially pr2_teleop_phantom or pr2_teleop_cockpit. The pr2_teleop monikor seems a bit problematic in that when folks implement new/different/better methods for teleop control. This just seems a reasonable juncture to think about that a little bit.
Question / concerns / comments
Enter your thoughts on the API and any questions / concerns you have here. Please sign your name. Anything you want to address in the API review should be marked down here before the start of the meeting.
Kevin
Merge
I think we should for dturtle. The existing pr2_teleop system should be deprecated. In order to make things easier, we may want to change the direction of the head tilt commands to make it easier for pr2_teleop users to convert.
We could rename the existing code to "pr2_teleop_simple.launch", in case we have concerns about interoperability.
Interoperability
We should add a mux and make sure it works with the nav system.
I don't think we should support allowing different programs to change what buttons do. We could provide a supported ROS API, for example publishing on 'my_arm_controller/position' for a particular button. Once we have a stable API, the user can change out the type of controller, etc.
Naming
pr2_teleop is fine with me. More complicated teleop systems can be easily named to distinguish themselves from pr2_teleop, like pr2_cockpit and teleop_3d.
Melonee
Naming
pr2_teleop_common or teleop_common might be better so that core infrastructure can be create and placed there so that people can build off of it
Interoperability
We should look at making some base "modules" or nodelets that are parameterized( buttons, controllers, etc) so people can import/run them to construct custom teleop nodes/packages, this will help with copied code everywhere.
Meeting agenda
- What is the consensus on the above issues?
- What changes need to be made?
- What should the release process look like?
Conclusion
Package status change mark change manifest)
Add run button
Add navigation mux
Release soon against cturtle as pr2_teleop_booth
Consider drop-in replacement for dturtle
Eitan needs to deal with his dependencies
Add some compelling stuff to roadmap
Send out an email to pr2 users on release
Migrate existing wg users to it to work out bugs