API review

Proposer: kwc

Present at review:

• Eric
• Tim
• Wim
• Bhaskara
• Tully
• Josh
• Eitan

Please review the ROSH Overview documentation for this review.

• Features marked EXPERIMENTAL will not be considered stable in ROS 1.4 but are open to review

• Features not documented will not be considered stable in ROS 1.4 but are open to review, assuming you know what they are
• ROSH plugins will be reviewed separately, though API for loading plugins is part of the review

I am hoping for:

• general user feedback: what works, what doesn't.
• what could make rosh a better shell?
• what could make rosh a better rospy replacement?
• is the rosh API consistent and friendly?
• what critical features are missing?

ROSH will be included in the "ros_comm" stack for ROS 1.4. The "stable" version of this API will be released against C Turtle, then rosh will be forked for final Diamondback integration.

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.

• Eric
  • Most of my feature requests are already ticketed.
  • The topics[0] vs. topics[1] syntax was definitely confusing
  • Major things that I found myself dropping out of rosh to do when it was my default shell:
    • Non ros-related things (e.g. editing files).
    • rostopic echo XXX | grep YYY or "rostopic list | grep rotated"
    • launching .launch files
    • running guis (e.g. rosrun dynamic_reconfigure reconfigure_gui)
  • Automatically importing rospy into the namespace would be nice. I found myself importing rospy to use various features (e.g. rates) much more frequently than I found myself importing any other components.
  • Similarly, importing roslib.load_manifest into the namespace by default would make grabbing one function from python code significantly quicker.
  • The other biggest problem I had was with the distinctions in various places between usable objects and informational objects (which print out right, but can't be used, such as with parameters). I don't know what the solution is here, but clear messages (such as the updated ones for parameters) help a lot here.
• bhaskara
  • what does rostype return for a service, parameter, or action?

    • kwc: ServiceDefinition, nothing, and TBD (actions out of scope, but whatever type class is the best index)

  • why is getting a parameter done via a function call, but setting one done via an assignment operator?
    • kwc: It's an artifact of what tab-completion will allow. In order to descend a namespace, I have to used a boxed object, which means you have to call a function to unbox.
  • Is dictionary assignment to a parameter namespace additive or overwriting?
    • kwc: overwriting
  • would be nice to have a wait_for_service
    • kwc: two options. (1) I explicitly expose this. (2) I make it the default behavior that rosh always waits for service on a service call.
  • is there an advertise_topic?
    • kwc: no, you advertise by publishing. Also, the default mode for publishing is latching.
  • So the example of launching nodes would be an error due to duplicate names, right?
  • rostype(obj, val): could this be set_rostype(obj, val), or obj.rostype=val ?

    • (kwc) set_rostype: more characters, but yes

    • (kwc) obj.rostype = val: no, it can't, because that would make 'rostype' a reserved name

  • Would be cool to not have all name references be global, e.g., have a current_namespace global variable (or a 'with Namespace("foo/bar")' syntax)

    • (kwc): Interesting. It might be possible to pull off with thread locals, but it comes at additional lookup expense. I wouldn't do it as a global variable, because that mucks up multithreaded code (smach has this issue). As an alternative, you get the same effect (though only per concept) by using local variables:

      ns = parameters.foo.bar
      ns.x = 1
      ns.y = 2
      
      or
      ns = 'foo/bar'
      parameters[ns].bar
      topics[ns].foo

• kwc:

  • autoheader(frame_id='base_link')

    • kwc: should the default frame_id be empty instead?

    • bhaskara: I think usage patterns vary pretty widely, so there's no one frame_id that would be a great default.

  • topics[0] vs. topics[1]

    • kwc: do people like having a non-blocking topics[0], or is it not worth the confusion?

      • bhaskara: I'd find this useful.
• gerkey:

  • Would be nice to have an explicit subscribe, to allow me to prepare to receive a message to be published in the future (e.g., an action result, if I'm poking an action server manually). I can get the same behavior via topics.foo[0] and discarding the result, but that doesn't feel natural to me.

    • kwc: I'll look into exposing this functionality

  • Would be nice to be able to easily check the publication rate of a topic, e.g., hz(topics.foo) or hz('foo'). If I can discover that foo is being published at 0.5Hz, then I'll know not to ask for topics.foo[0:100] unless I'm very patient.

    • kwc: this is on the todo list, though separately I need to fix things so topics.foo[0:100] returns immediately.

  • The docs for topics.foo[N] look like an inadvertent copy-n-paste from topics.foo[1].

  • Is there anything to be said about roshlets?
    • kwc: I've moved the roshlets documentation into the Overview
• joq:
  • This looks like a very useful idea, but I was unable to figure it out well enough to review the API.

  • For an outsider like me, the overview documentation needs an introduction explaining what this package does and how it fits into the system. I think it's an interactive python shell for access to ROS interfaces. It would help to know how it relates to rospy, whether it requires a running roscore, etc.

  • The Install doc could use an explanation of what to install for running it in cturtle and in unstable. That would help more users try it out.

  • I look forward to running this.
    • (kwc): thank you for the documentation comments. I'll definitely take care of these before the doc review.

Meeting agenda

To be filled out by proposer based on comments gathered during API review period

Conclusion

Package status change mark change manifest)

• Action items that need to be taken.

• Major issues that need to be resolved

API conditionally cleared:

• import rospy by default [DONE]

• changed topics[0] to topics[-1] (topics[LATEST]) [DONE]

• add explicit subscribe [DONE]

• move autoheader to rosh_geometry, possibly call nowheader or header [DONE - header]

• add wait_for_service [DONE]

• wait for services by default [DONE]

Fixes completed 2/13/2011