Improve device option docs, promote use of --verbose

[PeterBowman]

The yarpdev utility suggests using a --verbose CLI parameter in order to list all available options, their description, default value, etc. This comes in handy when working with our own devices, which undergo frequent changes in their usage (or, simply put, are barely or not documented at all). Traditionally, we introduced the --help parameter, but it's hard to maintain (needs hardcoding plus some care to keep it up to date).

YARP offers its own mechanisms for such tasks, tightly bound to the yarp::dev::PolyDriver class: the YarpDevMonitor utility (not shown in Doxygen generated docs nor even exported, check PolyDriver.cpp). Such instance is created by all PolyDriver objects and can be propagated to subdevices (either wrapped devices, or devices opened by other devices in their DeviceDriver::open method). Example:

• roboticslab-uc3m/kinematics-dynamics@fef5657 (wrapped device)
• roboticslab-uc3m/kinematics-dynamics@dcbd1d6 (network wrapper, important)

Try this via yarpdev --device ... with and without --verbose. Example:

yarpdev --device FakeControlboard --name /fake
yarpdev --device BasicCartesianControl --local /local --remote /fake --kinematics path.ini --verbose

Some thoughts:

• --help actually made the device stop and exit (but remember: std::exit is evil)
• need to review all check calls and add missing comments/defaults
• this is only valid for devices initialized via yarpdev, but we'll surely want to achieve the same results with RF modules

[jgvictores]

Looks good! I'm going to be touching RobotServer/RobotClient pair at https://github.com/asrob-uc3m/yarp-devices, would look like a nice opportunity to experiment!!

[PeterBowman]

How to make this work on RF modules (e.g. transCoords app):

int main()
{
    MyMonitor monitor; // implements yarp::os::SearchMonitor

    yarp::os::ResourceFinder rf;
    // tweak 'rf', e.g. set default context and so on
    rf.setMonitor(&monitor, "transCoords");

    roboticslab::TransCoords mod;
    mod.configure(rf); // check return value!

    // at this point, retrieve options stored in 'monitor' and
    // print them on stdout if --verbose was passed on

    return mod.runModule(); // no need to configure again
}

We'll need a class that implements yarp::os::SearchMonitor (no Doxygen, see definition) and provides a public method for obtaining option bottles. Compare YarpDevMonitor.

[jgvictores]

Thanks a lot for this info! Always loved the behavior, but found the intrinsics quite cryptic, especially for modules.

[PeterBowman]

Bad thing is, YARP should have already provided an implementation, otherwise we are forced to write a tiny library only for this and replicate the same code in several places... This is what yarpdev runs behind the scene: ref. The sample main function from my previous comment would check verbose and reuse that code to print the report (replace PolyDriver with MyMonitor in the function's signature).

[jgvictores]

Maybe we could develop some kind of helper class that could be PR'ed directly into YARP?

[PeterBowman]

Sure, the sooner the better. I'm afraid of those find_package(YARP 3.0.0 REQUIRED) in each single app (unless we stick to preprocessor conditionals).

[PeterBowman]

need to review all check calls and add missing comments/defaults

Speaking of the yarp::os::Searchable class (see inheritance diagram, it has several implementations), prefer

bool check(const ConstString &key, const ConstString &comment) const;
Bottle findGroup(const ConstString &key, const ConstString &comment) const;

instead of

check(const ConstString &key) const;
Bottle findGroup(const ConstString &key) const;

The former generate a report that may be used by the monitor in verbose mode. Also, the following are available:

bool check(const ConstString &key, Value *&result, const ConstString &comment="") const;
Value check(const ConstString &key, const Value &fallback, const ConstString &comment="") const;

[PeterBowman]

I'm going to list here all repos that have been improved by providing or expanding on option documentation (now passing appropriate defaults and description in config.check() and the like) and propagating it through device wrappers:

• roboticslab-uc3m/kinematics-dynamics@1f18d71
• roboticslab-uc3m/tools@babb13c
• roboticslab-uc3m/vision (no YARP devices here)
• roboticslab-uc3m/asibot-hmi (...and here)
• roboticslab-uc3m/speech (nothing to do here either, Espeak device is already documented)
• roboticslab-uc3m/openrave-yarp-plugins (no monitor available - plugin loader in OR environment)
• roboticslab-uc3m/yarp-devices
  • roboticslab-uc3m/yarp-devices@5377c2a (partially reverted due to [LaunchManipulation] --homePoss is not working  yarp-devices#175)
  • roboticslab-uc3m/yarp-devices@3d66a62

[jgvictores]

Thanks a lot for this!

[PeterBowman]

Sometimes, device options hide each other, like this:

if (config.check("optionA"))
{
    yarp::os::Value v = config.check("optionB", yarp::os::Value(MY_DEFAULT));
}

The check for optionB could be pulled out from its enclosing if clause so that it's parsed by --verbose regardless of the presence of optionA. Then, additional logic would take care of the implicit relationship between both options. Is this the way to go?

[PeterBowman]

Or, since I like simplicity:

if (config.check("optionA", "enables A (and additional options)"))
{
    // ...
}

Thus, users would be aware of the existence of hidden options (and how to enable them).

[PeterBowman]

Custom devices wrapped by controlboardwrapper2 cannot display options via --verbose due to an upstream bug (tracked at robotology/yarp#1608).

[PeterBowman]

Related upstream PR regarding --help: robotology/yarp#1473.

[PeterBowman]

I'm going to list here all repos that have been improved...

All set but openrave-yarp-plugins. @jgvictores I'd really appreciate if you could take a look at existing device options and review/fill their description fields :) (low priority).

[jgvictores]

Okay, it'll take a while, but I will find a moment to do it!

[PeterBowman]

yarp-devices still WIP due to roboticslab-uc3m/yarp-devices#175, marking as blocked.

[PeterBowman]

Note to self: use simpler unit notation and replace parens with square brackets. Example: [m/s] or [deg/s] instead of (meters/second or degrees/second).

[jgvictores]

Added some documentation on ORYP at roboticslab-uc3m/openrave-yarp-plugins@e0adfbc

[jgvictores]

Applied to asrob-uc3m/yarp-devices here; works, but ran into asrob-uc3m/yarp-devices#12

[PeterBowman]

I'd rather close roboticslab-uc3m/yarp-devices#175 and track that bug here. To sum up, PolyDriver::getValue does not work as expected when the instance has been configured and opened with a Searchable object to which a monitor has been attached to (usually via options.setMonitor(config.getMonitor())). See roboticslab-uc3m/yarp-devices#175 (comment) for full explanation. Might need to circumvent the way we use getValue in that repo (example) or enhance the value lookup mechanism upstream. For that reason, I'd keep the blocked label for now.

[PeterBowman]

There is no simple way to sort this out on the upstream side without proposing a breaking change of the (undocumented) SearchMonitor interface, which lacks option getters (check sources).

On our end, the CanBusControlboard device might hold a vector of Property instances besides the existing one for PolyDriver objects (ref) in order to store and access device configurations. This is a hack.

[jgvictores]

IMHO, a big issue is too much being done on the CanbusControlboard. Things such as home should be done within Technosoft open or new method which is involved for all devices and each one implements their behavior via polymorphism.

The only special case where I introduced the original ugly hackish code was connecting the CuiAbsolute to the TechnosoftIpos, but that (apart from different) could be done better.

[PeterBowman]

Custom devices wrapped by controlboardwrapper2 cannot display options via --verbose due to an upstream bug (tracked at robotology/yarp#1608).

Merged into YARP's master branch, scheduled for the 2.3.72.1 release.

[PeterBowman]

This issue has been stalled for too long, therefore...

I'd rather close roboticslab-uc3m/yarp-devices#175 and track that bug here.

I changed my mind (again) and opened roboticslab-uc3m/yarp-devices#207, in the only repo affected by said bug.

The list #49 (comment) is done except for a few undocumented config options in openrave-yarp-plugins. @jgvictores could you please take a look? Apart from that, I need to check whether any new devices that have been added in the past year conform to these new standards.

One last question: is the behavior of --verbose actually better than the old --help? Also, note to self: look for remaining occurrences of the latter.

Regarding RFModule: either open an issue upstream considering #49 (comment), or forget.

[jgvictores]

The list #49 (comment) is done except for a few undocumented config options in openrave-yarp-plugins. @jgvictores could you please take a look?

I have that repo pretty abandoned, but I'll take a look. Specifically, I had done advances on roboticslab-uc3m/openrave-yarp-plugins#82, raised its priority to get done with it.

One last question: is the behavior of --verbose actually better than the old --help? Also, note to self: look for remaining occurrences of the latter.

My intuition behind --help was to be implement a --verbose that additionally quits before running anything, mostly to be able to read the output before all the verbosity of actually running. Would implementing this explicitly make any sense, or do you think we can just force users to stop a running program and scroll up?

[PeterBowman]

There is not much to do at or-yp, anyway, just a few undocumented occurrences of .check( at OpenraveYarpPluginLoader.

Would implementing this explicitly make any sense, or do you think we can just force users to stop a running program and scroll up?

It's a pity we don't know the outcome of robotology/yarp#1473. Some thoughts:

• Duplication should be avoided, hence passing --help would need to invoke --verbose under the hood.
• --verbose itself is position-dependent, that is, if we fail to pass some check in DeviceDriver::open and end up in a return false; clause, all further instructions won't be reached, especially accounting for those documentation comments passed to Searchable::check. In other words, the output of --verbose may be incomplete.
• With that last point in mind, we could place all check in the beginning or the end of the ::open method: either parse all YARP options and store them in local variables before using them in conditional checks or whatever (i.e. register doc comments before doint anything else), or defer the early exit of a failed configuration (let return false; be the very last instruction in said method), respectively. Such a big change would enforce a new paradigm of writing YARP devices which I'm not sure to be fond of, but it's definitely doable.
• How do we really want --help to behave? Should it output additional, textual clues which are not specific to YARP options shown by --verbose? Must it actually propagate to subdevices, just like --verbose does, now?
• I'm not sure yet how to cope with the different behavior of PolyDriver and RFModule. Our implementation of a --help handler could differ between those, also we need to know whether YARP catches this option anywhere and enforces its own solution (see linked PR above).

[PeterBowman]

To sum up:

• passing monitor objects on PolyDriver instances: done, except for some leftovers (openrave-yarp-plugins, wink wink @jgvictores?) and possibly new & unaware code
• adding documentation parameter to check calls: same as above
• nasty getValue bug (Pass config options to CanBusControlboard's subdevices yarp-devices#207): solved
• passing monitor objects via RFModule: not done, propose upstream issue/PR
• --help versus --verbose: not sure how to proceed, we use neither of both anyway...

Closing due to inactivity/too broad a scope/lack of interest on tedious subtasks.