-
Notifications
You must be signed in to change notification settings - Fork 39
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
Allow (simulated) display to be used with ci builds #1
Conversation
I added Travis to this repo so moveit_ci can test moveit_ci - can you rebase your PR with latest commits before I merge this? Thanks |
I think this is now rebased (I haven't done this before). Please let me know if it didn't work. |
@@ -39,6 +43,9 @@ env: | |||
matrix: | |||
allow_failures: | |||
- env: ROS_DISTRO="kinetic" ROS_REPOSITORY_PATH=http://packages.ros.org/ros/ubuntu UPSTREAM_WORKSPACE=https://raw.githubusercontent.com/ros-planning/moveit_docs/kinetic-devel/moveit.rosinstall | |||
install: | |||
- export DISPLAY=':99.0' | |||
- Xvfb :99 -screen 0 1024x768x24 > /dev/null 2>&1 & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment here explaining the gist of what these two lines do?
@scottpaulin you rebased correctly, good job! unfortunately I realized I setup the travis script to test this incorrectly - when it says "All checks have passed" it is not actually testing your modifications but just the master version. Needed 9f3c97a Could you rebase again against master? My bad. Also don't forget to address my inline comments |
I have added comments and cleaned up the Xvfb command. It was specifying a screen resolution, but the default should work fine. Also rebased. I just added you as an admin to my fork in case there is a minor change you need to make in the next few days. We just got snow so I will be away skiing (not biking to the field though!). |
haha its ski season for you?? even our back country is dried up by now I'm not sure this PR fixes the desired issue - looking at the Travis build just now I still see
I don't really know, but perhaps this is an unavoidable limitation of a server? |
Yip! Southern hemisphere :) Sorry I broke this in the last commit (too keen to ski), but not in the way you describe. Did you change .travis.yml in moveit_ros to include the changes in the template from moveit_ci/Readme? It should look something like this.
Nah, I made sure it was working before making this PR. Xvfb is specifically for simulating displays on machines without screens. I will test it again when the ci build is passing and leave a comment here, before the Moveit! repo merge if possible. |
Ha, no I didn't. Can you fix the merge conflict? then I'll test it in the unified repo... |
Implemented according to the instructions at http://wiki.ros.org/docker/Tutorials/GUI. For this to work a display has to be set with xvfb (see the readme).
Use xvfb to simulate a display within Travis that can be passed into the docker container that is used for building the software under test. The display environment variable is also set. The instructions at https://docs.travis-ci.com/user/gui-and-headless-browsers/ didn't seem to work (got errors when trying to run "sh -e /etc/init.d/xvfb start" and exit code 127), but I think this solution achieves the same thing.
Cleaned up the Xvfb command (-screen option is not required) and added comments.
Accidentally removed & when cleaning up this template. Without the & the virtual display runs in the foreground (blocking) so stalls the build.
Accidentally deleted it when merging
Testing this PR here: https://travis-ci.org/davetcoleman/moveit/builds/150449446 |
Haha looks like we both had test builds running that resulted in build errors from the mesh_filter unit tests. At least it means that the display has been detected. The 'static const member variables must be constexpr' is a c++11 thing. I have fixed that build error in my fork which can later be a PR against moveit. The build output is here. I think there are a few more build errors because Google Test's EXPECT_TRUE explicitly requires a bool argument, but the mesh_filter tests are using it to check for nullptrs. I am happy to fix the build errors, should I raise it as an issue in Moveit? Sorry about the delay in handling this, I have been sick :(. |
Awesome that its working and catching bugs! Now your travis is showing another issue:
|
Hmm it looks like there might be two problems here 1) OpenGl can't find the graphics hardware drivers and 2) Xvfb possibly does not support the required frame buffer format. I am trying to solve 1) by installing the mesa drivers (see second answer), the build is here. When I tested it pre-PR it was timing out (which it does when the screen resolution is set) and I incorrectly thought it was due to the mesh_filter unit tests. |
Your build passed but still says "No display, will not configure tests for moveit_ros_perception/mesh_filter" |
Installing the mesa drivers got rid of these errors:
Free glut now seems to be having trouble opening the display: |
What a headache! I do not have much experience with X server stuff |
Hey @scottpaulin, any progress on fixing this test on Travis? |
Unfortunately not. I am out of ideas |
I have done plenty of Googling-Stackoverflow on this and am super stuck. Should I just reject this PR and close it? |
Perhaps @ruffsl has advice on adding a virtual display/GPU since he's worked extensively with Docker containers. Maybe we need something like https://github.com/NVIDIA/nvidia-docker |
The nvidia-docker plugin is relevant if the host has nvidia hardware and drivers. Does your travis provider use servers with nvidia GPUs? Is the docker engine running within a VM, and does the VM have have access to the PCI passthrough for the GPU? I'm not sure of the shape of the stack of Travis's CI stack. I am yet unsure how you could swap the call to the Last summer I was playing around getting gazebo to work on a headless amazon aws instance with nvidia gpus. I was running gzclient remotely from my local workstation, but I needed the gzserver to have a virtual display so that ogre could render the camera views in simulation, with the GPU acceleration speeding up the render rates. Here is my (perhaps dated, before the nvidia-docker plugin came around) notes on what I did to get it working: I'd imagine getting the integrated intel graphics to work might be simpler, no need for drivers and plugins the mount devices, just the correct packages. I think someone added some intel info to the wiki i started here: Perhaps @scpeters or @hugomatic might have some more incite, as I recall they were working around some of these issues more recently with cloud based simulation development. |
We have been using nvidia-docker locally with quite a bit of success, though I haven't used it in a CI context yet. We had talked about supporting Intel GPU's as well, but have done very little testing in that regard. I believe the plan for intel is to try to match the driver of the host and the container. |
I've got xvfb running in docker. See #33. |
This now allows a display to be used in ci testing. I tested this with the moveit_ros repo, the mesh_filter tests are no longer disabled meaning there is a display detected.
I found the xvfb instructions on someone else's repository (lost the link) after the instructions from travis here didn't work. As I understand it xvfb is used to simulate a display on virtual frame buffer 99. I think 99 is used because it is less likely to be used for something else (see here).
The change to travis.sh allows the display simulated by xvfb to be used in Docker. This was implemented following the wiki instructions here (thanks for the link @davetcoleman !).