API review

Proposer: your name here

Present at review:

• Radu
• List reviewers

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.

Radu

• Not clear what wge100_sim does

  • Blaise: Part of the test suite, not documented, not part of the API.

• There's 3 wge100_camera nodes

  • Blaise: Killed one, renamed the obsolete one (but needed by qualification) to _old, and the new one is in place. Will kill the old one when qualification is fixed. Not documented, not part of the API.

• When a wge100_camera_node is ran, the output is:

[ WARN] 1263844248.404912000: rmem_max is 131071. Buffer overflows and packet loss may occur. Minimum recommended value is 20000000. Updates may not take effect until the driver is restarted. See http://pr.willowgarage.com/wiki/errors/Dropped_Frames_and_rmem_max for details. [ERROR] 1263844248.606210000: Matching URL any:// : No cameras matched the URL.

• Blaise: Sounds normal. The warning tells you the camputer is badly configured and tells you how to fix it. The error tells you you don't have any cameras in sight.

• Not clear what read_all_flash does

  • Blaise: Not documented, not part of the API. (Useful for debugging)

• What is companding? We could add a few more words about that on the wiki page.
  • ~companding (bool, default: True)
    • Turns on companding

  • Blaise: Added a brief blurb on companding beside the ~companding parameter. (a non-linear intensity scaling to improve sensitivity in dark areas)

• Should ~trig_timestamp_topic (str) be a parameter?

  • Blaise: What else could it be?

• doc/api/diagnostic_msgs/html/srv/SetCameraInfo.html doesn't work from http://wiki/wge100_camera. Is that obsolete? Should the service be ~set_camera_info btw?

• Are all topics relative? ~image_raw vs camera/image_raw

  • Blaise: The camera works in 4 namespaces:

    • The / namespace for diagnostics
    • The camera namespace for the primary imager (see the camera API on the camera_drivers page)
    • The camera_alternate for the alternate imager (didn't provide a second set_camera_info to avoid name conflicts when you remap camera and camera_alternate to the same place)
    • The ~ namespace for its non standardized parameters and services.
• camera/image_info and camera/image_raw are duplicate on the wiki.

  • Blaise: One of each should be camera_alternate.

• No documentation for the advanced command-line tools (http://wiki/wge100_camera#Advanced_Command-Line_Tools)

  • Blaise: Fixed the link.

Blaise

• Forgot to document the wge100_camera_multiconfigurator.

  • Blaise: Done

Vijay

• It's not clear what is the relationship camera_info has with some of the parameters: mirror_x, mirror_y, horizontal_offset, vertical_offset, vertical_binning, horizontal_binning, width, height.

  • Blaise: I have put a blurb on that in the description of Region Of Interest Parameters.

• It might be cleaner to make two param namespaces alternate and primary. Not sure if this would be more or less confusing.

  • Blaise: Dynamic_reconfigure can't currently atomically change parameters in different namespaces so that is out for now. Besides, it makes the parameters that are shared less clear. I have grouped the parameters in the documentation, which is probably sufficient for comprehension.

Ethan

• Perhaps instead of two param namespaces just having the root namespace and an alternate subnamespace would be cleanest.

  • Blaise: Can't currently be handled by dynamic reconfigure (see Vijay's comment). But in any case, would alternate/exposure really be clearer than exposure_alternate?

• Setting the camera IP is a little unclear at least to me. Does it happen whenever an access using the @camera_ip format is performed? If so why the distinction between set_name reporting the current settings and changing the current settings? Is it that the new_name form modifies the flash (and hence the default value after power on?) I think this distinction needs to at least be clearer in the documentation. What is the motivation for having something that looks like an address specifier have a side effect like modifying the current IP? Is there any actual modification that happens (it's reset each time a new tool is used apparently, so is this just a way of specifying an IP to use for the duration of use of a specific tool? What if there are multiple tools in use at once?)? Why is it that tools need to be able to set the camera to a specific IP for the duration of their interaction with it?

  • Blaise: I reworked the camera URL section, and added a section on concurrent access. Does this clarify your questions?

• This is more of a documentation issue, but there are so many parameters that I'm not really sure as a user where to start changing things to get the best images for my task. Perhaps this is a good target for a tutorial. It also seems like some of the parameters fall in to categories like "expert_mode" (things a user probably never wants to change like packet_offset, binning, and the horiz and vert offsets) and "camera_synchronization" (the various trig parameters, the alternates). Maybe making this explicit (at least in the documentation, maybe with param namespaces) would help make the large number of parameters more digestible.

  • Blaise: I think that the parameters should be a bit clearer now that they are categorized. I am also going to point to a section in pr2_camera_synchronization that explains what you want to be touching on the PR2. If you aren't on the PR2, there aren't really any advanced parameters. You'll need most of them to get high quality images going. The only exception is the alternate parameters if you aren't using the alternate register set. But they are already clumped out of the way at the bottom of the parameter list.

• Are there any NON-dynamically reconfigurable params?

  • Blaise: No. However, I always group the dynamically reconfigurable parameters that way as a way of making it evident that these parameters are dynamically reconfigurable.

• Again a non-API comment, but a tutorial on using dynparam load to set the camera to a known state in a launch file would be useful. This seems like one of the more common use cases (but maybe it's biased because that's something I'm using).

  • Blaise: I have added a link near the top to some hokuyo_node tutorials that illustrate exactly that.

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

---