Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid hardcoded absolute paths in .ini files #61

Closed
PeterBowman opened this issue May 3, 2018 · 12 comments
Closed

Avoid hardcoded absolute paths in .ini files #61

PeterBowman opened this issue May 3, 2018 · 12 comments

Comments

@PeterBowman
Copy link
Member

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
Copy link
Member

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.

👍

@jgvictores
Copy link
Member

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

@PeterBowman
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member Author

BTW we got rid of long absolute paths pointing at OpenRAVE environment descriptor files at roboticslab-uc3m/openrave-yarp-plugins#99.

@PeterBowman PeterBowman self-assigned this Mar 17, 2020
PeterBowman added a commit to roboticslab-uc3m/teo-configuration-files that referenced this issue Mar 17, 2020
@PeterBowman
Copy link
Member Author

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:

  • Callers (anyone invoking this module) avoids passing an extra --kinematics-context option.
  • Devs are told to install robot-specific kinematics in a common context name, no ambiguities allowed.

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

@jgvictores
Copy link
Member

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

That's perfect, thanks a lot!

PeterBowman added a commit to roboticslab-uc3m/asibot-configuration-files that referenced this issue Mar 17, 2020
PeterBowman added a commit to roboticslab-uc3m/amor-configuration-files that referenced this issue Mar 17, 2020
PeterBowman added a commit to roboticslab-uc3m/teo-bimanipulation that referenced this issue Mar 17, 2020
@PeterBowman
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants