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 tutorials, add default velocity/acceleration scaling factors #468

Merged
merged 9 commits into from
Mar 16, 2020

Conversation

felixvd
Copy link
Contributor

@felixvd felixvd commented Mar 7, 2020

Description

For this PR, I added two explicit mentions of the default velocity/acceleration scaling factors.

While I was editing the tutorials, I started getting distracted and added some documentation for the joint sliders and the cartesian path checkbox. Those features are great and deserve some love. I also added a few lines with an example that I remember missing, and which have had a lot of views on ROS answers.

I kept the commits separate, but I didn't bother filing a separate PR.

One issue: I added a .webm file as an animation, but I don't know how to integrate it, and build_locally.sh didn't work in the container for me. Would be grateful for pointers.

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • While waiting for someone to review your request, please consider reviewing another open pull request to support the maintainers

Copy link
Contributor

@v4hn v4hn left a comment

Choose a reason for hiding this comment

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

I kept the commits separate, but I didn't bother filing a separate PR.

This should have been separate requests indeed...

Concerning the default_scaling_factor, this is not really what I had in mind (I thought of a separate tutorial to set the parameters in the yaml or in code).
Still, it probably suffices 👍

Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

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

The images rviz_plan_cartesian.png and rviz_plan_free.png are mixed up.

@felixvd
Copy link
Contributor Author

felixvd commented Mar 13, 2020

Nice catch. I updated this.

@v4hn v4hn merged commit ab37d33 into moveit:master Mar 16, 2020
@v4hn
Copy link
Contributor

v4hn commented Mar 16, 2020

@felixvd indeed, the webm embedding did not work as expected:
https://ros-planning.github.io/moveit_tutorials/doc/quickstart_in_rviz/quickstart_in_rviz_tutorial.html

Please create a follow-up pull-request to address this.

@felixvd
Copy link
Contributor Author

felixvd commented Mar 16, 2020 via email

@v4hn
Copy link
Contributor

v4hn commented Mar 16, 2020

Alternatively, if you can tell me how to get build-locally.sh to run, I could try to inline the video.

It works just fine for me, but of course I don't run your system/container. What's the error message?

When I tried converting it to a GIF, it became 3 times larger and 10 times as ugly.

The size is not a problem from my point of view, but I agree with ugly.
How about a clickable image of the process instead?
I do not particularly like to have people click on a link to see the GUI.

@rhaschke
Copy link
Contributor

For build_locally.sh you need to install rosdoc_lite and then source your ROS environment.

@felixvd
Copy link
Contributor Author

felixvd commented Mar 20, 2020

After creating a fresh moveit/moveit:master-source container on Ubuntu 18.04 (docker pull moveit/moveit:master-source) and running apt install rosdoc-melodic-rosdoc-lite snapd-xdg-open xdg-utils inside it, I get these errors. I don't find the html files in the build directory afterwards.

@v4hn
Copy link
Contributor

v4hn commented Mar 20, 2020

The console output looks pretty much like ~/ws_moveit/src/moveit_tutorials/build/html/index.html should exist now.
The errors only complain about no browser being available (fair enough for the container).

@felixvd
Copy link
Contributor Author

felixvd commented Mar 20, 2020

My bad, I was looking in ~ws_moveit/build/moveit_tutorials/ 🙄

Will fix soon.

@felixvd felixvd mentioned this pull request Mar 23, 2020
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants