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

Update tutorial readme melodic #429

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

HiroIshida
Copy link
Contributor

I updated readme. The original one is confusing especially for melodic user. The updated one avoid confusion by checking $ROS_DISTRO. Also, originally, the workspace created in the example is ros/kinetic and is confusing for melodic user. Hence, I changed it to catkin_ws.

Copy link
Member

@k-okada k-okada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please cleanup commit log

@HiroIshida HiroIshida force-pushed the update-tutorial-readme-melodic branch from a38fcd8 to fe8f7ab Compare April 5, 2020 12:51
mkdir -p ~/catkin_ws/src && cd ~/catkin_ws
catkin init
wstool init src
wstool merge -t src https://raw.githubusercontent.com/jsk-ros-pkg/jsk_pr2eus/tree/master/pr2eus_tutorials/pr2eus_tutorials.rosinstall
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HiroIshida Why we install source after sudo apt install ros-$ROS_DISTRO-pr2eus-tutorials at https://github.com/jsk-ros-pkg/jsk_pr2eus/pull/429/files#diff-d9d1ac6a1d0d20412ad36c7e287f0a27R13

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the original README file, the both section Using pre-build package and Using source package do exit, so I just put both of them with no thought. In the original one, I guess the it was written like that so that a user can select from the two options.

However, I tried the option Using pre-build package, then when run roslaunch pr2eus_tutorials pr2_tabletop_sim.launch physics:=dart of here, I got the following error:

root@0628585147d9:/home$ roslaunch pr2eus_tutorials pr2_tabletop_sim.launch physics:=dart
... logging to /root/.ros/log/1c0079ba-7a99-11ea-b073-0242ac110004/roslaunch-0628585147d9-28828.log
Checking log directory for disk usage. This may take a while.
Press Ctrl-C to interrupt
Done checking log file disk usage. Usage is <1GB.

Resource not found: pr2_gazebo
ROS path [0]=/opt/ros/melodic/share/ros
ROS path [1]=/opt/ros/melodic/share
The traceback for the exception was written to the log file

Apparently, pr2_gazebo package is missed in the apt-released version. Also, another command from here does not work because pr2-tabletop-object-grasp.l does not exist in the package.

Thus I now conclude to remove the option of Using pre-build package.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please make sure that you ran rosdep install ... command, pr2_gazebo is set run_depends at https://github.com/jsk-ros-pkg/jsk_pr2eus/blob/master/pr2eus_tutorials/package.xml#L24, so it should be installed.
And my original question is "why you need pr2eus from source"?

@@ -0,0 +1,4 @@
- git:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wnen you create .rosinstall file, please add why you need that package from source, with PR URLs. see -> https://github.com/jsk-ros-pkg/jsk_demos/blob/master/detect_cans_in_fridge_201202/detect_cans_in_fridge_201202.rosinstall

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

Successfully merging this pull request may close these issues.

3 participants