search

lbr-stack / lbr_fri_ros2_stack

ROS 1/2 integration for KUKA LBR IIWA 7/14 and Med 7/14
https://lbr-stack.readthedocs.io/en/latest/
Apache License 2.0
121 stars 34 forks source link

Docker #151

Closed mhubii closed 4 months ago

mhubii commented 5 months ago

Refers to #149

mhubii commented 4 months ago

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!

Nicolai-98 commented 4 months ago

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: mkdir -p lbr-stack/src && cd lbr-stack. This directory does not exist yet when cloning the repository. This is why I suggested copying them after following the quick start by using: 

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. I am not 100% sure what you mean by separating docker instructions from build instructions. Could you clarify?

mhubii commented 4 months ago

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 setup.py turn shell scripts into entry points? But then that's also strange because one would have to install the package prior to building a container.

mhubii commented 4 months ago

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 !

Nicolai-98 commented 4 months ago

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.