Conversation
b84bba5 to
863235e
Compare
414c328 to
099f22d
Compare
c76cb53 to
170f897
Compare
170f897 to
34de7b3
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #748 +/- ##
==========================================
+ Coverage 65.33% 69.11% +3.78%
==========================================
Files 78 82 +4
Lines 3392 3681 +289
==========================================
+ Hits 2216 2544 +328
+ Misses 1176 1137 -39 ☔ View full report in Codecov by Sentry. |
|
Tip For best results, initiate chat on the files or code changes.
Just let me know what you'd like me to do! |
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
WalkthroughThe PR introduces a new manipulation demo Docker environment alongside supporting documentation and scripts. It adds Fast DDS configuration and Poetry caching optimizations to the main Dockerfile, creates a new manipulation-demo Dockerfile with vcs import and streamlit entrypoint, simplifies demo documentation, and includes a model weights download script. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/demos/manipulation.md`:
- Around line 92-129: Fix typos and unify terminology and image tag in the
manipulation demo steps: correct "manipulaiton" to "manipulation" in the docker
build step (the line containing the docker build command for
rai-manipulation-demo:jazzy), change "environmental variable" to "environment
variable" in the line with export OPENAI_API_KEY (the export OPENAI_API_KEY=...
line), update the docker run example to reference the full image tag
"rai-manipulation-demo:humble" instead of "rai:humble" (the docker run -p
8501... line), and remove the extra space before the colon in the web interface
URL line so it reads "http://localhost:8501".
🧹 Nitpick comments (3)
src/rai_extensions/rai_perception/rai_perception/scripts/download_weights.py (1)
17-21: Ensure agents are stopped even if initialization fails.
IfGroundedSamAgent()raises,GroundingDinoAgentnever gets stopped. Atry/finallyavoids resource leaks.♻️ Proposed fix
if __name__ == "__main__": - agent1 = GroundingDinoAgent() - agent2 = GroundedSamAgent() - agent1.stop() - agent2.stop() + agent1 = agent2 = None + try: + agent1 = GroundingDinoAgent() + agent2 = GroundedSamAgent() + finally: + if agent2: + agent2.stop() + if agent1: + agent1.stop()docker/Dockerfile.manipulation-demo (1)
34-35: Useexecin ENTRYPOINT to forward signals.
Withoutexec, Bash stays PID 1 and may not forward SIGTERM cleanly.♻️ Proposed fix
-ENTRYPOINT ["bash", "-c", ". setup_shell.sh && streamlit run examples/manipulation-demo-streamlit.py \"$@\""] +ENTRYPOINT ["bash", "-c", ". setup_shell.sh && exec streamlit run examples/manipulation-demo-streamlit.py \"$@\""]docker/Dockerfile (1)
82-84: Confirm that forcing Fast DDS is intended for all containers.
If other DDS implementations are supported, consider documenting how to overrideRMW_IMPLEMENTATIONat runtime.
| 1. Set up docker as outlined in the [docker setup guide](../setup/setup_docker.md). | ||
|
|
||
| 2. Enable X11 access for the docker container: | ||
| 2. Build docker manipulaiton demo image: | ||
|
|
||
| ```shell | ||
| xhost +local:root | ||
| docker build -t rai-manipulation-demo:jazzy --build-arg ROS_DISTRO=jazzy -f docker/Dockerfile.manipulation-demo . | ||
| ``` | ||
|
|
||
| 3. Run the docker container with the following command: | ||
| 3. Enable X11 access for the docker container: | ||
|
|
||
| ```shell | ||
| docker run --net=host --ipc=host --pid=host -e ROS_DOMAIN_ID=$ROS_DOMAIN_ID -e DISPLAY=$DISPLAY -v /tmp/.X11-unix:/tmp/.X11-unix --gpus all -it rai:jazzy # or rai:humble | ||
| xhost +local:root | ||
| ``` | ||
|
|
||
| !!! tip "NVIDIA Container Toolkit" | ||
|
|
||
| In order to use the `--gpus all` flag, the [NVIDIA Container Toolkit](https://docs.nvidia.com/datacenter/cloud-native/container-toolkit/install-guide.html) must be installed on the host machine. | ||
|
|
||
| 4. (Inside the docker container) By default, RAI uses OpenAI as the vendor. Thus, it is necessary | ||
| to set the `$OPENAI_API_KEY` environmental variable. The command below may be utilized to set | ||
| the variable and add it to the container's `.bashrc` file: | ||
| 4. Set the `$OPENAI_API_KEY` environmental variable. | ||
|
|
||
| ```shell | ||
| export OPENAI_API_KEY=YOUR_OPEN_AI_API_KEY | ||
| echo "export OPENAI_API_KEY=$OPENAI_API_KEY" >> ~/.bashrc | ||
| ``` | ||
|
|
||
| !!! note AI vendor change | ||
|
|
||
| The default vendor can be changed to a different provider via the [RAI configuration tool](../setup/install.md#15-configure-rai) | ||
|
|
||
| 5. After this, follow the steps in the [Local Setup](#local-setup) from step 2 onwards. | ||
| 5. Run the docker container with the following command: | ||
|
|
||
| !!! tip "New terminal in docker" | ||
| ```shell | ||
| docker run -p 8501:8501 -e DISPLAY=$DISPLAY -e OPENAI_API_KEY=$OPENAI_API_KEY -v /tmp/.X11-unix:/tmp/.X11-unix --gpus all -it rai-manipulation-demo:jazzy # or rai:humble | ||
| ``` | ||
|
|
||
| !!! tip "NVIDIA Container Toolkit" | ||
|
|
||
| In order to open a new terminal in the same docker container, you can use the following command: | ||
| In order to use the `--gpus all` flag, the [NVIDIA Container Toolkit](https://docs.nvidia.com/datacenter/cloud-native/container-toolkit/install-guide.html) must be installed on the host machine. | ||
|
|
||
| ```shell | ||
| docker exec -it <container_id> bash | ||
| ``` | ||
| 6. To access the demo web interface, open your web browser and navigate to : | ||
| ``` | ||
| http://localhost:8501 | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the file
cat -n docs/demos/manipulation.md | sed -n '90,130p'Repository: RobotecAI/rai
Length of output: 1648
Fix typos, terminology, image tag reference, and formatting.
Line 94: "manipulaiton" is a typo. Line 106: use "environment variable" (standard terminology). Line 119: the comment references "rai:humble" but should match the full image tag format "rai-manipulation-demo:humble". Line 126: remove the extra space before the colon.
📝 Proposed fix
-2. Build docker manipulaiton demo image:
+2. Build docker manipulation demo image:
-4. Set the `$OPENAI_API_KEY` environmental variable.
+4. Set the `$OPENAI_API_KEY` environment variable.
- docker run -p 8501:8501 -e DISPLAY=$DISPLAY -e OPENAI_API_KEY=$OPENAI_API_KEY -v /tmp/.X11-unix:/tmp/.X11-unix --gpus all -it rai-manipulation-demo:jazzy # or rai:humble
+ docker run -p 8501:8501 -e DISPLAY=$DISPLAY -e OPENAI_API_KEY=$OPENAI_API_KEY -v /tmp/.X11-unix:/tmp/.X11-unix --gpus all -it rai-manipulation-demo:jazzy # or rai-manipulation-demo:humble
-6. To access the demo web interface, open your web browser and navigate to :
+6. To access the demo web interface, open your web browser and navigate to:🤖 Prompt for AI Agents
In `@docs/demos/manipulation.md` around lines 92 - 129, Fix typos and unify
terminology and image tag in the manipulation demo steps: correct "manipulaiton"
to "manipulation" in the docker build step (the line containing the docker build
command for rai-manipulation-demo:jazzy), change "environmental variable" to
"environment variable" in the line with export OPENAI_API_KEY (the export
OPENAI_API_KEY=... line), update the docker run example to reference the full
image tag "rai-manipulation-demo:humble" instead of "rai:humble" (the docker run
-p 8501... line), and remove the extra space before the colon in the web
interface URL line so it reads "http://localhost:8501".
Purpose
Update base dockerfile
Add dockerfile for manipulation demo
Proposed Changes
waiting for services....(not sure about this though)Issues
Testing
Run manipulation demo in docker following the docs
Summary by CodeRabbit
New Features
Documentation
Chores