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

Hardware ZED Camera Node Part 1 (Installation Overhaul) #91

Merged
merged 39 commits into from
Jul 25, 2024

Conversation

alberthli
Copy link
Contributor

@alberthli alberthli commented Jul 24, 2024

This PR begins the process of implementing a ZED2 Camera Node in Obelisk. We start by overhauling the way dependencies are managed/installed with conditional builds and configuring the packages that we build based on the initial setup flags.

Closes #87.

DONE

Installation

The major change to installation is that we now expose build args set when running setup.sh to the Dockerfile, which means the image now conditionally builds system-level dependencies based on environment variables on the system. Example usage:

cd $OBELISK_ROOT

# this configures environment variables (including in `docker/.env`) to build the container with certain deps
source setup.sh --recommended --docker-zed

# this now conditionally builds things like the basic system-level deps or zed deps into the container
# based on options passed to setup.sh
cd docker
docker compose -f docker-compose.yml run --build obelisk
  • modifications to setup.sh
    • add a --zed and a --basic flag. these flags will set OBELISK_BASIC=true and OBELISK_ZED=true.
    • copy the install_sys_deps.sh script into the docker/ directory
  • modifications to install_sys_deps.sh
    • expose a --basic and --zed flag. Basic will install the stuff that's in there now. Zed will install the ZED SDK.
  • modifications to Dockerfile and docker-compose yamls
    • expose the OBELISK_BASIC and OBELISK_ZED environment variables as build args
    • based on their value, pass --basic or --zed conditionally into the install_sys_deps.sh script, which should be added to the image
  • move cyclone performance optimizations to install script
  • rename docker_setup.sh to user_setup.sh, move it to scripts directory, and copy it into the docker directory upon running setup.sh
  • Separated out the leap hand deps such that you have to opt into them, i.e., bash install_system_deps.sh --leap
  • zed python sdk is now installed upon running obk if it wasn't installed already and if the sdk files have been downloaded

Usage

  • removed the messages-build and ros-build pixi tasks in favor of the obk-build and obk alias. These tasks are now called messages-build-ci and ros-build-ci, and are only used for CI
  • [CRITICAL] Aliases now selectively build ROS packages based on the flags passed to setup.sh on the host system (for example, if we specify --leap, we install the leap-related packages in obelisk, and if we don't, we exclude those packages). More concrete example:
# OPTION 1
# on host
source setup.sh --recommended
# in pixi shell
obk-clean
obk  # 15 packages finished

# OPTION 2
# on host
source setup.sh --recommended --leap
# in pixi shell
obk-clean
obk  # 17 packages finished

File Structure

  • Reorganized LEAP hand python/cpp code to all sit in one unified package (also slightly updated naming for clarity). Future: reorganize example code (like example controllers) in a separate workspace for cleanliness.

TODOs

Functional

  • [Follow-up PR] Code an ObeliskSensor called Zed2Sensors which initializes and synchronizes several ZED camera outputs, publishing an ObkImage message.

Future Issues

  • Organize example code in a separate workspace (also in this repo) that the end user can build on top of obelisk_ws to reduce package clutter. Issue Organize Example Code #94.
  • Clearly separate robot-specific packages from generics in the package file structure. Issue Clearly separate robot packages from generics #95.
  • Certain install paths seem broken as reported by Gavin:
    • source setup.sh --recommended --zed --leap --install-sys-deps apparently broken on local system
    • source setup.sh --recommended --zed --leap --install-sys-deps-docker fails to install deps in the docker filesystem correctly
  • Fix nvidia-container-toolkit install for systems that don't yet even have nvidia drivers (so nvidia-smi can't be run). Issue Fix nvidia-container-toolkit installation #96.

@alberthli alberthli changed the title Hardware ZED Camera Node Hardware ZED Camera Node Part 1 (Installation Overhaul) Jul 25, 2024
@alberthli alberthli marked this pull request as ready for review July 25, 2024 07:24
@alberthli alberthli requested review from Zolkin1 and gavin-hyl July 25, 2024 07:24
Copy link
Collaborator

@Zolkin1 Zolkin1 left a comment

Choose a reason for hiding this comment

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

Reviewed all changes, but I haven't run any code, so I'm not approving yet. I'll run it first thing this morning.

Overall everything looks reasonable, but it would be good to update the downstream example repo with how to use this now. I think that is a pretty good test bed to be sure everything runs and to provide a real world-ish use case example.

@@ -10,33 +10,59 @@ Obelisk should be used as a dependency for external robot control code that is w
3. Use Obelisk in a project that uses `pixi`.

### Initial Setup
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be worth clarifying that if the user wants to use Obelisk in a custom docker container (i.e. not the one we provide) then they will want to clone this inside the docker container and run the command that modify the "system level deps", because all of the conditional building is really for the dev container and presumably the user won't want to dev their robot stack in the Obelisk dev container.

On the flip side maybe this is an overreach of the docs, but I think it might make the intended use clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made a halfway attempt at commenting on this - if we need further clarity, let's resolve in a future PR

@alberthli alberthli requested a review from Zolkin1 July 25, 2024 22:12
Copy link
Collaborator

@Zolkin1 Zolkin1 left a comment

Choose a reason for hiding this comment

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

lgtm

@alberthli alberthli merged commit 2d4d44c into main Jul 25, 2024
1 check passed
@alberthli alberthli deleted the hardware-zed-cams branch July 25, 2024 22:14
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.

Allow end user to have more control over hardware-specific dependencies
2 participants