clearpathrobotics / robot_upstart

ROS package of helper functions related to bringing up roslaunch on system startup.
BSD 3-Clause "New" or "Revised" License
196 stars 96 forks source link

Added "--env" option to add custom export statement in job-start #44

Closed kmalhan closed 7 years ago

kmalhan commented 8 years ago

When "--env" option is specified, user can add custom export statement to job-start script using "name=value" format. This option supports setting multiple environment variables. If this option is not selected, no custom export will be added to job-start.

Example usage

rosrun robot_upstart install ros_package/path/to/launch --env LIDAR_ENABLE=1

This will add following line to just before launching launch file in job-start.

export LIDAR_ENABLE=1

This option is useful when launch file depends on specific environment variable to launch optional components.

Please review and provide feedback.

mikepurvis commented 8 years ago

Hmmm. I'm not completely opposed to including this functionality, but I will say that the original intent was to solve this a different way, which is to create a wrapper setup.bash, by convention /etc/ros/setup.bash, populated like so:

export ROBOT_SETUP=/etc/ros/setup.bash
export LIDAR_ENABLE=1

source /opt/ros/indigo/setup.bash

Then source that, and when you run the install script after, it will use value of the ROBOT_SETUP variable as the environment for the upstart job, meaning that's a single place where you can always put environment overrides.

kmalhan commented 8 years ago

What we are trying to achieve using --env option is to make user specific environment variable (like $HOME, $USER) available during launch from robot_upstart.

When user (kmalhan) runs following robot_upstart install command,

rosrun robot_upstart install ros_package/path/to/launch --env HOME=$HOME 

It adds following lines to install-job, which enables launch file to include user-specific launch file inside $HOME/.ros/

export HOME=/home/kmalhan

When I try to perform same functionality using --setup option with following wrapper setup bash script, HOME environment variable is not set to /home/kmalhan. This causes launch fail as job cannot find included launch files.

export HOME=$HOME

source /opt/ros/indigo/setup.bash
mikepurvis commented 8 years ago

At the time that the setup.bash file is being sourced, it no longer knows what your current HOME value is, so you'd need to make it explicit, so:

export ROBOT_SETUP=/etc/ros/setup.bash
export LIDAR_ENABLE=1
export HOME=/home/kmalhan

source /opt/ros/indigo/setup.bash

Remember that the upstart job starts running before anyone has logged in at all— it would have no idea which HOME to use short of being told in advance, either via the wrapped setup file, or by baking it into the start script (as proposed in this PR).

kmalhan commented 8 years ago

Thanks for the feedback. We'll try to use the method you have suggested.

kmhallen commented 7 years ago

@mikepurvis We used the method you suggested. You can go ahead an decline this PR.