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

Docker script #20

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Docker script #20

wants to merge 11 commits into from

Conversation

AlfredMoore
Copy link
Collaborator

add a module to use bash script to config and set up docker container. Note: This container uses zsh instead of bash, so you should source devel/setup.zsh instead of source devel/setup.bash

@AlfredMoore AlfredMoore self-assigned this Oct 29, 2024
Copy link
Collaborator

@schromya schromya left a comment

Choose a reason for hiding this comment

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

Overall, this looks super cool!

However, run_simulation.md is failing although it previously worked in the other docker setup. I tested adding the missing dependencies and modifying file paths (mentioned in my other comments) but it is still its failing. This should be fixed along with the other comments I've added before this is merged to main

ros-docker-dev/enterpoint.sh Outdated Show resolved Hide resolved
ros-docker-dev/README.md Outdated Show resolved Hide resolved
ros-docker-dev/README.md Outdated Show resolved Hide resolved
ros-docker-dev/configs/oh-my-zsh.zshrc Outdated Show resolved Hide resolved
ros-docker-dev/configs/oh-my-zsh.zshrc Outdated Show resolved Hide resolved
ros-docker-dev/panda-noetic.Dockerfile Outdated Show resolved Hide resolved
ros-docker-dev/panda-noetic.Dockerfile Outdated Show resolved Hide resolved
ros-docker-dev/README.md Outdated Show resolved Hide resolved
doc/setup.md Show resolved Hide resolved
### End ##############################################################
######################################################################


Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to add:

RUN rosdep update
RUN rosdep install --from-paths src --ignore-src --rosdistro noetic  -y --skip-keys libfranka

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since we volume the NIST folder, it would not show up during the image building process. It would only show in the docker run or docker execute. So user should do rosdep inside of the container.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should just take out all instructions for running the old way to avoid confusion. It will help us avoid having to maintain 2 different ways to run things. Thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Totally agree. This docker can be used as a alternative or for someone interests to dive in. It would be better to separate it with the main thread.

doc/setup.md Outdated Show resolved Hide resolved
doc/setup.md Outdated Show resolved Hide resolved
ros-docker-dev/configs/oh-my-zsh.zshrc Outdated Show resolved Hide resolved
doc/setup.md Outdated Show resolved Hide resolved
@AlfredMoore AlfredMoore requested a review from schromya November 19, 2024 20:40
@AlfredMoore
Copy link
Collaborator Author

The zsh and regular does not work well in the realtime environment. The c++ realtime library keeps making errors in the regular user even if the user has been added to the realtime group.
So if you want to run in a realtime env, please run with option -r.

The link to the docker-script has been moved to the first item of setup.md/Note

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.

2 participants