robotology / yarp

YARP - Yet Another Robot Platform
http://www.yarp.it
Other
524 stars 195 forks source link

yarp-config robot --import fails with multiple YARP_DATA_DIRS directories #419

Open traversaro opened 9 years ago

traversaro commented 9 years ago

Scenario: Ubuntu 12.04, I have the YARP_DATA_DIRS env variable setted to :

icub@macsi03:~/.local/share/yarp/robots/iCubParis02$ echo $YARP_DATA_DIRS
/home/icub/software/compile/share/yarp:/home/icub/software/compile/share/iCub:/home/icub/software/src/codyco-superbuild/_build/install/share/codyco

The yarp data directory robots directory does not contain a robots subdirectory, while both the iCub and the codyco one contain a robots/iCubParis02 subdirectory.

I have a file (model.urdf) that is contained in the codyco robots/iCubParis02 subdirectory, but if I try to import it I get this error:

icub@macsi03:~/.local/share/yarp/robots/iCubParis02$ yarp-config robot --import iCubParis02 model.urdf
Error in checking properties for /home/icub/software/compile/share/iCub/robots/iCubParis02/model.urdf
ERRORS OCCURRED WHILE IMPORTING FILES FOR ROBOT iCubParis02

If I manually set the YARP_DATA_DIRS env variable to point to just the codyco data directory, everything works fine:

icub@macsi03:~/.local/share/yarp/robots/iCubParis02$ YARP_DATA_DIRS=/home/icub/software/src/codyco-superbuild/_build/install/share/codyco yarp-config robot --import iCubParis02 model.urdf
Copied selected files to /home/icub/.local/share/yarp/robots/iCubParis02
elen4 commented 9 years ago

I am not sure that it is correct to have multiple robots or contexts directories with the same name at the same "level"... It feels pretty "dangerous", as you might end up reading some files from one of the "installed" folder (i.e., iCub), and other ones from another folder (i.e., codyco), and they might be inconsistent. I think this is why the RF only expects one "installed" context or robot folder. Unfortunately I don't see anything in the specs about it :confused: Is it common that different packages install robots and contexts directory with the same name?

traversaro commented 9 years ago

Yes, actually we are using robots directory in codyco YARP_DATA_DIRS in all codyco software configuration (see https://github.com/robotology/codyco-modules/issues/71 ) : ( . The basic idea is that we have configuration file describing the available joints/motors/sensors for each robot, so using the robots directory (or an equivalent system) is a strict requirement for us. Working with relatively "low-level" controllers, having robot-specific gains (given the difference in motors/mechanics/electronics in different robots) is another typical requirement.

However I want to stress that the ResourceFinder is working fine with multiple robots directories with the same name at the same "level", it is only the yarp-config utility that is not properly importing the files.

elen4 commented 9 years ago

I see, still I don't know if it's more of a bug the RF working fine with multiple robots directories (I guess that it works, especially if files in each directory have different names, but the behavior is undefined), or yarp-config only considering the first directory. I think that yarp-config can be easily modified to behave as you expect (in src/yarp/yarpcontextutils.cpp, lines 620 and 624 could call the RF as in line 603 instead of "composing" the full path+name of installed files), but actually forcing the user to explicitly set which installation directory to consider when importing (as you did in your workaround) seems a bit safer to me. Bottom line, I think that the behavior when multiple directories with the same name at the same level are present should be defined in the specs (maybe it's already there but I did not find it... @drdanz any idea?), then changes to RF or yarp-config should be made accordingly.

lornat75 commented 9 years ago

@traversaro: can those information be added to the icub-main repository in the robot directory?

Otherwise I think the mechanism you describe is useful.

traversaro commented 9 years ago

@lornat75 : that is what I was initially doing, and for something it definitely make sense (for example: urdf models).

However for other configuration files (namely yarpWholeBodyInterface, wholeBodyDynamicsTree conf and also torqueBalancing gains) it does not make sense to distribute the configuration files separately from the software that uses those configuration file. As soon as you do a modification in the software (for example a feature that requires some new configuration parameter) you want to ensure that the user has this new configuration files, and trying to enforce this across two different repositories is an ill posed problem.

lornat75 commented 9 years ago

@traversaro: looks reasonable. In your view the import should merge files form both source to the same "local" directory?

traversaro commented 9 years ago

@lornat75 : When I opened this issue I was thinking more about the "single file" import, so I will describe that use case, and what I was naively expecting.

The ResourceFinder already has an explicit search path hierarchy. Into this hierarchy, the directories indicated in $YARP_DATA_DIRS are inserted preserving the order in which they are listed in $YARP_DATA_DIRS, as for example happens for PATH and LD_LIBRARY_PATH. When I was doing

yarp-config robot --import iCubParis02 model.urdf

I was expecting the ResourceFinder to search in the installed robot directories the first one which contained iCubParis02/model.urdf, and import it in the .local robot directories.

I didn't think up a behavior for the "mass import" option, that I always "considered harmful". A possible behavior is to import all the files, and in the case of name clashes give the priority to the file that is upper in the search hierarchy (i.e. comes before in YARP_DATA_DIRS list).

I don't know what the "merge" command is doing, and the help is a bit confused ("merge differences in selected files-directories".. merge where? ) so I am not sure what an appropriate behavior would be.

elen4 commented 9 years ago

@traversaro the yarp-config TYPE --merge command modifies the "user" files, to reflect changes that may have occurred in installed files (i.e., after installing newer versions) after they were imported to the user home (e.g., typically new parameters are added, as new functionalities are implemented). I think @lornat75 was using the term merge generically, to understand what users (you :smiley: ) expect and/or need in the multiple-installed-directories-with-the-same-name case. The behavior you suggest makes sense to me, I'm just saying it would be a nice idea to write it down explicitly in the docs (and warn the users about possible inconsistencies when importing multiple files, as they might be picked up from different directories).

lornat75 commented 9 years ago

I meant "copies files from multiple directories into the same one".