[Documentation] [TitleIndex] [WordIndex

API review

Proposer: E. Gil Jones

Present at review:

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://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:

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

Conclusion

Package status change mark change manifest)



2019-08-17 13:06