-
Notifications
You must be signed in to change notification settings - Fork 55
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
Docker #151
Docker #151
Conversation
…ner. Updated README.
#149 Added Dockerfile and scripts. Updated README. thank you @Nicolai-98 for doing this so quickly!
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.
looks good overall, re-iterate on file placement
hi @Nicolai-98 , thank you again for contributing this. I now have time to review properly and would like to merge into humble. do you think it makes sense to put the shell scripts / the dockerfile into the root folder, since they are no ros package. Also, in terms of build instructions, does it make sense to separate docker instructions? Thank you for the input and the help! |
Hi! I'm always happy to be able to contribute. Ideally you want the docker related files in the lbr-stack directory that is created using the quick start instructions from the ReadMe: cp -r src/lbr_fri_ros2_stack/lbr_humble_docker/* .
chmod +x container_build.sh
sudo ./container_build.sh To answer the first question: The files can also be put into the root directory as long as they end up in the correct directory after building the project. This allows an easy workflow where you only have to use the container_build script once and then you can start a container whenever you need it using the container_start script. |
okay got it! By that I mean just running with / without Docker. Hm okay, would it be useful to turn these shell scripts into entry points a la ros2 run lbr_docker build_docker that would justify a package, right? Or is that rather pointless. Can |
okay I tested it and it works, got to double check on real robot. But I would say lets merge this PR ASAP and go from here. Thank you again @Nicolai-98 ! |
Its my pleasure. I think the current setup with the changes you made is perfectly fine. I don't think turning the shell scripts into entry points adds any extra value. |
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.
Looks good and tidy!
RUN rosdep install -i --from-paths src --rosdistro ${ROS_DISTRO} -y | ||
|
||
# "--symlink-install" allows the code in the locally mounted volume ./src to be adjusted without rebuilding | ||
RUN source /opt/ros/${ROS_DISTRO}/setup.bash && \ |
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.
okay so I cleaned some dependencies, as these are mostly in the base images already.
One more question before merge @Nicolai-98: I don't use Docker a lot, so please let me know if you need scipy / numpy / vim / python3 is python etc installed, so it is more convenient for your use.
There will likely be a couple changes moving forward but this is a great first step.
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.
Sorry I went ahead and merged this anyways already. Thank you again! You are now listed among contributors.
Please feel free to create new PRs against humble
if you see fit.
Refers to #149