roboticslab-uc3m / questions-and-answers

A place for general debate and question&answer
https://robots.uc3m.es/developer-manual/appendix/repository-index.html
2 stars 0 forks source link

Avoid hardcoded absolute paths in .ini files #61

Closed PeterBowman closed 4 years ago

PeterBowman commented 6 years ago

Following the lines of #52, I'd be tempted to mimick the CMake configure_file mechanism in several absolute paths found throughout out <robot>-configuration-files repos. Example:

<module>
        <name>yarpdev</name>
        <parameters>--device BasicCartesianControl --name /teoSim/rightArm/CartesianControl --from /usr/local/share/teo-configuration-files/contexts/kinematics/rightArmKinematics.ini --angleRepr axisAngle --robot remote_controlboard --local /BasicCartesianControl/teoSim/rightArm --remote /teoSim/rightArm</parameters>
        <node>localhost</node>
</module>

Note the /usr/local/share/... part. Since moving everything into a cmake/templates/ in the form of a *.xml.in would mess the directory structure of the affected repositories, YARP context paths might come into play as a better approach, e.g. ... --context teo-kinematics --from rightArmKinematics.ini. Keep in mind that context names should be unique, hence the teo- prefix.

jgvictores commented 6 years ago

YARP context paths might come into play as a better approach

Yes, that looks elegant.

Keep in mind that context names should be unique, hence the teo- prefix.

:+1:

jgvictores commented 6 years ago

Mmm thinking again, anything really wrong with non-unique kinematics?

PeterBowman commented 6 years ago

Mmm thinking again, anything really wrong with non-unique kinematics?

YARP will find the first context directory and .ini file that match the --context and --from requirements, respectively. However, we adopted the kinematics context in our three robot config repos, and two of them (ASIBOT and AMOR) already hold a armKinematics.ini.

Per yarp-config context --list:

**LOCAL USER DATA:
**SYSADMIN DATA:
**INSTALLED DATA:
* Directory : /usr/local/share/yarp/contexts
yarpdataplayer
* Directory : /usr/local/share/amor/contexts
kinematics
openrave
* Directory : /usr/local/share/asibot/contexts
kinematics
openrave
webInterface
[...]

Per yarp resource --context kinematics --from armKinematics.ini:

||| configuring
||| added context kinematics
||| default config file specified as armKinematics.ini
||| checking [/home/bartek/git/roboticslab/amor-configuration-files/scripts/bash/armKinematics.ini] (pwd)
||| checking [/home/bartek/.config/yarp/robots/default] (robot YARP_CONFIG_HOME)
||| checking [/home/bartek/.local/share/yarp/robots/default] (robot YARP_DATA_HOME)
||| checking [/etc/xdg/xdg-ubuntu/yarp/robots/default] (robot YARP_CONFIG_DIRS)
||| checking [/usr/share/upstart/xdg/yarp/robots/default] (robot YARP_CONFIG_DIRS)
||| checking [/etc/xdg/yarp/robots/default] (robot YARP_CONFIG_DIRS)
||| checking [/usr/share/ubuntu/yarp/robots/default] (robot YARP_DATA_DIRS)
||| checking [/usr/share/gnome/yarp/robots/default] (robot YARP_DATA_DIRS)
||| checking [/usr/local/share/yarp/robots/default] (robot YARP_DATA_DIRS)
||| checking [/usr/share/yarp/robots/default] (robot YARP_DATA_DIRS)
||| checking [/var/lib/snapd/desktop/yarp/robots/default] (robot YARP_DATA_DIRS)
||| checking [/usr/share/ubuntu/yarp/config/path.d] (robot path.d YARP_DATA_DIRS)
||| checking [/usr/share/gnome/yarp/config/path.d] (robot path.d YARP_DATA_DIRS)
||| checking [/usr/local/share/yarp/config/path.d] (robot path.d YARP_DATA_DIRS)
||| found /usr/local/share/yarp/config/path.d
||| checking [/usr/local/share/amor/robots/default] (robot yarp.d)
||| checking [/usr/local/share/asibot/robots/default] (robot yarp.d)
||| checking [/usr/local/share/asrob-yarp-devices/robots/default] (robot yarp.d)
||| checking [/usr/local/share/rd/robots/default] (robot yarp.d)
||| checking [/usr/local/share/roboticslab-kinematics-dynamics/robots/default] (robot yarp.d)
||| checking [/usr/local/share/roboticslab-openrave-yarp-plugins/robots/default] (robot yarp.d)
||| checking [/usr/local/share/roboticslab-speech/robots/default] (robot yarp.d)
||| checking [/usr/local/share/roboticslab-tools/robots/default] (robot yarp.d)
||| checking [/usr/local/share/roboticslab-vision/robots/default] (robot yarp.d)
||| checking [/usr/local/share/roboticslab-yarp-devices/robots/default] (robot yarp.d)
||| checking [/usr/local/share/teo-openrave-models/robots/default] (robot yarp.d)
||| checking [/home/bartek/.config/yarp/contexts/kinematics] (context YARP_CONFIG_HOME)
||| checking [/home/bartek/.local/share/yarp/contexts/kinematics] (context YARP_DATA_HOME)
||| checking [/etc/xdg/xdg-ubuntu/yarp/contexts/kinematics] (context YARP_CONFIG_DIRS)
||| checking [/usr/share/upstart/xdg/yarp/contexts/kinematics] (context YARP_CONFIG_DIRS)
||| checking [/etc/xdg/yarp/contexts/kinematics] (context YARP_CONFIG_DIRS)
||| checking [/usr/share/ubuntu/yarp/contexts/kinematics] (context YARP_DATA_DIRS)
||| checking [/usr/share/gnome/yarp/contexts/kinematics] (context YARP_DATA_DIRS)
||| checking [/usr/local/share/yarp/contexts/kinematics] (context YARP_DATA_DIRS)
||| checking [/usr/share/yarp/contexts/kinematics] (context YARP_DATA_DIRS)
||| checking [/var/lib/snapd/desktop/yarp/contexts/kinematics] (context YARP_DATA_DIRS)
||| checking [/usr/local/share/amor/contexts/kinematics] (context yarp.d)
||| found /usr/local/share/amor/contexts/kinematics
||| checking [/usr/local/share/asibot/contexts/kinematics] (context yarp.d)
||| found /usr/local/share/asibot/contexts/kinematics
||| checking [/usr/local/share/asrob-yarp-devices/contexts/kinematics] (context yarp.d)
||| checking [/usr/local/share/rd/contexts/kinematics] (context yarp.d)
||| checking [/usr/local/share/roboticslab-kinematics-dynamics/contexts/kinematics] (context yarp.d)
||| checking [/usr/local/share/roboticslab-openrave-yarp-plugins/contexts/kinematics] (context yarp.d)
||| checking [/usr/local/share/roboticslab-speech/contexts/kinematics] (context yarp.d)
||| checking [/usr/local/share/roboticslab-tools/contexts/kinematics] (context yarp.d)
||| checking [/usr/local/share/roboticslab-vision/contexts/kinematics] (context yarp.d)
||| checking [/usr/local/share/roboticslab-yarp-devices/contexts/kinematics] (context yarp.d)
||| checking [/usr/local/share/teo-openrave-models/contexts/kinematics] (context yarp.d)
||| checking [/usr/local/share/amor/contexts/kinematics/armKinematics.ini] (context)
||| found /usr/local/share/amor/contexts/kinematics/armKinematics.ini
||| finding file [from]
"/usr/local/share/amor/contexts/kinematics/armKinematics.ini"

As you see, only the first item is picked even if two occurrences were found. I thought that unique context names may be preferable over unique .ini file names in terms of readability (only one prefix in the directory name instead of prefixing every file).

jgvictores commented 6 years ago

I thought that unique context names may be preferable over unique .ini file names in terms of readability (only one prefix in the directory name instead of prefixing every file).

Ok, makes perfect sense!

PeterBowman commented 6 years ago

There is also another option to yarp-config: robot. That is, call yarp-config robot --import amor prior to working with AMOR and so on. Not sure about potential caveats, though, I've never used this.

PeterBowman commented 4 years ago

To sum up: prefix context file names with the robot name (e.g. teo-). Thus, the following (ref):

<module>
  <name>BasicCartesianControl</name>
  <parameters>--from /usr/local/share/teo-configuration-files/contexts/kinematics/fixedTrunk-leftArm-fetch-kinematics.ini ...</parameters>
  <node>localhost</node>
  <deployer>yarpdev</deployer>
</module>

would become:

<module>
  <name>BasicCartesianControl</name>
  <parameters>--from teo-fixedTrunk-leftArm-fetch-kinematics.ini ...</parameters>
  <node>localhost</node>
  <deployer>yarpdev</deployer>
</module>

This is entirely dependent on the implementation of each module. Here, BasicCartesianControl should either parse --context kinematics or perform file look-up on this default path (preferable); also, additional mechanisms exist, i.e. this device accepts the --kinematics option, which we rarely use.

@jgvictores do you agree?

jgvictores commented 4 years ago

I think so. Maybe we could close this issue because too broad then?

  1. Update http://robots.uc3m.es/gitbook-developer-manual/programming-with-yarp.html (perma) to make more emphasis on using YARP mechanisms for resources (installation, finding).
  2. Check all CMake resource installations and all YARP module finding. <-- way too tedious
PeterBowman commented 4 years ago

Check all CMake resource installations and all YARP module finding. <-- way too tedious

I think it's not that hard at all, the only occurrence of hardcoded paths I could find in teo-config-files is the bcc-kinematics thing. Same goes for amor-/asibot-config-files, probably.

PeterBowman commented 4 years ago

BTW we got rid of long absolute paths pointing at OpenRAVE environment descriptor files at https://github.com/roboticslab-uc3m/openrave-yarp-plugins/issues/99.

PeterBowman commented 4 years ago

To sum up: prefix context file names with the robot name (e.g. teo-).

I didn't notice my original proposal was to rename contexts instead of individual files. However, now I realize modules can hardcode this context name behind the scenes (ref). It kinda makes sense doing so since:

@jgvictores OK with https://github.com/roboticslab-uc3m/teo-configuration-files/commit/a89480fa727433826e875cd606c843586cacb4a7?

jgvictores commented 4 years ago

@jgvictores OK with roboticslab-uc3m/teo-configuration-files@a89480f?

That's perfect, thanks a lot!

PeterBowman commented 4 years ago

Done, now we point at filenames instead of full paths (see linked commits).