roboticslab-uc3m / yarp-devices

A place for YARP devices
https://robots.uc3m.es/yarp-devices/
9 stars 7 forks source link

Phase out launchCanBus in favor of yarprobotinterface #271

Open PeterBowman opened 9 months ago

PeterBowman commented 9 months ago

Our launchCanBus app is the main entry point to orchestrating CAN control boards and their wrapped raw subdevices according to the requested configuration in the .ini format. It was devised as an ad hoc solution, apparently superior to the (at the time undocumented) yarprobotinterface tool, see https://github.com/roboticslab-uc3m/yarp-devices/issues/237. Yet times have changed and we already benefit from the latter when using Gazebo. In my experience, the syntax feels cleaner and the document structure favors adhering to the increasingly more common device attach methodology for wrappers, while doing so in a descriptive fashion (this is currently encoded in launchCanBus's code). Besides, there would be a sensible gain in flexibility we haven't experienced yet in terms of file transclusion via <xi:include> and the handy tag mechanism described here (watch out for conditionally enabled ROS(2) stuff). There are lots of XML file examples at robotology/robots-configuration.

Proposal: drop launchCanBus in favor of yarprobotinterface, migrate our INI files to the XML format supported by the latter.

PeterBowman commented 7 months ago

Parameters can be passed to each device like follows:

config.xml:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE robot PUBLIC "-//YARP//DTD yarprobotinterface 3.0//EN" "http://www.yarp.it/DTD/yarprobotinterfaceV3.0.dtd">

<robot name="teo" build="1" portprefix="/teo" xmlns:xi="http://www.w3.org/2001/XInclude">
    <devices>
        <xi:include href="./teo-devices.xml" />
    </devices>
</robot>

teo-devices.xml:

<?xml version="1.0" encoding="UTF-8"?>

<device name="my-device" type="MyDevice">
    <param name="name1">value1</param>
    <group name="GROUP1">
        <param name="name2">value2</param>
    </group>
    <paramlist name="networks">
        <elem name="head_joints1">( 0  1  0  1 )</elem>
        <elem name="head_joints2">( 2  5  0  3 )</elem>
    </paramlist>
    <!-- <param name="name1">value3</param> --> <!-- ERROR: duplicate param name -->
    <xi:include href="./params.xml" />
    <params>
        <param name="name2">value4</param>
        <group name="GROUP2">
            <param name="name3">value5</param>
        </group>
    </params>
</device>

params.xml:

<?xml version="1.0" encoding="UTF-8"?>

<params>
    <param name="name4">value5</param>
    <group name="GROUP3">
        <param name="name5">value6</param>
    </group>
</params>

The <params> element in params.xml can be omitted, i.e. inner elements need not to be wrapped by it.

See also this example project with a dummy plugin used to show how parameters are passed to it during initial configuration. Usage: yarprobotinterface --config config.xml. Add --dryrun to test the validity of XML syntax, and --verbose to generate a config_preprocessor_log.xml file with all transclusions in place.

PeterBowman commented 7 months ago

To increase flexibility, e.g. launch different robot parts depending on a set of context files as in launchCanBus, use --from. The main difference is that said context files must live in share/yarp/robots/ instead of share/yarp/contexts/. I found out that you could place them in contexts/ anyway and spawn devices through yarprobotinterface --context mycontext --from mycnf.ini. However, the path passed to the config parameter is always interpreted as local to the current working directory yarprobotinterface is called from. On the other hand, placing those config files in the robot directory actually resolves all paths relative to said directory.

For instance, let's say we have mycfg.ini placed in share/yarp/robots/teo/ with these contents:

config "config.xml"

The command YARP_ROBOT_NAME=teo yarprobotinterface --from mycfg.ini will load share/yarp/robots/teo/config.xml. If --from is omitted, it will assume there is a yarprobotinterface.ini file there instead and read from it.

PeterBowman commented 7 months ago

Given:

<!-- config.xml -->
<!-- ... -->
<param name="name1" extern-name="name2">value</param>
<!-- ... -->

yarprobotinterface --config config.xml gives (name1 value)

while

yarprobotinterface --config config.xml --name2 other_value gives (name1 other_value)

PeterBowman commented 7 months ago
<xi:include href="./test1.xml" enabled_by="a1 a2" />
<xi:include href="./test2.xml" disabled_by="b1 b2" />

<param name="default" extern-name="custom">()</param>

Since <param> elements must not be empty and have a name attribute, they can be initialized to an empty list and later overridden through an external configuration option via extern-name.

PeterBowman commented 7 months ago

It is possible to invoke yarprobotinterface's mechanism programatically (tutorial). The following example also shows how to pass enable/disable tags and custom properties, including a blob.

main.cpp (link against the YARP::YARP_robotinterface component)

#include <array>

#include <yarp/os/LogStream.h>
#include <yarp/os/ResourceFinder.h>
#include <yarp/robotinterface/XMLReader.h>

int main()
{
    auto & rf = yarp::os::ResourceFinder::getResourceFinderSingleton();
    auto path = rf.findFileByName("config.xml");

    if (path.empty())
    {
        yError() << "Could not find config.xml";
        return 1;
    }

    std::array arr {16, 22, 37, 46, 53};
    auto * arrPtr = &arr;

    yarp::os::Property options;
    options.put("enable_tags", yarp::os::Value::makeList("a1"));
    options.put("myparam", yarp::os::Value::makeList("1 8 9"));
    options.put("myexternblob", yarp::os::Value::makeBlob(&arrPtr, sizeof(arrPtr)));

    yarp::robotinterface::XMLReader reader;
    auto result = reader.getRobotFromFile(path, options);

    if (!result.parsingIsSuccessful)
    {
        yError() << "Could not parse config.xml";
        return 1;
    }

    bool ok = true;

    ok &= result.robot.enterPhase(yarp::robotinterface::ActionPhaseStartup);
    ok &= result.robot.enterPhase(yarp::robotinterface::ActionPhaseInterrupt1);
    ok &= result.robot.enterPhase(yarp::robotinterface::ActionPhaseShutdown);

    return ok ? 0 : 1;
}

MyDevice.cpp

#include "MyDevice.hpp"

#include <array>
#include <iostream>

#include <yarp/os/Bottle.h>

bool MyDevice::open(yarp::os::Searchable & config)
{
    yarp::os::Bottle b("{" + yarp::os::Bottle(config.toString()).findGroup("myblob").tail().toString() + "}");
    const auto * arr = *reinterpret_cast<std::array<int, 5> * const *>(b.get(0).asBlob());

    for (const auto & elem : *arr)
    {
        std::cout << elem << std::endl;
    }

    return true;
}

Note the mess around passing a blob to the underlying device through yarprobotinterface. The convolute reinterpret_cast is intentional, passing a pointer to a pointer avoids copying and logging a long memory map, instead of that we log its address (that's how it's currently done, see launchCanBus and CanBusBroker). However, parsing a blob does not work as expected due to missing {} braces, which I presume are skipped at some point (parens () for lists are passed correctly, though).

Test project: yarprobotinterface.zip.

Edit: blob support proposed at https://github.com/robotology/yarp/pull/3066.

PeterBowman commented 7 months ago

The enable_tags and disable_tags were probably designed to enable/disable individual devices or entire groups of them if they share a common topic (e.g. a robot limb, or a NWS such as ROS). Our use case is somewhat particular in that sometimes we want to disable specific devices (nodes) that are part of a logical sequence of devices, e.g. only one joint of a limb. This conflicts with yarprobotinterface's attach mechanism, which doesn't allow attaching disabled (= missing) devices. Apart from that, it is up to the mapper/wrapper device to decide whether all registered devices need to be attached at once (e.g. controlboardremapper's attachAll fails if any device previously passed to --axesNames is not included) or if it is permitted to pass unknown devices.

For instance, this is not allowed if --disable_tags (no_mydevice):

<devices>
    <xi:include href="./mydevice.xml" disabled_by="no_mydevice" />
    <device name="mywrapper" type="mywrapper_nws">
        <action phase="startup" level="1" type="attach">
            <param name="device">mydevice</param>
        </action>
    </device>
</devices>

The mydevice.xml file containing the mydevice device is skipped in the preprocessing step, hence yarprobotinterface does not know how to attach it to mywrapper.

It could be circumvented like so:

<devices>
    <xi:include href="./mydevice.xml" disabled_by="no_mydevice" />
    <device name="mywrapper" type="mywrapper_nws">
        <action phase="startup" level="1" type="attach">
            <xi:include href="./mydevice-param.xml" disabled_by="no_mydevice" />
        </action>
    </device>
</devices>

With mydevice-param.xml:

<params>
    <param name="device">mydevice</param>
</params>

So I was thinking of a similar mechanism to conditionally disable broken nodes through --disable_tags and internally replacing them with FakeJoint. And it is an awful idea since it would require creating 28 dummy XML files such as mydevice-param.xml only for the iPOS nodes, and another 28 for the Cui ones if the attach methodology is also extended to these device types.

To avoid this madness, I'm considering an extern-name attribute to also allow passing external configuration (in the vein of enable_tags/disable_tags) directly to mappers/wrappers, e.g. --disable_nodes (id20-ipos), so they can decide what to do during the attach phase (e.g. replace said node with a FakeJoint instance).

PeterBowman commented 7 months ago

So I was thinking of a similar mechanism to conditionally disable broken nodes through --disable_tags and internally replacing them with FakeJoint. And it is an awful idea since it would require creating 28 dummy XML files such as mydevice-param.xml only for the iPOS nodes, and another 28 for the Cui ones if the attach methodology is also extended to these device types.

Besides, CanBusBroker would need to store its own wrapped instances of FakeJoint, while the whole point of this issue is migrating from a subdevice paradigm to the attach mechanism.

To avoid this madness, I'm considering an extern-name attribute to also allow passing external configuration (in the vein of enable_tags/disable_tags) directly to mappers/wrappers, e.g. --disable_nodes (id20-ipos), so they can decide what to do during the attach phase (e.g. replace said node with a FakeJoint instance).

Less messy, but forces CanBusBroker to understand more logic for parsing additional config options, and of course, wrapping is exceptionally allowed for FakeJoints.

I'm considering this intermediate solution:

<devices>
    <xi:include href="./idxx-ipos.xml" disabled_by="no_idxx" />
    <xi:include href="./fake-idxx-ipos.xml" enabled_by="no_idxx" />
    <device name="mywrapper" type="mywrapper_nws">
        <action phase="startup" level="1" type="attach">
            <param name="device">idxx</param>
        </action>
    </device>
</devices>

<!-- idxx-ipos.xml -->
<device name="idxx" type="TechnosoftIpos"></device>

<!-- fake-idxx-ipos.xml -->
<device name="idxx" type="FakeJoint"></device>

Despite requiring a lot of boilerplate XML files, CanBusBroker has no knowledge nor any extra logic for specifically dealing with fake devices, and the enabling/disabling can be done via enable_tags/disable_tags.

Since TechnosoftIpos can also be requested to attach a fake device, instead of creating a FakeEncoder that implements a limited set of interfaces, FakeJoint could be used here as well and renamed to something more generic, e.g. FakeControl or FakeCanNodeRaw (?).

Edit: I forgot that for this to work, one would need to issue yarprobotinterface with options --enable_tags (no_idxx) --disable_tags (no_idxx). Not so cool anymore.

PeterBowman commented 7 months ago

Recap of available options:

  1. Current solution (launchCanBus): let wrapper devices instantiate their own subdevices and pass a master robot configuration pointer throughout the whole call chain.
  2. Introduce the attach mechanism, read configuration from XML (a.k.a. theyarprobotinterface way):
    1. Instantiate either real or fake devices through the yarprobotinterface caller class, let the user control this via --enable_tags AND --disable_tags. See previous comment, having to use both options at once feels clumsy and counterintuitive.
    2. Instantiate fake devices through the wrapper device (CanBusBroker, TechnosoftIpos) again:
      1. Merge the capabilities of disable_tags and extern-name: use the former to disable unwanted real devices, pass the latter to wrapper devices to instantiate the requested fake devices (i.e. extern-name="disable_tags").
      2. If the corresponding configuration option is supplied, instantiate both real devices AND fake devices, even though only the latter are used. Since we'd like to share the real joint name with the fake device (for yarpmotorgui to work), this information can be queried during the attach phase using the pointer to the real device, and passed to the fake device on its initial configuration.
PeterBowman commented 7 months ago

I'm going with 2.ii.b. To ease the handling of fake devices, I'm no longer supporting CanBusFake to focus mainly on FakeJoint instead. A virtual CAN controller may be used for testing, see https://github.com/roboticslab-uc3m/yarp-devices/issues/251#issuecomment-884794430:

sudo ip link add dev can0 type vcan
sudo ip link set can0 up

Please do note default WSL(2) does not support virtual CAN. A custom kernel must be compiled for this to work, see https://github.com/microsoft/WSL/issues/5533.

Besides that, since real devices might be replaced by fake ones, but instantiated anyway, the former should never perform any CAN comms nor start threads in DeviceDriver::open. Those actions should be deferred to ICanBusSharer::initialize instead, which is invoked upon device attach. In other words, open should be cheap and the configured device might not be used even if created and attached to the CAN broker.