search

NVIDIA-ISAAC-ROS / isaac_ros_common

Common utilities, packages, scripts, Dockerfiles, and testing infrastructure for Isaac ROS packages.
https://developer.nvidia.com/isaac-ros-gems
Other
197 stars 140 forks source link

Fix isaac_ros-dev path by using relative path to script #68

Closed j3soon closed 8 hours ago

j3soon commented 1 year ago

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:

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.

Double quotation marks are required to correctly mount paths with special characters. (Tested with paths including whitespace characters)

YuZhong-Chen commented 1 year ago

@j3soon Hi, I encountered the same issue when I launched the script, and the fix worked perfectly for me. Thank you so much!

j3soon commented 1 year ago

Just confirmed the issue persists in the latest release 94dfe8d.

hemalshahNV commented 1 year ago

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.

output.patch

j3soon commented 1 year ago

Hi @hemalshahNV,

I just verified that the provided patch (output.patch) does not fix the issue. Please see the commit history and comments for more details. Specifically, see this commit for the required changes to output.patch.

Thank you!

wjj1008 commented 3 months ago

@hemalshahNV do we have the CI/CD system to validate the patch?

j3soon commented 8 hours ago

This has been fixed since version 3.0.2, closing this issue.