Fix isaac_ros-dev path by using relative path to script #68
Conversation
|
@j3soon |
|
Just confirmed the issue persists in the latest release 94dfe8d. |
|
I have had a chance to get back to this. I've adapted your changes, @j3soon, to make one less assumption about the path as an absolute fallback. Please take a look. |
Originally, the `ISAAC_ROS_DEV_DIR` variable will be incorrectly set to `$(pwd)`, i.e., `<SOME_PATH>/isaac_ros-dev/src/isaac_ros_common`, when launching the script with: ```sh cd <SOME_PATH>/isaac_ros-dev/src/isaac_ros_common && \ ./scripts/run_dev.sh ``` The correct path should be `<SOME_PATH>/isaac_ros-dev`, which is fixed in this commit using related path to script: `$ROOT/../../..`, i.e., `<SOME_PATH>/isaac_ros-dev/src/isaac_ros_common/scripts/../../..`. The `LFS_FILES_STATUS` should be fixed accordingly by `cd` into `$ISAAC_ROS_DEV_DIR/src/isaac_ros_common` before checking the git lfs status. When mounting the directory into the docker container, double quotation marks are required to correctly mount paths with special characters.
Co-authored-by: Hemal Shah <[email protected]>
scripts/run_dev.sh
Outdated
| @@ -26,12 +26,23 @@ fi | |||
|
|
|||
| ISAAC_ROS_DEV_DIR="$1" | |||
| if [[ -z "$ISAAC_ROS_DEV_DIR" ]]; then | |||
| ISAAC_ROS_DEV_DIR="$HOME/workspaces/isaac_ros-dev" | |||
| ISAAC_ROS_DEV_DIR_DEFAULTS=("$HOME/workspaces/isaac_ros-dev" "/workspaces/isaac_ros-dev") | |||
There was a problem hiding this comment.
I wonder if the /workspaces/isaac_ros-dev here is a typo. Since the instructions here mentioned the use of /ssd/workspaces/isaac_ros-dev on Jetson platforms.
scripts/run_dev.sh
Outdated
| if [[ ! -d "$ISAAC_ROS_DEV_DIR" ]]; then | ||
| ISAAC_ROS_DEV_DIR="$ROOT/../../.." | ||
| ISAAC_ROS_DEV_DIR=$(realpath "$ROOT/../../") |
There was a problem hiding this comment.
Moving up two directories is unfortunately not enough (and will cause error) as mentioned in this comment.
$ROOT/../../is<SOME_PATH>/isaac_ros-dev/src$ROOT/../../../is<SOME_PATH>/isaac_ros-dev
The latter is required for correctly mounting the volume for docker:
isaac_ros_common/scripts/run_dev.sh
Line 184 in 94dfe8d
Moving up two directories (instead of three) would cause an error that can be reproduced by following the nvblox tutorial. Instead of working in ~/workspaces/isaac_ros-dev, we use ~/projects/isaac_ros-dev instead.
In step 5, the following error occurs:
admin@ubuntu:/workspaces/isaac_ros-dev$ ls
isaac_ros_common
admin@ubuntu:/workspaces/isaac_ros-dev$ rosdep install -i -r --from-paths src --rosdistro humble -y --skip-keys "libopencv-dev libopencv-contrib-dev libopencv-imgproc-dev python-opencv python3-opencv nvblox"
given path 'src' does not exist
This error is due to mounting the wrong directory ($ROOT/../../) into the container. This could be fixed by mounting the correct directory ($ROOT/../../../).
If we want to support arbitrary directory hierarchy (e.g., without the src directory), we must somehow traverse upwards in the directory structure and compare each directory name with "isaac_ros-dev" in order to locate it.
scripts/run_dev.sh
Outdated
| git rev-parse &>/dev/null | ||
| if [[ $? -eq 0 ]]; then | ||
| LFS_FILES_STATUS=$(cd "$ISAAC_ROS_DEV_DIR/src/isaac_ros_common" && git lfs ls-files | cut -d ' ' -f2) | ||
| LFS_FILES_STATUS=$(cd $ISAAC_ROS_DEV_DIR && git lfs ls-files | cut -d ' ' -f2) |
There was a problem hiding this comment.
The cd $ISAAC_ROS_DEV_DIR part should be changed to cd $ROOT/...
|
Hi @hemalshahNV, I just verified that the provided patch ( Thank you! |
|
@hemalshahNV do we have the CI/CD system to validate the patch? |
Originally, the
ISAAC_ROS_DEV_DIRvariable will be incorrectly set to$(pwd), i.e.,<SOME_PATH>/isaac_ros-dev/src/isaac_ros_common, when launching the script with:The correct path should be
<SOME_PATH>/isaac_ros-dev, which is fixed in this commit using related path to script:$ROOT/../../.., i.e.,<SOME_PATH>/isaac_ros-dev/src/isaac_ros_common/scripts/../../...The
LFS_FILES_STATUSshould be fixed accordingly bycdinto$ISAAC_ROS_DEV_DIR/src/isaac_ros_commonbefore checking the git lfs status.Double quotation marks are required to correctly mount paths with special characters. (Tested with paths including whitespace characters)