Skip to content

Update tutorial readme melodic#429

Open
HiroIshida wants to merge 5 commits intojsk-ros-pkg:masterfrom
HiroIshida:update-tutorial-readme-melodic
Open

Update tutorial readme melodic#429
HiroIshida wants to merge 5 commits intojsk-ros-pkg:masterfrom
HiroIshida:update-tutorial-readme-melodic

Conversation

@HiroIshida
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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