API review

Proposer: your name here

Present at review:

  • List reviewers

Package proposal

This is simply a shared data type to allow a catch ros::Exception to catch anything ROS related. It is considered good practice to have a hierarchy of exceptions to allow easy catching of a broad range of exceptions but not all. This would be the base class for all ROS applications. It inherits from std::runtime_exception which provides a string constructor and a what() accessor for ease of giving useful errors.

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.

Proposal Comments

Eric:

  • Why are we deriving from runtime_error instead of directly from exception?
  • This might make sense to combine with e.g. rosthread into a set of basic utilities to avoid dependencies on huge numbers of packages (although this will likely we brought in via roscpp in most places anyway, so this may not matter...)

Josh:

  • It'd be nice to have a variadic constructor as well, allowing things like: throw ros::Exception("Component %s failed to initialize", name.c_str());
  • Eric: I agree that having lots of small packages doesn't make a whole lot of sense... I'm not sure this should be combined with rosthread though, as one is platform agnostic and the other is not. Maybe a rosstd package for things like rosexception, and a rosplatform package for things like rosthread, potentially shared memory handling, etc?
  • Ken: I like 'rosstd' for C++ library code. I'm not sure the value of rosstd+rosplatform, mainly because I see it creating more confusion than the separation is worth. We're talking about a relatively small amount of code, correct?

Tully

  • runtime_error provides the method what() to read back a string passed to the constructor. There are quite a few pitfalls doing your own allocation of messages. And this has them all done for us.
  • I do like the idea of a single c++ ros tools package. If we're promoting boost to first class we should consider deprecating rosthread.
  • Asking around It looks like we should do maybe rostools_cpp or rostools itself in a cpp directory.
    • If this is to be unified it should probably pull in rostime
    • And if we're doing Time and Duration I'd like to suggest removing the NSec constructor and require Time().fromNSec(20202)

Meeting agenda

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

Conclusion

Package status change (also mark change on PackageStatusDict page)

  • /!\ Action items that need to be taken.

  • {X} Major issues that need to be resolved

  • {X} Need to determine whether we can safely throw exceptions across module boundaries; if not, this goes away

    • We don't seem to be able to. It works fine in gazebo but not in realtime.
  • /!\ Consider making each package define its own exception base class, rather than having a common ros_exception.

Result: gone


Wiki: rosexception/2008-12-10_API_Review (last edited 2009-08-14 20:51:42 by localhost)